-
-
Notifications
You must be signed in to change notification settings - Fork 345
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
Fix and expand DateTime class, address 64-bit time_t issues #2762
Merged
Conversation
This file contains 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
Correct implementation available in newlib.
Doesn't account for century
"Seconds, minutes, hours and days may be any value, e.g. to calculate the value for 300 days since 1970 (epoch), set day=300" This requires a more appropriate parameter type (int) and casting so calculations are 64-bit.
Results are not always correct. e.g. Out by a whole day for `0x66152e16`. Code in `gmtime_r` markedly different, and not bloaty.
Windows does have `gmtime` but doesn't behave like glibc/newlib does. Safest to just copy the code from newlib. https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/gmtime_r.c
Generate test data using python script Check dates before 1970 Include 64-bit-only tests
PR Summary
Please note, all the new and modified functionalities aim to enhance the time tracking and calculation aspects of the application, while increasing the productivity by automating several processes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR improves test coverage for the
DateTime
class and adds a number of necessary fixes. It also adds some obvious omissions.Compare day/month names without case-sensitivity
Allows more use cases when converting strings.
Ensure host builds (linux) use 64-bit
time_t
Defaults to 32-bit if not specified by
_TIME_BITS
.Now consistent with embedded toolchains which use fixed size (all but IDF 4.x are 64-bit).
See issue #2758.
Document and add static check for time_t range
CI will fail build if incorrect.
For legacy toolchains (Windows host, IDF 4.x) will fail if it ever gets fixed as a reminder.
mktime
broken by axtls. Correct implementation available in newlib.Calls esp8266 ROM function which doesn't support 64-bit time_t.
Fix
isLeapYear
implementationDoesn't account for century.
Add enumeration for Month value
For backward compatibility methods use standard types, but makes code easier to read and avoids off-by-one errors in application code.
Fix
toUnixTime
to accommodate negative valuestime_t
can represent dates before 1 Jan 1970.Fix
toUnixTime
parameter handling outside normal rangeComment reads: "Seconds, minutes, hours and days may be any value, e.g. to calculate the value for 300 days since 1970 (epoch), set day=300"
This requires a more appropriate parameter type (int) and casting so calculations are 64-bit.
Apply
const
to DateTime methodsAs appropriate.
Add
fromISO8601
methodComplements
toISO8601
. Update Basic_DateTime sample so strings can be interpreted.Expand HTTP string conversion
Various RFC versions suggest more relaxed interpretation of strings.
Accept e.g. "Sun" or "Sunday". Also "Sunasdlfkj" but do we care?
Make leading day of week optional, since the value gets discarded anyway
Fold whitespace
Fix
setTime
,fromUnixTime
methodsResults are not always correct. e.g. Out by a whole day for
0x66152e16
.Code in
gmtime_r
markedly different, and not bloaty, so just use that.Provide
gmtime_r
implementation for Windows host buildsWindows does have
gmtime
but doesn't behave like glibc/newlib does.Safest to just copy the code from newlib.
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/time/gmtime_r.c
Use DateTime to produce default IFS TimeStamp string
Just output UTC, localising will likely be wrong.
Include milliseconds with '%T' format if non-zero
Consistent with ISO time strings.
This is different from libC
strftime
behaviour, but thenstruct tm
doesn't have a fractional seconds field.Update tests
Generate test data using python script
Check dates before 1970
Include 64-bit-only tests
Verify
setTime
calls with out-of-range offsetsPull out some utility methods. Some internal functions could be useful so add those as static methods:
bool isLeapYear(uint16_t year);
uint8_t getMonthDays(uint8_t month, uint16_t year);
String getLocaleDayName(uint8_t day);
String getLocaleMonthName(uint8_t month);
String getIsoDayName(uint8_t day);
String getIsoMonthName(uint8_t month);
uint16_t getDaysInYear(uint16_t year);
Replace helper macros with inline functions