Back
[08:35:23] <dgarr> seb_kuzminsky: i am interested in the streamer patch with man page add here:
http://www.panix.com/~dgarrett/stuff/0001-streamer.c-add-clock-clock-mode-pins.patch ok for 2.7?
[08:40:41] <jepler> > in addition there are two new pins to enable clocking streamer.
[08:40:49] <jepler> I don't think it makes sense in a comment to call the pins "new"
[08:41:19] <jepler> so I'd slightly reword the comment
[08:42:01] <jepler> + /* keep last two clock states to get all possible clock edges */
[08:42:01] <jepler> + myclockedge=((myclockedge<<1) | (*(str->clock) & 1)) & 3;
[08:42:01] <jepler> + /* are we enabled? - generate doclock if enabled and right mode */
[08:42:01] <jepler> + doclk=0;
[08:42:02] <jepler> + if ( *(str->enable) ) {
[08:42:06] <jepler> inconsistent indentation
[08:42:29] <jepler> in that whole added block
[08:43:28] <jepler> - tmpin += fifo->depth;
[08:43:29] <jepler> + tmpin += fifo->depth;
[08:43:35] <jepler> whitespace-only change
[08:45:56] <jepler> I think the logic is OK but I would like to see the code style cleaned up before it's merged.
[08:46:24] <jepler> also attention will be required when merging to master, as seb_kuzminsky has converted the streamer docs from man to asciidoc. I volunteer to do that.
[09:54:17] <dgarr> seb_kuzminsky: revised per jepler's remarks (to the extent i comprehend them):
http://www.panix.com/~dgarrett/stuff/v2-0001-streamer.c-add-clock-clock-mode-pins.patch
[10:02:06] <jepler> ah the code is wrong :-/
[10:02:13] <jepler> myclockedge needs to be a member of streamer_t
[10:02:22] <jepler> otherwise it is not going to work for multiple instances
[10:03:09] <dgarr> ok -- thanks for looking at it, i give up on this one
[10:17:37] <dgarr> http://www.panix.com/~dgarrett/stuff/v3-0001-streamer.c-add-clock-clock-mode-pins.patch
[10:28:23] <seb_kuzminsky> dgarr: is this the patch from our feature-request tracker? (with your fixes as per jepler's review comments)
[10:28:57] <dgarr> yes
[10:29:19] <seb_kuzminsky> jepler: i'm pleased to discover that i merged that asciidoc manpage stuff (as opposed to a few dozen half-finished branches lying around in my private repos)
[10:29:24] <seb_kuzminsky> dgarr: cool
[10:29:29] <seb_kuzminsky> i'll take a look at you v3
[10:29:32] <seb_kuzminsky> *your
[10:30:13] <seb_kuzminsky> is "freerun" a word?
[10:30:21] <seb_kuzminsky> (it's used in the manpage)
[10:31:51] <cradek> maybe not, but I know what it means and it's pretty clear
[10:32:15] <seb_kuzminsky> mmmyep
[10:32:21] <cradek> I think I've heard it said in the electronics realm before
[10:32:46] <cradek> you could spell it free-run and then I'm pretty sure it's a word
[10:33:15] <cradek> (I'm ashamed I didn't look at the code but I'm commenting on grammar)
[10:34:09] <seb_kuzminsky> dgarr: there's still useless whitespace changes in the setting of str->empty and str->curr_depth
[10:34:38] <seb_kuzminsky> that's not a blocker but it's bad patch etiquette
[10:35:59] <seb_kuzminsky> that's one of the things i asked slavko to clean up before merging, which made him give up and walk away :-(
[10:38:21] <cradek> that sucks
[10:38:59] <cradek> it's really hard as a new contributor to understand why people are picky about that.
[10:39:27] <seb_kuzminsky> yeah, i can clearly remember not caring about stuff like that at all when i was new to programming
[10:39:33] <cradek> it's only after you are bitten by how whitespace changes can make all future merges painful FOREVER that you understand
[10:39:47] <cradek> and bisects, and diffs, and ...
[10:39:51] <seb_kuzminsky> and i feel strongly that it's important, now that i've had 20 years of experience
[10:39:56] <cradek> totally
[10:40:08] <cradek> but y'know, if our tools were better it wouldn't be so important, and that sucks too
[10:40:47] <pcw_home> yeah, where's the whitespace change ignore flag?
[10:40:47] <seb_kuzminsky> right
[10:40:50] <cradek> building a vc system on diff and patch which don't even understand programming languages is pretty silly
[10:41:09] <cradek> I mean I understand why you'd do it, because anything else would be harder and probably worse, but still :-P
[10:41:27] * cradek waves his arms
[10:41:59] <seb_kuzminsky> the irrelevant parts of the files, whitespace, brace-alignment, etc, should all be local rendering preferences, just like font size and color
[10:42:30] <cradek> haha sure, except when that's not what you want
[10:42:39] <dgarr> http://www.panix.com/~dgarrett/stuff/v4-0001-streamer.c-add-clock-clock-mode-pins.patch
[10:42:44] <seb_kuzminsky> except i guess it's nice for different people to be able to agree about things like line numbers
[10:43:18] <seb_kuzminsky> dgarr: thanks
[10:43:47] <seb_kuzminsky> if you do the one that jepler pointed out above (tmpin/fifo->depth), go ahead and push it to 2.7
[10:58:44] <KGB-linuxcnc> 03Dewey Garrett 052.7 9ea94bb 06linuxcnc 10docs/man/man9/streamer.9 10src/hal/components/streamer.c streamer.c: add clock,clock-mode pins * 14http://git.linuxcnc.org/?p=linuxcnc.git;a=commitdiff;h=9ea94bb
[10:58:49] <seb_kuzminsky> yay!!
[11:03:33] <jepler> thanks dgarr
[11:03:48] <jepler> /beer dgarr
[11:04:00] <cradek> oh I wish that worked
[11:04:05] <jepler> (hmmm now there's a project to get you internet famous)
[11:04:33] <jepler> you'd need a solenoid and a switch
[11:04:59] <seb_kuzminsky> http://www.beeradvocate.com/community/threads/1922-nobel-prize-winner-niels-bohr-got-a-house-with-free-beer.179713/
[11:05:28] <seb_kuzminsky> all this beer talk reminds me: it's friday! woooo
[11:05:31] <jepler> so that if the beer glass is present and has not been filled since it became present, the right tweet / email / irc message / bitcoin transaction will fill it
[11:06:22] <seb_kuzminsky> the buildmaster would automatically /beer the committer when a build succeeds
[11:07:55] <jepler> http://www.japantrendshop.com/asahi-beerbot-beer-pouring-robot-p-130.html
[11:07:58] <jepler> drat, sold out
[11:09:05] <jepler> https://www.youtube.com/watch?v=smmcude-8mU
[11:11:02] <jepler> https://www.youtube.com/watch?v=MltbAKFXRtk
[11:11:25] <jepler> so thirsty
[11:12:50] <jepler> it would be so much simpler for American Robot. Just have to pop the tab on a can and hand it to the person
[11:29:45] <linuxcnc-build> build #1277 of 1403.rip-wheezy-amd64 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1403.rip-wheezy-amd64/builds/1277 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:30:20] <linuxcnc-build> build #1468 of 1404.rip-wheezy-rtpreempt-amd64 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1404.rip-wheezy-rtpreempt-amd64/builds/1468 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:30:25] <linuxcnc-build> build #1277 of 1400.rip-wheezy-i386 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1400.rip-wheezy-i386/builds/1277 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:30:31] <jepler> well this doesn't look good
[11:30:45] <seb_kuzminsky> uuuhh
[11:31:18] <seb_kuzminsky> that's surprising
[11:31:28] <jepler> the tests all use streamer
[11:31:51] <linuxcnc-build> build #788 of 1402.rip-wheezy-rtpreempt-i386 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1402.rip-wheezy-rtpreempt-i386/builds/788 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:31:58] <jepler> abs.0 test is hung here too
[11:32:10] <jepler> actually rtapi_app seems to have crashed
[11:32:15] <jepler> [1803643.362991] rtapi_app[18734]: segfault at 0 ip 00007f11b4407e88 sp 00007f11b439fe68 error 4 in streamer.so[7f11b4407000+2000]
[11:32:25] <jepler> ah yes there too
[11:32:31] <seb_kuzminsky> oh
[11:32:32] <seb_kuzminsky> heh
[11:32:43] <jepler> no good deed
[11:33:18] <jepler> 203 *str->myclockedge=((*str->myclockedge<<1) | (*(str->clock) & 1)) & 3;
[11:33:20] <seb_kuzminsky> i looked for streamer tests in our test suite, and didnt find any, but i forgot that many of the tests of other comps *use* streamer to provide test patterns
[11:33:21] <jepler> (gdb) p str->myclockedge
[11:33:21] <jepler> $2 = (hal_s32_t *) 0x0
[11:33:28] <cradek> myclockedge
[11:33:30] <cradek> .. yeah
[11:33:41] <cradek> is that meant to be a pin?
[11:33:42] <jepler> myclockedge should probably just be an int
[11:33:55] <jepler> let me fix it
[11:36:12] <seb_kuzminsky> i guess i assumed dgarr had run the tests
[11:36:23] <jepler> today at $DAY_JOB I'm waiting for 12-minute links and then 10-minute copies over wireless to the test machine :-/
[11:36:34] <jepler> it turns out the OpenGL implementation on Microsoft Surface Pro 3 is poop
[11:37:32] <cradek> the screen on that looks nice. I wonder if you could eventually put a useful OS on it.
[11:38:16] <jepler> http://winaero.com/blog/how-to-install-linux-on-surface-pro-3/
[11:38:23] <linuxcnc-build> build #1307 of 1405.rip-wheezy-armhf is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1405.rip-wheezy-armhf/builds/1307 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:40:09] <jepler> (actually I don't know whether this is surface pro 3 or some other number of surface)
[11:41:59] <linuxcnc-build> build #3119 of 1300.rip-precise-i386 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1300.rip-precise-i386/builds/3119 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:42:02] <cradek> I think the reading of str->clock is wrong
[11:42:10] <linuxcnc-build> build #3121 of 1306.rip-precise-amd64 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1306.rip-precise-amd64/builds/3121 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:42:16] <linuxcnc-build> build #3120 of 1200.rip-lucid-i386 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1200.rip-lucid-i386/builds/3120 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:42:17] <cradek> or are hal_bit_t more like bools now so it's moot?
[11:42:40] <linuxcnc-build> build #2323 of 1301.rip-precise-rtai-i386 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1301.rip-precise-rtai-i386/builds/2323 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:42:58] <linuxcnc-build> build #3120 of 1202.rip-lucid-amd64 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1202.rip-lucid-amd64/builds/3120 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:43:12] <cradek> linuxcnc-build: shhh, we know, it'll be ok
[11:43:19] <cradek> jepler: ^
[11:43:41] <jepler> cradek: hal_bit_t is volatile bool *, and the value of a bool coerced to int (as in arithmetic) is 0 or 1. with a big "I think" caveat next to it
[11:44:21] <cradek> ok, that must be new, I know this was a bug in the past
[11:45:38] <jepler> I think that by using C99 bool that is no longer a concern
[11:45:49] <cradek> thanks for checking
[11:45:58] <linuxcnc-build> build #3120 of 1201.rip-lucid-rtai-i386 is complete: Failure [4failed compile runtests] Build details are at
http://buildbot.linuxcnc.org/buildbot/builders/1201.rip-lucid-rtai-i386/builds/3120 blamelist: Dewey Garrett <dgarrett@panix.com>
[11:48:36] <jepler> http://pastie.org/10111925
[11:48:55] <jepler> Runtest: 154 tests run, 153 successful, 1 failed + 0 expected
[11:48:55] <jepler> Failed:
[11:48:55] <jepler> /home/jepler/local/src/linuxcnc/tests/t0/nonrandom
[11:49:10] <jepler> http://pastie.org/10111927
[11:49:53] <seb_kuzminsky> that's one that occasionally fails, and i dont know why
[11:50:07] <seb_kuzminsky> another dropped mdi problem somewhere maybe? or a bug in that test?
[11:51:21] <KGB-linuxcnc> 03Jeff Epler 052.7 af5e850 06linuxcnc 10src/hal/components/streamer.c streamer: fix 'myclockedge' segfault * 14http://git.linuxcnc.org/?p=linuxcnc.git;a=commitdiff;h=af5e850
[11:51:40] <seb_kuzminsky> thx...
[11:51:46] <jepler> /beer jepler
[11:52:49] <KGB-linuxcnc> 03Chris Radek 05v2.5_branch 405ed60 06linuxcnc 10src/hal/hal_lib.c Fix a dubious type cast, from machinekit dac83399 * 14http://git.linuxcnc.org/?p=linuxcnc.git;a=commitdiff;h=405ed60
[11:52:50] <seb_kuzminsky> ///
[11:53:13] <jepler> I wonder why the F6 key on this (suface pro) keyboard has an ubuntu logo
[12:02:04] <Tom_itx> hah
[12:03:30] <Tom_itx> never noticed that before :)
[12:44:09] <andypugh> Is it excessive for a HAL component to use 1k for internal variables?
[12:45:46] <andypugh> shmen size seems to be 20Mb so 1k seems allowable.
[12:53:14] <archivist> should the startup measure the size of hal variables etc so shmem can be sized to suit ?
[13:05:19] <seb_kuzminsky> andypugh: most internal variables shouldnt live in shm, right? only hal pins & params, the things that come from hal_malloc
[13:18:50] <andypugh> Ah, yes. The per-instance storage is private isn’t it? It’s been a while since I thought about this stuff ;-)
[13:35:24] <jepler> andypugh, seb_kuzminsky: *in principle* 'private' stuff need not be in the hal shared memory area, but in practice comp puts everything there because it's marginally easier
[13:46:00] <jepler> when the comp philosophy was "this is for super simple components" this was fine, because if you're going to have an int or two of extra storage it's probably not a bad idea for it to be on the same cache line as some pin pointers
[13:46:15] <jepler> but since comp gets used for megacomplex components it's more questionable
[13:46:41] <jepler> still, I think it's been awhile since we've heard anybody running out of hal memory and we usually double it every release or two
[13:46:46] <jepler> just to avoid that kind of thing
[13:48:08] <andypugh> There is one particularly guilty party for the super-complex comp sin. :-)
[13:48:39] <andypugh> You will love the current one, I am embedding a programming language. :-)
[13:48:48] <andypugh> Not turing-complete, though.
[13:57:41] * cradek shivers
[14:04:06] <jepler> oh I've thought about doing that
[14:04:16] <jepler> it would have been rpn/forth-based
[14:04:35] <jepler> for those who found lut5 too limiting
[14:04:52] <cradek> https://en.wikipedia.org/wiki/SIOD
[14:05:52] <cradek> I've wanted to find a problem for this solution for many years
[14:06:53] <jepler> pretty sure you don't want a language with gc running in realtime
[14:07:06] <cradek> darn
[15:16:42] <andypugh> I may be exaggerating
[15:16:58] <andypugh> But it is extensible, so who knows?
[15:20:17] <andypugh> loadrt toolchanger config=“sequence=s1f1;r1s2t5” sets out-1 then waits for in-1 to be false, then resets out-1, sets out-2 and waits for in-5 to be true. A C-command in the sequence string operates a carousel toolchanger at any point.
[15:21:01] <andypugh> I _think_ this will cover a lot of situations, especially with G-code watching the pins and making moves as necessary.
[15:22:00] <andypugh> But I am, of course, mainly doing it because I famcied doing it. I don’t actually have a toolchanger.
[15:22:47] <andypugh> (ponder) Maybe I should split it into two components, “sequence” and “carousel” ?
[15:23:06] <cradek> how does carousel work?
[15:24:21] <andypugh> loadrt carousel config=“num_sense=5 code=gray pockets=16 bidirectional=false”
[15:26:04] <cradek> wow
[15:27:02] <andypugh> I think I will split it, the sequencer is probably useful for many things and it can fire-off the carousel easily.
[15:27:38] <JT-Shop> just ran the stencil font on the plasma running 2.7.0-pre6 and the velocity is dead on 60 IPM while cutting... just thought I'd let you guys know
[15:27:55] <andypugh> loadrt sequence config=“sequence=s1;r3 on_error=r1r2r3s5”
[15:28:09] <cradek> JT-Shop: sweet
[15:28:45] <cradek> andypugh: if you split them, someone could do the sequence part in ladder (where I think the visual representation of the program can really help) but still use your carousel part
[15:28:58] <JT-Shop> yea, I got a grin from ear to ear when it was running
[15:29:16] <andypugh> Yes, and HAL is meant to be small modules.
[15:30:12] <cradek> andypugh: I kind of hate to see you make a new kind of tool that solves some of the same things as ladder, but I understand why you want to
[15:30:27] <cradek> andypugh: down the road I'm on we'd be able to remove all the logic components like and2, too
[15:30:45] <cradek> andypugh: and I suppose nobody really wants to go down that road
[15:33:33] <andypugh> You mean adding logic syntax to “net” ?
[15:35:51] <skunksleep> JT-Shop: the new to is cool
[15:36:20] <skunksleep> Tp
[15:36:40] <andypugh> Maybe I should step back and think. I started to write a carousel comp, then decided that it should know how to unlock itself, and maybe move the arm, and start spindle alignment. Then I decided to make that part more configurable. Then decided to take the configurable bit out and standalone to be redundant with Ladder.
[15:42:12] <andypugh> There are no real downsides to a component existing, I suppose.
[15:51:13] <cradek> andypugh: yeah, aside from your personal support load
[16:25:29] <andypugh> I will just lie about the author
[16:33:50] <andypugh> Xubuntu 14.04 is going to be a compile-from-source isn’t it? Is it also a “patch your own kernel” ?
[16:51:38] <seb_kuzminsky> andypugh: well so far no one has made kernel packages for it, so yes, until someone does that
[16:53:07] <cradek> ubuntu is ditching apt, so people may as well switch to debian today
[16:54:02] <andypugh> Hmm.
[16:54:39] <cradek> as if you needed another reason
[16:56:15] <andypugh> I actually feel I _should_ stay with Ubuntu because Canonical are British.
[16:56:52] <cradek> oh is that why they default to the weird dictionary with all the extra vowels? :-)
[16:57:13] <andypugh> Yes.
[16:57:23] <cradek> but debian is from everywhere
[16:57:27] <cradek> be a citizen of the world
[16:57:59] <cradek> why do people say "we're all brothers" but not "we're all siblings"? people are weird.
[16:58:09] <andypugh> I am amazed how many little CAM companies are based in the UK
[16:58:25] <seb_kuzminsky> it'd be great is someone made snappy-clicky-whatever packages of linuxcnc for future ubuntus
[16:58:38] <seb_kuzminsky> *if
[16:58:40] <andypugh> (CAmBam, SheetCAM, Vectrix)
[16:59:02] <seb_kuzminsky> the freesteel guys are from brittain too, aren't they?
[16:59:47] <andypugh> Maybe. Rings a vague bell.
[17:00:08] <seb_kuzminsky> http://www.freesteel.co.uk
[17:01:31] <seb_kuzminsky> wow:
http://www.freesteel.co.uk/wpblog/2015/01/05/we-have-some-slicing/
[17:17:58] <andypugh> Ah, he’s one of us, you know:
http://www.linuxcnc.org/index.php/english/my-profile-usermenu-17/userprofile/goatchurch
[17:18:08] <cradek> woo I smell smart people
[17:18:34] <andypugh> Did you see my Slicer?
[17:18:49] <seb_kuzminsky> the dlp projector one? that was cool
[17:19:19] <andypugh> I still need to figure out how to convert 5th-order splines to polybeziers.
[17:27:23] <andypugh> cradek: Looking at the Freesteel blog they are Cavers with CUCC, which means they studied at Cambridge.
[17:33:43] <andypugh> Oh how I hate Groff/Troff
[17:36:56] <andypugh> man groff = Many pages that explain almost everything exept what the actual formatting codes are
[17:40:28] <cradek> I'm shocked there isn't one at
http://cheat-sheets.org
[17:43:41] <andypugh> I had to look at several web pages until I found one that reminded me that it is \fBmy word\fR to bold a few words and not a line. And this page suggests just starting a new line:
http://babbage.cs.qc.edu/courses/cs701/Handouts/man_pages.html
[18:23:37] <seb_kuzminsky> andypugh: i agree...
[18:24:54] <seb_kuzminsky> is this helpful to you?
http://git.linuxcnc.org/gitweb?p=linuxcnc.git;a=commitdiff;h=c7800455c8f8074149191b8fc343342f0fc8ec77;hp=20f52dd0e1d6f6f1fafec8b8aefe6df8e05fb224
[18:30:19] <andypugh> seb_kuzminsky: I don’t think so, I am trying to format the embedded documentation in .comp
[18:54:16] <jepler> if I had enough tuits, I'd rewrite halcompile from scratch
[18:54:21] <jepler> but my tuits remain severely depleted
[18:55:56] <jepler> andypugh: 'man 7 groff_man' (package groff on debian) is the thing I refer to for help with the manpage source format .. that and the things I think I know or remember.
[18:56:56] <jepler> and man 7 groff is stuff that is in groff independent of the 'man' package
[18:57:45] <jepler> so for instance, that's where \\fR / \\fB / \\fI are specified
[18:58:25] <jepler> though groff_man .IB / .BI / etc are probably better particularly since they don't have \\ in them
[18:59:03] <jepler> shrug, it's a markup; all markup sucks, but markup invented in the 70s sucks more
[18:59:07] <jepler> holy cow, don't google "roff", you get porn
[19:00:35] <andypugh> I have worked round it, but for some reason [16: ((personality & 0xF0) >> 8)] became [16: ((personality & 0xF0) > > 8)] in the C-code and the compiler got sad.
[19:19:00] <jepler> that's weird!
[19:19:15] <jepler> you might try writing / 16 or / 256
[19:19:20] <andypugh> I did :-)
[19:19:59] <andypugh> I am sure I have done exactly the same thing before, too.
[19:22:26] <jepler> andypugh:
http://emergent.unpythonic.net/files/sandbox/0001-WIP-halcompile-fix-parsing-of-and.patch
[19:23:18] <andypugh> Ah, so I can’t have done it before.
[19:24:00] <andypugh> Do you ever wish you had called it something other than “personality”? I know I do :-)
[19:24:36] <jepler> https://www.youtube.com/watch?v=UmCZ_xzZsFQ
[19:24:52] <andypugh> Thinking about it bldc just masks but doesn’t shift
[19:25:12] <jepler> andypugh: over and over and over again I'll be a fool for you
[19:25:42] <jepler> what more can I do?
[19:26:48] <andypugh> Halcompile is an inspired but of #define abuse. I am very impressed. And slightly appalled :-)
[19:27:21] <jepler> I'm frequently called "an inspired butt" so that's OK
[19:28:15] <andypugh> I do sometimes wonder if textual subsititution in the Python phase part might have been better?
[19:28:27] <jepler> the road not taken
[19:28:39] <andypugh> (first define “better”)
[19:32:50] <andypugh> infinite loops in realtime are such fun.
[19:33:22] <andypugh> (at least is’t a VM so easy to sort out)
[19:39:08] <andypugh> Confused:
[19:50:46] <andypugh> http://pastie.org/10112533
[19:55:25] <andypugh> Do I have to use strncomp to compare a single character in a modparam?
[20:00:04] <jepler> so you are expecting a parameter like sequence=a,s,d,f ?
[20:00:21] <jepler> so sequence[0] is a length-1 C string?
[20:01:46] <jepler> in that case, I'd expect after char *token = sequence[0]; that *token == 'a' is true
[20:02:41] <jepler> but I've just had a drink so my programming is worse than usual
[20:06:15] <mozmck> looked a little bit a "snappy", and it looks similar to proposals by the systemd guys, except probably not as intrusive.
[20:07:20] <andypugh> jepler:
http://pastie.org/10112551
[20:08:31] <andypugh> (I tried printing the %i format and compating to 115 and it stil failed the comparison)
[20:08:51] <andypugh> So I think perhaps I have forgotten C again.
[20:08:56] <jepler> Apr 24 17:44:33 ubuntu kernel: [ 2012.521704] invalid token in sequence must be S, R, T, F or C
[20:08:59] <jepler> Apr 24 17:44:33 ubuntu kernel: [ 2012.521706] *token = 4 pin = 0, num_in = 0, num_out = 1
[20:09:26] <jepler> it's complaining about the 4 and not the s, right?
[20:09:33] <jepler> Apr 24 17:44:33 ubuntu kernel: [ 2012.521704] invalid token in sequence must be S, R, T, F or C
[20:09:37] <jepler> Apr 24 17:44:33 ubuntu kernel: [ 2012.521706] *token = 4 pin = 0, num_in = 0, num_out = 1
[20:09:40] <jepler> pin = simple_strtol(token, NULL, 0);
[20:09:41] <andypugh> but it should have strtol the 4 after finding an s
[20:09:53] <jepler> don't you need to move 'token' forward after strtol?
[20:10:05] <jepler> you are expecting strtol to change its argument 'token"?
[20:10:18] <jepler> perhaps you need strtol(token, NULL, &token)
[20:11:00] <andypugh> Ah, thanks.
[20:11:06] <jepler> hmm simple_strtol won't build in uspace
[20:11:14] <andypugh> I had missed the printing order
[20:11:45] <jepler> and worse the kernel docs say 378 /* Obsolete, do not use. Use kstrto<foo> instead */
[20:12:00] <andypugh> when I have used simple_strtol before (a lot in hm2) it increments token
[20:12:57] <jepler> ok, we *do* have "simple_strtol" in uspace
[20:13:15] <jepler> and I do see there are statements like this in hostmot2:
[20:13:18] <jepler> src/hal/drivers/mesa-hostmot2/hostmot2.c: hm2->config.num_encoders = simple_strtol(token, NULL, 0);
[20:14:32] <jepler> but it looks like after each simple_strtol(token), it ends up back at the top of the loop
[20:14:36] <andypugh> I need to increment token _before_ simple_strtol
[20:14:37] <jepler> char *token = argv[i];
[20:14:59] <jepler> and gets the next argument, which has been split from config= on whitespace by rtapi_argv_split
[20:15:12] <jepler> pin = simple_strtol(token+1, NULL, &token)
[20:15:15] <jepler> maybe
[20:15:17] <andypugh> simple_strrol will see an “s” and quit
[20:16:00] <andypugh> ++token ?
[20:16:51] <andypugh> pin = simple_strtol(++token, NULL, 0) feels like it should pass-by value, somehow.
[20:17:02] <andypugh> But might actually be right.
[20:17:07] <jepler> I am reasonbly sure it doesn't
[20:17:35] <jepler> assuming that ..., &token *IS* necessary ...
[20:17:53] <jepler> I wouldn't write ++token because that reads like token is modified twice in the same statement
[20:18:31] <andypugh> Yes, I think I will just token++ before the strtol
[20:18:46] <andypugh> Simple, clear..
[20:18:49] <jepler> modifying a value twice in the same statement is in the category of forbidden things in C. As a concrete example, this is not OK in C: x += tab[i++] + tab[i++]
[20:19:46] <jepler> OK, if you want to use token++ then doing it as a separate statement makes me feel better
[20:22:35] <jepler> hm simple_strtol is deprecated and kstrtol does not have the same API
[20:22:42] <andypugh> That works now
[20:22:44] <jepler> we'll have to do something about that someday
[20:24:23] <andypugh> else if (*token == 'S' || *token++ == 's'){ would be an interesting bug with optimised interpretation of the ||
[20:24:41] <jepler> yup and yuck