Thanks for the reply. I figured that there will be a few issues, as this is not completed yet and I have not removed all the warnings yet. However, i did want to see your opinions before continuing further.
- For white spaces I use vim with expend tab and the auto indent. Do you guys have a vim config recommendations? - I do wish you would reconsider declaring variables at the smallest scope possible. This helps avoid problems in the future where erroneous values need to be debugged, since there are fewer number of statements where a variable can be touched. There are other benefits that you can get from here (http://lars-lab.jpl.nasa.gov/JPL_Coding_Standard_C.pdf page 13). However, from now own I will declare the variables at the top of the functions. - I should have removed all the printf functions, did I miss any (there were some in earlier commits before I read the HACKING file)? I will try to correct the rest of the issues within the next few days. Thanks again for the response. Lior. ________________________________ From: Uwe Hermann <[email protected]> To: Lior Elazary <[email protected]> Cc: "[email protected]" <[email protected]> Sent: Monday, January 7, 2013 6:37 PM Subject: Re: [sigrok-devel] link-mso19 drivers Hi, On Mon, Jan 07, 2013 at 08:16:50AM -0800, Lior Elazary wrote: > I have a patch for an initial working link-mso19 driver (generated with 'git > format-patch origin/master'). Awesome, thanks! > There is one small change to the serial driver to support flow control in the > config string, since this device uses xon/xoff. Yup, looks good, merged. Thanks! > The rest of the changes all protein to the link-mso19 driver (removed the old > link-mso19.* files and changes to api.c and protocol.c/h). Here are a few > things that still need to be addressed: Merged too, though this needed some fixes and will need a few more long-term. Here's a quick list of items to look out for in future changes. - The coding style / whitespace was completely off, please check HACKING for details and hints. As a bare minimum please use one tab for indentation, the rest is relatively minor stuff. Generally we use the Linux kernel coding style where applicable and where it makes sense (some parts are not really usable, but most of the stuff is). - Please try to avoid declaring variables in the middle of functions, we really want to avoid this style in libsigrok, it just generates additional work for us to remove it later. Declare all variables at the top of the functions, if possible. Thanks! - Don't use printf() for output, only sr_err(), sr_dbg() and so on. The permanently useful ones should also stay in the code (not be removed later), the user can control the log level / verbosity via the -l switch in sigrok-cli (for example). - Only put stuff into protocol.h which is absolutely needed in both api.c and protocol.c in general. Stuff that's only needed in protocol.c should be in that file only and marked static. - Please also check the remaining compiler warnings, I fixed a few of the simpler ones, but not yet all, feel free to check the rest; you probably know better which the correct fix for those would be. > 4) I tried to use pulse view, but I can not configure this device with the > gui (sigrok-cli works with no problems). Any ideas? Also pulse view does not > let you select 1024 samples, so for now I just added that ability. I tired > the other GUIs but they all don't work. Which gui at this point is the one > being developed for analog/digital and features? Bert will probably answer a few of your other questions in more detail, just a quick hint about this one: Currently the most usable GUI is PulseView for sure (though it's still in development, of course). It's also the only one for now which has initial analog signal support, the other GUIs are Logic-Analyzer only. Cheers, Uwe. -- http://hermann-uwe.de | http://sigrok.org http://randomprojects.org | http://unmaintained-free-software.org
------------------------------------------------------------------------------ Master Java SE, Java EE, Eclipse, Spring, Hibernate, JavaScript, jQuery and much more. Keep your Java skills current with LearnJavaNow - 200+ hours of step-by-step video tutorials by Java experts. SALE $49.99 this month only -- learn more at: http://p.sf.net/sfu/learnmore_122612
_______________________________________________ sigrok-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/sigrok-devel
