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

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

Closed
Thinkersbluff opened this issue Apr 6, 2021 · 16 comments
Labels
area:dwin-interop Area - DWIN interop t:bug Something isn't working

Comments

@Thinkersbluff
Copy link

Thinkersbluff commented Apr 6, 2021

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/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/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
Copy link

Ranney1 commented Apr 6, 2021

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.

@Sebazzz Sebazzz added area:dwin-interop Area - DWIN interop t:bug Something isn't working labels Apr 7, 2021
@Thinkersbluff
Copy link
Author

Thinkersbluff commented Apr 8, 2021

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
Copy link

Ranney1 commented Apr 8, 2021

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
Copy link

tomearp commented Apr 8, 2021

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
Copy link

Ranney1 commented Apr 8, 2021

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
Copy link

tomearp commented Apr 8, 2021

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
Copy link

Ranney1 commented Apr 8, 2021

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
Copy link

tomearp commented Apr 8, 2021

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
Copy link
Author

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
Copy link
Collaborator

Sebazzz commented Apr 10, 2021

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

It seems to me the change is caused by b35bfeb, 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
Copy link
Collaborator

Sebazzz commented Apr 10, 2021

Opened upstream issue: MarlinFirmware#21582

@Thinkersbluff
Copy link
Author

@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.

  • The files inside the .zip file are timestamped "2021-04-07 1:44pm" .
  • The .zip file itself is timestamped "2021-04-08 4:41pm"

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

@ritchiedc
Copy link

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 added a commit that referenced this issue Apr 11, 2021
@Sebazzz
Copy link
Collaborator

Sebazzz commented Apr 11, 2021

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

@Thinkersbluff
Copy link
Author

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
Copy link
Collaborator

Sebazzz commented Apr 13, 2021

This has been fully fixed by 5ea0cf7

@Sebazzz Sebazzz closed this as completed Apr 13, 2021
Sebazzz added a commit that referenced this issue Apr 25, 2021
(cherry picked from commit cadc76c)
Sebazzz added a commit that referenced this issue Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dwin-interop Area - DWIN interop t:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants