SpiBus -> SpiDevice

[CBJamo]

Should close #31. Will also conflict with #29, but because the change from SpiBus to SpiDevice also required switching to spi transactions, it should have a similar effect on perf as #29.

I've tested on my hardware (custom rp240 board and sx1262 boards), and it works here. I don't have an sx1261 or sx127x to test against, though I doubt this will cause issues for those parts. I'm more concerned about the stm32wl, as I don't know how it's weird internal spi interface works, and as such have marked this a draft.

This is also a breaking change that would require a major version bump.

[lucasgranberg]

This is the better solution. Use as much ready made code as possible. Maybe you could work together with @oll3 on this as you both seem to have figured it out independently.

[plaes]

Excellent! Tested this with SX1272 chip (PR #34) and seems to be working.

[vDorst]

Hmm I think https://github.com/embassy-rs/embassy/blob/main/embassy-lora/src/iv.rs#L223 also has to change.
I try to us it in a shared SPI environment with embassy-embedded-hal SpiDeviceWithConfig trait see examples here: https://docs.embassy.dev/embassy-embedded-hal/git/default/shared_bus/asynch/spi/index.html

But I hit the problem that GenericSx126xInterfaceVariant and SpiDeviceWithConfig both wants the chipselect pin.

This is my current code without this patch.
My code want chipselect here https://github.com/vDorst/picotracker/blob/main/rp2040/src/main.rs#L97
And I wrote a wrapper around SpiBusWithConfig https://github.com/vDorst/picotracker/blob/main/rp2040/src/main.rs#L749
Which did not need a chipselect.

So it seems that GenericSx126xInterfaceVariant is incompatiable/can't work with shared bus SpiDeviceWithConfig.

I use SpiDeviceWithConfig too switch SPI frequency, Display uses 64MHz and Lora chip 1 MHz

Now it is working but I have to update the dependency of three crates.
Modified code: vDorst/picotracker@4ad40bc

• embassy-lora , update dependency and remove the chip-select. See patch below
• lora-phy
• lorawan-device and lorawan

diff --git a/embassy-lora/Cargo.toml b/embassy-lora/Cargo.toml
index 846c3919..18d71034 100644
--- a/embassy-lora/Cargo.toml
+++ b/embassy-lora/Cargo.toml
@@ -27,5 +27,5 @@ embedded-hal-async = { version = "=1.0.0-rc.1" }
 embedded-hal = { version = "0.2", features = ["unproven"] }
 
 futures = { version = "0.3.17", default-features = false, features = [ "async-await" ] }
-lora-phy = { version = "2" }
-lorawan-device = { version = "0.11.0", default-features = false, features = ["async"], optional = true }
+lora-phy = { path = "../../lora-phy" }
+lorawan-device = {path = "../../rust-lorawan/device" , default-features = false, features = ["async"], optional = true }
diff --git a/embassy-lora/src/iv.rs b/embassy-lora/src/iv.rs
index d22beb33..267e717c 100644
--- a/embassy-lora/src/iv.rs
+++ b/embassy-lora/src/iv.rs
@@ -125,7 +125,6 @@ where
 /// Base for the InterfaceVariant implementation for an stm32l0/sx1276 combination
 pub struct Stm32l0InterfaceVariant<CTRL, WAIT> {
     board_type: BoardType,
-    nss: CTRL,
     reset: CTRL,
     irq: WAIT,
     rf_switch_rx: Option<CTRL>,
@@ -139,7 +138,6 @@ where
 {
     /// Create an InterfaceVariant instance for an stm32l0/sx1276 combination
     pub fn new(
-        nss: CTRL,
         reset: CTRL,
         irq: WAIT,
         rf_switch_rx: Option<CTRL>,
@@ -147,7 +145,6 @@ where
     ) -> Result<Self, RadioError> {
         Ok(Self {
             board_type: BoardType::Stm32l0Sx1276, // updated when associated with a specific LoRa board
-            nss,
             reset,
             irq,
             rf_switch_rx,
@@ -164,12 +161,6 @@ where
     fn set_board_type(&mut self, board_type: BoardType) {
         self.board_type = board_type;
     }
-    async fn set_nss_low(&mut self) -> Result<(), RadioError> {
-        self.nss.set_low().map_err(|_| NSS)
-    }
-    async fn set_nss_high(&mut self) -> Result<(), RadioError> {
-        self.nss.set_high().map_err(|_| NSS)
-    }
     async fn reset(&mut self, delay: &mut impl DelayUs) -> Result<(), RadioError> {
         delay.delay_ms(10).await;
         self.reset.set_low().map_err(|_| Reset)?;
@@ -220,7 +211,6 @@ where
 /// Base for the InterfaceVariant implementation for a generic Sx126x LoRa board
 pub struct GenericSx126xInterfaceVariant<CTRL, WAIT> {
     board_type: BoardType,
-    nss: CTRL,
     reset: CTRL,
     dio1: WAIT,
     busy: WAIT,
@@ -235,7 +225,6 @@ where
 {
     /// Create an InterfaceVariant instance for an nrf52840/sx1262 combination
     pub fn new(
-        nss: CTRL,
         reset: CTRL,
         dio1: WAIT,
         busy: WAIT,
@@ -244,7 +233,6 @@ where
     ) -> Result<Self, RadioError> {
         Ok(Self {
             board_type: BoardType::Rak4631Sx1262, // updated when associated with a specific LoRa board
-            nss,
             reset,
             dio1,
             busy,
@@ -262,12 +250,6 @@ where
     fn set_board_type(&mut self, board_type: BoardType) {
         self.board_type = board_type;
     }
-    async fn set_nss_low(&mut self) -> Result<(), RadioError> {
-        self.nss.set_low().map_err(|_| NSS)
-    }
-    async fn set_nss_high(&mut self) -> Result<(), RadioError> {
-        self.nss.set_high().map_err(|_| NSS)
-    }
     async fn reset(&mut self, delay: &mut impl DelayUs) -> Result<(), RadioError> {
         delay.delay_ms(10).await;
         self.reset.set_low().map_err(|_| Reset)?;

[CBJamo]

You're absolutely correct about needing a change in Embassy. I apologize for not noting and linking that initially. I made the exact same change to embassy here. The responsibility of controlling the chip select pin is moved from InterfaceVariant to SpiDevice.

This is a good reference for what the SpiDevice is responsible for. The SpiDeviceWithConfig type provided by embassy additionally changes mode/freq/etc, which is very similar to what your SpiBusWithConfig does.

This change should not require any changes downstream crates such as lorawan-rust, except to bump the version when everything is ready to be released. The interdependence of these 3 projects is currently a bit messy, and we've been discussing potential changes in the matrix channel. It's likely that embassy-lora and lora-phy will be merged, which will prevent this kind of confusion in the future.

[lucasgranberg]

lgtm, will be a major breaking change but that is okay