Skip to content

Conversation

@edgar-bonet
Copy link
Contributor

As reported in issue #287, the method RTC_DS3231::getTemperature() fails on negative temperatures. This is because the conversion from raw bytes to a float-typed temperature assumes the value is always positive, wheres the datasheet specifies it is stored as a signed, two's complement integer.

This pull request fixes the method by changing the conversion code to this:

  int16_t temp = uint16_t(buffer[0]) << 8 | buffer[1];
  return temp * (1 / 256.0);

Some points worth noting:

  • Casting buffer[0] to an unsigned type is meant to avoid signed integer overflow, which is undefined behavior in C++.

  • Assigning the result to a signed integer makes numbers with the most significant bit set be interpreted as negative. There is no extra work to do, as all current processors use two’s complement representation internally.

  • Multiplying by 1 / 256.0 (rather than 1 / 4.0) obviates the need for the 6-bit shift. Also, 1 / 256.0 being a compile-time constant, there is no division performed at run-time (which would be expensive).

  • This code also reduces the number of floating-point operations to only two (int-to-float conversion and multiplication), v.s. four in the original code (two int-to-float conversions, one multiplication and one addition).

Tested down to −27.00°C by @i440bx.

Fixes #287.

RTC_DS3231::getTemperature() assumes the temperature register stores a
positive integer whereas, according to the datasheet,[1] it is a signed
number in two’s complement format.

Fix the method by transferring this register into a signed 16-bit
integer. As the CPU uses two’s complement natively, no further
conversion is needed other than scaling by a factor (1/256.0).

Fixes adafruit#287.

[1] https://www.analog.com/media/en/technical-documentation/data-sheets/DS3231.pdf#page=15
@BillyGriffiths
Copy link

We ran into the same problem with negative temps. Do you have any idea when this will be merged?

@edgar-bonet
Copy link
Contributor Author

@BillyGriffiths: No idea. This repo seems practically unmaintained. The last change to the source code of the library was the merge of pull request #257, on 2022-08-04. Since then, it has only had minute changes that do not touch the source code of the library itself.

@dhalbert dhalbert requested a review from caternuson October 13, 2024 13:10
@tyeth
Copy link
Member

tyeth commented Jan 9, 2025

I've got some RTC breakouts on order, as we're adding Offline SD Logging to Wippersnapper firmware so at that point I'm going to go through a few of the more useful/simple PRs (time permitting) while testing.

@i440bx
Copy link

i440bx commented Jan 10, 2025

Would you read the initial posting you would see that the negative temperature problem was solved by fix #287.

BenUniqcode pushed a commit to BenUniqcode/RTClib that referenced this pull request Oct 1, 2025
It's basically the same but maybe a bit safer due to explicit conversion to 16-bit first.
BenUniqcode pushed a commit to BenUniqcode/RTClib that referenced this pull request Oct 1, 2025
BenUniqcode pushed a commit to BenUniqcode/RTClib that referenced this pull request Oct 1, 2025
My previous method worked, but this one is better so I used it in the DS3231 and 3232, and for consistency use a similar method here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RTC_DS3231.cpp getTemperature() doesn't provide for negative temps

4 participants