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

using M70/72 can unexpectaly start the spindle on abort or error. #2764

Open
c-morley opened this issue Nov 28, 2023 · 18 comments
Open

using M70/72 can unexpectaly start the spindle on abort or error. #2764

c-morley opened this issue Nov 28, 2023 · 18 comments

Comments

@c-morley
Copy link
Collaborator

If a user starts the spindle in MDI then stops the spindle with manual buttons, then uses a subprogram (such as probing routines) that record/reset codes with M70/M72 The spindle will start on abort or error.

I assume manual control doesn't reset the M5 in the internal settings.
Explicitly setting M5 just before M70 will work around the problem.

https://forum.linuxcnc.org/qtvcp/50729-qtdragonhd-versa-probe-failed-probe-turns-on-spindle?start=10#286614

complaint was on 2.9. master is the same - never tested 2.8.

@andypugh
Copy link
Collaborator

Yikes! The one time you really don't want the spindle to start is when probing.

Is this an issue with the probe routine, or do we think that M70/M72 should never re-start the spindle?

@rmu75
Copy link
Contributor

rmu75 commented Nov 28, 2023

"spindle stop with manual buttons" just sends a stop command to emctask, right? state in interpreter is not changed? so maybe the button should instead send "M5"?

@rmu75
Copy link
Contributor

rmu75 commented Nov 28, 2023

btw, we really need a "max allowed rpm" field in the tool table, shouldn't be too hard to implement

@c-morley
Copy link
Collaborator Author

c-morley commented Nov 28, 2023

The problem is that manual stop and m5 are not equivalent in internal settings.

A really great feature would the ability to see what is being recorded and or ability to record different code with different options.

But for now just deciding how to make manual stop equivalent to m5 would be great.

To be frank I'm not sure why we enforce manual/mdi modes in the controller.

@c-morley
Copy link
Collaborator Author

c-morley commented Nov 28, 2023

In reality we mix them in the GUI

@c-morley
Copy link
Collaborator Author

Sorry missed a message....

The problem is manual stop and m5 are not equivalent in the internal settings.

I'm not sure why we still have manual/MDI modes in linuxcncs motion controller. We mix the modes in the GUI all the time.

@rmu75
Copy link
Contributor

rmu75 commented Nov 28, 2023

Main difference seems to be teleop/free mode in manual vs. coordinated in mdi.

@rmu75
Copy link
Contributor

rmu75 commented Nov 28, 2023

The problem with M70/M72 seems to be that interp internal "struct setup" is not in sync with status in emcmot. Don't know how to solve that -- why not make "external buttons" send "M5"?

@c-morley
Copy link
Collaborator Author

Because you can't send M5 in Manual mode, only in MDI. Also the fix needs to be deeper then the GUI code (if that is what you mean) otherwise the problem is still lurking.

@rmu75
Copy link
Contributor

rmu75 commented Nov 28, 2023

the problem is that emctask receives a NML command from the GUI to stop the spindle and instructs emcmot to do so. the interpreter has a separate status and doesn't see that command, so if it is told to restore state, in on_abort, it will restore to it's view of the world which has the spindle running.

either the gui button has to go through the interpreter or it has to reset the interpreter.

@rmu75
Copy link
Contributor

rmu75 commented Nov 28, 2023

maybe make the button send M5 if in MDI mode and NML message if in manual? but that sounds like a hack and would need consistent impl. in every GUI. not a good solution.

@andypugh
Copy link
Collaborator

andypugh commented Dec 8, 2023

I wonder if there is a way to make a spindle-stop NML command set the _setup[2] to stopped(5)? (and whether that would work)

https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/rs274ngc/interp_write.cc#L156C1-L157C63

  emz[2] = (settings->spindle_turning[0] == CANON_STOPPED) ? 5 :   /* 2 spindle     */
    (settings->spindle_turning[0] == CANON_CLOCKWISE) ? 3 : 4;

The workaround in the probe scripts is fairly easy, and should be done anyway. I doubt that many probe with a spinning spindle.

@c-morley
Copy link
Collaborator Author

c-morley commented Dec 8, 2023

Some probes require spinning spindles.
M70 is supposed to record the current state and it does not.
Maybe a better way to say this is the interpreter is not up to date with linuxcnc's actual state.

is there not a sync state command that MDI commands should call first or manual commands call after?

@samcoinc
Copy link
Collaborator

samcoinc commented Dec 8, 2023 via email

@c-morley
Copy link
Collaborator Author

c-morley commented Dec 8, 2023

in this case it's not a que problem. the que is empty after the initial (user typed in) mdi command to start the spindle.
It's the manual button to stop the spindle that changes the actual state of the machine but the interpreter doesn't see it.
When the MDI command (m70) comes, it records it as the spindle is not stopped.

@andypugh
Copy link
Collaborator

andypugh commented Dec 9, 2023

This seems like a difficult thing to solve in the general case. Even my idea above of writing to the interpreter state from motion won't always work if you consider pluggable interpreters.

I think that this should be documented in the M70 documentation as a limitation of the code.

I noticed yesterday that Chris has added an M5 to the script, and I hope to push out a 2.9 update in the next few days to distribute both this and the run-from-line fix. However that is dependent on working out how to fix the buildbot builds.

@new952036
Copy link

@aman952036

1 similar comment
@new952036
Copy link

@aman952036

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

No branches or pull requests

5 participants