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

Timestamp to human readable date&time string (Martin Poloch home assignment) #1535

Closed
wants to merge 2 commits into from

Conversation

MartinoPolo
Copy link

Code uses utime module which in current version offers only timezone-dependant function to convert timestamp to datetime (utime.localtime). Tested only on core emulator with computer timezone set to UTC.
The mentioned function also looses precision of the conversion with increasing timestamp. The precision is much worse than 1 second around current timestamps and with future timestamps (in more than 100 years), the precision is less than a minute and even less.
Workaround was implemented so the current precision is 1 second for the next few decades but I still suggest further inspection.

@MartinoPolo MartinoPolo requested a review from matejcik as a code owner March 18, 2021 14:45
core/src/trezor/strings.py Show resolved Hide resolved
Comment on lines +93 to +95
first_daily_timestamp = utime.mktime(date)
# calculate correction of the midnight timestamp to the origiral
correction = timestamp - first_daily_timestamp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "correction" can be calculated more simply by taking correction = timestamp % 86400.

returned date (2021, 3, 18, 0, 0, 0, 3, 77, 0)
correction 31624 (~ 8:47:04)
"""
approx_datetime = utime.localtime(timestamp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the actual device this fails with:

AttributeError: 'module' object has no attribute 'localtime'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we got this working, I suspect that it might give an incorrect time on the device, because the STM32 implementation seems to be using 2000-01-01 as the beginning of the epoch as opposed to the UNIX implementation which uses 1970-01-01. It's hard to believe, but that's what I see in the C code. I didn't actually test.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, from my limited sources, I expect the behavior you described. Sorry for not mentioning it in the PR description.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the two implementations are really using different epochs, then I have doubts whether we should be using utime at all. We would then have to have a piece of code like "If not EMULATOR, then subtract 30 years", which just seems very wrong.

Secondly, there is #741, which indicates that there may have been some opposition to adding the utime module to the firmware build.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added PR #1548 to update uPy to 1.14 - among other things it introduces utime.gmtime() function which is exactly what we need here I suppose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw #741 is not opposed to adding utime, but adding full python's datetime. At that time there was probably no date processing in utime, but now there is.


def _calculate_timestamp_correction(timestamp: int) -> (Tuple[tuple, int]):
"""
utime module can't convert timestamp to datetime with seconds precision.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this problem occurs only in the UNIX implementation, because micropython quite pointlessly converts the timestamp to a C float which has a mantissa of 23 bits, causing the lower bits to get lost. The STM32 implementation reads it as an integer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually fixed upstream.

Copy link
Member

@prusnak prusnak Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we need to merge MicroPython 1.14 in before merging in this PR - #1548

@matejcik matejcik added the blocked Blocked by external force. Third party inputs required. label Mar 26, 2021
@tsusanka tsusanka added this to the backlog milestone May 20, 2021
@matejcik matejcik added core Trezor Core firmware. Runs on Trezor Model T and T2B1. enhancement and removed blocked Blocked by external force. Third party inputs required. labels Sep 24, 2021
@matejcik
Copy link
Contributor

we now have uPy 1.17 so we are able to continue on this

apart from Bitcoin lock-time, this should be used in Stellar timebounds

@matejcik matejcik requested review from prusnak and removed request for prusnak September 24, 2021 08:22
@tsusanka tsusanka removed this from the backlog milestone Oct 6, 2021
@matejcik
Copy link
Contributor

thanks @snapyy for your contribution, it was adapted and superseded by #1877

@matejcik matejcik closed this Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants