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

[BUG] M112 does not shut down motors #14863

Closed
maukcc opened this issue Aug 6, 2019 · 36 comments
Closed

[BUG] M112 does not shut down motors #14863

maukcc opened this issue Aug 6, 2019 · 36 comments

Comments

@maukcc
Copy link
Contributor

maukcc commented Aug 6, 2019

The M112 command stops heaters and movement.
It should also shut down the motors (like M84).
In current state stepper motors remain in "brake" mode, thus not resulting in a safe state of the machine.

@AnHardt
Copy link
Contributor

AnHardt commented Aug 6, 2019

What's unsafe with "braked" motors?

@InsanityAutomation
Copy link
Contributor

What's unsafe with "braked" motors?

If the M112 was triggered by say an estop button because someone got a hand (or beard) caught in a belt or pinched by a motor, that would leave them still stuck.

Common modality in industrial machine design. Zero energy in estopped state.

@AnHardt
Copy link
Contributor

AnHardt commented Aug 6, 2019

Feels wrong for a self-sagging z-axis.

@InsanityAutomation
Copy link
Contributor

Feels wrong for a self-sagging z-axis.

Proper design would be either proper gear reduction to prevent dropping or a rod lock device however on the scale of printers that may get to be cost prohibitive. In my opinion the Estop state should be as close to yanking the cord from the wall as we can get.

@maukcc
Copy link
Contributor Author

maukcc commented Aug 9, 2019

As emergency stop, code M112 needs to set everything to a zero energy state. It is not something that we would like to have, it is regulation.
For people with a self-sagging z-axis, we would suggest to use M410. As that probably will not be a machine that needs to comply to any regulations anyway :)

@InsanityAutomation
Copy link
Contributor

Fyi this behavior has been changed. Please test the latest snapshot and close this once you can confirm it performs as expected.

@maukcc
Copy link
Contributor Author

maukcc commented Sep 16, 2019

Thanks for the update.
In the mean time we have moved away from Marlin because of #13559
When that basic functionality gets fixed we will give it a try

@maukcc
Copy link
Contributor Author

maukcc commented Sep 23, 2019

tested with bugfix 2.0
motors are still in brake mode after m112

@boelle
Copy link
Contributor

boelle commented Oct 18, 2019

@InsanityAutomation i just tested with todays copy of marlin

M112 does do the kill but the stepper will whine and not be released like an M84 do

@boelle boelle changed the title [BUG] M112 does not shut down motors [BUG] [Bugfix 2.0.x] M112 does not shut down motors Nov 24, 2019
@boelle boelle changed the title [BUG] [Bugfix 2.0.x] M112 does not shut down motors [BUG] M112 does not shut down motors Dec 4, 2019
@boelle
Copy link
Contributor

boelle commented Dec 7, 2019

@maukcc since 2.0 was just released a few days ago has this changed this issue at all?

@maukcc
Copy link
Contributor Author

maukcc commented Dec 9, 2019

Tested with 2.0
It seems as if none of the emergency parser commands (M108, M112, M410), send from host, do anything.
There is also no feedback comming back from the printer with M108 and M410. this would be helpfull.

@boelle
Copy link
Contributor

boelle commented Jan 17, 2020

@maukcc @Vertabreak just did some test and the only thing that does not work as the wiki says are steppers, those are not disabled but everything else is

he will look at code and maybe comment on this later

@boelle boelle mentioned this issue Jan 17, 2020
@boelle
Copy link
Contributor

boelle commented Jan 17, 2020

#16592

@boelle
Copy link
Contributor

boelle commented Jan 20, 2020

@maukcc this was fixed ~7 hours ago, please retest with a new copy of bugfix 2.0.x

@maukcc
Copy link
Contributor Author

maukcc commented Jan 20, 2020

Still present in bugfix 2.0.x

@Vertabreak
Copy link
Contributor

is emergency parser on or off it can change how m112 works.

@maukcc
Copy link
Contributor Author

maukcc commented Jan 20, 2020

emergency parser is on and we do not think the emergency parser is working as it still executes all commands in buffer after a M112 is send to it.

@Vertabreak
Copy link
Contributor

Vertabreak commented Jan 20, 2020

the PR i submitted it functioned in all my tests but that is not what was done unsure if i should resubmit or try to discuss it at this point.
@thinkyhead maybe you have something to add?

@boelle
Copy link
Contributor

boelle commented Jan 30, 2020

a lot of updates and fixing has happend in the last week, is the problem still there?

@maukcc
Copy link
Contributor Author

maukcc commented Jan 31, 2020

with todays bugfix version the issue still persists

@sjasonsmith
Copy link
Contributor

@maukcc What control board are you using? I thought I had seen an issue with the emergency parser as well (not just M112), but haven't gone back to isolate the problem. It worked on my AVR board but not my LPC1768 board.

@sjasonsmith
Copy link
Contributor

You can ignore that last comment, the emergency parser is working on my LPC just fine.

It looks like one of the possible M112 code paths wasn't updated in the previous fix. Currently M112 doesn't kill the steppers if EMERGENCY_PARSER is enabled. I'm posting a PR to fix it right now.

@maukcc
Copy link
Contributor Author

maukcc commented Feb 5, 2020

With todays bugfix-2, this still is not fixed
we use an arduino due with these settings:
Configuration.zip
compiled with arduino ide 1.8.10

@MarlinFirmware MarlinFirmware deleted a comment from boelle Feb 7, 2020
@thinkyhead thinkyhead reopened this Feb 7, 2020
@thinkyhead
Copy link
Member

@maukcc — Re-flash and try again. This change tells kill to disable steppers.

    if (emergency_parser.killed_by_M112) kill(M112_KILL_STR, nullptr, true);

@maukcc
Copy link
Contributor Author

maukcc commented Feb 7, 2020

So you want me to do the same thing as 2 days ago, and expect different results? That would make me insane :) Ok, I will try, as that is right up my alley.
Tested......results are the same :)
Does kill actually disable steppers, or does it set enable pin to a certain state?(which isn't our disabled state)

also, the #define DEFAULT_STEPPER_DEACTIVE_TIME 120 function works after a M112

@sjasonsmith
Copy link
Contributor

@maukcc does it work if you disable EMERGENCY_PARSER?

I tested that it was working before posting that last fix, but I was on an LPC1768, not a DUE.

I can retest it using the current bf2 in my printer, but not until much later today.

@InsanityAutomation
Copy link
Contributor

Calling kill a flag is used to determine if steppers are killed. For killed by M112 processing from emergency parser or from the gcode M112 variant its sent as true. It should hit the same underlying code as M84 by the end of the trail. I dont have a Due to test unfortunately.

@sjasonsmith
Copy link
Contributor

I have a DUE but no way to connect drivers to it. I could use it and just probe the enable pins if I need to.

@maukcc
Copy link
Contributor Author

maukcc commented Feb 7, 2020

@sjasonsmith : with //#define EMERGENCY_PARSER it works!! Which is strange.

@InsanityAutomation
Copy link
Contributor

So its a due specific emergency parser issue apparently... Thanks, that helps narrow it down.
Not really strange as they execute totally different code paths.

@AnHardt
Copy link
Contributor

AnHardt commented Feb 7, 2020

Consider the DUE is special.
There are two possible paths for the host-Marlin communication - the nativ-cpu-USB-way and the native-cpu-UART->externel-USB-chip way.
I'd guess EMERGENCY_PARSER will work with the UART but not with the nativ-USB port.

@InsanityAutomation
Copy link
Contributor

@AnHardt there is explicit code for both interfaces present in the hal. Unfortunately Marcio would likely be the only one with extensive experience on this and given the state of lulzbot right now and that he is working a full-time job outside of this industry currently his availability to answer that will be quite limited.... I'll forward this to him and see if he has any input.

@sjasonsmith
Copy link
Contributor

I just verified that it is still working on an LPC1768 with EMERGENCY_PARSER, so it seems there is a DUE-specific issue.

@ellensp
Copy link
Contributor

ellensp commented Mar 21, 2020

I did some testing on this today, just with a standard DUE board.
Using current bugfix 2.0.x, with EMERGENCY_PARSER and SERIAL_PORT -1, SERIAL_PORT_2 0
With using virtual serial over USB (native USB) M112 works.
Using Serial 0 via usb/serial chip (programming interface) M112 is ignored.

It looks like there is an issue in the DUE hardware Serial implementation.

@sjasonsmith
Copy link
Contributor

I'm closing this with the assumption it has been fixed, since there have been no comments since the fix was merged nearly 3 months ago.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants