Skip to content

Commit

Permalink
refactor(i2c): use fugit instead of a custom type for kilohertz
Browse files Browse the repository at this point in the history
  • Loading branch information
@ROMemories
ROMemories committed Sep 20, 2024
1 parent 8fd9cf4 commit 0972799
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 78 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions src/riot-rs-embassy-common/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ workspace = true

[dependencies]
defmt = { workspace = true, optional = true }
fugit = { workspace = true, optional = true }
embassy-futures = { workspace = true }
embassy-time = { workspace = true }
embedded-hal = { workspace = true }
Expand All @@ -21,6 +22,6 @@ embedded-hal-async = { workspace = true }
external-interrupts = []

## Enables I2C support.
i2c = []
i2c = ["dep:fugit"]

defmt = ["dep:defmt"]
defmt = ["dep:defmt", "fugit?/defmt"]
5 changes: 3 additions & 2 deletions ...t-rs-embassy-common/src/i2c/controller.rs → ...-embassy-common/src/i2c/controller/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use embassy_time::Duration;

pub use embedded_hal::i2c::Operation;
pub use fugit::KilohertzU32 as Kilohertz;

/// Timeout value for I2C operations.
///
Expand Down Expand Up @@ -43,10 +44,10 @@ macro_rules! impl_i2c_from_frequency_up_to {
fn from(freq: riot_rs_embassy_common::i2c::controller::Frequency) -> Self {
match freq {
riot_rs_embassy_common::i2c::controller::Frequency::_100k => {
Frequency::UpTo100k(100)
Frequency::UpTo100k($crate::i2c::controller::Kilohertz::kHz(100))
}
riot_rs_embassy_common::i2c::controller::Frequency::_400k => {
Frequency::UpTo400k(400)
Frequency::UpTo400k($crate::i2c::controller::Kilohertz::kHz(400))
}
}
}
Expand Down
14 changes: 0 additions & 14 deletions src/riot-rs-embassy-common/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,3 @@ pub use embassy_futures;
pub use embassy_time;

pub use embedded_hal_async;

/// Represents a frequency expressed in kilohertz.
// Do not implement From<u32>, we want to enforce using the constructor for instantiation.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
#[expect(non_camel_case_types)]
pub struct kHz(pub u32);

impl kHz {
/// Returns the frequency in kilohertz.
pub const fn khz(self) -> u32 {
self.0
}
}
1 change: 1 addition & 0 deletions src/riot-rs-embassy/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ defmt = [
"embassy-net?/defmt",
"embassy-time?/defmt",
"embassy-usb?/defmt",
"riot-rs-embassy-common/defmt",
"riot-rs-esp/defmt",
"riot-rs-nrf/defmt",
"riot-rs-rp/defmt",
Expand Down
49 changes: 26 additions & 23 deletions src/riot-rs-embassy/src/i2c/controller/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use embassy_sync::blocking_mutex::raw::CriticalSectionRawMutex;

use crate::arch;

pub use riot_rs_embassy_common::{i2c::controller::*, kHz};
pub use riot_rs_embassy_common::i2c::controller::*;

/// An I2C driver implementing [`embedded_hal_async::i2c::I2c`].
///
Expand All @@ -30,8 +30,8 @@ pub type I2cDevice = InnerI2cDevice<'static, CriticalSectionRawMutex, arch::i2c:
/// Assuming the architecture is only able to do 100 kHz and 400 kHz (not 250 kHz):
///
/// ```
/// # use riot_rs_embassy::{arch, i2c::controller::{highest_freq_in, kHz}};
/// let freq = const { highest_freq_in(kHz(100)..=kHz(250)) };
/// # use riot_rs_embassy::{arch, i2c::controller::{highest_freq_in, Kilohertz}};
/// let freq = const { highest_freq_in(Kilohertz::kHz(100)..=Kilohertz::kHz(250)) };
/// assert_eq!(freq, arch::i2c::controller::Frequency::_100k);
/// ```
///
Expand All @@ -40,10 +40,10 @@ pub type I2cDevice = InnerI2cDevice<'static, CriticalSectionRawMutex, arch::i2c:
/// This function is only intended to be used in a `const` context.
/// It panics if no suitable frequency can be found.
pub const fn highest_freq_in(
range: core::ops::RangeInclusive<riot_rs_embassy_common::kHz>,
range: core::ops::RangeInclusive<riot_rs_embassy_common::i2c::controller::Kilohertz>,
) -> arch::i2c::controller::Frequency {
let min = range.start().0;
let max = range.end().0;
let min = range.start().to_kHz();
let max = range.end().to_kHz();

assert!(max >= min);

Expand Down Expand Up @@ -100,27 +100,30 @@ mod tests {

#[test]
fn test_valid_highest_freq_in() {
const FREQ_0: arch::i2c::controller::Frequency = highest_freq_in(kHz(50)..=kHz(150));
const FREQ_1: arch::i2c::controller::Frequency = highest_freq_in(kHz(100)..=kHz(100));
const FREQ_2: arch::i2c::controller::Frequency = highest_freq_in(kHz(50)..=kHz(100));
const FREQ_3: arch::i2c::controller::Frequency = highest_freq_in(kHz(50)..=kHz(400));
const FREQ_4: arch::i2c::controller::Frequency = highest_freq_in(kHz(100)..=kHz(400));
const FREQ_5: arch::i2c::controller::Frequency = highest_freq_in(kHz(300)..=kHz(400));
const FREQ_6: arch::i2c::controller::Frequency = highest_freq_in(kHz(100)..=kHz(450));
const FREQ_7: arch::i2c::controller::Frequency = highest_freq_in(kHz(300)..=kHz(450));
use arch::i2c::controller::Frequency;
use riot_rs_embassy_common::i2c::controller::Kilohertz;

const FREQ_0: Frequency = highest_freq_in(Kilohertz::kHz(50)..=Kilohertz::kHz(150));
const FREQ_1: Frequency = highest_freq_in(Kilohertz::kHz(100)..=Kilohertz::kHz(100));
const FREQ_2: Frequency = highest_freq_in(Kilohertz::kHz(50)..=Kilohertz::kHz(100));
const FREQ_3: Frequency = highest_freq_in(Kilohertz::kHz(50)..=Kilohertz::kHz(400));
const FREQ_4: Frequency = highest_freq_in(Kilohertz::kHz(100)..=Kilohertz::kHz(400));
const FREQ_5: Frequency = highest_freq_in(Kilohertz::kHz(300)..=Kilohertz::kHz(400));
const FREQ_6: Frequency = highest_freq_in(Kilohertz::kHz(100)..=Kilohertz::kHz(450));
const FREQ_7: Frequency = highest_freq_in(Kilohertz::kHz(300)..=Kilohertz::kHz(450));

// The only available values in the dummy arch are 100k and 400k.
assert_eq!(FREQ_0, arch::i2c::controller::Frequency::_100k);
assert_eq!(FREQ_1, arch::i2c::controller::Frequency::_100k);
assert_eq!(FREQ_2, arch::i2c::controller::Frequency::_100k);
assert_eq!(FREQ_3, arch::i2c::controller::Frequency::_400k);
assert_eq!(FREQ_4, arch::i2c::controller::Frequency::_400k);
assert_eq!(FREQ_5, arch::i2c::controller::Frequency::_400k);
assert_eq!(FREQ_6, arch::i2c::controller::Frequency::_400k);
assert_eq!(FREQ_7, arch::i2c::controller::Frequency::_400k);
assert_eq!(FREQ_0, Frequency::_100k);
assert_eq!(FREQ_1, Frequency::_100k);
assert_eq!(FREQ_2, Frequency::_100k);
assert_eq!(FREQ_3, Frequency::_400k);
assert_eq!(FREQ_4, Frequency::_400k);
assert_eq!(FREQ_5, Frequency::_400k);
assert_eq!(FREQ_6, Frequency::_400k);
assert_eq!(FREQ_7, Frequency::_400k);

// FIXME: add another test to check when max < min
// and with
// const FREQ_0: arch::i2c::controller::Frequency = highest_freq_in(kHz(50)..=kHz(80));
// const FREQ_0: Frequency = highest_freq_in(Kilohertz::kHz(50)..=Kilohertz::kHz(80));
}
}
0 src/riot-rs-nrf/src/i2c/controller.rs → src/riot-rs-nrf/src/i2c/controller/mod.rs
View file
File renamed without changes.
34 changes: 17 additions & 17 deletions src/riot-rs-rp/src/i2c/controller.rs → src/riot-rs-rp/src/i2c/controller/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use embassy_rp::{
i2c::{InterruptHandler, SclPin, SdaPin},
peripherals, Peripheral,
};
use riot_rs_embassy_common::impl_async_i2c_for_driver_enum;
use riot_rs_embassy_common::{i2c::controller::Kilohertz, impl_async_i2c_for_driver_enum};

const KHZ_TO_HZ: u32 = 1000;

Expand All @@ -19,7 +19,7 @@ pub struct Config {
impl Default for Config {
fn default() -> Self {
Self {
frequency: Frequency::UpTo100k(100),
frequency: Frequency::UpTo100k(Kilohertz::kHz(100)),
}
}
}
Expand All @@ -32,34 +32,34 @@ impl Default for Config {
#[repr(u32)]
pub enum Frequency {
/// Standard mode.
UpTo100k(u32), // FIXME: use a range integer?
UpTo100k(Kilohertz), // FIXME: use a ranged integer?
/// Fast mode.
UpTo400k(u32), // FIXME: use a range integer?
UpTo400k(Kilohertz), // FIXME: use a ranged integer?
}

impl Frequency {
pub const fn first() -> Self {
Self::UpTo100k(1)
Self::UpTo100k(Kilohertz::kHz(1))
}

pub const fn last() -> Self {
Self::UpTo400k(400)
Self::UpTo400k(Kilohertz::kHz(400))
}

pub const fn next(self) -> Option<Self> {
match self {
Self::UpTo100k(f) => {
if f < 100 {
if f.to_kHz() < 100 {
// NOTE(no-overflow): `f` is small enough due to if condition
Some(Self::UpTo100k(f + 1))
Some(Self::UpTo100k(Kilohertz::kHz(f.to_kHz() + 1)))
} else {
Some(Self::UpTo400k(self.khz() + 1))
Some(Self::UpTo400k(Kilohertz::kHz(self.khz() + 1)))
}
}
Self::UpTo400k(f) => {
if f < 400 {
if f.to_kHz() < 400 {
// NOTE(no-overflow): `f` is small enough due to if condition
Some(Self::UpTo400k(f + 1))
Some(Self::UpTo400k(Kilohertz::kHz(f.to_kHz() + 1)))
} else {
None
}
Expand All @@ -70,27 +70,27 @@ impl Frequency {
pub const fn prev(self) -> Option<Self> {
match self {
Self::UpTo100k(f) => {
if f > 1 {
if f.to_kHz() > 1 {
// NOTE(no-overflow): `f` is large enough due to if condition
Some(Self::UpTo100k(f - 1))
Some(Self::UpTo100k(Kilohertz::kHz(f.to_kHz() - 1)))
} else {
None
}
}
Self::UpTo400k(f) => {
if f > 100 + 1 {
if f.to_kHz() > 100 + 1 {
// NOTE(no-overflow): `f` is large enough due to if condition
Some(Self::UpTo400k(f - 1))
Some(Self::UpTo400k(Kilohertz::kHz(f.to_kHz() - 1)))
} else {
Some(Self::UpTo100k(self.khz() - 1))
Some(Self::UpTo100k(Kilohertz::kHz(self.khz() - 1)))
}
}
}
}

pub const fn khz(self) -> u32 {
match self {
Self::UpTo100k(f) | Self::UpTo400k(f) => f,
Self::UpTo100k(f) | Self::UpTo400k(f) => f.to_kHz(),
}
}
}
Expand Down
36 changes: 18 additions & 18 deletions src/riot-rs-stm32/src/i2c/controller.rs → src/riot-rs-stm32/src/i2c/controller/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use embassy_stm32::{
time::Hertz,
Peripheral,
};
use riot_rs_embassy_common::impl_async_i2c_for_driver_enum;
use riot_rs_embassy_common::{i2c::controller::Kilohertz, impl_async_i2c_for_driver_enum};

/// I2C bus configuration.
#[non_exhaustive]
Expand All @@ -21,7 +21,7 @@ pub struct Config {
impl Default for Config {
fn default() -> Self {
Self {
frequency: Frequency::UpTo100k(100),
frequency: Frequency::UpTo100k(Kilohertz::kHz(100)),
sda_pullup: false,
scl_pullup: false,
}
Expand All @@ -36,34 +36,34 @@ impl Default for Config {
#[repr(u32)]
pub enum Frequency {
/// Standard mode.
UpTo100k(u32), // FIXME: use a range integer?
UpTo100k(Kilohertz), // FIXME: use a ranged integer?
/// Fast mode.
UpTo400k(u32), // FIXME: use a range integer?
UpTo400k(Kilohertz), // FIXME: use a ranged integer?
}

impl Frequency {
pub const fn first() -> Self {
Self::UpTo100k(1)
Self::UpTo100k(Kilohertz::kHz(1))
}

pub const fn last() -> Self {
Self::UpTo400k(400)
Self::UpTo400k(Kilohertz::kHz(400))
}

pub const fn next(self) -> Option<Self> {
match self {
Self::UpTo100k(f) => {
if f < 100 {
if f.to_kHz() < 100 {
// NOTE(no-overflow): `f` is small enough due to if condition
Some(Self::UpTo100k(f + 1))
Some(Self::UpTo100k(Kilohertz::kHz(f.to_kHz() + 1)))
} else {
Some(Self::UpTo400k(self.khz() + 1))
Some(Self::UpTo400k(Kilohertz::kHz(self.khz() + 1)))
}
}
Self::UpTo400k(f) => {
if f < 400 {
if f.to_kHz() < 400 {
// NOTE(no-overflow): `f` is small enough due to if condition
Some(Self::UpTo400k(f + 1))
Some(Self::UpTo400k(Kilohertz::kHz(f.to_kHz() + 1)))
} else {
None
}
Expand All @@ -74,27 +74,27 @@ impl Frequency {
pub const fn prev(self) -> Option<Self> {
match self {
Self::UpTo100k(f) => {
if f > 1 {
if f.to_kHz() > 1 {
// NOTE(no-overflow): `f` is large enough due to if condition
Some(Self::UpTo100k(f - 1))
Some(Self::UpTo100k(Kilohertz::kHz(f.to_kHz() - 1)))
} else {
None
}
}
Self::UpTo400k(f) => {
if f > 100 + 1 {
if f.to_kHz() > 100 + 1 {
// NOTE(no-overflow): `f` is large enough due to if condition
Some(Self::UpTo400k(f - 1))
Some(Self::UpTo400k(Kilohertz::kHz(f.to_kHz() - 1)))
} else {
Some(Self::UpTo100k(self.khz() - 1))
Some(Self::UpTo100k(Kilohertz::kHz(self.khz() - 1)))
}
}
}
}

pub const fn khz(self) -> u32 {
match self {
Self::UpTo100k(f) | Self::UpTo400k(f) => f,
Self::UpTo100k(f) | Self::UpTo400k(f) => f.to_kHz(),
}
}
}
Expand All @@ -104,7 +104,7 @@ riot_rs_embassy_common::impl_i2c_from_frequency_up_to!();
impl From<Frequency> for Hertz {
fn from(freq: Frequency) -> Self {
match freq {
Frequency::UpTo100k(f) | Frequency::UpTo400k(f) => Hertz::khz(f),
Frequency::UpTo100k(f) | Frequency::UpTo400k(f) => Hertz::khz(f.to_kHz()),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/i2c-controller/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use riot_rs::{
log::{debug, info},
EXIT_SUCCESS,
},
i2c::controller::{highest_freq_in, kHz, I2cDevice},
i2c::controller::{highest_freq_in, I2cDevice, Kilohertz},
};

const LIS3DH_I2C_ADDR: u8 = 0x19;
Expand All @@ -36,7 +36,7 @@ pub static I2C_BUS: once_cell::sync::OnceCell<
#[riot_rs::task(autostart, peripherals)]
async fn main(peripherals: pins::Peripherals) {
let mut i2c_config = arch::i2c::controller::Config::default();
i2c_config.frequency = const { highest_freq_in(kHz(100)..=kHz(400)) };
i2c_config.frequency = const { highest_freq_in(Kilohertz::kHz(100)..=Kilohertz::kHz(400)) };
debug!("Selected frequency: {}", i2c_config.frequency);

let i2c_bus = pins::SensorI2c::new(peripherals.i2c_sda, peripherals.i2c_scl, i2c_config);
Expand Down

0 comments on commit 0972799

Please sign in to comment.