CR6Community / Marlin

This Marlin fork has the goal of cleaning-up the source code changes for the CR-6 so it can be merged upstream. We also want to extend the functionality to make it fully functional
GNU General Public License v3.0
474 stars 82 forks source link

CF6 is not enforcing MAXTEMP for any nozzle heater settings or gcodes like M303 #237

Closed Thinkersbluff closed 3 years ago

Thinkersbluff commented 3 years ago

Description

I have confirmed by testing that CF6 Final does not prevent users from specifying nozzle temperatures higher than MAXTEMP-HOTEND_OVERSHOOT.

I can not test whether it protects against out of limit temperatures for a Chamber heater or a Laser Cooler, since I don't have either, but I have confirmed that it does still prevent users from entering bed temperatures higher than MAXTEMP-BED_OVERSHOOT.

This issue may be an upstream bug introduced into Marlin bugfix2.0, I don't know, but since it creates a potential safety issue for CF6 users, I have flagged it here and have updated the Release Notes to incorporate a new Safety Warning and a Known Issues entry to document it.

NOTE: There may be a related error in the Configuration Files included with CF6-Final, since these lines (added to Configuration.h in (Marlin Issue#21273)[https://github.com/MarlinFirmware/Marlin/pull/21273]) are missing from those files:

"/**

  • Thermal Overshoot
  • During heatup (and printing) the temperature can often "overshoot" the target by many degrees
  • (especially before PID tuning). Setting the target temperature too close to MAXTEMP guarantees
  • a MAXTEMP shutdown! Use these values to forbid temperatures being set too close to MAXTEMP. /

    define HOTEND_OVERSHOOT 15 // (°C) Forbid temperatures over MAXTEMP - OVERSHOOT

    define BED_OVERSHOOT 10 // (°C) Forbid temperatures over MAXTEMP - OVERSHOOT"

I did try recompiling the BTT MB/Stock TFT firmware.bin after editing the Configuration.h to add the above lines, but that by itself has not fixed the bug.

Steps to Reproduce

  1. Select any UI function that allows specifying a nozzle temperature (e.g. Setup->Temperature->Nozzle)
  2. Enter 285
  3. Printer begins heating the nozzle.

Expected behavior:

Printer should NOT accept any nozzle temperature higher than 260. (275-15)

Actual behavior:

Printer allows entry of 285C and implements the command.

Additional Information

I first found this problem while using a CR6-SE running "CR6Comm-CF6-FINAL-btt-skr-cr6-with-stock-creality-tft-2021-03-27-15-50". I was not able to solve this problem by recompiling the Source Code.zip files after adding the missing lines to Configuration.h from (Marlin Issue#21273)[https://github.com/MarlinFirmware/Marlin/pull/21273]

M303 E0 S285 also initiates a nozzle PID, but it should cancel the M303 and produce a message saying PID Too High. M303 E-1 S120 does cancel the M303 and does produce a message saying PID Too High.

Ranney1 commented 3 years ago

I see in the Marlin documentation that this info is also used for thermal runaway. I've just tested the thermal runaway for hotend and bed. Both are working. So it is 'just' max temp safety. (i've set all max temps in FW to 50. It is indeed no problem to heat to 100 and stay there for 10 min. Maybe that we can edit DGUS to not allow input higher than 255 or something like that. That is the most common error i think.

Thinkersbluff commented 3 years ago

According to the Marlin documentation in Configuration.h, Marlin is supposed to turn off the nozzle heater if it reaches the MAXTEMP value.

"Setting the target temperature too close to MAXTEMP guarantees a MAXTEMP shutdown! Use these values to forbid temperatures being set too close to MAXTEMP."

Being able to sit at 100 for 10 minutes when MAXTEMP has been defined as 50 is definitely evidence that the MAXTEMP shut down is not working. Thinkyhead thought we would have unexpected shutdowns if we set that value to 260 & then commanded 260.

Ranney1 commented 3 years ago

I've got the idea that the thermistor error for the hotend (min temp error) is also not working. For the bed it is working. Can you check that @Thinkersbluff ?

tomearp commented 3 years ago

I've got the idea that the thermistor error for the hotend (min temp error) is also not working. For the bed it is working. Can you check that @Thinkersbluff ?

Just tested unplugging the thermistor (BTT SKR CR6 board, reports -15c for me with thermistor unplugged) with the hotend heated to 130c (target was set by tapping on the nozzle icon on the first screen and entering a value from there). After 60 seconds i was presented with the Thermal Runaway screen. I plugged the thermistor back in and power cycled the printer without delay. The hotend temperature had fallen to 67c, so even before the thermal runaway was triggered, the power had been removed from the hotend heater, as expected.

I also tested unplugging the thermistor for shorter periods of time, and each time i plugged it back in the hotend temperature had fallen, indicating that power is being removed as soon as the thermistor is seen as unplugged. Each time the thermistor was plugged back in the printer resumed reheating the nozzle to my 130c target.

Ranney1 commented 3 years ago

Thanks for testing! I saw that the thermal runaway is working. But if you unplugg the hotbed thermistor and set an temp, you trigger inmediatly an 'Thermistor error' by #define BED_MINTEMP 0 For the hotend it looks only the 'thermal runaway' is working and not the 'thermistor error' by #define HEATER_0_MINTEMP 5

What is your idea?

tomearp commented 3 years ago

If I unplug the thermistor and then set a target of 130c from the screen, after 60 seconds I get the Heating Failed screen. Judging by the nozzle temperature reported when I plug the thermistor back in and power cycle the printer, it made no attempt to heat during that time.

Ranney1 commented 3 years ago

Ok, so it looks like the thermal runaway is working for hotend and bed but the 'thermistor error' is only working for the bed. Right?

What I did: Disconnect the hotend and bed (thermistor and heaters)

  1. set hotend temp (current temp = -15) ==> Thermal runaway is triggered after set time out of range (a minute or so)
  2. set bed temp to 50 (current temp = -15) ==> Thermistor error is triggered direct (in a sec)
tomearp commented 3 years ago

Yes I agree with that, just tested it on mine.

Unplugging the bed thermistor and setting a target temperature triggers the Thermistor Error screen immediately. Hotend does not behave the same.

Tried some earlier versions of the CF, the issue starts at CF6 pre3

Thinkersbluff commented 3 years ago

UPDATE - Thanks to @tomearp and @Ranney1 for the additional testing.

Today, I compiled the latest Marlin bugfix2.0 for my Ender 3 (w/BTT SKR Turbo MB and BLT) and confirmed that the MAXTEMP protections against setting Nozzle Temp > 260 and against M303 E0 S261 ("Too High") both WORK on the Ender 3.

This suggests to me that the issue is somehow an issue with CF6, not an upstream Marlin bug.

Sebazzz commented 3 years ago

@Thinkersbluff Are you positive you are on latest bugfix-2.0.x?

It seems to me the change is caused by b35bfeb1c3f1271016227a1d79dcb552e67e3cb9, which is one of the commits leading up to pre3.

This default value is kept: image

_MINMAX_TEST(0, MAX) fails and therefore _TEMP_MAX_E(0); doesn't get expanded and compiled. The _MINMAX_TEST macro is looking for TEMP_SENSOR_0THERMISTOR_ID which doesn't exist. TEMP_SENSOR_0_THERMISTOR_ID does exist.

Sebazzz commented 3 years ago

Opened upstream issue: https://github.com/MarlinFirmware/Marlin/issues/21582

Thinkersbluff commented 3 years ago

@Thinkersbluff Are you positive you are on latest bugfix-2.0.x?

I was...

I have a BTT SKR mini Turbo motherboard, so I used the example configuration files. The example platformio.ini file had the wrong environment in it (mega2560 instead of LPC1769, so I had to edit that to compile. The Configuration.h file is also missing that new definition of the two overshoot values.

Now I see that the config files show "#define CONFIGURATION_ADV_H_VERSION 020007" instead of 020008, so I am baffled what went wrong with my download.

I am not seeing evidence of b35bfeb in the files, though.??

ritchiedc commented 3 years ago

The example files are not up to date. At least the adv for 4.5.2 still has the old single define for MEATPACK and not the newer two defines for two ports. In this case it's commented out so it doesn't cause problems but you have to be careful using them.

Sebazzz commented 3 years ago

It is regardless of the configuration files, it is an upstream issue.

Thinkersbluff commented 3 years ago

It is regardless of the configuration files, it is an upstream issue.

Yep. I figured that when recompiling with a corrected configuration.h did not fix it. I mentioned the configs issue in case it was related. Do you need me to open a separate issue on that?

Sebazzz commented 3 years ago

This has been fully fixed by 5ea0cf7303