Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove iocontrol, and move its functionality into task. #2497

Merged
merged 10 commits into from
Sep 26, 2023

Conversation

rene-dev
Copy link
Member

This is an experimental branch I'm working on, eventually leading to a much better tooltable. The idea of this particular PR is to move everything that IO does directly into task.
The reason for IO is historic, and predates HAL. All iocontrol does is set hal pins, and do some tool logic.
There is no reason I can see to have a separate process, it is all fast and non blocking.
This is a lot cleaner, faster(iocontrol is slow, everything always has to wait for IO), more maintainable,
and removes 3 out of 6 NML channels. There is a lot more NML related cleanup to do, all the structs are still there.
Moving 1150 lines of iocontrol to taskclass code only grows taskclass by 200 lines.

also closes #2467

it passes all the tests, should not change any functionality, and does not require any configuration changes.

@SebKuzminsky
Copy link
Collaborator

I have not read this PR yet but I am in favor of the overall idea of deleting IO and moving IO's work into Task.

@andypugh
Copy link
Collaborator

One possible reason to keep IO as a separate module is if there is a wish to change the way that something is handled.
We have, for example, iov2 which can be chosen in the INI and does some slightly different things (mainly handling toolchanger aborts better, I believe)
For example, could an alternative iocontrol change the tool change behaviour so that T is all that is needed (lathe style) so that a single INI change could configure this rather than requiring a complex (and finicky) remapping config.

@SebKuzminsky
Copy link
Collaborator

Hmm, good point...
Though i think i'd prefer a single best-of-breed implementation (inside Task or inside IO) with configuration knobs, rather than a separate implementation for these different behaviors.

@rene-dev
Copy link
Member Author

rene-dev commented May 19, 2023

all that isn't handled in IO, and can't be done in IO. if you want to to do that, the best place is a remap(there is a lathe style remap example).
IOv2 only adds faults. I dont think its very useful for the reasons below, but can be added easily to this branch.
there is taskmodule, but its broken and abandoned(this branch doesnt affect taskmodule). Its also not very useful, because it doesn't let you change any actual behavior of what is going on, and requires deep knowledge of task.
remap is the better way, and the way every other control does it.
look at this comment:
https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/task/emccanon.cc#L2948-L2954
most machines require movement for a toolchange, so it cannot be done in IO/HAL.
I think there should be only one well supported and working way of implementing toolchanges.
I believe anything you can do in IO you can do in a remap much easier.
unlike remaps, taskmodule or IOv2 never took off.

@rene-dev
Copy link
Member Author

the lathe style toolchange config is here, and still works fine in this branch.
https://github.com/LinuxCNC/linuxcnc/tree/master/configs/sim/axis/lathe-fanucy

@hansu
Copy link
Member

hansu commented May 20, 2023

There is no reason I can see to have a separate process, it is all fast and non blocking.
This is a lot cleaner, faster(iocontrol is slow, everything always has to wait for IO), more maintainable,

Sounds good to be able to run with a process less 👍

@andypugh
Copy link
Collaborator

the lathe style toolchange config is here, and still works fine in this branch.
https://github.com/LinuxCNC/linuxcnc/tree/master/configs/sim/axis/lathe-fanucy

Yes, but, that's a remap. And those are fussy to get working. (for normal users)

I don't actually know if switchable IO modules are any sort of solution to this class of problems. (and at this point you probably do)

@rene-dev
Copy link
Member Author

Yes, but, that's a remap. And those are fussy to get working. (for normal users)

I agree, and I dislike that fact very much. Remaps are such an important feature. I have seen users moving
to other controllers just because toolchangers are so hard in linuxcnc. We probably all agree, that a few lines of gcode macro are easier to understand to the average user than 1000+ lines of c code. I would love to fix the fussy parts about remaps, and make toolchangers work properly, and easier to understand.
imho There should be one working, documented, and supported way of doing things.
example: currently Im aware of 4 different ways of accessing the tool number from the change macro. why.

I don't actually know if switchable IO modules are any sort of solution to this class of problems. (and at this point you probably do)

It is not. Linuxcnc had modular and python extendable! IO for 10+ years, and I'm not aware of even a single example making use of it.

@SebKuzminsky
Copy link
Collaborator

2fec4a1 is my favorite commit ever.

@forrestg
Copy link

For users is irelevant that IO control executes any main task or IOcontrol, IOcontrolv2. If I writing tool change I only need any understandable interface.

For example
output hal pin - indicate start of change
u32 - tool number to change
u32 - next tool number
input hal pin - indicate end of change

typical user write gcode macro and for complicated tool changer is classic ladder and hal.
If iocontrol will be at task and working with tool table will be easier it helps me lot, becouse I think I must rewrite all tooltable interface becouse tooltable structure havent index. Insteed of using pocket number. Bad idea.

@rene-dev rene-dev mentioned this pull request Sep 25, 2023
@rene-dev
Copy link
Member Author

there is one issue Im working on while aborting a tool change. I will fix that, and rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

race between EMC_AUX_ESTOP_OFF and EMC_TOOL_ABORT
7 participants