Conversation
edgar-bonet
left a comment
There was a problem hiding this comment.
I didn't test it yet, but I fear that the various += operations are prone to overflow. Maybe the intermediate calculations should be done on 16-bit local variables.
RTClib.cpp
Outdated
| // make month valid to prevent getDaysInMonth() returning 0 | ||
| // Otherwise, infinite loop possible | ||
| if (m > 12) { | ||
| temp = m % 12; |
There was a problem hiding this comment.
This is probably meant to be m / 12
| temp = m % 12; | |
| temp = m / 12; |
RTClib.cpp
Outdated
| m++; | ||
| if (m > 12) { | ||
| yOff++; | ||
| m--; |
There was a problem hiding this comment.
This should be m = 1 or m -= 12
| m--; | |
| m -= 12; |
|
I did a small test that shows the effect of the The first one is the correct, expected behavior. The second one is incorrect: it should return 2000-01-01T04:16:00. Edit: the case when either the month or the day is zero is also mishandled: Month 0 should presumably be understood as December from the previous year, but As a side note, I find the name of the method ( |
RTClib.cpp
Outdated
| */ | ||
| /**************************************************************************/ | ||
| bool isLeapYear(uint16_t year) { | ||
| return year % 400 == 0 || (year % 4 == 0 && year % 100 != 0); |
There was a problem hiding this comment.
Simplified isLeapYear() and made it static to prevent misuse.
Fixed bug with `fixDateTime()` overflowing DateTime data members. Simplified `isLeapYear()` function for efficiency at the cost of not working for years of multiples of 100 and not multiples fo 400. Also wrote warning for limitation. Made `isleapYear()` static to avoid misuse. Rewrote warning for `fixDateTime()` to specify that the year becomes 2100 if fixing DateTime requires setting the year to before 2000.
Wrote
fixDateTime()method: attempts to fix invalid DateTime object by decrementing invalid time/date components and incrementing next components to compensate. E.g. If DateTime object represents the 32 July 2019 then runningfixDateTime()will change it to 1 August 2019.Also wrote
getDaysInMonth()andisLeapYear()functions. These are used byfixDateTime()hence the inclusion.