[BUG] Compilation error PROBE_TEMP_COMPENSATION

[hannesweisbach]

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

PROBE_TEMP_COMPENSATION is configurable in Configuration_adv.h, for example:

 #define PTC_SAMPLE_START  30.0f
 #define PTC_SAMPLE_RES    5.0f

If values are not defined, sane defaults are substituted in probe_temp_comp.h:

#ifndef PTC_SAMPLE_RES
  #define PTC_SAMPLE_RES 5
#endif
#ifndef PTC_SAMPLE_START
  #define PTC_SAMPLE_START 30
#endif

Obviously, values in Configuration_adv.h are defined as floats, whereas the substitute values are ints. Because these values are used to brace-initialize a variable of type celsius_t, defining these macros as floats results in a compilation error, because they need to narrowed from float to celsius_t (which, apparently is type short int):

In file included from Marlin/src/feature/probe_temp_comp.cpp:27:0:
Marlin/src/feature/probe_temp_comp.h:89:1: error: narrowing conversion of '5.0e+0f' from 'float' to 'celsius_t {aka short int}' inside { } [-Wnarrowing]
 };
 ^
Marlin/src/feature/probe_temp_comp.h:89:1: error: narrowing conversion of '2.5e+1f' from 'float' to 'celsius_t {aka short int}' inside { } [-Wnarrowing]
Marlin/src/feature/probe_temp_comp.h:89:1: error: narrowing conversion of '8.0e+1f' from 'float' to 'celsius_t {aka short int}' inside { } [-Wnarrowing]
Marlin/src/feature/probe_temp_comp.h:89:1: error: narrowing conversion of '5.0e+0f' from 'float' to 'celsius_t {aka short int}' inside { } [-Wnarrowing]
Marlin/src/feature/probe_temp_comp.h:89:1: error: narrowing conversion of '5.0e+1f' from 'float' to 'celsius_t {aka short int}' inside { } [-Wnarrowing]
compilation terminated due to -fmax-errors=5.
*** [.pio/build/mks_robin_nano35/src/src/feature/probe_temp_comp.cpp.o] Error 1

Aside from the facts, that I'm not a huge fan of repeated values (Configuration_adv.h and probe_temp_comp.h) and macros (not type safe), I'd be willing to fix this. I'm just not sure how:

Do you want the values in Configuration_adv.h to be int?
Do you want probe_temp_comp.h to include static_cast to celsius_t?
Something else?

Bug Timeline

Unknown

Expected behavior

Successful compile with default values un-commented in Configuration_adv.h.

Actual behavior

Compilation error:

Steps to Reproduce

1. Activate PROBE_TEMP_COMPENSATION
2. Un-comment, for example, #define PTC_SAMPLE_START 30.0f in Configuration_adv.h
3. Compile.

Version of Marlin Firmware

2.0.x-bugfix, master as of yesterday

Printer model

Two Trees Sapphiro Pro

Electronics

Stock for the printer, but modified, MKS Robin Nano

Add-ons

Inductive Z probe with temperature sensor (aka: PINDA v2)

Your Slicer

Cura

Host Software

OctoPrint

Additional information & file uploads

No response

[ellensp]

"Comment out, for example, #define PTC_SAMPLE_START 30.0f in Configuration_adv.h"
I think you mean un comment

[hannesweisbach]

Yes, you are right! Remove the // ;)

I edited the report to reflect that correction.

[slowbro]

I guess the question here is should probe temp compenation a) use celsius_float_t to emulate the old code, or b) use celsius_t, for which the default config just needs to be updated (+ maybe something in SanityCheck)?

I think it may be helpful to have granularity here due to the scale of things.. small probe, so small changes in temp may be more noticable?

[hannesweisbach]

Oh, I wasn't aware celsius_float_t even existed ;)

I don't really think it's needed here though.I doubt the thermistor is neither accurate nor precise in fractional degrees over the temperature span used. Not to mention ADC, noise, etc …

Just for reference, some measurements though. I have a none-name inductive probe with temp sensor and I have:

• less than 40µm per +5 Kelvin deflection in the bed (~ 3.6µm/K after linear regression)
• less than 300µm per +5 Kelvin drift in the probe (~ 32µm/K after linear regression)

For me it seems, fractions of a degree wouldn't be necessary. I don't know if there are other kinds of temperature compensated probes which need more resolution.

[slowbro]

#22130 merged; this can be closed.

[github-actions]

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.