From 4760615fc94359bb9edb09ae5d89aa406de1bbec Mon Sep 17 00:00:00 2001 From: Stephen Waits Date: Tue, 7 Apr 2026 09:48:25 -0600 Subject: [PATCH] refactor: LoRaBoard trait with full associated types + BoardParts - LoRaBoard trait now has associated types: RadioParts, CommParts, DisplayParts, DisplayDriver - into_parts() is on the trait (enforced by compiler, not implicit convention) - BoardParts struct replaces tuple return from into_parts() - mac_address() stays on trait, MAC also in BoardParts for convenience - Removed mac field from DisplayParts (display task calls Board::mac_address()) - DisplayDriver associated type requires DrawTarget --- firmware/src/board/esp32s3.rs | 1 - firmware/src/board/heltec_v3.rs | 31 ++++++++------ firmware/src/board/heltec_v4.rs | 32 ++++++++------ firmware/src/board/rak_wisblock_4631.rs | 57 ++++++++++++++----------- firmware/src/board/traits.rs | 43 +++++++++++++------ firmware/src/board/wio_tracker_l1.rs | 51 ++++++++++++---------- firmware/src/display.rs | 2 +- firmware/src/main.rs | 15 +++---- 8 files changed, 134 insertions(+), 98 deletions(-) diff --git a/firmware/src/board/esp32s3.rs b/firmware/src/board/esp32s3.rs index dfb6d59a..34c2670b 100644 --- a/firmware/src/board/esp32s3.rs +++ b/firmware/src/board/esp32s3.rs @@ -44,7 +44,6 @@ pub struct UartParts { pub struct DisplayParts { pub i2c: DisplayI2c, - pub mac: [u8; 6], } // ── Display driver ────────────────────────────────────────────────── diff --git a/firmware/src/board/heltec_v3.rs b/firmware/src/board/heltec_v3.rs index 8b717262..2e506ddc 100644 --- a/firmware/src/board/heltec_v3.rs +++ b/firmware/src/board/heltec_v3.rs @@ -1,17 +1,15 @@ use esp_hal::gpio::{Level, Output, OutputConfig}; use super::esp32s3; -use super::traits::LoRaBoard; +use super::traits::{BoardParts, LoRaBoard}; use crate::hal::esp32s3 as mcu; -#[allow(unused_imports)] // Re-exported for other modules (display, radio, uart tasks) +#[allow(unused_imports)] // Re-exported for other modules (display, radio, host tasks) pub use super::esp32s3::{ create_display, DisplayDriver, DisplayI2c, DisplayParts, RadioDriver, RadioParts, UartDriver, UartParts, }; -// ── Board init ────────────────────────────────────────────────────── - pub struct Board { p: esp_hal::peripherals::Peripherals, } @@ -20,6 +18,11 @@ impl LoRaBoard for Board { const NAME: &'static str = "Heltec V3"; const TX_POWER_RANGE: (i8, i8) = (-9, 22); + type RadioParts = RadioParts; + type CommParts = UartParts; + type DisplayParts = DisplayParts; + type DisplayDriver = DisplayDriver; + fn init() -> Self { let p = esp_hal::init(esp_hal::Config::default()); Self { p } @@ -28,21 +31,19 @@ impl LoRaBoard for Board { fn mac_address() -> [u8; 6] { mcu::mac_address() } -} -impl Board { - pub fn into_parts(self) -> (RadioParts, UartParts, Option) { + fn into_parts(self) -> BoardParts { let p = self.p; mcu::start_timer(p.TIMG0); // Vext power: GPIO36, active LOW to enable peripherals let vext = Output::new(p.GPIO36, Level::Low, OutputConfig::default()); - core::mem::forget(vext); // hold pin low permanently; drop would reset it + core::mem::forget(vext); let spi_bus = mcu::init_spi(p.SPI2, p.DMA_CH0, p.GPIO9, p.GPIO10, p.GPIO11); let radio = esp32s3::init_radio(spi_bus, p.GPIO8, p.GPIO12, p.GPIO14, p.GPIO13); - let uart = UartParts { + let host = UartParts { driver: mcu::init_uart(p.UART0, p.GPIO43, p.GPIO44), }; @@ -51,12 +52,16 @@ impl Board { esp_hal::delay::Delay::new().delay_millis(10); display_rst.set_high(); esp_hal::delay::Delay::new().delay_millis(10); - core::mem::forget(display_rst); // hold reset high permanently + core::mem::forget(display_rst); let i2c = mcu::init_i2c(p.I2C0, p.GPIO17, p.GPIO18); - let mac = Self::mac_address(); - let display = Some(DisplayParts { i2c, mac }); + let display = Some(DisplayParts { i2c }); - (radio, uart, display) + BoardParts { + radio, + host, + display, + mac: Self::mac_address(), + } } } diff --git a/firmware/src/board/heltec_v4.rs b/firmware/src/board/heltec_v4.rs index 607e90be..e51c7e3d 100644 --- a/firmware/src/board/heltec_v4.rs +++ b/firmware/src/board/heltec_v4.rs @@ -1,17 +1,15 @@ use esp_hal::gpio::{Level, Output, OutputConfig}; use super::esp32s3; -use super::traits::LoRaBoard; +use super::traits::{BoardParts, LoRaBoard}; use crate::hal::esp32s3 as mcu; -#[allow(unused_imports)] // Re-exported for other modules (display, radio, usb tasks) +#[allow(unused_imports)] // Re-exported for other modules (display, radio, host tasks) pub use super::esp32s3::{ create_display, DisplayDriver, DisplayI2c, DisplayParts, RadioDriver, RadioParts, UsbDriver, UsbParts, }; -// ── Board implementation ──────────────────────────────────────────── - pub struct Board { p: esp_hal::peripherals::Peripherals, } @@ -20,6 +18,11 @@ impl LoRaBoard for Board { const NAME: &'static str = "Heltec V4"; const TX_POWER_RANGE: (i8, i8) = (-9, 22); + type RadioParts = RadioParts; + type CommParts = UsbParts; + type DisplayParts = DisplayParts; + type DisplayDriver = DisplayDriver; + fn init() -> Self { let p = esp_hal::init(esp_hal::Config::default()); Self { p } @@ -28,24 +31,21 @@ impl LoRaBoard for Board { fn mac_address() -> [u8; 6] { mcu::mac_address() } -} -impl Board { - pub fn into_parts(self) -> (RadioParts, UsbParts, Option) { + fn into_parts(self) -> BoardParts { let p = self.p; mcu::start_timer(p.TIMG0); // Vext power: GPIO36, active LOW to enable peripherals let vext = Output::new(p.GPIO36, Level::Low, OutputConfig::default()); - core::mem::forget(vext); // hold pin low permanently; drop would reset it + core::mem::forget(vext); let spi_bus = mcu::init_spi(p.SPI2, p.DMA_CH0, p.GPIO9, p.GPIO10, p.GPIO11); let radio = esp32s3::init_radio(spi_bus, p.GPIO8, p.GPIO12, p.GPIO14, p.GPIO13); // Note: switches internal USB PHY from Serial-JTAG to OTG. - // espflash --monitor will stop working after this point. - let usb = UsbParts { + let host = UsbParts { driver: mcu::init_usb(p.USB0, p.GPIO20, p.GPIO19), }; @@ -54,12 +54,16 @@ impl Board { esp_hal::delay::Delay::new().delay_millis(10); display_rst.set_high(); esp_hal::delay::Delay::new().delay_millis(10); - core::mem::forget(display_rst); // hold reset high permanently + core::mem::forget(display_rst); let i2c = mcu::init_i2c(p.I2C0, p.GPIO17, p.GPIO18); - let mac = Self::mac_address(); - let display = Some(DisplayParts { i2c, mac }); + let display = Some(DisplayParts { i2c }); - (radio, usb, display) + BoardParts { + radio, + host, + display, + mac: Self::mac_address(), + } } } diff --git a/firmware/src/board/rak_wisblock_4631.rs b/firmware/src/board/rak_wisblock_4631.rs index e9808fe4..561a519a 100644 --- a/firmware/src/board/rak_wisblock_4631.rs +++ b/firmware/src/board/rak_wisblock_4631.rs @@ -9,7 +9,7 @@ use embassy_time::Delay; use lora_phy::iv::GenericSx126xInterfaceVariant; use lora_phy::sx126x::{self, Sx1262, Sx126x}; -use super::traits::LoRaBoard; +use super::traits::{BoardParts, LoRaBoard}; use crate::hal::nrf52840 as mcu; // ── Concrete peripheral types ─────────────────────────────────────── @@ -27,6 +27,23 @@ pub type DisplayDriver = ssd1306::Ssd1306Async< ssd1306::mode::BufferedGraphicsModeAsync, >; +// ── Peripheral bundles ────────────────────────────────────────────── + +pub struct RadioParts { + pub driver: RadioDriver, + pub delay: Delay, +} + +pub struct UsbParts { + pub driver: UsbDriver, +} + +pub struct DisplayParts { + pub i2c: DisplayI2c, +} + +// ── Display init ──────────────────────────────────────────────────── + pub async fn create_display(i2c: DisplayI2c) -> Option { use ssd1306::mode::DisplayConfigAsync; use ssd1306::prelude::{Brightness, DisplayRotation}; @@ -46,22 +63,6 @@ pub async fn create_display(i2c: DisplayI2c) -> Option { Some(display) } -// ── Peripheral bundles ────────────────────────────────────────────── - -pub struct RadioParts { - pub driver: RadioDriver, - pub delay: Delay, -} - -pub struct UsbParts { - pub driver: UsbDriver, -} - -pub struct DisplayParts { - pub i2c: DisplayI2c, - pub mac: [u8; 6], -} - // ── Board init ────────────────────────────────────────────────────── pub struct Board { @@ -72,6 +73,11 @@ impl LoRaBoard for Board { const NAME: &'static str = "RAK WisBlock 4631"; const TX_POWER_RANGE: (i8, i8) = (-9, 22); + type RadioParts = RadioParts; + type CommParts = UsbParts; + type DisplayParts = DisplayParts; + type DisplayDriver = DisplayDriver; + fn init() -> Self { let p = embassy_nrf::init(Default::default()); Self { p } @@ -80,10 +86,8 @@ impl LoRaBoard for Board { fn mac_address() -> [u8; 6] { mcu::mac_address() } -} -impl Board { - pub fn into_parts(self) -> (RadioParts, UsbParts, Option) { + fn into_parts(self) -> BoardParts { let p = self.p; // ── SPI bus for SX1262 ────────────────────────────────── @@ -95,7 +99,6 @@ impl Board { let nss = Output::new(p.P1_10, Level::High, OutputDrive::Standard); let spi_device = SpiDevice::new(spi_bus, nss); - // ── SX1262 control pins ───────────────────────────────── let reset = Output::new(p.P1_06, Level::High, OutputDrive::Standard); let dio1 = Input::new(p.P1_15, Pull::Down); let busy = Input::new(p.P1_14, Pull::Down); @@ -117,7 +120,7 @@ impl Board { // ── USB ───────────────────────────────────────────────── let vbus = mcu::alloc_vbus_detect(true, false); - let usb = UsbParts { + let host = UsbParts { driver: Driver::new(p.USBD, mcu::Irqs, vbus), }; @@ -126,9 +129,13 @@ impl Board { let i2c = embassy_nrf::twim::Twim::new( p.TWISPI0, mcu::Irqs, p.P0_13, p.P0_14, twim::Config::default(), twim_buf, ); - let mac = Self::mac_address(); - let display = Some(DisplayParts { i2c, mac }); + let display = Some(DisplayParts { i2c }); - (radio, usb, display) + BoardParts { + radio, + host, + display, + mac: Self::mac_address(), + } } } diff --git a/firmware/src/board/traits.rs b/firmware/src/board/traits.rs index f1f61142..0f6d8ffb 100644 --- a/firmware/src/board/traits.rs +++ b/firmware/src/board/traits.rs @@ -1,31 +1,48 @@ //! Board abstraction trait. //! -//! Every board must implement [`LoRaBoard`]. The compiler enforces completeness — -//! missing constants or methods are compile errors, not runtime surprises. -//! -//! Board modules also export concrete types (`RadioParts`, `UsbParts`, -//! `DisplayParts`, `RadioDriver`, `UsbDriver`, `DisplayI2c`) that the tasks -//! use directly. The trait verifies metadata and initialization; the types -//! verify hardware compatibility. +//! Every board must implement [`LoRaBoard`] with concrete associated types +//! for its radio, host communication, and display peripherals. The compiler +//! enforces completeness — missing types or methods are compile errors. //! //! See `src/board/PORTING.md` for a step-by-step guide to adding a new board. +use embedded_graphics::draw_target::DrawTarget; +use embedded_graphics::pixelcolor::BinaryColor; + /// Compile-time contract for a DongLoRa board. -/// -/// Every board's `Board` struct must implement this trait. It verifies that -/// the board provides all required metadata and initialization methods. -/// The tasks use the board's concrete types directly (not the trait's -/// associated types) because Embassy tasks must be concrete. -pub trait LoRaBoard { +pub trait LoRaBoard: Sized { /// Human-readable board name (shown on display splash screen). const NAME: &'static str; /// TX power range in dBm (min, max) for this board's radio + PA. const TX_POWER_RANGE: (i8, i8); + /// Radio peripheral bundle (driver + delay). + type RadioParts; + + /// Host communication peripheral bundle (USB or UART driver). + type CommParts; + + /// Display peripheral bundle (I2C bus for display init). + type DisplayParts; + + /// Concrete display driver type (must implement DrawTarget for rendering). + type DisplayDriver: DrawTarget; + /// Initialize the board hardware. fn init() -> Self; /// Read the board's unique hardware address (MAC, device ID, etc.). fn mac_address() -> [u8; 6]; + + /// Decompose initialized board into peripheral bundles for each task. + fn into_parts(self) -> BoardParts; +} + +/// Peripheral bundles for each firmware task, produced by [`LoRaBoard::into_parts`]. +pub struct BoardParts { + pub radio: R, + pub host: C, + pub display: Option, + pub mac: [u8; 6], } diff --git a/firmware/src/board/wio_tracker_l1.rs b/firmware/src/board/wio_tracker_l1.rs index b1ac908b..13b181c2 100644 --- a/firmware/src/board/wio_tracker_l1.rs +++ b/firmware/src/board/wio_tracker_l1.rs @@ -9,7 +9,7 @@ use embassy_time::Delay; use lora_phy::iv::GenericSx126xInterfaceVariant; use lora_phy::sx126x::{self, Sx1262, Sx126x}; -use super::traits::LoRaBoard; +use super::traits::{BoardParts, LoRaBoard}; use crate::hal::nrf52840 as mcu; // ── Concrete peripheral types ─────────────────────────────────────── @@ -23,18 +23,6 @@ pub type DisplayI2c = mcu::I2cBus; pub type DisplayDriver = crate::driver::sh1106::Sh1106; -pub async fn create_display(i2c: DisplayI2c) -> Option { - let mut display = crate::driver::sh1106::Sh1106::new(i2c, 0x3D); - - embassy_time::Timer::after_millis(100).await; - if display.init().await.is_err() { - defmt::error!("SH1106 display init failed"); - return None; - } - let _ = display.set_brightness(0xFF).await; - Some(display) -} - // ── Peripheral bundles ────────────────────────────────────────────── pub struct RadioParts { @@ -48,7 +36,20 @@ pub struct UsbParts { pub struct DisplayParts { pub i2c: DisplayI2c, - pub mac: [u8; 6], +} + +// ── Display init ──────────────────────────────────────────────────── + +pub async fn create_display(i2c: DisplayI2c) -> Option { + let mut display = crate::driver::sh1106::Sh1106::new(i2c, 0x3D); + + embassy_time::Timer::after_millis(100).await; + if display.init().await.is_err() { + defmt::error!("SH1106 display init failed"); + return None; + } + let _ = display.set_brightness(0xFF).await; + Some(display) } // ── Board init ────────────────────────────────────────────────────── @@ -61,6 +62,11 @@ impl LoRaBoard for Board { const NAME: &'static str = "Wio Tracker L1"; const TX_POWER_RANGE: (i8, i8) = (-9, 22); + type RadioParts = RadioParts; + type CommParts = UsbParts; + type DisplayParts = DisplayParts; + type DisplayDriver = DisplayDriver; + fn init() -> Self { let mut config = embassy_nrf::config::Config::default(); config.hfclk_source = embassy_nrf::config::HfclkSource::ExternalXtal; @@ -72,10 +78,8 @@ impl LoRaBoard for Board { fn mac_address() -> [u8; 6] { mcu::mac_address() } -} -impl Board { - pub fn into_parts(self) -> (RadioParts, UsbParts, Option) { + fn into_parts(self) -> BoardParts { let p = self.p; // ── SPI bus for SX1262 ────────────────────────────────── @@ -87,7 +91,6 @@ impl Board { let nss = Output::new(p.P1_14, Level::High, OutputDrive::Standard); let spi_device = SpiDevice::new(spi_bus, nss); - // ── SX1262 control pins ───────────────────────────────── let reset = Output::new(p.P1_07, Level::High, OutputDrive::Standard); let dio1 = Input::new(p.P0_07, Pull::Down); let busy = Input::new(p.P1_10, Pull::Down); @@ -111,7 +114,7 @@ impl Board { // ── USB ───────────────────────────────────────────────── let vbus = mcu::alloc_vbus_detect(true, true); - let usb = UsbParts { + let host = UsbParts { driver: Driver::new(p.USBD, mcu::Irqs, vbus), }; @@ -124,9 +127,13 @@ impl Board { let i2c = embassy_nrf::twim::Twim::new( p.TWISPI0, mcu::Irqs, p.P0_06, p.P0_05, twim::Config::default(), twim_buf, ); - let mac = Self::mac_address(); - let display = Some(DisplayParts { i2c, mac }); + let display = Some(DisplayParts { i2c }); - (radio, usb, display) + BoardParts { + radio, + host, + display, + mac: Self::mac_address(), + } } } diff --git a/firmware/src/display.rs b/firmware/src/display.rs index 998daa30..8e064cae 100644 --- a/firmware/src/display.rs +++ b/firmware/src/display.rs @@ -108,7 +108,7 @@ pub async fn display_task( ) { // Format MAC address as "XX:XX:XX:XX:XX:XX" let mut mac_str: heapless::String<18> = heapless::String::new(); - let m = parts.mac; + let m = Board::mac_address(); let _ = core::fmt::Write::write_fmt( &mut mac_str, format_args!( diff --git a/firmware/src/main.rs b/firmware/src/main.rs index 62479bbd..02c1aaa4 100644 --- a/firmware/src/main.rs +++ b/firmware/src/main.rs @@ -77,27 +77,24 @@ cfg_if::cfg_if! { #[cfg(not(test))] async fn run(spawner: Spawner) { let board = ::init(); - let mac = board::Board::mac_address(); - let (radio, comm, display) = board.into_parts(); - - let has_display = display.is_some(); + let parts = board.into_parts(); spawner - .spawn(radio::radio_task(radio, &COMMANDS, &RESPONSES, &STATUS)) + .spawn(radio::radio_task(parts.radio, &COMMANDS, &RESPONSES, &STATUS)) .expect("spawn radio_task"); spawner .spawn(host::host_task( - comm, + parts.host, &COMMANDS, &RESPONSES, &DISPLAY_COMMANDS, - has_display, - mac, + parts.display.is_some(), + parts.mac, )) .expect("spawn host_task"); - if let Some(dp) = display { + if let Some(dp) = parts.display { spawner .spawn(display::display_task(dp, &STATUS, &DISPLAY_COMMANDS)) .expect("spawn display_task");