Marlin Firmware 1.1.6v1 source code bug

Discussions about ideaMaker and other printing software.
Jetguy
Posts: 2697
Joined: Tue Mar 22, 2016 1:40 am
Location: In a van, down by the river

Marlin Firmware 1.1.6v1 source code bug

Postby Jetguy » Sat Nov 25, 2017 6:22 pm

Just wanted to let the group know i have found some issues in some specific safety code that is implemented in the latest Marlin 1.1.6 rev1 release. This was discovered while modifying this firmware to run a delta printer but it has nothing to do with delta.

So here is the bug:
If you change the temp sensor type and ADC pin port connection in the source, then when you set a heated bed temp, you will find that the temp resets back to 0 in the firmware with no error messages. Again, the basic symptom is that you send marlin a heated bed set temp command and instead of showing it as a target temp, it resets target to 0 making using a heated bed all but impossible.


Again, please understand the context and relevancy to Raise 3D N series users. So far as configured for the N series and using the supplied binaries built using this source, I am not yet aware of any reports of this bug with the heated bed temp resetting to 0 with no warning or error messages. What I'm saying is, I still advocate flashing and updating to 1.1.6 for all N series printers as 1.1.1 has known issues fixed in 1.1.6. It is better, you really do need to run it. But, what I am also saying is to the larger N series owner community is that if running 1.1.6 rev1 updated firmware, you happen to see a situation where you set bed temp and see it revert back to 0 with no error messages, either in a print or from using the LCD control panel, this is a symptom of this underlying code issue I found. Cannot stress enough, I have not seen this invoke on a Raise 3D printer and all of mine are running 1.1.6rev1 firmware. I am asking for users running this firmware just to make a note and report here if you do ever see the bed setpoint temp revert to 0 for any reason. The kicker is, the serial log may not immediately let you know this happened if trying to debug.

The basic problem is that safety code not part of normal release open source marlin was added to the temperature.cpp in the source code.
The way this is checking is using the raw ADC values against hard coded values inline in the code. While I understand the performance reasons to use raw value against a hard set value, in practical use, there is zero reason for this kind of way of setting up a safety check. Second that is, this is highly sensitive to any spurious EMI and RFI captured by raw reads and not filtered. Then, just the punch in face is that it's hard coded and ONLY valid for a specific temp sensor type. So what happens is, should you change the type of the extruder temp sensor from thermocouple to thermistor- this in the most insane way causes the heated bed safety code to fail and auto reset the setpoint temp to 0. But just to be an extra level of wrong, it then throws no error message to the serial port and thus serial log.

Again, what made this interesting was a few facts:
#1 it does not use the min temp and max temp variables in configuration.h
#2 it uses raw values for speed, but because of this has no filtering on an analog pin making it likely to trigger false positives.
#3 while I encourage safety code, the rate this code needs invoked should be a lot more sensible and speed is not the primary concern.
#4 this is NOT the thermal runaway code!!!! I say again, this is not invoked, disabled or related to the thermal runaway code.
#5 if it is safety code, not throwing an error message when invoked is one of the worst things I have ever seen in code.

Again, what was observed is that if you change the extruder temperature sensor type to any thermistor type, this then in the most insane way causes some spurious ADC reads and that then causes this safety code in the setpoint of the heated bed to reset to 0, meanwhile never giving any indication this condition was met.

Just did a diff using notepad++ to compare the current source to the one we edited:
temperature.cpp
Line 549 is commented and this is the line that should report a message on the serial port when the function is invoked.
// _temp_error(-1, PSTR(MSG_MAXTEMP_BED_OFF), PSTR(MSG_ERR_MAXTEMP_BED));

Definitely should be uncommented as this being commented made debugging a royal pain.

Then down starting at line 1601:
This is the original code
/* No bed MINTEMP error? */
#if defined(BED_MAXTEMP) && (TEMP_SENSOR_BED != 0)
if (!(current_temperature_bed_raw MAXTEST bed_maxttemp_raw)) {
target_temperature_bed = 0;
// SERIAL_PROTOCOL(current_temperature_bed_raw);
bed_max_temp_error();
}
#endif


And this is the way my friend changed the code to not error out:
/* No bed MINTEMP error? */
#if defined(BED_MAXTEMP) && (TEMP_SENSOR_BED != 0)
if (!(current_temperature_bed MAXTEST BED_MAXTEMP)) {
target_temperature_bed = 0;
// SERIAL_PROTOCOL(current_temperature_bed_raw);
bed_max_temp_error();
}
#endif


I'm not saying this is the best way to fix this or that this is some problem and Raise 3D N series owners should not use firmware version 1.1.6.
I am saying that this needs looked at more and refined in the next release of firmware.

Jetguy
Posts: 2697
Joined: Tue Mar 22, 2016 1:40 am
Location: In a van, down by the river

Re: Marlin Firmware 1.1.6v1 source code bug

Postby Jetguy » Tue Nov 28, 2017 3:09 am

No replies? Nobody else is looking at this?

Again, I'm not saying users should not use this version of firmware, but i would hope we can come up with a revision here and "fix" this to an acceptable standard soon. It's not that complicated.

User avatar
Vicky@Raise3D
Posts: 4138
Joined: Fri Mar 25, 2016 3:54 am

Re: Marlin Firmware 1.1.6v1 source code bug

Postby Vicky@Raise3D » Tue Nov 28, 2017 8:03 am

Have already forward your information and suggestions above to software team. The problem you mentioned is under debugging these days.


Return to “Software”

Who is online

Users browsing this forum: No registered users and 3 guests