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] Laser turns on and cannot be turned off after gcode script is finished (Marlin 2.0.6) #20182

Closed
ryanaukes opened this issue Nov 17, 2020 · 13 comments

Comments

@ryanaukes
Copy link

Bug Description

I am trying to get the Marlin 2.0.6 inline laser control function to work for a while now. I found (possibly) a bug. I am using the Marlin Laser function and enabled inline laser control. When I run a script which ends with a M5 command, when the script ends, the laser turns of but then turns on again. After that, I cannot turn it off again with a M5 command, but I first have to turn it on again to be able to turn it off.

Configuration Files

Marlin.zip

Steps to Reproduce

  1. Download Marlin 2.0.6
  2. Uncomment '#define LASER_FEATURE' and '#define LASER_POWER_INLINE'
  3. Run a gcode file ending with a M5 command

Expected behavior:

The laser turns of after the gcode file has finished and stays off.

Actual behavior:

When the script ends, the laser turns of but then turns on again. After that, I cannot turn it off again with a M5 command, but I first have to turn it on again to be able to turn it off.

Additional Information

This is one of the code files I used:

smiley INLINE.zip

Thanks in advance,
Ryan

@thisiskeithb
Copy link
Member

Please download bugfix-2.0.x to test with the latest code and let us know if you're still having this issue.

@shitcreek
Copy link
Contributor

shitcreek commented Nov 19, 2020

This issue still exists in the latest bugfix
The problem is that LASER_POWER_INLINE affects the normal behavior of M3 and M5. It expects a follow up G move command to turn off the laser. Ending the file with M5 I does not turn off the laser as well. The work around is to end the file with a G0 or G28 if you are using LASER_MOVE_G0_OFF or LASER_MOVE_G28_OFF
There is also no need to start the file with an M3 as you have LASER_MOVE_POWER
My PR isn't able to fix this issue, but I've noted its trickiness.

@Zumili
Copy link

Zumili commented Nov 26, 2020

Hey guys, something new about this issue? Because I am facing the same problem right now with the latest bugfix version.

@shitcreek
Copy link
Contributor

@Zumili - this issue hasn't been resolved. But in the meantime, you can just enable LASER_MOVE_G0_OFF and/or LASER_MOVE_G28_OFF and end your file with a G28 or G0

@github-actions
Copy link

This issue has had no activity in the last 30 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 7 days.

@ryanaukes
Copy link
Author

@shitcreek Is there any progress on the inline laser draft you were working on? Is it still being worked on or will it be in the future?

I have been trying a lot of different configurations lately, but whatever I do there is something that is not completely how I like it. Now I am using inline laser control, but when engraving images it stutters. Can this be due to the planner? Or is it more likely that the controller can't handle the gcode? I am using LASER_POWER_INLINE_TRAPEZOID_CONT since I had the problem where I would get burnt spots at the end of lines, but when I turn off LASER_POWER_INLINE_TRAPEZOID_CONT, I still get the stuttering motion.

@shitcreek
Copy link
Contributor

@ryanaukes Sorry I've been busy with other work. There has been some new laser stuff that's been pushed, and there is also my PR that needs some polishing. I haven't experienced any stuttering. What's your setup like? Have you tried just adding a couple extra G0/G1 moves without laser to your end script?

@bugfixin
Copy link

bugfixin commented Jan 8, 2021

I'll throw my hat in the ring and say I'm experiencing the same issue with 2.0.7.2. I notice that the PWM pin goes low enough that my actual laser tube stops successfully firing, but the SPINDLE_LASER_ENA_PIN stays active (low) until I turn it on with M3 (S1) and back off with M5.

A simple G0 or G28 does not deactivate the SPINDLE_LASER_ENA_PIN - in fact, either of those actually make that pin active when it's already inactive.

I suppose it raises the question of what SPINDLE_LASER_ENA_PIN vs SPINDLE_LASER_PWM_PIN actually signify, and if a PWM of 0 should deactivate the laser or set it to min PWM?

Let me know if I should make another issue - this seemed like the same issue to me so I didn't want to flood more in.
Configs:
Marlin.zip

@bugfixin
Copy link

bugfixin commented Jan 9, 2021

I did some digging - it appears that stepper.cpp calls set_ocr_power() -

static inline void set_ocr_power(const uint8_t ocr) { power = ocr; set_ocr(ocr); }
this calls set_ocr(), which directly sets the SPINDLE_LASER_ENA_PIN active, regardless of the power it's being set to.

This is different from apply_power() within the spindle_laser.cpp which checks for a 0-power state and chooses to set SPINDLE_LASER_ENA_PIN based on that state.

M5 calls set_enabled() which in sequence calls apply_power()

cutter.set_enabled(false);
- but it seems that if the apply_power() function has already been called with 0 power, it will short-circuit due to its check that power is being changed (
if (opwr == last_power_applied) return;
), completely unaware that the ENA pin has been made active via the above set_ocr_power > set_ocr codepath above.

There are probably 100 different ways to fix this - does it make sense to remove the activation of the ENA pin within set_ocr? does it make sense to replace all of the set_ocr_power calls with calls to apply_power instead? does it make sense to add another zero-power-check to set_ocr so that it itself chooses how to handle (de)activation of the ENA pin?

I think my personal preference would be to remove set_ocr_power and push it all to apply_power, but that may have performance concerns within the stepper loop.

I'm new to the project so I don't want to go throwing out wild pull requests without some guidance on the vibe.

P.S. During this dive I've also noticed the stepper isr can call set_ocr_power even when SPINDLE_LASER_PWM is disabled (meaning set_ocr_power won't be declared in spindle_laser.h). This just makes me more inclined to think that set_ocr_power shouldn't be directly called by the stepper isr

bugfixin added a commit to bugfixin/Marlin that referenced this issue Feb 3, 2021
Changes the stepper loop to use the full apply_power function instead of set_ocr_power()
@github-actions
Copy link

github-actions bot commented Feb 9, 2021

This issue has had no activity in the last 30 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 7 days.

@bugfixin
Copy link

bugfixin commented Feb 9, 2021

This is still an issue - waiting on PR approval

@thisiskeithb
Copy link
Member

Since this bug is fixed in #22690, I'll close this issue.

@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 28, 2022
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

6 participants