[BUG] Adafruit MAX31865 causes serious delays on SKR 1.4 (bugfix-2.0) #21638
Comments
|
The adafruit MAX31865 library implements readRTD() as follows: So, there's a blocking delay of 75msec in every invocation, several times per second. It's really impressive that nobody else has complained about this till now. There are two possible paths: one (which I'll try first) is to create a new fork of the adafruit library that implements a non blocking tryReadRTD() method, that will use millis to advance to the next step each time it is invoked, and return a fresh value only when the necessary delays have been observed. The other one is to implement the non-blocking MAX31865.readRTD() function in Marlin. I'm not sure about the policies regarding such an implementation, so I'd like some feedback here. |
What setup are you using? Which printer? |
|
It's a custom build, based on SKR 1.4. I used to have an e3d analog amplifier for my PT100 sensor, but I smh managed to blow that up, so I recently went for an adafruit MAX31865. Thanks to your great guide (kudos for that!) I found out all the quirks for installing the new amplifier (in fact, and since my board is crammed with sensors and most pins are in use, I've installed the adafruit on the same SPI with my reprap display, just using P0.03 for CS, with no problems). But when I tried it out, I noticed large delays in all movement (homing, calibration, printing). Yesterday I dug into your adafruit library and found the delays I describe above. I made some changes to the library in order to remove the delays (https://github.com/zeleps/Adafruit-MAX31865-V1.1.0-Mod-M.git) and replace them with timed waits and everything works fine now. Only thing is that I occasionally read a 0xffff from the RTDMSB register, I'll probably tackle the odd error by repeating the last read once or twice before sending a reading error and halting the printer. I'm really wondering how these delays worked for you. Marlin reads RTD from the sensor several times per second, and the 75msec delay is substantial and clearly visible. |
|
It looks like its set to one shot mode, so you have to wait for it all the time. What about always on? |
As I understand, the bias current heats the sensor, thus making reading inaccurate, so the library probably adopted this on/off strategy to keep heating at a minimum, but I guess that the reading is so frequent that the sensor will heat up anyway. The code you sent is for MAX31856, not 865, but I'll try that suggestion (leaving the sensor on) and see what happens. In my trials yesterday, I found out that after some actual printing time, the sensor starts returning 0xFFFF until reset (power toggle). Reading the fault register, I'm getting 0x80 (MAX31865_FAULT_HIGHTHRESH). This doesn't seem to happen when not printing. I'll make some more tests to figure out the issue. |
|
Ok, I read a bit more about MAX31865 and decided that 1-shot mode is futile in the case of hotends, since reading is too frequent to prevent the bias current heating effect, plus the fact that bias current affects room temperatures, not 200°C temperatures. So I changed my MAX31865 version of the library to work in auto mode, and to simply read the rtd register, which is efficient, quick and simple. And apparently works just as fine (@descipher thanks for the tip!) While printing, I still get some random read errors, which would normally cause a _temp_error(). Since they are single read errors I chose to ignore them at the MAX31865 library level (under certain conditions, eg. up to 5 consecutive errors or if the temp change is unnaturally big within a small period of time). I'll investigate the odd error issue a bit more and come back. Meanwhile, since nobody else has complained about these issues, I'm planning to keep my solution to myself. If more people are interested in them, we can continue this discussion. |
|
I like your solution, but you will get temperature reading errors because
if you read the data "too fast" the MAX31865 sensor needs additional time
to "settle out" before you take the actual reading.
Why do you think reading the temperatures faster is better? Temperature
changes do not occur faster than 1 sec.
Using a non-blocking wait is the correct answer!!
If I remember correctly, my modified library was based on the library
created by Adafruit for the MAX31865. You probably are not even aware that
before I did my modified library for the MAX31865 no one using a SKR
1.3/1.4 board could not use a PT100 with a MAX31865 board . The code would
not even compile or run.
So one shot mode, with non-blocking wait between temperature readings, is
in my humble opinion, is the correct answer. When I get back into town, I
will look at your code to see how you implemented the non-blocking function
and add it to my library.
When I tried to read the MAX31865 temperature readings faster, I
experienced the temperature reading errors as you did.
Marlin has the ability to reduce temperature reading errors by using the
Marlin THERMOCOUPLE_MAX_ERRORS variable in configuation_adv.h file. It is
defaulted to 15. So, if Marlin experiences MAXTEMP or MINTEMP errors that
are less then 15 the 3D printer is not halted.
Try raising the THERMOCOUPLE_MAX_ERRORS to see if it helps reduce the
detections of MAX31865 temperature reading errors. Use one shot mode. Add
non-blocking waits between readings. Adjust the non-blocking wait until the
temperature error readings go away. I never really test the wait time to
see what the minimum would be to stop the errors in the temperature
readings. But you should not need to drop bad temperature reading inside
the library, because Marlin has already done this.
When I get back I will add your non-blocking waits to my library. My 3D
printer right now is torn apart for upgrades. When I get it all back
together I will adjust my non-blocking waits when I can test it on a full
blown working 3D printer. But I will say this again, temperature does not
changes in the span of milliseconds it changes in "number of seconds". My
mistake was using blocking waits.
…On Mon, Apr 26, 2021 at 9:36 AM zeleps ***@***.***> wrote:
Closed #21638 <#21638>.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#21638 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AH7LC2NYW27VOZ5QHQAXG2TTKVT5FANCNFSM43DBRRMQ>
.
|
|
Ok, this seems to be an issue with temperature.cpp. I'm not the one polling for temperature, it's Marlin. Check this log from my serial port (each line is an invocation to readRTD()): That's 92 times in a second. This basically happens because Marlin invokes readRTD() from within analog_to_celsius_hotend(). Also, THERMOCOUPLE_MAX_ERRORS is used only when calling read_max_tc(), but as you can see in temperature.cpp: right after reading raw values, marlin re-reads the temperature from within analog_to_celsius_hotend(). And there is no error checking there, the temperature read is used as-is. Obviously this is wrong and it should be averted. Also, my solution has some assumptions (like a default value of 20°C is hardcoded as a default return value when there are no readings) which need to be fixed. I do wonder though, why are you using 1-shot reading instead of auto-convert? |
|
Ok, I think whoever put this line there: did not expect temperature() to re-read the RTD register. I've changed temperature() to use the lastRead value and everything looks much better now. I'll cleanup the code later and upload it to my fork. |
MAX6675_HEAT_INTERVAL is the Marlin variable which decides how fast to do the ADC reading in Marlin. MAX6675_HEAT_INTERVAL is located on the second line in Temperature::read_max_tc() routine. The reason I use one shot is because you do not need to constantly read the temperature every millisecond. Since the temperature readings are occurring slowly (as compared to real-time readings) I choose to use one shot. Also the original Adafruit library performed its temperature readings this way. They developed the MAX31865 board so I followed their lead. Also look at the Marlin variable MAX_CONSECUTIVE_LOW_TEMPERATURE_ERROR_ALLOWED in temperature.cpp code around line 2609 - 2628. I see what you are saying now, I believe you are correct, that re-reading the RTD register twice in a row should not occur. But you should not be getting any faulty temperature readings from the MAX31865 sensor. There has to be a combination of these changes that gives you the speed you need, and temperature readings that are not faulty. Let me know when you solve it, I will use your library when you are done testing when I get my printer put back together. Also inform me if after you update your library if you get any faulty temperature readings. Thanks for these ideas. |
|
92 samples per second! That's 10.8ms per sample. The device cannot deliver it that fast. Continuous conversion (60Hz notch) = 16.7ms |
I believe the 92 samples per sec included two reading for RTD. After @zeleps changes the library to reuse the previous RTD in the temperature routine of the library the samples will be cut in half. So really Marlin is doing 92/2=46 which would be a sample every 21.73ms. But from the data sheet on page 3 using single shot says the MAXIMUM conversion time is 55 ms. So if you want to have it done faster maybe switching to continuous conversion which has a MAXIMUM conversion time of 17.6ms. So the following changes need to be done:
That should solve the problems |
Yes, I concur with this assessment. |
|
@thisiskeithb this is NOT a BUG in Marlin code!! The Marlin code does not need to be changed. This is the problem with the MAX31865 library. @zeleps will make the changes and test it out for the user define library for the MAX31865. All Marlin did was allow the user to add their own user defined library call for MAX31865, MAX31855 and MAX6675 sensors. So this is NOT a bug for the Marlin code, this is a bug in the user defined libraries. Once I get back I will use @zeleps MAX31865 library and make changes to remove blocking waits in the MAX31855 and MAX6675 user libraries and recheck the MAX31855 and MAX6675 user libraries for conversion limits again. |
thisiskeithb
removed
Bug: Confirmed !
Needs: Patch
Sounds good. I’ll close this then. |
|
Removing readRTD() from temperature() dropped RTD reads during print from ~100/sec to 4/sec (observing MAX6675_HEAT_INTERVAL ). So, no need to change Marlin there. Also, at this rate, the errors almost vanished (but I still got one in an hour-long print). But on the other hand, read_max_tc() has an issue with LIB_ADAFRUIT_MAX31865. Instead of using readRTD_with_Fault(), which would trigger the THERMOCOUPLE_MAX_ERRORS, uses readFault(), which is done before readRTD(), so any faults readRTD() produces will go unchecked until the next read_max_tc() invocation (causing other temperature checks to fail and ending up to a _temp_error()). So, my proposal regarding Marlin, is to use LIB_ADAFRUIT_MAX31865 in exactly the same way as LIB_USR_MAX31865 in temperature.cpp, and this does require a few modifications to Marlin code (@thisiskeithb, I think this calls for a re-opening of the issue). As for the non-blocking readRTD() (readRTD_with_Fault() if the above changes are applied), the major issue is that temperature is actually polled on every third invocation. If readRTD_with_Fault() remains blocking, we still have an unnecessary 75msec block. So, either we improvise a mechanism to advance readRTD_with_Fault()'s state independently, or switch it to auto, which imho is a clean solution with no visible drawbacks. I can uncerstand that the original library used 1-shot, but that's an unoptimized library (well, it does block :) ) that -probably- tries to keep the PT100 sensor cooler at room temperatures (by disabling the bias current and thus enchancing accuracy in room temperatures). In our case (hotend monitoring), we use the sensor at temperatures >200°C, where the heat accumulated by the bias current is negligible, thus the extra commands to enable bias and configure 1-shot are just redundant. I'll shape up my changes (probably tomorrow) and give them to you @GadgetAngel for review. |
|
Since this isn’t a Marlin bug, it’d be best to continue discussion over at the Marlin Forum hosted on RepRap.org or the Marlin Firmware Discord server. |
@zeleps @descipher let's move this discussion over to GadgetAngel/Adafruit-MAX31865-V1.1.0-Mod-M#1. This is an issues section under my MAX31865 library. I would have opened one under @zeleps MAX31865 library but his repository does not have an issues section. So for further discussion, let continue this over at my MAX31865 issues library repository site: GadgetAngel/Adafruit-MAX31865-V1.1.0-Mod-M#1 If Marlin needs to be changed we will submit a merge request when the time comes. |
|
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. |
Did you test the latest
bugfix-2.0.xcode?Yes, and the problem still exists.
Bug Description
Installing an Adafruit MAX31865 on my SKR 1.4 (as per GadgetAngel's instructions) caused big delays in almost any function. Homing pauses for 3-4 seconds before starting, and another 4'' before deploying bltouch. Prints pause for a few seconds at random steps and ultimately fail with MAXTEMP. Changing TEMP_SENSOR_0 back to 5 (my previous setup) without physically changing any connections brings speed back to normal (temperature readings are incorrect, of course).
I put some timing code for testing and found out that max865ref.readRTD() takes some 70-90msec to complete, so does max31865_0.temperature(MAX31865_SENSOR_OHMS_0, MAX31865_CALIBRATION_OHMS_0). This seems to be excessive, and makes the sensor practically useless.
I installed the sensor on the LCD's SPI at first, using pin 0.03 for CS, then changed it to on-board SPI (0.07-0.08-0.09) with 0.26 for CS. Results were the same. I'm not using SD for printing, I print directly from OctoPrint.
Bug Timeline
Issue started immediately after installing MAX31865.
Expected behavior
Little, if any, delays.
Actual behavior
Long, noticeable and ultimately fatal delays
Steps to Reproduce
No response
Version of Marlin Firmware
latest bugfix 2.0
Printer model
Custom
Electronics
SKR 1.4
Add-ons
bltouch, Adafruit MAX31865 on T0, type 5 thermistor on T1.
Your Slicer
Cura
Host Software
OctoPrint
The text was updated successfully, but these errors were encountered: