Merge pull request #37 from plaes/rework-boardtype

Drop global BoardType enum in favor of driver-specific configuration struct

README.md

@@ -58,13 +58,7 @@ Example RadioKind implementations and ancillary information:
 
 ## LoRa board-specific support
 
-LoRa boards use LoRa chip features differently.  To suppport these variations within a radio kind implementation, BoardType and ChipType are available:
-
-- <a href="https://github.com/embassy-rs/lora-phy/blob/main/src/mod_params.rs">scroll to BoardType and ChipType</a>.
-
-One can add a LoRa board (the board name includes the chip type in case the board may include a range of chip types) and the ChipType, then modify the radio kind processing to support board-specific features.  The ChipType is used for generic checks, alleviating the need to add a new board type check in places where a generic check will do.  BoardType checks only need to be implemented where the specificity is board-related.  There are examples of each type of check here:
-
-- <a href="https://github.com/embassy-rs/lora-phy/blob/main/src/sx1261_2/mod.rs">search for BoardType and ChipType</a>.
+Board-specific configuration can be handled via the chip driver specific Config struct.
 
 ## Chat
 

src/lib.rs

@@ -61,11 +61,6 @@ where
         Ok(lora)
     }
 
-    /// Get the board type of the LoRa board
-    pub fn get_board_type(&self) -> BoardType {
-        self.radio_kind.get_board_type()
-    }
-
     /// Create modulation parameters for a communication channel
     pub fn create_modulation_params(
         &mut self,

src/mod_params.rs

@@ -5,7 +5,7 @@ pub use lora_modulation::{Bandwidth, CodingRate, SpreadingFactor};
 /// Errors types reported during LoRa physical layer processing
 #[allow(clippy::upper_case_acronyms)]
 #[derive(Debug, defmt::Format, PartialEq)]
-#[allow(dead_code, missing_docs)]
+#[allow(missing_docs)]
 pub enum RadioError {
     SPI,
     Reset,
@@ -41,7 +41,6 @@ pub enum RadioError {
     DutyCycleRxContinuousUnsupported,
     CADUnexpected,
     RngUnsupported,
-    BoardTypeUnsupportedForRadioKind,
 }
 
 /// Status for a received packet
@@ -52,52 +51,6 @@ pub struct PacketStatus {
     pub snr: i16,
 }
 
-/// LoRa boards supported by this crate.
-/// In addition, custom boards (possibly proprietary) can be supported by using the custom board and chip types and
-/// external implementations of the RadioKind and (in some cases) InterfaceVariant traits.  For instance:
-/// let iv = ExternalInterfaceVariantImpl::new(..params...)
-/// LoRa::new(ExternalRadioKindImpl::new(BoardType::CustomBoard, spi, iv), ...other_params...)
-#[derive(Clone, Copy, PartialEq)]
-#[allow(missing_docs)]
-pub enum BoardType {
-    CustomBoard,
-    GenericSx1261, // placeholder for Sx1261-specific features
-    HeltecWifiLoraV31262,
-    RpPicoWaveshareSx1262,
-    Rak4631Sx1262,
-    Rak3172Sx1262,
-    Stm32l0Sx1276,
-    Stm32wlSx1262,
-}
-
-/// LoRa chips supported by this crate
-#[derive(Clone, Copy, PartialEq)]
-#[allow(missing_docs)]
-pub enum ChipType {
-    CustomChip,
-    Sx1261,
-    Sx1262,
-    Sx1276,
-    Sx1277,
-    Sx1278,
-    Sx1279,
-}
-
-impl From<BoardType> for ChipType {
-    fn from(board_type: BoardType) -> Self {
-        match board_type {
-            BoardType::CustomBoard => ChipType::CustomChip,
-            BoardType::GenericSx1261 => ChipType::Sx1261,
-            BoardType::HeltecWifiLoraV31262 => ChipType::Sx1262,
-            BoardType::RpPicoWaveshareSx1262 => ChipType::Sx1262,
-            BoardType::Rak4631Sx1262 => ChipType::Sx1262,
-            BoardType::Rak3172Sx1262 => ChipType::Sx1262,
-            BoardType::Stm32l0Sx1276 => ChipType::Sx1276,
-            BoardType::Stm32wlSx1262 => ChipType::Sx1262,
-        }
-    }
-}
-
 /// The state of the radio
 #[derive(Clone, Copy, defmt::Format, PartialEq)]
 #[allow(missing_docs)]

src/mod_traits.rs

@@ -5,8 +5,6 @@ use crate::mod_params::*;
 /// Functions implemented for an embedded framework for an MCU/LoRa chip combination
 /// to allow this crate to control the LoRa chip.
 pub trait InterfaceVariant {
-    /// Set the LoRa board type
-    fn set_board_type(&mut self, board_type: BoardType);
     /// Reset the LoRa chip
     async fn reset(&mut self, delay: &mut impl DelayUs) -> Result<(), RadioError>;
     /// Wait for the LoRa chip to become available for an operation
@@ -42,8 +40,6 @@ pub enum IrqState {
 /// Functions implemented for a specific kind of LoRa chip, called internally by the outward facing
 /// LoRa physical layer API
 pub trait RadioKind {
-    /// Get the specific type of the LoRa board (for example, Stm32wlSx1262)
-    fn get_board_type(&self) -> BoardType;
     /// Create modulation parameters specific to the LoRa chip kind and type
     fn create_modulation_params(
         &self,

src/sx1261_2/mod.rs

@@ -31,10 +31,31 @@ const SX126X_MAX_LORA_SYMB_NUM_TIMEOUT: u8 = 248;
 // Time required for the TCXO to wakeup [ms].
 const BRD_TCXO_WAKEUP_TIME: u32 = 10;
 
+/// Supported SX126x chip variants
+#[derive(Clone, PartialEq)]
+pub enum Sx126xVariant {
+    /// Semtech SX1261
+    Sx1261,
+    /// Semtech SX1261
+    Sx1262,
+    /// STM32WL System-On-Chip with SX126x-based sub-GHz radio
+    Stm32wl, // XXX: Drop and switch to board-specific configuration?
+             // STM32 manuals don't really specify which sx126x chip is used.
+             // Original code in set_tx_power_and_ramp_time assumes Sx1262-specific power rates
+}
+
+/// Configuration for SX126x-based boards
+pub struct Config {
+    /// LoRa chip variant on this board
+    pub chip: Sx126xVariant,
+    /// Configuration for TCXO and its voltage selection
+    pub txco_ctrl: Option<TcxoCtrlVoltage>,
+}
+
 /// Base for the RadioKind implementation for the LoRa chip kind and board type
 pub struct SX1261_2<SPI, IV> {
-    board_type: BoardType,
     intf: SpiInterface<SPI, IV>,
+    config: Config,
 }
 
 impl<SPI, IV> SX1261_2<SPI, IV>
@@ -43,10 +64,9 @@ where
     IV: InterfaceVariant,
 {
     /// Create an instance of the RadioKind implementation for the LoRa chip kind and board type
-    pub fn new(board_type: BoardType, spi: SPI, mut iv: IV) -> Self {
-        iv.set_board_type(board_type);
+    pub fn new(spi: SPI, iv: IV, config: Config) -> Self {
         let intf = SpiInterface::new(spi, iv);
-        Self { board_type, intf }
+        Self { intf, config }
     }
 
     // Utility functions
@@ -156,10 +176,6 @@ where
     SPI: SpiDevice<u8>,
     IV: InterfaceVariant,
 {
-    fn get_board_type(&self) -> BoardType {
-        self.board_type
-    }
-
     fn create_modulation_params(
         &self,
         spreading_factor: SpreadingFactor,
@@ -233,7 +249,7 @@ where
 
     // Use DIO2 to control an RF Switch, depending on the board type.
     async fn init_rf_switch(&mut self) -> Result<(), RadioError> {
-        if self.board_type != BoardType::Stm32wlSx1262 {
+        if self.config.chip != Sx126xVariant::Stm32wl {
             let op_code_and_indicator = [OpCode::SetRFSwitchMode.value(), true as u8];
             self.intf.write(&op_code_and_indicator, false).await?;
         }
@@ -289,30 +305,19 @@ where
     }
 
     async fn set_oscillator(&mut self) -> Result<(), RadioError> {
-        // voltage used to control the TCXO on/off from DIO3
-        let voltage = match self.board_type {
-            BoardType::CustomBoard | BoardType::Stm32l0Sx1276 => {
-                return Err(RadioError::BoardTypeUnsupportedForRadioKind);
-            }
-            BoardType::Rak3172Sx1262 => {
-                // uses XTAL instead of TCXO
-                return Ok(());
-            }
-            BoardType::GenericSx1261
-            | BoardType::RpPicoWaveshareSx1262
-            | BoardType::Rak4631Sx1262
-            | BoardType::Stm32wlSx1262 => TcxoCtrlVoltage::Ctrl1V7,
-            BoardType::HeltecWifiLoraV31262 => TcxoCtrlVoltage::Ctrl1V8,
-        };
-        let timeout = BRD_TCXO_WAKEUP_TIME << 6; // duration allowed for TCXO to reach 32MHz
-        let op_code_and_tcxo_control = [
-            OpCode::SetTCXOMode.value(),
-            voltage.value() & 0x07,
-            Self::timeout_1(timeout),
-            Self::timeout_2(timeout),
-            Self::timeout_3(timeout),
-        ];
-        self.intf.write(&op_code_and_tcxo_control, false).await
+        if let Some(voltage) = self.config.txco_ctrl {
+            let timeout = BRD_TCXO_WAKEUP_TIME << 6; // duration allowed for TCXO to reach 32MHz
+            let op_code_and_tcxo_control = [
+                OpCode::SetTCXOMode.value(),
+                voltage.value() & 0x07,
+                Self::timeout_1(timeout),
+                Self::timeout_2(timeout),
+                Self::timeout_3(timeout),
+            ];
+            self.intf.write(&op_code_and_tcxo_control, false).await?;
+        }
+
+        Ok(())
     }
 
     // Set the power regulators operating mode to DC_DC.  Using only LDO implies that the Rx/Tx current is doubled.
@@ -355,8 +360,9 @@ where
             false => RampTime::Ramp200Us, // for instance, on initialization
         };
 
-        let chip_type: ChipType = self.board_type.into();
-        if chip_type == ChipType::Sx1261 {
+        // TODO: Switch to match so all chip variants are covered
+        let chip_type = &self.config.chip;
+        if chip_type == &Sx126xVariant::Sx1261 {
             if !(-17..=15).contains(&output_power) {
                 return Err(RadioError::InvalidOutputPower);
             }
@@ -812,7 +818,8 @@ where
         self.intf.write(&op_code_and_masks, false).await
     }
 
-    /// Process the radio IRQ.  Log unexpected interrupts, but only bail out on timeout.  Packets from other devices can cause unexpected interrupts.
+    /// Process the radio IRQ. Log unexpected interrupts, but only bail out on timeout.
+    /// Packets from other devices can cause unexpected interrupts.
     async fn process_irq(
         &mut self,
         radio_mode: RadioMode,
@@ -965,8 +972,10 @@ where
     /// The random numbers produced by the generator do not have a uniform or Gaussian distribution.
     /// If uniformity is needed, perform appropriate software post-processing.
     async fn get_random_number(&mut self) -> Result<u32, RadioError> {
-        // The stm32wl often returns 0 on the first random number generation operation.  Documentation for the stm32wl does not recommend LNA register modification.
-        if self.board_type == BoardType::Stm32wlSx1262 {
+        // The stm32wl often returns 0 on the first random number generation operation.
+        // Documentation for the stm32wl does not recommend LNA register modification.
+        // XXX: Ideally this should result in a compile-time error...
+        if self.config.chip == Sx126xVariant::Stm32wl {
             return Err(RadioError::RngUnsupported);
         }
         self.set_irq_params(None).await?;

src/sx1261_2/radio_kind_params.rs

@@ -208,7 +208,6 @@ impl CalibrationParams {
 }
 
 #[derive(Clone, Copy)]
-#[allow(dead_code)]
 pub enum TcxoCtrlVoltage {
     Ctrl1V6 = 0x00,
     Ctrl1V7 = 0x01,

src/sx1276_7_8_9/mod.rs

@@ -19,10 +19,20 @@ const TCXO_FOR_OSCILLATOR: u8 = 0x10u8;
 // Frequency synthesizer step for frequency calculation (Hz)
 const FREQUENCY_SYNTHESIZER_STEP: f64 = 61.03515625; // FXOSC (32 MHz) * 1000000 (Hz/MHz) / 524288 (2^19)
 
+/// Supported SX127x chip variants for further device-specific customizations
+/// Currently SX1276, SX1277, SX1278 and SX1279 are supported
+pub enum Sx127xVariant {}
+
+/// Configuration for SX127x-based boards
+pub struct Config {
+    /// LoRa chip variant on this board
+    pub chip: Sx127xVariant,
+}
+
 /// Base for the RadioKind implementation for the LoRa chip kind and board type
 pub struct SX1276_7_8_9<SPI, IV> {
-    board_type: BoardType,
     intf: SpiInterface<SPI, IV>,
+    _config: Config,
 }
 
 impl<SPI, IV> SX1276_7_8_9<SPI, IV>
@@ -31,10 +41,9 @@ where
     IV: InterfaceVariant,
 {
     /// Create an instance of the RadioKind implementation for the LoRa chip kind and board type
-    pub fn new(board_type: BoardType, spi: SPI, mut iv: IV) -> Self {
-        iv.set_board_type(board_type);
+    pub fn new(spi: SPI, iv: IV, _config: Config) -> Self {
         let intf = SpiInterface::new(spi, iv);
-        Self { board_type, intf }
+        Self { intf, _config }
     }
 
     // Utility functions
@@ -80,10 +89,6 @@ where
     SPI: SpiDevice<u8>,
     IV: InterfaceVariant,
 {
-    fn get_board_type(&self) -> BoardType {
-        self.board_type
-    }
-
     fn create_modulation_params(
         &self,
         spreading_factor: SpreadingFactor,