Adjustable precision in M105 temperature report

[FanDjango]

This PR does not fix a bug. It is entirely cosmetic and low priority

This concerns

// For serial echo, the number of digits after the decimal point
//#define SERIAL_FLOAT_PRECISION 4

When it is enabled by removing the comments, a possible problem ensues.

There are a few places in the code where SERIAL_ECHO(...) is used to display a value of type float.

Thus, for example, the temperature status messages look like this:

Recv:  T:21.17 /0.0000 B:21.04 /0.0000 @:0 B@:0
Recv:  T:21.00 /0.0000 B:20.82 /0.0000 @:0 B@:0
Recv:  T:21.07 /0.0000 B:20.96 /0.0000 @:0 B@:0

SERIAL_ECHO is pretty baseline and does not respect SERIAL_FLOAT_PRECISION.

I have not looked at the other (few) places where this constellation also exists.

Since there are only a few places, it seems simpler to recode them instead of messing with serial.h and overloading SERIAL_ECHO to handle floats correctly, just like SERIAL_ECHOPAIR and its brethren do.

So here is the proposed change to temperature.cpp, the first such occurence I have noticed.

The temperature message then looks like this:

Recv:  T:21.1523 /0.0000 B:20.9765 /0.0000 @:0 B@:0
Recv:  T:21.2695 /0.0000 B:21.2109 /0.0000 @:0 B@:0
Recv:  T:21.3281 /0.0000 B:21.0546 /0.0000 @:0 B@:0

[FanDjango]

I am aware that this message especially is very important to many clients, such as Octoprint.

Although Octoprint seems happy enough with all the above shown formats as well as the default one, this might not be true for others.

So an alternative thought, also easy to code a change for, is to have the temperature message stay at 2 digit precision, regardless of the precision setting in the configuration.

In that case, the title of this PR should then be "DONT respect PRECISION in temperature message" and the code needs to be changed in the other direction.

I don't know if anyone even uses this configuration option. In many places in the code where it is deemed necessary, an echo of a float will have an explicit precision set for it, also ignoring the configured precision.

I would like to invite comments and discussion on this.

[thisiskeithb]

I don't know if anyone even uses this configuration option.

SERIAL_FLOAT_PRECISION 4 is used by BigTreeTech's TFT firmware.

[FanDjango]

I don't know if anyone even uses this configuration option.

SERIAL_FLOAT_PRECISION 4 is used by BigTreeTech's TFT firmware.

Well, I am not 100% sure, but it looks as if they are getting this format:
T:21.17 /0.0000 B:21.04 /0.0000 @:0 B@:0

And depending on their code, changing this (for better or worse) might break things.

[sjasonsmith]

@thisiskeithb would you be able to test this with their TFT display, to see if it works there?

And depending on their code, changing this (for better or worse) might break things.

The RepRap Wiki M105 has this to say:

Note that temperatures can be reported as integers or floats. There sadly are a lot of interpretations of how an M105 response should look like across firmware variants, making parsing them potentially tricky.

If clients understand a variety of firmware's responses, they will hopefully use proper numeric parsing techniques and not rely on fixed-width output.

[sjasonsmith]

I looked a little closer at the code. I had no idea that SERIAL_ECHO behaved completely differently from SERIAL_ECHOPAIR.

That is a very unfortunate bit of naming confusion, since their names imply they would do the same thing, just for one versus multiple items.

From my perspective, I am fine with your change as a simple solution to this. @thinkyhead would have to state whether those two macros behave differently intentionally or should be re-worked to have the same result.

[FanDjango]

I should add, this is definitely a candidate to put on a back burner. I just came across it by chance. Given previous reactions when message formats changed, I would be leery of ad-hoc actions.

Sadly, there is no "back-burner" list of things that "we know about, but more important things have precedence". This would be a candidate for such a list. And if it ever becomes important, bingo, it's there and ready for some kind of action.

[sjasonsmith]

I should add, this is definitely a candidate to put on a back burner.

It is a simple change, so it should hopefully be easy to get a "Yes/No" answer quickly.

[thisiskeithb]

It is a simple change, so it should hopefully be easy to get a "Yes/No" answer quickly.

I've asked someone more familiar with the TFT code to give it a test/review. I'll let you know what I hear!

[thinkyhead]

It may be argued that temperature fluctuations under 0.01 degrees are completely irrelevant, and the 'float precision' setting might just be too broad if it will apply to everything, including areas where it really doesn't matter. That said, we also don't want to apply a precision of 1 to output where that would be too little precision. So, wherever the float-precision setting is applied, it should probably be bracketed in constrain().

[FanDjango]

So, wherever the float-precision setting is applied, it should probably be bracketed in constrain()

So an alternative thought, also easy to code a change for, is to have the temperature message stay at 2 digit precision, regardless of the precision setting in the configuration.

Most places already are constrained.

Only a few "would need to be" looked at. What irritated me, was

SERIAL_FLOAT_PRECISION 4 is used by BigTreeTech's TFT firmware.

especially because I would strongly assume that they have been getting the strange mixed format up to now.

Either both values have the same precision or both are constrained to 2 dp's regardless of settings, the mixed one grates on the eyes.