classicrocker883 / MRiscoCProUI

This is optimized firmware for Voxelab Aquila & Ender3 V2/S1 3D printers.
https://classicrocker883.github.io/
Other
74 stars 17 forks source link

[BUG] LASER_SYNCHRONOUS_M106_M107 breaks fan control menu #85

Closed Nazar78 closed 6 months ago

Nazar78 commented 8 months ago

Did you test the latest release build?

Yes, and the problem still exists.

Bug Description

Compiling the firmware with LASER_SYNCHRONOUS_M106_M107 enabled breaks fan control menu. Changing the values in the fan in Control > Temperature menu doesn't change the actual fan speed nor it was reflected correctly in the display status. However the fan control and display continue to work normally with M106 from both the gcode or external commands like OctoPrint.

This issue is somewhat similar to https://github.com/MarlinFirmware/Marlin/issues/22815.

Printer Model

Voxelab Aquila

Model Type

X2

Your Mainboard

Aquila N32

Other Mainboard Type

No response

Add-ons that could be involved

BL-Touch

ProUI?

ProUI

Bed Leveling

BLT - BL Touch Bilinear mesh

Did you include your own configuration files?

Additional information & file uploads

No response

classicrocker883 commented 8 months ago

in lcd/e3v2/proui/dwin.cpp try changing this to

  void ApplyFanSpeed() { thermalManager.set_fan_speed(0, MenuData.Value); TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS));}

otherwise if that doesnt work, you can try

  void ApplyFanSpeed() { TERN(LASER_SYNCHRONOUS_M106_M107, 
      MString<20> cmd;
      cmd.setf(cmd, F("M106 S%i"),  MenuData.Value);
      queue.inject(&cmd);
      planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS);,
      thermalManager.set_fan_speed(0, MenuData.Value);)}

if one or the other works let me know. or if any other changes can be made.

Nazar78 commented 8 months ago

in lcd/e3v2/proui/dwin.cpp try changing this to

  void ApplyFanSpeed() { thermalManager.set_fan_speed(0, MenuData.Value); TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS));}

This works, thanks! I can now change the fan speed in the Control > Temperature menu.

However right at boot up, the physical fan speed stays at full blast although the display status states FAN=0. This with a laser module attached meaning it will also blast the laser at full power. My laser module doesn't boot in PWM mode, I need to activate it with a button, but it's still dangerous when the laser is already armed and at any unfortunate circumstances the printer firmware crashed and rebooted with full blast fan/beam.

I've tried inserting the TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS)) into the Marlin\src\lcd\marlinui.cpp with its included header #include "../../module/planner.h" however I'm lost at the error identifier "planner" is undefined.

Do you have any idea how to fix this too? I can always change back the speed to 1 then 0 in the menu after start up but this is clearly a bug.

classicrocker883 commented 8 months ago

can you post more about the error? like the log of it from terminal a simply copy past will do.

and where are you pasting that to marlinui.cpp?

inorder for "planner" to work you need to add something like Planner planner;

offhand at the moment not sure exactly but it's some Planner or planner something class or struct

However right at boot up, the physical fan speed stays at full blast although the display status states FAN=0.

was this happening before or after that change?

Nazar78 commented 8 months ago

Sorry for the confusion about the startup fan. The second issue doesn't exist in this fork, the fan is as expected not running at startup. I was using the Ender3V2S1 fork to test which appears to have this physical fan blasted at startup when using the CV_LASER_MODULE with N32G455RE. I'll have to figure out myself or open the issue in that fork or it would be helpful if you have any idea where the fan speed is applied or can be applied at startup. I've tried adding it here but it still doesn't work:

https://github.com/classicrocker883/MRiscoCProUI/blob/bdee95a11c872af9392cac7f71bbc56d1609fc5a/Marlin/src/module/temperature.cpp#L453

  + TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS));
    TERN_(REPORT_FAN_CHANGE, report_fan_speed(fan));

That said, the first issue is still valid for this fork and your advise on the fix works so you can include it in the next release.

classicrocker883 commented 8 months ago

to set fanspeed 0 at startup, just add this to void DWIN_InitScreen in dwin.cpp
thermalManager.set_fan_speed(0, 0);
TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS));

  #endif
  thermalManager.set_fan_speed(0, 0);
  TERN_(LASER_SYNCHRONOUS_M106_M107, planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS));
  LCD_MESSAGE(WELCOME_MSG);
}

like so - at the end of the function.
not sure if its LASER_SYNCHRONOUS_M106_M107 or CV_LASER_MODULE for TERN

to clarify the fixes, would you mind listing what should be changed? it'll help me know what is what.

```diff
-code before change = 'hello world!'
+code after change = 'hello your world!'
``` `

this is the syntax if you wish to use.

Nazar78 commented 8 months ago

to set fanspeed 0 at startup, just add this to void DWIN_InitScreen in dwin.cpp

I've tested this before, it still activated the fan full blast for few secs at boot then only turned off when the menu appears. This is enough to accidentally burn an indent on my PEI sheet with a 20 watt laser module.

The closest I can get to make the fan spin the least is in:

https://github.com/classicrocker883/MRiscoCProUI/blob/fe7f18c0d078842c2bcd464f522ad5fb2ac241c0/Marlin/src/MarlinCore.cpp#L1320C28-L1320C28

  SETUP_RUN(settings.first_load());   // Load data from EEPROM if available (or use defaults)
                                      // This also updates variables in the planner, elsewhere
+
+  #if ENABLED(LASER_SYNCHRONOUS_M106_M107)  // Zero fans speed at boot with LASER_SYNCHRONOUS_M106_M107
+    thermalManager.zero_fan_speeds();
+    planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS);
+  #endif

It would be be great though if it doesn't spin at all at startup. I'll try to dig further, thanks for the hints.

classicrocker883 commented 6 months ago
  SETUP_RUN(settings.first_load());   // Load data from EEPROM if available (or use defaults)
                                      // This also updates variables in the planner, elsewhere
+
+  #if ENABLED(LASER_SYNCHRONOUS_M106_M107)  // Zero fans speed at boot with LASER_SYNCHRONOUS_M106_M107
+    thermalManager.zero_fan_speeds();
+    planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS);
+  #endif

It would be be great though if it doesn't spin at all at startup. I'll try to dig further, thanks for the hints.

I dont know if set_fan_speed(0, 0); vs zero_fan_speeds(); makes a difference, but
I was able to add this to proui/dwin.cpp - line ~2100

void DWIN_InitScreen() {
...
- TERN_(LASER_SYNCHRONOUS_M106_M107, thermalManager.set_fan_speed(0, 0);
- planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS);)
+ #if ENABLED(LASER_SYNCHRONOUS_M106_M107)  // Zero fans speed at boot with LASER_SYNCHRONOUS_M106_M107
+  thermalManager.zero_fan_speeds();
+  planner.buffer_sync_block(BLOCK_BIT_SYNC_FANS);
+ #endif
  LCD_MESSAGE(WELCOME_MSG);
}

[!NOTE] the thing about adding to settings.cpp is iffy. anytime you either add or change the structure, and do not change the EEPROM_VERSION, it throws an error during startup. its more of a warning rather than impact anything, but its best to leave this file untouched as possible. I suppose changing some things is fine - things that are already there, but it does say // Change EEPROM version if the structure changes, so I believe adding things or taking other stuff away will have that result.

github-actions[bot] commented 2 months ago

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.