Accepting NaN values causes unexpected results

[cadriel]

It's currently possible to send NaN in console, as per below;

SET_HEATER_TEMPERATURE HEATER=extruder TARGET=NaN

In this case, Klipper starts heating - and will error once it exceeds the configured max temp.

Secondarily, Javascript reject's a NaN in its JSON.parse - causing many more issues client side. For example, in Fluidd's case the temperature value stop updating, so the user is unaware of what his / her heater is doing.

Can include a log here if you like, but suspect this is clear enough.

@Arksine @KevinOConnor

[klipper-gitissuebot]

Hi @cadriel,

It did not look like there was a Klipper log file attached to this ticket. The log file has been engineered to answer common questions the Klipper developers have about the software and its environment (software version, hardware type, configuration, event timing, and hundreds of other questions).

Unfortunately, too many people have opened tickets without providing the log. That consumes developer time; time that would be better spent enhancing the software. If this ticket references an event that has occurred while running the software then the Klipper log must be attached to this ticket. Otherwise, this ticket will be automatically closed in a few days.

For information on obtaining the Klipper log file see: https://github.com/KevinOConnor/klipper/blob/master/docs/Contact.md

The log can still be attached to this ticket - just add a comment and attach the log to that comment.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

[cadriel]

Keeping klipper-bot happy ;)

klippy.log

Note, this is a fresh log - and I sent two commands after startup;

SET_HEATER_TEMPERATURE HEATER=extruder TARGET=NaN
SET_HEATER_TEMPERATURE HEATER=extruder TARGET=0

I don't see the second one in the log tho, so I'm not sure what's up with that.

[bevanweiss]

There's probably a few special cases here...
NaN, +Inf, -Inf
These are 'valid' floating point values, but do not represent a numeric value. Which also means that they can't be captured by JSON.

I would argue where ever JSON stringify-ing occurs, these should be converted as:
NaN => null
+Inf => Max.Float (i.e. +1.70141183E+38)
-Inf => Min.Float (i.e. -1.70141183E+38)

Hopefully the other side would then appropriately handle these (the max/min should be fine... null would probably need special handling on retrieving from JSON).

On the Klipper internal, I think things should handle NaN 'correctly' (in addition to the +Inf / -Inf values).
+Inf / -Inf are probably fine, as they typically work in comparisons.
But all comparisons with NaN will fail, so if the NaN is the target temperature, and the logic is 'if temperature > target then turn off' this will fail, and will always be on.
If the logic was instead 'if temperature < target then turn on' it should have been 'fine' (it would never turn on).

An explicit test for whether a setpoint is NaN and performs the 'safest' action in that event is probably prudent... but it does seem like a lot of work.