Closed CBJamo closed 10 months ago
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.
Excellent! Tested this with SX1272 chip (PR #34) and seems to be working.
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: https://github.com/vDorst/picotracker/commit/4ad40bc31434c8d38aa39051c9ab6f415ac44196
embassy-lora
, update dependency and remove the chip-select. See patch belowlora-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)?;
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.
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.