Enabling LASER_POWER_INLINE breaks M3 S gcodes

[descipher]

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

When setting #define LASER_POWER_INLINE any cutter M3 S command no longer sets laser power.

Specifically when the INLINE code does not parse moves with an inline parameter the cutter.unitPower global is set to 0. This action prevents any M3 standard power settings.

M3-M5.cpp
calls inline_disable() when no I or O parameter is matched.

// Force disengage planner power control
static inline void inline_disable() {
  isReady = false;
  **unitPower = 0;**
  planner.laser_inline.status.isPlanned = false;
  planner.laser_inline.status.isEnabled = false;
  planner.laser_inline.power = 0;
}

Bug Timeline

Unknown, likely since the INLINE feature was added.

Expected behavior

Power control should be available for non inline laser/cutter gcodes.

Actual behavior

Laser does not power on with any basic M3 gcode e.g. M3 S80.

Steps to Reproduce

#define LASER_POWER_INLINE
issue any M3 Snn

Version of Marlin Firmware

bugfix-2.0.x (2.0.9.0)

Printer model

Custom

Electronics

Mega2560/Ramps1.4

Add-ons

No response

Your Slicer

No response

Host Software

No response

Additional information & file uploads

No response

[descipher]

Configuration.zip

[descipher]

Checking deeper, unitPower is not the reason for this issue.
Any G1 move will kill a previous M3 S80 command while laser_inline.status.isPlanned is set true.
To reproduce the following can be used

M3 S80
G1 X5 <- this will shut the laser off

planner.cpp Planner::_populate_block
..
// Update block laser power
#if ENABLED(LASER_POWER_INLINE)
laser_inline.status.isPlanned = true; <- commenting out this line stops the behavior
block->laser.status = laser_inline.status;
block->laser.power = laser_inline.power;
#endif

[descipher]

This feature #define LASER_POWER_INLINE_TRAPEZOID is what sets the laser to 0 on any subsequent G1 move.
I was under the impression that we should only have inline mode when the parser sees gcodes with an I command.
Inline is always on in this case when defined. With Trap enabled it uses secondary laser power variables which do not match the M3 Snn setting even when no I command was parsed.

Digging further.

[descipher]

This issue has been brought up in #18965.
When issuing:"
M3 S100
G1 X5
The M3 S100 power should not be disabled when a move is issued unless the laser is in inline mode.
At no point was this set. I am closer to solving this.

The main problem is that inline is set to always on in the planner code by doing a laser_inline.status.isPlanned=true in every loop. The isPlanned flag grants control with planner movement and laser power. The second issue is the set_inline_enabled(bool) function does not set the isPlanned flag which appears to be the correct way to set it.

Digging further.

[descipher]

One can certainly see why there are issues here:
cutter.set_inline_enabled(false); // Laser power in inline mode

The function set_inline_enabled(false) enables inline mode? This is so misleading! I will fix this madness :-/

[descipher]

Ok, I have a solution, needs more testing, I will gen a PR and link it to this issue.

Basically by setting planner.laser_inline.status.isPlanned as true or false within the M3,M4 and M5 gcodes it resolved the G1..G5 moves disabling the basic M3-5 mode. There are two redundant inline disable functions, will remove the duplicate and setup set_inline_enabled(bool) as the control call.

[descipher]

There are clearly incomplete functions within the inline laser power features as M4 does not appear to be implemented. Yet it is commented in the headers:

/**
 * Laser:
 *  M3 - Laser ON/Power (Ramped power) 
 *  M4 - Laser ON/Power (Continuous power) <- This is not functional in the current code.
 *

It would greatly benefit Marlin to implement M4 as a dynamic mode like the more popular GRBL 1.1 laser mode code.
I will look at it and see if that's an option here.

Also I see no real benefit to the use of M3 Onnn I to activate PWM on the output and based on the available software e.g. Lightburn, LaserWEB, LaserGRBL etc. it is not supported by any of them. PWM is actually set with Snnn so why the extra function was created is not clear to me. The M3 Onnn code also does not work with CUTTER_POWER_UNIT display functions of PERCENT or RPM. It only functions for PWM255 and requires work arounds to display a non PWM value.

Does any one have an issue with deprecation of the M3 Onnn SCODE?

[descipher]

@thinkyhead do you have any info/positioning on the laser inline functions?

[Fusseldieb]

Do you think fixing this issue should fix mine, too?

Eventually just calling inline_disable() should temporarily fix mine, no?

[descipher]

Yes it will. That change is in the soon to be issued in a PR which will address the inline and standard mode conflicts. This will not address the pause problem, that's more involved as there are too many ways one can pause the system. I may possibly use the marlin state function global to address the pause problem. I have not decided yet.

[thinkyhead]

Our friend @shitcreek may have some insights and some familiarity with recent laser code updates, plus may even have a laser to test with. I do not feel confident about my qualifications to touch this code.

[shitcreek]

@descipher @Fusseldieb - take a look at my PR: #20202
Recently moved, and so the new workshop is still in the works.

[descipher]

I have read that PR, it will not merge without some conflict updates. Could move some of the trap improvement functions over to a PR that addresses the incompatibility and flow safety control with inline vs standard mode. However I don’t agree with the M3 delay work around though.

[descipher]

I am looking at a way to define a relative power factor based on current stepper movement speed. This would allow dynamic power and a different way to handle the acceleration to power relation.

[Fusseldieb]

@descipher @Fusseldieb - take a look at my PR: #20202
Recently moved, and so the new workshop is still in the works.

Should I try to turn off the chiller and see if it turns off the laser, using this PR?

[github-actions]

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

[github-actions]

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.