From 44933ffdce139e237d837d5db4ca8839b02f33e6 Mon Sep 17 00:00:00 2001 From: chrysn Date: Tue, 1 Oct 2024 10:31:07 +0200 Subject: [PATCH] feat(device-id): Move ID data into type Previously, the type was just a never-instanciated-even-though-you-could type to put things into a trait. Now it *is* the return type, opening up for further evolution through provided methods. --- src/riot-rs-embassy-common/src/identity.rs | 61 ++++++++++++++-------- src/riot-rs-nrf/src/identity.rs | 18 ++++--- src/riot-rs-stm32/src/identity.rs | 13 +++-- src/riot-rs/src/identity.rs | 3 +- 4 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/riot-rs-embassy-common/src/identity.rs b/src/riot-rs-embassy-common/src/identity.rs index 94d92df3c..0e7ae4aeb 100644 --- a/src/riot-rs-embassy-common/src/identity.rs +++ b/src/riot-rs-embassy-common/src/identity.rs @@ -2,21 +2,23 @@ //! //! See `riot_rs::identity` for general documentation. -/// Describes how a board produces unique identifiers. -pub trait DeviceId { - /// Opaque type representing a unique-ish identifier of a board. - /// - /// See [`.get()`](Self::get) for precise semantics. - /// - /// # Open questions - /// - /// Do we have any concrete serializers we want to prescribe? Or should we switch to a const - /// size ([u8; N]) anyway? - type DeviceId: core::fmt::Debug + defmt::Format; - +/// Trait desribing the unique identifier available on a board. +/// +/// # Evolution +/// +/// In its current state, this type is mainly a wrapper around a binary identifier with a +/// length constant at build time. +/// +/// As it is used more, additional methods can be provided for concrete types of identifiers, such +/// as MAC addresses. By default, those would be generated in some way from what is available in +/// the identifier -- but boards where the identifier already *is* a MAC address (or possibly a +/// range thereof) can provide their official addresses. +pub trait DeviceId: Sized + core::fmt::Debug + defmt::Format { /// Error type indicating that no identifier is available. /// - /// This is encouraged to be [`core::convert::Infallible`] where possible. + /// This is the return type of the [`::get()`][Self::get] constructor. + /// + /// It is encouraged to be [`core::convert::Infallible`] where possible. /// /// # Open questions /// @@ -24,14 +26,16 @@ pub trait DeviceId { /// on how to report "Not yet available"? type Error: core::error::Error + defmt::Format; - /// Obtains a unique identifier of the device. + /// Some `[u8; N]` type, returned by [`.bytes()`][Self::bytes]. /// - /// A successful return value is, within the scope of the active board, reasonably unique. (The - /// "reasonably" part is what allows this to be used with devices whose vendors do not assign - /// serial numbers but large (>= 64 bits) random identifiers). + /// # Evolution /// - /// This value may be obtained from the processor itself, or from a peripheral that is connected to - /// the processor as part of the board. + /// On the long run, it will be preferable to add a `const BYTES_LEN: usize;` and enforce the + /// type `[u8; Self::BYTES_LEN]` as the return value of [`.bytes(_)]`][Self::bytes]. This can + /// not be done yet as it depends on the `generic_const_exprs` featureVg + type Bytes: AsRef<[u8]>; + + /// Obtains a unique identifier of the device. /// /// For the type implementing this trait at its conventional position /// `riot_rs::arch::identity::DeviceId`, a convenience function to call it exists at @@ -50,25 +54,36 @@ pub trait DeviceId { /// # Errors /// /// This prodcues an error if no device ID is available on this board, or is not implemented. - fn get() -> Result; + fn get() -> Result; + + /// The device identifier in serialized bytes format. + fn bytes(&self) -> Self::Bytes; } /// A type implementing [`DeviceId`] that always errs. /// /// This can be used both on architectures that do not have a unique identifier on their boards, /// and when it has not yet been implemented. +#[derive(Debug, defmt::Format)] pub struct NoDeviceId( + core::convert::Infallible, core::marker::PhantomData, ); impl DeviceId for NoDeviceId { - type DeviceId = core::convert::Infallible; - type Error = E; - fn get() -> Result { + // We could also come up with a custom never type that AsRef's into [u8], but that won't fly + // once there is a BYTES_LEN. + type Bytes = [u8; 0]; + + fn get() -> Result { Err(Default::default()) } + + fn bytes(&self) -> [u8; 0] { + match self.0 {} + } } /// Error indicating that a [`DeviceId`] may be available on this platform, but is not implemented. diff --git a/src/riot-rs-nrf/src/identity.rs b/src/riot-rs-nrf/src/identity.rs index 1623b2097..e348096df 100644 --- a/src/riot-rs-nrf/src/identity.rs +++ b/src/riot-rs-nrf/src/identity.rs @@ -1,14 +1,10 @@ -pub struct DeviceId; +#[derive(Debug, defmt::Format)] +pub struct DeviceId(u64); impl riot_rs_embassy_common::identity::DeviceId for DeviceId { - /// The DEVICEID from the FICR peripheral. - /// - /// The two-word value is interpreted as a 64-bit value as per the product specification. - type DeviceId = u64; - type Error = core::convert::Infallible; - fn get() -> Result { + fn get() -> Result { // Embassy does not wrap the FICR register, and given that all we need from there is a register // read that is perfectly fine to do through a stolen register, let's do that rather than // thread the access through several layers. @@ -23,6 +19,12 @@ impl riot_rs_embassy_common::identity::DeviceId for DeviceId { let low = ficr.deviceid[0].read().bits(); let high = ficr.deviceid[1].read().bits(); - Ok((u64::from(high) << u32::BITS) | u64::from(low)) + Ok(Self((u64::from(high) << u32::BITS) | u64::from(low))) + } + + type Bytes = [u8; 8]; + + fn bytes(&self) -> Self::Bytes { + self.0.to_le_bytes() } } diff --git a/src/riot-rs-stm32/src/identity.rs b/src/riot-rs-stm32/src/identity.rs index 89619cd3c..3ad4d6dfb 100644 --- a/src/riot-rs-stm32/src/identity.rs +++ b/src/riot-rs-stm32/src/identity.rs @@ -1,11 +1,16 @@ -pub struct DeviceId; +#[derive(Debug, defmt::Format)] +pub struct DeviceId(&'static [u8; 12]); impl riot_rs_embassy_common::identity::DeviceId for DeviceId { - type DeviceId = &'static [u8; 12]; + type Bytes = &'static [u8; 12]; type Error = core::convert::Infallible; - fn get() -> Result { - Ok(embassy_stm32::uid::uid()) + fn get() -> Result { + Ok(Self(embassy_stm32::uid::uid())) + } + + fn bytes(&self) -> Self::Bytes { + self.0 } } diff --git a/src/riot-rs/src/identity.rs b/src/riot-rs/src/identity.rs index c21cc453f..d37a0f49b 100644 --- a/src/riot-rs/src/identity.rs +++ b/src/riot-rs/src/identity.rs @@ -28,7 +28,6 @@ pub use riot_rs_embassy_common::identity::DeviceId; use crate::arch::identity::DeviceId as ArchDeviceId; /// Obtains a unique identifier of the device. -pub fn device_identity( -) -> Result<::DeviceId, ::Error> { +pub fn device_identity() -> Result::Error> { riot_rs_embassy::arch::identity::DeviceId::get() }