From 8265b09b03e169d84b8f951c7d214f9dfed8ff1c Mon Sep 17 00:00:00 2001 From: kellda <59569234+kellda@users.noreply.github.com> Date: Tue, 28 Jun 2022 14:47:30 +0200 Subject: [PATCH 1/3] Use sealed traits to prevent marker trait implementations on arbitrary types --- tm4c-hal/src/i2c.rs | 4 +- tm4c-hal/src/lib.rs | 12 ++-- tm4c-hal/src/serial.rs | 139 +++++++++++++++++++++------------------- tm4c123x-hal/src/i2c.rs | 10 +-- tm4c123x-hal/src/lib.rs | 8 +++ tm4c123x-hal/src/spi.rs | 38 +++++------ tm4c129x-hal/src/i2c.rs | 10 +-- tm4c129x-hal/src/lib.rs | 8 +++ tm4c129x-hal/src/spi.rs | 38 +++++------ 9 files changed, 148 insertions(+), 119 deletions(-) diff --git a/tm4c-hal/src/i2c.rs b/tm4c-hal/src/i2c.rs index 1c9fdc9..0cfb7d4 100644 --- a/tm4c-hal/src/i2c.rs +++ b/tm4c-hal/src/i2c.rs @@ -28,14 +28,14 @@ macro_rules! i2c_pins { sda: [$(($($sdagpio: ident)::*, $sdaaf: ident)),*], ) => { $( - unsafe impl SclPin<$I2Cn> for $($sclgpio)::*> + impl SclPin<$I2Cn> for $($sclgpio)::*> where T: OutputMode, {} )* $( - unsafe impl SdaPin<$I2Cn> for $($sdagpio)::*> + impl SdaPin<$I2Cn> for $($sdagpio)::*> where T: OutputMode, {} diff --git a/tm4c-hal/src/lib.rs b/tm4c-hal/src/lib.rs index 790e1a5..c74bfb2 100644 --- a/tm4c-hal/src/lib.rs +++ b/tm4c-hal/src/lib.rs @@ -191,6 +191,8 @@ macro_rules! gpio_macro { _mode: PhantomData, } + impl crate::Sealed for $PXi {} + impl $PXi where MODE: IsUnlocked { /// Configures the pin to serve as alternate function 1 through 15. /// Disables open-drain to make the output a push-pull. @@ -525,6 +527,8 @@ macro_rules! uart_hal_macro { ($( $UARTX:ident: ($powerDomain:ident, $uartX:ident), )+) => { + $crate::uart_traits_macro!(); + $( impl Serial<$UARTX, TX, RX, RTS, CTS> { /// Configures a UART peripheral to provide serial communication @@ -780,7 +784,7 @@ macro_rules! uart_pin_macro { tx: [$(($($txgpio: ident)::*, $txaf: ident)),*], ) => { $( - unsafe impl CtsPin<$UARTn> for $($ctsgpio)::*> + impl CtsPin<$UARTn> for $($ctsgpio)::*> where T: OutputMode, { @@ -791,7 +795,7 @@ macro_rules! uart_pin_macro { )* $( - unsafe impl RtsPin<$UARTn> for $($rtsgpio)::*> + impl RtsPin<$UARTn> for $($rtsgpio)::*> where T: OutputMode, { @@ -802,14 +806,14 @@ macro_rules! uart_pin_macro { )* $( - unsafe impl RxPin<$UARTn> for $($rxgpio)::*> + impl RxPin<$UARTn> for $($rxgpio)::*> where T: OutputMode, {} )* $( - unsafe impl TxPin<$UARTn> for $($txgpio)::*> + impl TxPin<$UARTn> for $($txgpio)::*> where T: OutputMode, {} diff --git a/tm4c-hal/src/serial.rs b/tm4c-hal/src/serial.rs index d30db1b..4b9705e 100644 --- a/tm4c-hal/src/serial.rs +++ b/tm4c-hal/src/serial.rs @@ -1,80 +1,89 @@ //! Serial code that is generic to both the TM4C123 and TM4C129, such as the pin traits. -/// TX pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait TxPin {} +// The macro is required for the "sealed trait" pattern to work: +// the traits and the gpios have to be defined in the same crate -/// RX pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait RxPin {} +///! An internal macro to generate the UART traits +#[macro_export] +macro_rules! uart_traits_macro { + () => { + /// TX pin + pub trait TxPin: crate::Sealed {} -/// CTS pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait CtsPin { - /// Enables the CTS functionality if a valid pin is given (not `()`). - fn enable(&mut self, _uart: &mut UART); -} + /// RX pin + pub trait RxPin: crate::Sealed {} -/// DCD pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait DcdPin { - /// Enables the DCD functionality if a valid pin is given (not `()`). - fn enable(&mut self, _uart: &mut UART); -} + /// CTS pin + pub trait CtsPin: crate::Sealed { + /// Enables the CTS functionality if a valid pin is given (not `()`). + fn enable(&mut self, _uart: &mut UART); + } -/// DSR pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait DsrPin { - /// Enables the DSR functionality if a valid pin is given (not `()`). - fn enable(&mut self, _uart: &mut UART); -} + /// DCD pin + pub trait DcdPin: crate::Sealed { + /// Enables the DCD functionality if a valid pin is given (not `()`). + fn enable(&mut self, _uart: &mut UART); + } -/// DTR pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait DtrPin { - /// Enables the DTR functionality if a valid pin is given (not `()`). - fn enable(&mut self, _uart: &mut UART); -} + /// DSR pin + pub trait DsrPin: crate::Sealed { + /// Enables the DSR functionality if a valid pin is given (not `()`). + fn enable(&mut self, _uart: &mut UART); + } -/// RI pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait RiPin { - /// Enables the RI functionality if a valid pin is given (not `()`). - fn enable(&mut self, _uart: &mut UART); -} + /// DTR pin + pub trait DtrPin: crate::Sealed { + /// Enables the DTR functionality if a valid pin is given (not `()`). + fn enable(&mut self, _uart: &mut UART); + } -/// RTS pin - DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait RtsPin { - /// Enables the RTS functionality if a valid pin is given (not `()`). - fn enable(&mut self, _uart: &mut UART); -} + /// RI pin + pub trait RiPin: crate::Sealed { + /// Enables the RI functionality if a valid pin is given (not `()`). + fn enable(&mut self, _uart: &mut UART); + } -unsafe impl TxPin for () {} + /// RTS pin + pub trait RtsPin: crate::Sealed { + /// Enables the RTS functionality if a valid pin is given (not `()`). + fn enable(&mut self, _uart: &mut UART); + } -unsafe impl RxPin for () {} + impl TxPin for () {} -unsafe impl CtsPin for () { - fn enable(&mut self, _uart: &mut U) { - // Do nothing - } -} -unsafe impl DcdPin for () { - fn enable(&mut self, _uart: &mut U) { - // Do nothing - } -} -unsafe impl DsrPin for () { - fn enable(&mut self, _uart: &mut U) { - // Do nothing - } -} -unsafe impl DtrPin for () { - fn enable(&mut self, _uart: &mut U) { - // Do nothing - } -} -unsafe impl RiPin for () { - fn enable(&mut self, _uart: &mut U) { - // Do nothing - } -} -unsafe impl RtsPin for () { - fn enable(&mut self, _uart: &mut U) { - // Do nothing - } + impl RxPin for () {} + + impl CtsPin for () { + fn enable(&mut self, _uart: &mut U) { + // Do nothing + } + } + impl DcdPin for () { + fn enable(&mut self, _uart: &mut U) { + // Do nothing + } + } + impl DsrPin for () { + fn enable(&mut self, _uart: &mut U) { + // Do nothing + } + } + impl DtrPin for () { + fn enable(&mut self, _uart: &mut U) { + // Do nothing + } + } + impl RiPin for () { + fn enable(&mut self, _uart: &mut U) { + // Do nothing + } + } + impl RtsPin for () { + fn enable(&mut self, _uart: &mut U) { + // Do nothing + } + } + }; } /// writeln!() emits LF chars, so this is useful diff --git a/tm4c123x-hal/src/i2c.rs b/tm4c123x-hal/src/i2c.rs index 16a8e4d..c593fb9 100644 --- a/tm4c123x-hal/src/i2c.rs +++ b/tm4c123x-hal/src/i2c.rs @@ -5,6 +5,7 @@ use crate::{ hal::blocking::i2c::{Read, Write, WriteRead}, sysctl::{self, Clocks}, time::Hertz, + Sealed, }; use cortex_m::asm::delay; @@ -21,12 +22,11 @@ pub struct I2c { pub pins: PINS, } -// FIXME these should be "closed" traits -/// SCL pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait SclPin {} +/// SCL pin +pub trait SclPin: Sealed {} -/// SDA pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait SdaPin {} +/// SDA pin +pub trait SdaPin: Sealed {} i2c_pins!(I2C0, scl: [(gpiob::PB2, AF3)], sda: [(gpiob::PB3, AF3)],); i2c_pins!(I2C1, scl: [(gpioa::PA6, AF3)], sda: [(gpioa::PA7, AF3)],); diff --git a/tm4c123x-hal/src/lib.rs b/tm4c123x-hal/src/lib.rs index 12137ba..89bed4e 100644 --- a/tm4c123x-hal/src/lib.rs +++ b/tm4c123x-hal/src/lib.rs @@ -32,6 +32,14 @@ pub use crate::tm4c123x::interrupt; use embedded_hal as hal; +use sealed::Sealed; +mod sealed { + // To prevent implementation of `*Pin` traits on arbitrary types + pub trait Sealed {} + + impl Sealed for () {} +} + pub mod gpio; pub mod hib; pub mod i2c; diff --git a/tm4c123x-hal/src/spi.rs b/tm4c123x-hal/src/spi.rs index 8f30b64..4e02b53 100644 --- a/tm4c123x-hal/src/spi.rs +++ b/tm4c123x-hal/src/spi.rs @@ -13,6 +13,7 @@ use crate::{ sysctl, sysctl::Clocks, time::Hertz, + Sealed, }; use tm4c123x::{SSI0, SSI1, SSI2, SSI3}; @@ -24,35 +25,34 @@ pub enum Error { _Extensible, } -// FIXME these should be "closed" traits -/// SCK pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait SckPin {} +/// SCK pin +pub trait SckPin: Sealed {} -/// MISO pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait MisoPin {} +/// MISO pin +pub trait MisoPin: Sealed {} -/// MOSI pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait MosiPin {} +/// MOSI pin +pub trait MosiPin: Sealed {} // SSI0 -unsafe impl SckPin for PA2> where T: OutputMode {} -unsafe impl MisoPin for PA4> where T: OutputMode {} -unsafe impl MosiPin for PA5> where T: OutputMode {} +impl SckPin for PA2> where T: OutputMode {} +impl MisoPin for PA4> where T: OutputMode {} +impl MosiPin for PA5> where T: OutputMode {} // SSI1 -unsafe impl SckPin for PD0> where T: OutputMode {} -unsafe impl MisoPin for PD2> where T: OutputMode {} -unsafe impl MosiPin for PD3> where T: OutputMode {} +impl SckPin for PD0> where T: OutputMode {} +impl MisoPin for PD2> where T: OutputMode {} +impl MosiPin for PD3> where T: OutputMode {} // SSI2 -unsafe impl SckPin for PB4> where T: OutputMode {} -unsafe impl MisoPin for PB6> where T: OutputMode {} -unsafe impl MosiPin for PB7> where T: OutputMode {} +impl SckPin for PB4> where T: OutputMode {} +impl MisoPin for PB6> where T: OutputMode {} +impl MosiPin for PB7> where T: OutputMode {} // SSI3 -unsafe impl SckPin for PD0> where T: OutputMode {} -unsafe impl MisoPin for PD2> where T: OutputMode {} -unsafe impl MosiPin for PD3> where T: OutputMode {} +impl SckPin for PD0> where T: OutputMode {} +impl MisoPin for PD2> where T: OutputMode {} +impl MosiPin for PD3> where T: OutputMode {} /// SPI peripheral operating in full duplex master mode pub struct Spi { diff --git a/tm4c129x-hal/src/i2c.rs b/tm4c129x-hal/src/i2c.rs index 4510eaf..6fcb75b 100644 --- a/tm4c129x-hal/src/i2c.rs +++ b/tm4c129x-hal/src/i2c.rs @@ -5,6 +5,7 @@ use crate::{ hal::blocking::i2c::{Read, Write, WriteRead}, sysctl::{self, Clocks}, time::Hertz, + Sealed, }; use cortex_m::asm::delay; @@ -21,12 +22,11 @@ pub struct I2c { pub pins: PINS, } -// FIXME these should be "closed" traits -/// SCL pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait SclPin {} +/// SCL pin +pub trait SclPin: Sealed {} -/// SDA pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait SdaPin {} +/// SDA pin +pub trait SdaPin: Sealed {} i2c_pins!(I2C0, scl: [(gpiob::PB2, AF2)], sda: [(gpiob::PB3, AF2)],); i2c_pins!(I2C1, scl: [(gpiog::PG0, AF2)], sda: [(gpiog::PG1, AF2)],); diff --git a/tm4c129x-hal/src/lib.rs b/tm4c129x-hal/src/lib.rs index 73a6de0..562642f 100644 --- a/tm4c129x-hal/src/lib.rs +++ b/tm4c129x-hal/src/lib.rs @@ -32,6 +32,14 @@ pub use tm4c_hal::{bb, delay, time}; #[cfg(feature = "rt")] pub use crate::tm4c129x::interrupt; +use sealed::Sealed; +mod sealed { + // To prevent implementation of `*Pin` traits on arbitrary types + pub trait Sealed {} + + impl Sealed for () {} +} + pub mod gpio; pub mod hib; pub mod i2c; diff --git a/tm4c129x-hal/src/spi.rs b/tm4c129x-hal/src/spi.rs index 01b6a79..0900e85 100644 --- a/tm4c129x-hal/src/spi.rs +++ b/tm4c129x-hal/src/spi.rs @@ -12,6 +12,7 @@ use crate::{ hal::spi::{FullDuplex, Phase, Polarity}, sysctl::{self, Clocks}, time::Hertz, + Sealed, }; use nb; @@ -24,35 +25,34 @@ pub enum Error { _Extensible, } -// FIXME these should be "closed" traits -/// SCK pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait SckPin {} +/// SCK pin +pub trait SckPin: Sealed {} -/// MISO pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait MisoPin {} +/// MISO pin +pub trait MisoPin: Sealed {} -/// MOSI pin -- DO NOT IMPLEMENT THIS TRAIT -pub unsafe trait MosiPin {} +/// MOSI pin +pub unsafe trait MosiPin: Sealed {} // SSI0 -unsafe impl SckPin for PA2> where T: OutputMode {} -unsafe impl MisoPin for PA4> where T: OutputMode {} -unsafe impl MosiPin for PA5> where T: OutputMode {} +impl SckPin for PA2> where T: OutputMode {} +impl MisoPin for PA4> where T: OutputMode {} +impl MosiPin for PA5> where T: OutputMode {} // SSI1 -unsafe impl SckPin for PD0> where T: OutputMode {} -unsafe impl MisoPin for PD2> where T: OutputMode {} -unsafe impl MosiPin for PD3> where T: OutputMode {} +impl SckPin for PD0> where T: OutputMode {} +impl MisoPin for PD2> where T: OutputMode {} +impl MosiPin for PD3> where T: OutputMode {} // SSI2 -unsafe impl SckPin for PB4> where T: OutputMode {} -unsafe impl MisoPin for PB6> where T: OutputMode {} -unsafe impl MosiPin for PB7> where T: OutputMode {} +impl SckPin for PB4> where T: OutputMode {} +impl MisoPin for PB6> where T: OutputMode {} +impl MosiPin for PB7> where T: OutputMode {} // SSI3 -unsafe impl SckPin for PD0> where T: OutputMode {} -unsafe impl MisoPin for PD2> where T: OutputMode {} -unsafe impl MosiPin for PD3> where T: OutputMode {} +impl SckPin for PD0> where T: OutputMode {} +impl MisoPin for PD2> where T: OutputMode {} +impl MosiPin for PD3> where T: OutputMode {} /// SPI peripheral operating in full duplex master mode pub struct Spi { From eaa461823ded9f91f66bdd84872aabc7070a979d Mon Sep 17 00:00:00 2001 From: kellda <59569234+kellda@users.noreply.github.com> Date: Tue, 28 Jun 2022 14:49:31 +0200 Subject: [PATCH 2/3] Do not re-export internal macros --- tm4c123x-hal/src/i2c.rs | 2 +- tm4c123x-hal/src/serial.rs | 3 ++- tm4c129x-hal/src/serial.rs | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tm4c123x-hal/src/i2c.rs b/tm4c123x-hal/src/i2c.rs index c593fb9..fb8a735 100644 --- a/tm4c123x-hal/src/i2c.rs +++ b/tm4c123x-hal/src/i2c.rs @@ -12,7 +12,7 @@ use cortex_m::asm::delay; use tm4c123x::{I2C0, I2C1, I2C2, I2C3}; pub use tm4c_hal::i2c::Error; -pub use tm4c_hal::{i2c_busy_wait, i2c_hal, i2c_pins}; +use tm4c_hal::{i2c_busy_wait, i2c_hal, i2c_pins}; /// I2C peripheral operating in master mode pub struct I2c { diff --git a/tm4c123x-hal/src/serial.rs b/tm4c123x-hal/src/serial.rs index c70eb71..65c2e16 100644 --- a/tm4c123x-hal/src/serial.rs +++ b/tm4c123x-hal/src/serial.rs @@ -4,7 +4,8 @@ #![allow(clippy::too_many_arguments)] pub use tm4c123x::{UART0, UART1, UART2, UART3, UART4, UART5, UART6, UART7}; -pub use tm4c_hal::{serial::*, uart_hal_macro, uart_pin_macro}; +pub use tm4c_hal::serial::NewlineMode; +use tm4c_hal::{uart_hal_macro, uart_pin_macro}; #[rustfmt::skip] use crate::{ diff --git a/tm4c129x-hal/src/serial.rs b/tm4c129x-hal/src/serial.rs index 6a9671c..c64bb22 100644 --- a/tm4c129x-hal/src/serial.rs +++ b/tm4c129x-hal/src/serial.rs @@ -15,7 +15,8 @@ use nb::{self, block}; use void::Void; pub use tm4c129x::{UART0, UART1, UART2, UART3, UART4, UART5, UART6, UART7}; -pub use tm4c_hal::{serial::*, uart_hal_macro, uart_pin_macro}; +pub use tm4c_hal::serial::NewlineMode; +use tm4c_hal::{uart_hal_macro, uart_pin_macro}; /// Serial abstraction pub struct Serial { From 6da1970873325b9f0136538d4b280bcc7b94c07d Mon Sep 17 00:00:00 2001 From: kellda <59569234+kellda@users.noreply.github.com> Date: Wed, 6 Jul 2022 13:22:27 +0200 Subject: [PATCH 3/3] Update changelogs --- tm4c-hal/README.md | 2 +- tm4c123x-hal/README.md | 3 ++- tm4c129x-hal/README.md | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tm4c-hal/README.md b/tm4c-hal/README.md index 9e34b7b..123f110 100644 --- a/tm4c-hal/README.md +++ b/tm4c-hal/README.md @@ -9,7 +9,7 @@ depending on your processor. ### Unreleased Changes ([Source](https://github.com/rust-embedded-community/tm4c-hal/tree/master/tm4c-hal) [Diff](https://github.com/rust-embedded-community/tm4c-hal/compare/tm4c-hal-0.4.1...master)) -* No changes +* Implement use of sealed traits by downstream crates (i.e. `tm4c123x-hal` and `tm4c129x-hal`) ### v0.4.1 ([Source](https://github.com/rust-embedded-community/tm4c-hal/tree/tm4c-hal-0.4.1/tm4c-hal) [Diff](https://github.com/rust-embedded-community/tm4c-hal/compare/tm4c-hal-0.4.1...tm4c-hal-0.4.0)) diff --git a/tm4c123x-hal/README.md b/tm4c123x-hal/README.md index c42bd6a..47f72a0 100644 --- a/tm4c123x-hal/README.md +++ b/tm4c123x-hal/README.md @@ -10,7 +10,8 @@ ### Unreleased Changes ([Source](https://github.com/rust-embedded-community/tm4c-hal/tree/master/tm4c123x-hal) [Diff](https://github.com/rust-embedded-community/tm4c-hal/compare/tm4c123x-hal-0.10.2...master)) -* No changes +* Use sealed traits for `*Pin` marker traits +* Do not reexport `tm4c-hal` macros ### v0.10.2 ([Source](https://github.com/rust-embedded-community/tm4c-hal/tree/tm4c123x-hal-0.10.2/tm4c123x-hal) [Diff](https://github.com/rust-embedded-community/tm4c-hal/compare/tm4c123x-hal-0.10.2...tm4c123x-hal-0.10.1)) diff --git a/tm4c129x-hal/README.md b/tm4c129x-hal/README.md index d370064..9311569 100644 --- a/tm4c129x-hal/README.md +++ b/tm4c129x-hal/README.md @@ -10,7 +10,8 @@ ### Unreleased Changes ([Source](https://github.com/rust-embedded-community/tm4c-hal/tree/master/tm4c129x-hal) [Diff](https://github.com/rust-embedded-community/tm4c-hal/compare/tm4c129x-hal-0.9.2...master)) -* No changes +* Use sealed traits for `*Pin` marker traits +* Do not reexport `tm4c-hal` macros ### v0.9.2 ([Source](https://github.com/rust-embedded-community/tm4c-hal/tree/tm4c129x-hal-0.9.2/tm4c129x-hal) [Diff](https://github.com/rust-embedded-community/tm4c-hal/compare/tm4c129x-hal-0.9.2...tm4c129x-hal-0.9.1))