On Thu, Nov 21, 2013 at 02:42:19PM +0100, Bence Kov??cs wrote: > Hello, > > We added support for a new RS485 Teach Pendant module for GM6-PCI card > driver. Also fixed bugs and improved documentation. > > I uploaded modifications to GITHUB: > https://github.com/BenceKovacs/linuxcnc-mirror/ > Branch of modification is: gm_teachpedant
Hi Bence, I have a few comments: I think your thread ordering is still not right: I believe it should be read inputs, then run motion's two threads, then write outputs. If you get rid of the -1s and just do the addfs in that order you'll be set. I see one code weirdness: you use hal_bit_t as a generic boolean type - usually these are just used for the "bit" type of HAL pin. Could you fix that minor thing? Also since release managers read mostly the commit logs when building the changelog for a release announcement, would you please remove the changelog from the source file and put it in the commit message? We don't really have file-level changelogs: that's a pre-version-control practice. Part of your commit message is "add errata to doc" but I don't see that in your commit. Also, a style comment: in the future it would be best if each git commit only did one thing; especially a bugfix commit should be separate from a commit that adds a new feature. One reason for this is that it might be appropriate to pull only the bugfix part into a stable release branch. Thanks for your contribution - after these little fixes I'll merge it for you. Chris ------------------------------------------------------------------------------ Shape the Mobile Experience: Free Subscription Software experts and developers: Be at the forefront of tech innovation. Intel(R) Software Adrenaline delivers strategic insight and game-changing conversations that shape the rapidly evolving mobile landscape. Sign up now. http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk _______________________________________________ Emc-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/emc-developers
