Max31865 min_value and max_value not set

General Discussion
1

I’m not sure if this is the correct place to post this. Opening a github issue does not seem to be possible.

This minimal config does not work;

[mcu]
serial:/dev/serial/by-id/usb-Klipper_rp2040_...

[printer]
kinematics:none
max_accel:1
max_velocity:1

[temperature_sensor test]
sensor_type:MAX31865
sensor_pin:gpio1
spi_bus:spi0a
rtd_nominal_r:1000
rtd_reference_r:2200
rtd_num_of_wires:4

The problem is that for this configuration min_value and max_value are not set for the max38165 initialization on the rp2040 (both set to 0). This triggers a mcu shutdown because the reported value by the max31865 is bigger than max_value.

Commenting out the min_value and max_value checks in src/thermocouple.c fixed the problem. But I guess the issue should be fixed somewhere in the klippy code.

Edit:
This is the relevant code in src/thermocouple.c;

    /* check the result and stop if below or above allowed range */
    if (fault || value < spi->min_value || value > spi->max_value) {
        spi->invalid_count++;
        if (spi->invalid_count < spi->max_invalid)
            return;
        try_shutdown("Thermocouple reader fault");
    }

The error message is not really helpful for identifying the problem. I had to use the source code to find out what was happening.

2

Not sure, I understand the issue. Why don’t you just specify min_value / max_value?

3

The min_value and max_value are used in the query_thermocouple command that is send to the mcu (see klippy/extras/spi_temperature.py);

    self.mcu.add_config_cmd(
        "query_thermocouple oid=%u clock=%u rest_ticks=%u"
        " min_value=%u max_value=%u max_invalid_count=%u" % (
            self.oid, clock, self._report_clock,
            self.min_sample_value, self.max_sample_value,
            MAX_INVALID_COUNT), is_init=True)

min_sample_value and max_sample_value are internal to the SensorBase class and cannot be set in the klipper config directly. Somehow these values are kept at zero for the minimal config I posted.

4

I still don’t get it.
Temperature sensors are applicable to following config sections:

  • [extruder]
  • [heater_bed]
  • [heater_generic]
  • [temperature_sensor]

Each of these section require a min_value / max_value as mandatory config item. One of these section without a min_value / max_value is incomplete and thus not valid. There is nothing like a “stand alone” sensor.
Also all error checking and safety mechanism rely on these values.

For me it looks like as “by design”.

5

Thanks for pointing me to these config sections!

Actually, min_temp and max_temp are optional for the [temperature_sensor] section.

In klippy/extras/temperatur_sensor.py I found the following code in the __init__;

    self.min_temp = config.getfloat('min_temp', KELVIN_TO_CELSIUS, minval=KELVIN_TO_CELSIUS)
    self.max_temp = config.getfloat('max_temp', 99999999.9, above=self.min_temp)
    self.sensor.setup_minmax(self.min_temp, self.max_temp)

So by default the values are set to -273.15 and 9999999.9.

MAX31865 implementation

In klipper/extras/spi_temperature class SensorBase;

def setup_minmax(self, min_temp, max_temp):
    adc_range = [self.calc_adc(min_temp), self.calc_adc(max_temp)]
    self.min_sample_value = min(adc_range)
    self.max_sample_value = max(adc_range)

In klipper/extras/spi_temperature class MAX31865;

def calc_adc(self, temp):
    # Calculate relative resistance via Callendar-Van Dusen formula:
    #  resistance = rtd_nominal_r * (1 + CVD_A * temp + CVD_B * temp**2)
    R_div_nominal = 1. + CVD_A * temp + CVD_B * temp * temp
    adc = int(R_div_nominal / self.adc_to_resist_div_nominal + 0.5)
    adc = max(0, min(MAX31865_ADC_MAX, adc))
    adc = adc << 1 # Add fault bit

So it did the adc calculation for -273.15 and 9999999.9 and in both cases int(R_div_nominal / self.adc_to_resist_div_nominal + 0.5) results in a negative number. This is than set to 0 on the next line.

Conclusion

Even though min_temp and max_temp are optional, the default values do not work with the MAX31865.

Maybe the calc_adc function should be updated to return 65534 (max value reported by max31865) if it detects temp==99999999.9? (Kind of ugly off course. Breaks if default value changes.)
Or always return 65534 in case of an overflow. (probably undesired)

Or maybe the default value should be None? But than setup_minmax for all sensor types should be updated.

Anyway, thanks for pointing me to min_temp and max_temp! Adding that to my config will probably fix the issue (cannot test it right now).

6

I agree, when comparing with the code, Klipper could be more precise.
Nevertheless, according to the documentation these are mandatory values and when properly set, I do not see a “real world” problem.

7

One more possible fix;

The Callendar-Van Dusen formula becomes negative at 1.7 million °C. So setting the default max_temp from 99999999.9 to “only” 1000000 would also fix the problem. I think this would be a pretty decent solution since klipper will probably never be used for nuclear fusion setups.

8

IMO, there is nothing to fix, but I refer it to @koconnor for final judgement.

9

I made a pull request: Limit maximum temperature in MAX31865.calc_adc() to melting point of platinum by DavidvtWout · Pull Request #6320 · Klipper3d/klipper · GitHub

1 Like
10

Thanks. The proposed change seems fine to me.

The intent is for heaters to require a min_temp/max_temp specified. However, the intent for a temperature_sensor was to not require an explicit min_temp/max_temp. That is, it was felt that a heater would need a min/max setting to ensure safety, but it was not felt that a simple temperature sensor by itself would require that safety check.

Cheers,
-Kevin

2 Likes