RTC_DS3231: correctly handle negative temperatures#303
Open
edgar-bonet wants to merge 1 commit intoadafruit:masterfrom
Open
RTC_DS3231: correctly handle negative temperatures#303edgar-bonet wants to merge 1 commit intoadafruit:masterfrom
edgar-bonet wants to merge 1 commit intoadafruit:masterfrom
Conversation
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
|
We ran into the same problem with negative temps. Do you have any idea when this will be merged? |
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. |
Member
|
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. |
|
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As reported in issue #287, the method
RTC_DS3231::getTemperature()fails on negative temperatures. This is because the conversion from raw bytes to afloat-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:
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 than1 / 4.0) obviates the need for the 6-bit shift. Also,1 / 256.0being 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.