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

Fix and expand DateTime class, address 64-bit time_t issues #2762

Merged
merged 22 commits into from
Apr 12, 2024

Conversation

mikee47
Copy link
Contributor

@mikee47 mikee47 commented Apr 12, 2024

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 implementation

Doesn'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 values

time_t can represent dates before 1 Jan 1970.

Fix toUnixTime parameter handling outside normal range

Comment 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 methods

As appropriate.

Add fromISO8601 method

Complements 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 methods

Results 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 builds

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

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 then struct 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 offsets

Pull 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

mikee47 added 22 commits April 10, 2024 16:05
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
Copy link

what-the-diff bot commented Apr 12, 2024

PR Summary

  • New Files Added

    • Sming/Arch/Host/Components/libc/gmtime_r.c and Sming/Arch/Host/Components/libc/include/time.h have been added. These files help with time calculations and formatting.
    • Test data for date and time operations has been created in the new tests/HostTests/include/DateTimeData.h file. A new directory tests/HostTests was established for us to better verify our code's functionality.
    • A script called datetime-test.py has been added to automate testing of date and time operations.
  • Changes to Existing Files

    • The compiler instructions in Sming/Arch/Host/build.mk now includes the -D_TIME_BITS=64 flag, allowing more precise time tracking.
    • The time.c file in replacements has been updated to improve our time-value operations, making them more reliable and efficient.
    • The DateTime.h file in Sming/Core now contains additional functions and parameters for date and time tracking, and enhancements to existing functions. This provides broader functionality around date and time operations.
    • Updated documentation in the README.rst file in docs/source/upgrading to inform users about the 64-bit time_t implementation and the PartitionStream breaking change.
    • Enhanced user interaction in samples/Basic_DateTime/app/application.cpp by adding a new function showTime().
    • DateTime.cpp has been improved to work with the FlashString and DateTimeData functionality, which enhances performance and storage utilization.

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.

@slaff slaff added this to the 5.2.0 milestone Apr 12, 2024
@slaff slaff merged commit 3c39c29 into SmingHub:develop Apr 12, 2024
46 checks passed
@mikee47 mikee47 deleted the feature/DateTime-ISO branch April 12, 2024 11:46
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.

2 participants