Export tmc2240 driver temperature

[KevinOConnor]

This is a proposal to report the tmc2240 driver temperature in a new "temperature" field in the "tmc2240 stepper_x" status.

The temperature reporting in the tmc2240 is a nice feature - it would be nice to be able to see this temperature on the common Klipper frontends. So, in some sense it might be better to export this information using an existing "temperature_sensor" report. However, the tmc drivers are a bit unique in that the information is only queried when the driver is "enabled". (When the driver is not enabled, Klipper wont query the information because it is common for users to disable motor power, which disables communication with the driver.) Exporting the information in the "tmc" status section makes this easier, but it would require front-end changes to display the temperature.

@Arksine , @meteyou , @pedrolamas - do you have any thoughts on this?

This PR only updates the tmc2240 (which provides a full temperature report). It would also be possible to export a pseudo-temperature on the other tmc drivers. Other tmc drivers generally report "temperature warning" flags. It would be possible to map those flags to this new "temperature" field. If we did this, most drivers would normally report 0 degrees, until a temperature warning occurred - in which case the reported temperature would report 100 (or 120, 147, etc. depending on the tmc driver).

@leptun , @bigtreetech - fyi.

-Kevin

[meteyou]

Thx for the ping! I would prefer to use the temperature attribute, when a real temperature exists. Like the tmc2240 and have the warning/error flag separated from the temperature.

[pedrolamas]

Well, the advantage of having it exported in temperature_sensor would be that in theory no changes would be required on the frontend code, however we know that is not the case as the temperature might not be present if the stepper is disabled.

As such, my vote goes into adding the temperature directly to the tmc... object state and we can add some code to read that in and handle it when it is disabled.

[leptun]

I agree with both @meteyou and @pedrolamas about having the temperature in the tmc object.
Having the warning/error flags separate from the temperature is also a good idea imo since all drivers have this feature. Not sure how the UI could implement the warning, but it's better to have it as an option compared to now where the warning flags are completely useless.

What I'm curious about is what should be done with these warning flags? The tmc2130 and tmc2660 have fixed otpw/ot thresholds. tmc5160 has a fixed otpw and a configurable ot (4 levels). tmc2208/2209 has some configurability of the otpw/ot thresholds as well (4 levels). tmc2240 has a fixed ot level, but a fully configurable otpw threshold. In PR #6128 I make the otpw threshold configurable for tmc2240, but that still leaves the other drivers unconfigurable. Should anything be done about this?

[meteyou]

@leptun First, I would only include the driver flags and display the status such as "normal/cold", "warning", or "error: overheated". Can you read the otpw value from the registers and convert this threshold value for the configurable thresholds into degrees Celsius? If so, this temperature could also be displayed in the GUI.

Of course, this function could be extended to make the threshold configurable via the interface if Klipper allows this. Currently, I don't see the advantage for the user of being able to configure this easily. I think this is more of a "pro" function. It's enough if it can be done via register values in the printer.cfg.

[KevinOConnor]

FYI, the warning flags are already exported in the tmc objects - they are in the "drv_status" field ( https://www.klipper3d.org/Status_Reference.html#tmc-drivers ).

The flag to temperature mapping is:
tmc2660: "otpw" -> driver temperature >= 100 degrees
tmc2130, tmc5160: "otpw" -> 120
tmc2208, tmc2209: "t120" -> 120, "t143" -> 143, "t150" -> 150, "t157" -> 157

The tmc2208/9 also have an "otpw" field, but it's a configurable alias for the t120-t157 fields, so I don't think there is any gain in using that flag on those drivers as it is just as easy to access the unaliased temperature flags.

As indicated at the top of this PR, we could map the above flags to the new "temperature" field. It sounds like that is not desirable. We could also make some new field (eg, "coarse_temperature") with the mapping (if that makes it easier for the frontends).

-Kevin

[KevinOConnor]

To expand on the "drv_status" field, the following was part of a test I ran yesterday (on the code from this PR):
{"params":{"status":{"tmc2240 manual_stepper test":{"drv_status": {"csactual":31,"sg_result":489,"stallguard":1,"stst":1},"temperature":50.78}},"eventtime":3690601.460312802}}

If a tmc2208 driver got warm I'd expect it to report something like: {"drv_status": {..., "t120":1}}

If a tmc2208 went over 143 degrees I'd guess both the "t120" and "t143" fields would be set simultaneously - I'm not sure.

Cheers,
-Kevin

[leptun]

If a tmc2208 went over 143 degrees I'd guess both the "t120" and "t143" fields would be set simultaneously - I'm not sure.

I think you are correct. That's also what I understood from the datasheet.

What about tmc2240? The default value of OVERTEMPPREWARNING_VTH is for otpw at 120C. Do we want to make this at all configurable or not? We can read adc_temp register directly for the temperature, but maybe it makes sense to get the drv_status::otpw also configurable if it's possible. If not, at the very least, I'd like to initialize this field to the default value during init in case it was set to something else previously.

[KevinOConnor]

What about tmc2240?

Right, the "otpw" flag is also available on tmc2240. Like tmc2208/9 it doesn't provide any value as it's simpler to directly use the raw temperature.

If you want my 2 cents, I don't see a value in configuring otpw, as it doesn't provide any new information to Klipper. I'd say better to keep the code simple.

Cheers,
-Kevin

[leptun]

Well, in that case I'll close my PR.

If you want my 2 cents, I don't see a value in configuring otpw, as it doesn't provide any new information to Klipper. I'd say better to keep the code simple.

Not entirely true. At least for tmc2209, I read that when an OT event occurs, the driver is disabled until the temperature drops and OTPW is cleared. So otpw adds hysteresis to the overheat event. Not sure if regular users would want to configure this, but it's not completely useless even if you don't read that flag.

[KevinOConnor]

So otpw adds hysteresis to the overheat event.

otpw can also be routed to an irq pin on some drivers. In both cases, a configurable "otpw" has no value in the context of Klipper.

Cheers,
-Kevin

[KevinOConnor]

Thanks for the feedback. I have committed this change. (There is now a "temperature" field available for tmc2240 drivers; no additional logic has been added for the otpw and tXXX fields.)

-Kevin