Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTC_DS3231: correctly handle negative temperatures #303

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
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
2 participants