Skip to content

Conversation

@Ctru14
Copy link

@Ctru14 Ctru14 commented Jan 14, 2026

Add conversion implementations between Datetime and u64 by wrapping the to/from_unix_time_seconds() methods

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds trait implementations to enable conversions between Datetime and u64 types using Rust's From and Into traits, wrapping the existing from_unix_time_seconds() and to_unix_time_seconds() methods.

Changes:

  • Adds From<u64> for Datetime implementation for converting Unix timestamps to datetime objects
  • Adds From<Datetime> for u64 implementation for converting datetime objects to Unix timestamps

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}
}

impl From<Datetime> for u64 {
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 a lossy operation, so I'm not sure if we want to allow this as an implicit operation - it truncates any sub-second component. Explicitly calling a 'to_seconds' method makes it pretty clear that this is happening, but if an into() can do it, that's pretty easy to do accidentally / miss at the call site.

We're not currently using sub-second precision on imxrt, but we could conceivably use it on other platforms for e.g. millisecond support on an ACPI time and alarm device. Why do we want to do this?

Copy link
Author

Choose a reason for hiding this comment

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

OpenDevicePartnership/embassy-imxrt#542 (comment) this option came up for a Datetime set API

Copy link
Author

Choose a reason for hiding this comment

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

But fair point, while one method could be made cleaner the tradeoff to lose clarity may not be worth it

@Ctru14 Ctru14 closed this Jan 15, 2026
@felipebalbi felipebalbi reopened this Jan 15, 2026
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.

3 participants