From c145065642ead28687876f2b7da638f665c31c08 Mon Sep 17 00:00:00 2001 From: Stephen Waits Date: Tue, 7 Apr 2026 10:47:03 -0600 Subject: [PATCH] feat: Heltec V3 native USB, remove all dead_code suppressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - heltec_v3 now uses USB OTG (requires hardware mod: solder R29/R3) - heltec_v3_uart added as fallback for stock boards - Removed every #[allow(dead_code)] — fixed root causes instead: - Comm types (UsbParts/UartParts) moved to individual board files - USB/UART init removed from hal (boards call esp-hal directly) - SH1106 driver gated on boards that use it - UART task sends DisplayCommand::Reset at startup (variant now used) - ErrorCode::CrcError removed (SX1262 drops bad-CRC silently) - check/clippy now use --release (no more Xtensa dev-mode warnings) - Updated README with hardware mod docs and board table --- firmware/justfile | 4 +-- firmware/src/board/esp32s3.rs | 16 ++-------- firmware/src/board/heltec_v3.rs | 21 ++++++++++--- firmware/src/board/heltec_v3_uart.rs | 18 ++++++++--- firmware/src/board/heltec_v4.rs | 20 +++++++++--- firmware/src/channel.rs | 1 - firmware/src/driver/mod.rs | 2 +- firmware/src/hal/esp32s3.rs | 47 ++-------------------------- firmware/src/host/uart.rs | 7 ++++- firmware/src/protocol.rs | 4 +-- 10 files changed, 61 insertions(+), 79 deletions(-) diff --git a/firmware/justfile b/firmware/justfile index cb60c4d2..34880717 100644 --- a/firmware/justfile +++ b/firmware/justfile @@ -57,11 +57,11 @@ check-all: # Check a single board compiles check board: - @just _cargo {{board}} check + @just _cargo {{board}} "check --release" # Run clippy on a single board clippy board: - @just _cargo {{board}} "clippy" "-- -D warnings" + @just _cargo {{board}} "clippy --release" "-- -D warnings" # Build release firmware and copy to builds/ with a readable name build board profile="release": diff --git a/firmware/src/board/esp32s3.rs b/firmware/src/board/esp32s3.rs index f6b522eb..1a1cda5c 100644 --- a/firmware/src/board/esp32s3.rs +++ b/firmware/src/board/esp32s3.rs @@ -1,7 +1,8 @@ //! Shared types and peripheral wiring for ESP32-S3 boards. //! //! Delegates MCU-level init to `hal::esp32s3` and defines peripheral-level -//! types (RadioDriver, Parts structs) used by ESP32-S3 board files. +//! types (RadioDriver, display) used by ESP32-S3 board files. +//! Each board defines its own comm types (UsbParts/UartParts) directly. use embassy_embedded_hal::shared_bus::asynch::spi::SpiDevice; use embassy_sync::blocking_mutex::raw::NoopRawMutex; @@ -15,8 +16,7 @@ use crate::hal::esp32s3 as mcu; // ── Re-export MCU types ───────────────────────────────────────────── -#[allow(unused_imports)] // Each board uses one of USB/UART, not both -pub use mcu::{I2cBus as DisplayI2c, SpiBus, UartDriver, UsbOtgDriver as UsbDriver}; +pub use mcu::{I2cBus as DisplayI2c, SpiBus}; // ── Concrete peripheral types ─────────────────────────────────────── @@ -32,16 +32,6 @@ pub struct RadioParts { pub delay: Delay, } -#[allow(dead_code)] // Not used by heltec_v3_uart -pub struct UsbParts { - pub driver: UsbDriver, -} - -#[allow(dead_code)] // Only used by heltec_v3_uart -pub struct UartParts { - pub driver: UartDriver, -} - pub struct DisplayParts { pub i2c: DisplayI2c, } diff --git a/firmware/src/board/heltec_v3.rs b/firmware/src/board/heltec_v3.rs index 005bc8db..b84d134a 100644 --- a/firmware/src/board/heltec_v3.rs +++ b/firmware/src/board/heltec_v3.rs @@ -7,17 +7,22 @@ //! For stock (unmodified) V3 boards, use the `heltec_v3_uart` feature instead. use esp_hal::gpio::{Level, Output, OutputConfig}; +use static_cell::StaticCell; use super::esp32s3; use super::traits::{BoardParts, LoRaBoard}; use crate::hal::esp32s3 as mcu; -#[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, + create_display, DisplayDriver, DisplayParts, RadioDriver, RadioParts, }; +pub type UsbDriver = esp_hal::otg_fs::asynch::Driver<'static>; + +pub struct UsbParts { + pub driver: UsbDriver, +} + pub struct Board { p: esp_hal::peripherals::Peripherals, } @@ -53,9 +58,15 @@ impl LoRaBoard for Board { 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_inst = esp_hal::otg_fs::Usb::new(p.USB0, p.GPIO20, p.GPIO19); + static EP_OUT_BUF: StaticCell<[u8; 1024]> = StaticCell::new(); + let ep_out_buf = EP_OUT_BUF.init([0u8; 1024]); let host = UsbParts { - driver: mcu::init_usb(p.USB0, p.GPIO20, p.GPIO19), + driver: esp_hal::otg_fs::asynch::Driver::new( + usb_inst, + ep_out_buf, + esp_hal::otg_fs::asynch::Config::default(), + ), }; // SSD1306 display reset: pulse GPIO21 low->high before I2C init diff --git a/firmware/src/board/heltec_v3_uart.rs b/firmware/src/board/heltec_v3_uart.rs index 662bb7f1..90330b7f 100644 --- a/firmware/src/board/heltec_v3_uart.rs +++ b/firmware/src/board/heltec_v3_uart.rs @@ -12,12 +12,16 @@ use super::esp32s3; use super::traits::{BoardParts, LoRaBoard}; use crate::hal::esp32s3 as mcu; -#[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, + create_display, DisplayDriver, DisplayParts, RadioDriver, RadioParts, }; +pub type UartDriver = esp_hal::uart::Uart<'static, esp_hal::Async>; + +pub struct UartParts { + pub driver: UartDriver, +} + pub struct Board { p: esp_hal::peripherals::Peripherals, } @@ -51,8 +55,14 @@ impl LoRaBoard for Board { 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); + + use esp_hal::uart::{Config as UartConfig, Uart}; let host = UartParts { - driver: mcu::init_uart(p.UART0, p.GPIO43, p.GPIO44), + driver: Uart::new(p.UART0, UartConfig::default()) + .expect("UART init") + .with_tx(p.GPIO43) + .with_rx(p.GPIO44) + .into_async(), }; // SSD1306 display reset: pulse GPIO21 low->high before I2C init diff --git a/firmware/src/board/heltec_v4.rs b/firmware/src/board/heltec_v4.rs index e51c7e3d..1d201472 100644 --- a/firmware/src/board/heltec_v4.rs +++ b/firmware/src/board/heltec_v4.rs @@ -1,15 +1,20 @@ use esp_hal::gpio::{Level, Output, OutputConfig}; +use static_cell::StaticCell; use super::esp32s3; use super::traits::{BoardParts, LoRaBoard}; use crate::hal::esp32s3 as mcu; -#[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, + create_display, DisplayDriver, DisplayParts, RadioDriver, RadioParts, }; +pub type UsbDriver = esp_hal::otg_fs::asynch::Driver<'static>; + +pub struct UsbParts { + pub driver: UsbDriver, +} + pub struct Board { p: esp_hal::peripherals::Peripherals, } @@ -45,8 +50,15 @@ impl LoRaBoard for Board { 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. + let usb_inst = esp_hal::otg_fs::Usb::new(p.USB0, p.GPIO20, p.GPIO19); + static EP_OUT_BUF: StaticCell<[u8; 1024]> = StaticCell::new(); + let ep_out_buf = EP_OUT_BUF.init([0u8; 1024]); let host = UsbParts { - driver: mcu::init_usb(p.USB0, p.GPIO20, p.GPIO19), + driver: esp_hal::otg_fs::asynch::Driver::new( + usb_inst, + ep_out_buf, + esp_hal::otg_fs::asynch::Config::default(), + ), }; // SSD1315 display reset: pulse GPIO21 low->high before I2C init diff --git a/firmware/src/channel.rs b/firmware/src/channel.rs index 5ea7e14a..e431aaa1 100644 --- a/firmware/src/channel.rs +++ b/firmware/src/channel.rs @@ -14,7 +14,6 @@ use crate::protocol::{Command, RadioConfig, Response}; /// Display commands routed from host_task to display_task. #[derive(Debug, Clone, Copy, defmt::Format)] -#[allow(dead_code)] // Reset is only used by USB boards (DTR disconnect detection) pub enum DisplayCommand { On, Off, diff --git a/firmware/src/driver/mod.rs b/firmware/src/driver/mod.rs index cbba9cfc..f895c6c9 100644 --- a/firmware/src/driver/mod.rs +++ b/firmware/src/driver/mod.rs @@ -1,2 +1,2 @@ -#[allow(dead_code)] +#[cfg(feature = "wio_tracker_l1")] pub mod sh1106; diff --git a/firmware/src/hal/esp32s3.rs b/firmware/src/hal/esp32s3.rs index eea960f5..db5f9b5b 100644 --- a/firmware/src/hal/esp32s3.rs +++ b/firmware/src/hal/esp32s3.rs @@ -1,8 +1,8 @@ //! ESP32-S3 MCU initialization primitives. //! -//! Provides low-level bus and peripheral init for ESP32-S3 boards. +//! Provides low-level bus init for ESP32-S3 boards. //! Board files call these with specific pins and construct higher-level -//! drivers (Sx126x, SSD1306, etc.) themselves. +//! drivers (Sx126x, SSD1306, USB, UART, etc.) themselves. use embassy_sync::blocking_mutex::raw::NoopRawMutex; use esp_hal::dma::{DmaRxBuf, DmaTxBuf}; @@ -17,10 +17,6 @@ use static_cell::StaticCell; pub type SpiBus = SpiDmaBus<'static, esp_hal::Async>; pub type I2cBus = I2c<'static, esp_hal::Async>; -#[allow(dead_code)] // Used by USB boards, not UART boards -pub type UsbOtgDriver = esp_hal::otg_fs::asynch::Driver<'static>; -#[allow(dead_code)] // Used by UART boards, not USB boards -pub type UartDriver = esp_hal::uart::Uart<'static, esp_hal::Async>; // ── Timer ─────────────────────────────────────────────────────────── @@ -33,8 +29,6 @@ pub fn start_timer(timg0: esp_hal::peripherals::TIMG0<'static>) { // ── SPI bus ───────────────────────────────────────────────────────── /// Initialize SPI2 with DMA and wrap in a shared bus (StaticCell + Mutex). -/// -/// Returns a reference to the shared SPI bus suitable for `SpiDevice::new()`. pub fn init_spi( spi2: esp_hal::peripherals::SPI2<'static>, dma_ch0: esp_hal::peripherals::DMA_CH0<'static>, @@ -80,43 +74,6 @@ pub fn init_i2c( .into_async() } -// ── USB OTG ───────────────────────────────────────────────────────── - -/// Initialize USB OTG FS driver for CDC-ACM. -#[allow(dead_code)] // Used by USB boards, not UART boards -pub fn init_usb( - usb0: esp_hal::peripherals::USB0<'static>, - dp: esp_hal::peripherals::GPIO20<'static>, - dm: esp_hal::peripherals::GPIO19<'static>, -) -> UsbOtgDriver { - let usb_inst = esp_hal::otg_fs::Usb::new(usb0, dp, dm); - static EP_OUT_BUF: StaticCell<[u8; 1024]> = StaticCell::new(); - let ep_out_buf = EP_OUT_BUF.init([0u8; 1024]); - esp_hal::otg_fs::asynch::Driver::new( - usb_inst, - ep_out_buf, - esp_hal::otg_fs::asynch::Config::default(), - ) -} - -// ── UART ──────────────────────────────────────────────────────────── - -/// Initialize UART0 for boards with USB-UART bridge chips (e.g. CP2102). -#[allow(dead_code)] // Used by UART boards, not USB boards -pub fn init_uart( - uart0: esp_hal::peripherals::UART0<'static>, - tx: esp_hal::peripherals::GPIO43<'static>, - rx: esp_hal::peripherals::GPIO44<'static>, -) -> UartDriver { - use esp_hal::uart::{Config as UartConfig, Uart}; - - Uart::new(uart0, UartConfig::default()) - .expect("UART init") - .with_tx(tx) - .with_rx(rx) - .into_async() -} - // ── MAC address ───────────────────────────────────────────────────── /// Read the factory-programmed MAC address from eFuse. diff --git a/firmware/src/host/uart.rs b/firmware/src/host/uart.rs index d01146f4..a5a8fa1c 100644 --- a/firmware/src/host/uart.rs +++ b/firmware/src/host/uart.rs @@ -6,7 +6,7 @@ use defmt::warn; use embassy_executor::task; use embedded_io_async::Write; -use crate::channel::{CommandChannel, DisplayCommandChannel, ResponseChannel}; +use crate::channel::{CommandChannel, DisplayCommand, DisplayCommandChannel, ResponseChannel}; use super::framing::{self, CobsDecoder, MAX_FRAME}; #[task] @@ -20,6 +20,11 @@ pub async fn host_task( ) { let (mut rx, mut tx) = parts.driver.split(); + // UART has no DTR — signal the display that a host is present at boot. + if has_display { + display_commands.send(DisplayCommand::Reset).await; + } + let mut read_buf = [0u8; 64]; let mut write_buf = [0u8; MAX_FRAME]; let mut cobs_encode_buf = [0u8; MAX_FRAME]; diff --git a/firmware/src/protocol.rs b/firmware/src/protocol.rs index 3cf01d52..a21731b7 100644 --- a/firmware/src/protocol.rs +++ b/firmware/src/protocol.rs @@ -277,12 +277,11 @@ impl Response { #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(not(test), derive(defmt::Format))] #[repr(u8)] -#[allow(dead_code)] // CrcError reserved per PROTOCOL.md (error code 3) pub enum ErrorCode { InvalidConfig = 0, RadioBusy = 1, TxTimeout = 2, - CrcError = 3, + // 3 reserved (was CrcError — SX1262 silently drops bad-CRC packets) NotConfigured = 4, NoDisplay = 5, } @@ -686,7 +685,6 @@ mod tests { (ErrorCode::InvalidConfig, 0), (ErrorCode::RadioBusy, 1), (ErrorCode::TxTimeout, 2), - (ErrorCode::CrcError, 3), (ErrorCode::NotConfigured, 4), (ErrorCode::NoDisplay, 5), ] {