-
Notifications
You must be signed in to change notification settings - Fork 5
Fix getTime() timezone #28
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
base: main
Are you sure you want to change the base?
Conversation
Memory usage change @ 3b31d79
Click for full report table
Click for full report CSV
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but please add a comment here and mention that this function will try to sync with an NTP server to get the current time.
@pennam Actually this is already implemented in the function |
@pennam We would need to change |
To keep backward compatibility I've added a flag to getCellularTime() adding the possibility for the user the get localtime or utc time 4b7a446 I'm splitting PR #29 to reduce the scope and make it more simple to review |
To avoid duplication of NTPSync code we can add a private static method that only takes care of the NTPsync call and is called by both getTime() and getCellularTime(). |
@sebromero please take a look at #32 🙂 |
with this changes we ansure getTime is always using UTC timezone and we can check NTP is synced comparing output with UNIX epoch.