DEVICE_DISCONNECTED_C as -255 exceeds int8_t storage

[bobwolff68]

DEVICE_DISCONNECTED_C was defined as -127 which has no issues around storage size. However, a change to make this -255 causes warnings and a compiler 'conversion' of -255 ==> 1.

This change has created warnings in what was a totally clean build library. And it may also cause real problems in the disconnected case.
int8_t DallasTemperature::getHighAlarmTemp(const uint8_t* deviceAddress)
This function returns an 8-bit signed value who's range is -127 to 128 but the DEVICE_DISCONNECTED_C is now -255 (used to be -127).

[RobTillaart]

See commit - ac1eb7f#commitcomment-75490966

[RobTillaart]

(from the commit comments)

Some options

1. revert the values and accept that they are in range of the MAX31850
2. create a separate class for MAX31850
3. create a derived class for MAX31850
4. replace all int8_t code into int16_t in the library.

Option 1 is the quick fix.

Drawback is that -127°C is within the MAX31850 range. Applications that use these low temperatures (liquid N2 ?) would possibly be affected, however:

According to the datasheet - https://datasheets.maximintegrated.com/en/ds/MAX31850-MAX31851.pdf -

The converters resolve temperatures to 0.25°C, allow readings
as high as +1768°C and as low as -270°C, and exhibit
thermocouple accuracy of 8 LSBs (2°C) for temperatures
ranging from -200°C to +700°C

So even the current DEVICE_DISCONNECTED_C -255 is technically in the range of the MAX31850 converter.

[milesburton]

We could throw in an Option 5 and use a C++ Macro to enable those who need such a broad range of temperatures? I presume those who need -200C are likely outliers.

…

On Tue, 7 Jun 2022 at 16:59, Rob Tillaart ***@***.***> wrote: (from the commit comments) Some options 1. revert the values and accept that they are in range of the MAX31850 2. create a separate class for MAX31850 3. create a derived class for MAX31850 4. replace all int8_t code into int16_t in the library. Option 1 is the quick fix. Drawback is that -127°C is within the MAX31850 range. Applications that use these low temperatures (liquid N2 ?) would possibly be affected, however: According to the datasheet - https://datasheets.maximintegrated.com/en/ds/MAX31850-MAX31851.pdf - *The converters resolve temperatures to 0.25°C, allow readings as high as +1768°C and as low as -270°C, and exhibit thermocouple accuracy of 8 LSBs (2°C) for temperatures ranging from -200°C to +700°C* So even the current *DEVICE_DISCONNECTED_C -255* is technically in the range of the MAX31850 converter. — Reply to this email directly, view it on GitHub <#219 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJGHM5T2NVCK5GQWW3LKTLVN5WWLANCNFSM5YDJWOKQ> . You are receiving this because you are subscribed to this thread.Message ID: <milesburton/Arduino-Temperature-Control-Library/issues/219/1148863258 @github.com>

[RobTillaart]

A macro sounds like a better fix than reverting. +1

[bobwolff68]

Hey guys - Any chance of getting a decision on the solution here and getting it published? I'd love to not fork to get this settled in my own builds. Thanks!

[milesburton]

I think a macro is the way to go, unfortunately I don't have an IC to actually test the change so I'm hesitant to raise a PR until I know it won't nuke anything.

If you want to make the change Rob and I can give you a review?

[bobwolff68]

Well, for me it's not a quandry - I'd just pick option 1 and revert the change. :) But if you're going to cope with this otherwise, I'd think the 16-bit change would be proper but I'm not clear on whether a 2-byte value would be problematic for other reasons. But I'm concerned that the macro would map things that are technically still out of bounds. That seems like just askin' for trouble down the line. Right?

[milesburton]

Let me grab a branch and I'll get back to you

…

On Tue, 5 Jul 2022 at 07:12, Robert Wolff ***@***.***> wrote: Well, for me it's not a quandry - I'd just pick option 1 and revert the change. :) But if you're going to cope with this otherwise, I'd think the 16-bit change would be proper but I'm not clear on whether a 2-byte value would be problematic for other reasons. But I'm concerned that the macro would map things that are technically still out of bounds. That seems like just askin' for trouble down the line. Right? — Reply to this email directly, view it on GitHub <#219 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJGHM5K3SPSJF7XDVOAAVDVSPG5HANCNFSM5YDJWOKQ> . You are receiving this because you commented.Message ID: <milesburton/Arduino-Temperature-Control-Library/issues/219/1174656800@ github.com>

[milesburton]

Take two. Helps if I don't mix up my data types: #224

[milesburton]

@bobwolff68 Change merged. Let me know if that solves your problem. I'll close the issue for now (we'll revisit the Macro when I get a test setup on the bench)

[bobwolff68]

Thanks, Miles. Re-compiled with the change - zero warnings is a beautiful thing. Let me know when the macro version comes aroun d- happy to give that one a spin as well.

…

--bob

On Tue, Jul 5, 2022 at 4:23 AM Miles Burton ***@***.***> wrote: Closed #219 <#219> as completed. — Reply to this email directly, view it on GitHub <#219 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFYP5VHFWY5MMJQ7GWDYHTVSQLMRANCNFSM5YDJWOKQ> . You are receiving this because you were mentioned.Message ID: <milesburton/Arduino-Temperature-Control-Library/issue/219/issue_event/6933610430 @github.com>