On 12/19/2014 04:02 PM, Andrew wrote:
> 2014-11-20 21:23 GMT+02:00 Sebastian Kuzminsky <[email protected]>:
>>
>>
>> If you prepare your work as a git branch (or a patch file) against the
>> linuxcnc "master" branch and send it to the list, we'll review it for you.
>>
>>
> Please review the attached patch. I'd also like to update sim-hexapod
> config if this goes OK.
> Thanks!
Looks pretty good to me!
Your patch applies cleanly to master, builds, the
sim/axis/vismach/hexapod-sim config runs, and the new hal pins show up.
I didn't try changing the hal pins, because i dont really know how
they're supposed to work.... This is awesome.
Some feedback on the patch:
* haldata does not need to be dynamically allocated, it would be simpler
to make it statically allocated. The field a of the struct, the pins
themselves, would still be dynamically allocated by hal_pin_new.
* In the definition of struct haldata, you have a number of fields that
are arrays of 6, like this:
hal_float_t *basex[6];
It would be more clear what's going on if they were arrays of
NUM_STRUTS, like this:
hal_float_t *basex[NUM_STRUTS];
* The haldata fields base[xyz] and platform[xyz] are all initialized
twice in rtapi_app_main(): once to 0 immediately after
hal_pin_float_newf(), then again to the DEFAULT_* values from the header
file after all the pins are created. Probably the first one should just
be removed.
* The genhexkins_init() function isnt really an init function, it might be
clearer to call it genhexkins_read_hal_pins().
* There are some inconsistencies in indentation. Two places that i
noticed are in genhexkins_init() and rtapi_app_main().
* The manpage should be updated.
Nice work, this is a good improvement. Thanks for working on it!
--
Sebastian Kuzminsky
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Emc-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/emc-developers
