Hi Chris, Thank you for your review and comments! We have made the suggested modifications a placed it to the same place: > https://github.com/BenceKovacs/linuxcnc-mirror/ > Branch of modification is: gm_teachpedant
We will place one modification in one commit in the future. We are quite new to LinuxCNC source, so any comment or recommendation is welcome! Do you have info about next Live CD? Thanks for your work again, Bence 2013/11/21 Chris Radek <[email protected]> > 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 > ------------------------------------------------------------------------------ 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
