On 05/31/2016 03:54 PM, Sebastian Kuzminsky wrote: > On 05/31/2016 02:00 PM, John Morris wrote: >> I've enabled remap of the M62 thru M66 codes [1]. Is this of interest >> for merge into LinuxCNC? > > In my opinion: yes.
Great! Then in addition to the below, I went ahead and rounded out the patch by adding M67 and M68. > > >> Seb, I borrowed your 'statbuffer-g5x-abort' test for this. Proper >> testing of calling the original e.g. M64 from within the remapped M64 >> needs bath interp and HAL, plus the connecting machinery, so it was a >> timely example for me. Thanks! > > I see that you import hal, but i don't see you use it. You're verifying > the dout settings by looking in the stat buffer, which is probably fine, > and which *should* agree with Motion's digital-out hal pins. > > If you wanted to add that to the test, take a look at how (for example) > the io-startup test does it. I think this improvement is optional, > since that's not at the core of what the test is testing. I meant to do this before, but hadn't found the right example to crib from. ;) Done. > > >> The M66 test is also a good example of how to remap a code that returns >> INTERP_EXECUTE_WAIT. >> >> [1]: https://github.com/LinuxCNC/linuxcnc/commit/ffac8964 > > This is very cool. I very much appreciate your thorough testing and > bugfixing in this tricky area. > > > Looking at the history of ffac8964 i see a commit from Norbert > (d3820714) that's not in any mainline branch, but that also looks like > it doesn't belong with the remap work you're doing. There's a commit in > master (389bba5ff) that matches it. An off-by-one rebase perhaps? Yup. Fixed. > > I see that you brought back to life a commented-out call to > read_inputs() in Inter::synch(), which seems right to me (though i dont > know the details). Do you know why it was disabled? > > The `read_inputs()` call was moved back and forth between `Interp::synch()` and `Interp::_read()` in 424eb992 and earlier in e200945b. I didn't try to understand why, and lazily decided to enable it in `synch()`. That works, but makes lot more calls than necessary. Better is to call `read_inputs()` from `interp_python.cc` in `Interp::pycall()` before calling `generator_next()`. That ensures that if the generator function needs the result of the M66 after continuing from the `yield`, it's available. I added this case to the unit test, since users may need this in python remaps. Thanks for looking at this, Seb! John ------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e _______________________________________________ Emc-developers mailing list Emc-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/emc-developers
