flipperzero-rs / flipperzero

Rust on the Flipper Zero
MIT License
459 stars 31 forks source link

Inconsistent use of `furi::time::Duration` and `core::time::Duration` #128

Open dcoles opened 5 months ago

dcoles commented 5 months ago

Currently we have some APIs that use core::time::Duration (e.g. flipperzero::furi::thread::sleep) and some that use flipperzero::furi::time::Duration (e.g. flipperzero::gpio::i2c). This isn't great since it leads to a rather confusing situation where you have two incompatible Duration implementations in scope.

My understanding that the main reason that flipperzero::furi::time exists is that there is no Instant implementation in core::time and to better align with the kernel's tick resolution.

A quick comparison:

core::time::Duration flipperzero::furi::time::Duration flipperzero::furi::time::Instant
Representation { secs: u64, nanos: u32 } { ticks: u32 } { ticks: u32 }
Size 12 bytes 4 bytes 4 bytes
Limits ~5.8¹¹ years ~49.7 days[^1] ± ~24.9 days[^1]
Resolution 1 ns 1 ms[^1] 1 ms[^1]

Consumers:

Function Limits Resolution
furi_delay_us(u32) ~1.2 hours 1 μs
furi_delay_ms(u32) ~49.7 days 1 ms
furi_delay_tick(u32) ~49.7 days[^1] 1 ms[^1]
furi_hal_i2c_tx(..., u32) ~49.7 days 1 ms

[^1]: Assuming the default configTICK_RATE_HZ_RAW = 1000

dcoles commented 5 months ago

Having a Furi specific Duration/Instant type that works on ticks seems to be quite useful, especially for some of the more low-level APIs. I wonder if we want to rename these FuriDuration and FuriInstant and provide conversion functions (e.g. FuriDuration::try_from(Duration)).