machinekit-hal
machinekit-hal copied to clipboard
machinekit
remove params in hostmot2 and other components/drivers
I recently made a python only configuration with a mesa 5i20 card and ran into the problem that the cython bindings don't support params, and for setting up stepgen i needed to change params.
I gave a quick try at changing them to pins, and my immediate problem is solved. However, it would be good to have these params removed as to better work with setting up a configuration with python only.
I ran into some problem with the table, and since that was part of my problem I reverted only these back to params for the time being, to be solved when time permits. https://github.com/luminize/machinekit/commit/03621c6e3427178cc8f0de165bbbcc4a8579d490 https://github.com/luminize/machinekit/commit/b1b4a1371987b5ccc9478a9aa92e5d2ed5d7be27
since there could be more components which use params, this issue could gather them and provide some examples on how to change params to pins (variable to pointer and array to pointer array).
In the yet unmerged HAL work on components etc, all support for params in instantiable components has been removed. Even in the current .icomps, params have been replaced by IO pins eg https://github.com/machinekit/machinekit/blob/master/src/hal/i_components/thc.icomp#L77
As I understand it params came into being, to sidestep the fact that pins could not be read/write at some early stage, this does not pertain now and having all values held in pins removes another layer of unnecessary complexity in HAL
Ok, that's good to know. That leaves us with drivers. I didn't have the courage to look at the corresponding files (hostmot2.h and stepgen.c) in LinuxCNC to see if changes needed to be brought over to Machinekit (very out of my comfort zone).
@ArcEye the reason was really default value semantics of pins during linking - we did a change to the HAL pin linking semantics (still lcnc) like so:
- a pin can have a default value
- linking a signal to the first OUT or IO pin will make the signal inherit the pin's default value
- unlinking a OUT or IO pin would leave the pin with the last value the signal had
this made params superfluous since there is no functional advantage to them anymore, plus they cannot be linked
@mhaberler has already converted stepgen into an instantiated component in the new branch
a pin can have a default value
That rings a bell now;)
@ArcEye the stepgen I was referring to is the mesa hardware stepgen. That needs info on step length for example, and those were params.
it is very hard to find any actual change in all this whitespace diff noise
anyway, for example:
// No point coming back unless something changes
- inst->written_step_type = inst->hal.pin.step_type;
- inst->hal.pin.table[4] = (((inst->hal.pin.table[0] ^ inst->hal.pin.table[1])
- ^ inst->hal.pin.table[2]) ^ inst->hal.pin.table[3]);
+ inst->written_step_type = *inst->hal.pin.step_type;
+ inst->hal.param.table[4] = (((inst->hal.param.table[0] ^ inst->hal.param.table[1])
+ ^ inst->hal.param.table[2]) ^ inst->hal.param.table[3]);
(while I do not understand the change from pin to param):
a pin is a pointer to a value a param is a value
therefore mass-changing param to pin will break the value semantics - you need to deref a pin, you need not deref a param
(while I do not understand the change from pin to param):
It originally was a param, so I mass changed the params to pins in the first commit. then I dereferenced the pins in the second commit (fix my initial mistake) but I couldn't get the dereferencing of the the pointer array members (the table) going (just my knowledge hiatus). Since I needed to read up on that and had no need to set these params with python I decided to leave these as params until I knew how to properly do the dereferencing :)
it is very hard to find any actual change in all this whitespace diff noise
Yes, you're right, I need to fix that. Not succeeded yet.
https://github.com/machinekit/machinekit/pull/1020 takes care of hm2/stepgen
I will look if this is still needed, since some changes have been made in the past. I know mhaberler made some changes regarding the stepgen, encoder and muxed encoder. Not sure if something is left. This means going thru the code in search for params which have been left over
Hopefully not too many
But note @machinekoder 's PR #1344
It failed a couple of times, but the last one was just on runtests, which I fixed and sent him a PR for If he refreshes the PR it should now work and take care of that one