On Sat, May 30, 2020 at 11:18 PM Gerhard Sittig <gerhard.sit...@gmx.net> wrote: > How many sigrok drivers have you created before that? This one
Just this one, if not counting changes/patch to scpi-dmm to add GW-Instek bench meter support... Got an idea earlier this week to hook up all various test equipment on my bench to RasberryPi, and that lead to Sigrok, and then that lead into pulling libsigrok sources to see how much work would it be to get my bench meter and electronic load supported... And here we are :) > Are models using different UART bitrates or frame parameters? If > these are default values, the driver could encode them, and users > need not specify them. Simplifies the invocation. But you might > as well have provided them for demonstration. No issue there. Units seem to have 9600/8n1 factory default (only bitrate is configurable). Driver has default setting so that don't have to always specify "serialcomm". (need to check that I set it to 9600 and not 38400 that I use...) Btw, is there convention how to only specify bitrate (speed) on command line? > This suggests that you have the commits in a git repo, and have > an account for a service. Instead of "hiding" the patches in a > gist, would it not be easier to push the commits to a public > repo? I didn't really have a git repo (I had "read-only" clone from of the official repo), but I have now created one: https://github.com/tjko/libsigrok.git > Things that come to mind: > - Does your driver *depend* on the serial transport in the build > instructions? So that it automatically gets deselected when an > essential dependency is missing, and won't break the build. I added "serial_comm" as dependency in configure.ac, that's what you meant, right? > - You need not re-invent endianess conversion. Macros like RL16() > have been around for ages, recently read_u16le() got added. Thanks, I'll take a look and use those. > - Check whitespace, sometimes operands and operators "run into > each other", which is harder to read. > - Check data types, use of "char" where bytes get processed is > unexpected. Prefer C99 uint8_t etc types over "unsigned char" > stuff to even better reflect intent? Thanks, I'll do some cleanup. > - Check text line length, and indentation depth. There may be > different styles preferred by different persons. Let's hear > what others say. I personally like to see the structure first > and not indent continuation lines _that_ deep. That's Emacs (learned to use Emacs in late 80's, and it's hard to change habits :) I noticed Debian (Rasbian) finally had "linux kernel" mode included by default, but seems like its still kind of "broken".... I don't suppose there published Emacs settings/config that produces layout preferred by the project? > - Check portability. Do you fill in structs and then send _these_ > to the wire? Might be convenient but need not work everywhere. > It's probably better to accept the tedium and properly stream > requests and responses for improved reliability. Notice that > the tedium is less with the recently introduced conversion > helpers which also advance their position in the stream. Code should be portable (?) I'm aware of dangers different cpu architectures, but yes I'm "cheating" little bit by using a struct, but the struct is (on purpose) just an array of bytes. This allows referencing positions in the buffer conveniently... I can't think of portability issues with this approach, or am I missing something? Do you have reference/link to documentation on these conversion helpers? (those sound interesting) > Are you aware of a wiki page for this load? Want to update or > create one? I can't promise that I have time, but if you create entry under sigrok wiki for these, I could be tempted to make some updates... -- Timo <t...@iki.fi> _______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
