Skip to content
This repository has been archived by the owner on Dec 27, 2023. It is now read-only.

added preamble deadline for proper LoRaWAN RX1+RX2 reception #33

Merged
merged 3 commits into from
Oct 20, 2023
Merged

added preamble deadline for proper LoRaWAN RX1+RX2 reception #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ce8128
added preamble deadline for proper LoRaWAN RX1+RX2 reception
ilya-epifanov Oct 8, 2023
1f95715
renamed DesiredIrqState to TargetIrqState, removed unused variant
ilya-epifanov Oct 11, 2023
b7daa95
Merge branch 'main' into preamble-deadline
lucasgranberg Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,16 +209,27 @@ where
self.radio_kind.do_tx(timeout_in_ms).await?;
match self
.radio_kind
.process_irq(self.radio_mode, self.rx_continuous, &mut self.delay, None, None)
.process_irq(
self.radio_mode,
self.rx_continuous,
TargetIrqState::Done,
&mut self.delay,
None,
None,
)
.await
{
Ok(()) => Ok(()),
Ok(TargetIrqState::Done) => {
self.radio_mode = RadioMode::Standby;
Ok(())
}
Err(err) => {
self.radio_kind.ensure_ready(self.radio_mode).await?;
self.radio_kind.set_standby().await?;
self.radio_mode = RadioMode::Standby;
Err(err)
}
Ok(_) => unreachable!(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this is actually unreachable. What if you start an RX before, drop the future, and then try to TX? Couldn't the irq trigger with TargetIrqState::Preamble in that case? Feels safer if this was somewhat ignored or returned an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

process_irq will only return TargetIrqState::PreambleReceived if explicitly asked to. In this case it's asked to only return TargetIrqState::Done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking again I see what you mean. Looks good to me then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking about changing the API to be a little bit more type-stateful. That'd resolve some issues like device being left in an inconsistent state after cancellation of a future. Also, it'd remove confustion between tx interrupts and rx interrupts.

Copy link
Collaborator

@lucasgranberg lucasgranberg Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a proper chat about thr overall layout of this crate. If you have the time you could join us at #public-lora-wan-rs:matrix.org

}
}

Expand Down Expand Up @@ -290,22 +301,39 @@ where
rx_pkt_params: &PacketParams,
receiving_buffer: &mut [u8],
) -> Result<(u8, PacketStatus), RadioError> {
let IrqState::RxDone(len, status) = self.rx_until_state(rx_pkt_params, receiving_buffer, TargetIrqState::Done).await? else {
unreachable!();
};
Ok((len, status))
}

/// Obtain the results of a read operation
pub async fn rx_until_state(
&mut self,
rx_pkt_params: &PacketParams,
receiving_buffer: &mut [u8],
target_rx_state: TargetIrqState,
) -> Result<IrqState, RadioError> {
match self
.radio_kind
.process_irq(
self.radio_mode,
self.rx_continuous,
target_rx_state,
&mut self.delay,
self.polling_timeout_in_ms,
None,
)
.await
{
Ok(()) => {
let received_len = self.radio_kind.get_rx_payload(rx_pkt_params, receiving_buffer).await?;
let rx_pkt_status = self.radio_kind.get_rx_packet_status().await?;
Ok((received_len, rx_pkt_status))
}
Ok(actual_state) => match actual_state {
TargetIrqState::PreambleReceived => Ok(IrqState::PreambleReceived),
TargetIrqState::Done => {
let received_len = self.radio_kind.get_rx_payload(rx_pkt_params, receiving_buffer).await?;
let rx_pkt_status = self.radio_kind.get_rx_packet_status().await?;
Ok(IrqState::RxDone(received_len, rx_pkt_status))
}
},
Err(err) => {
// if in rx continuous mode, allow the caller to determine whether to keep receiving
if !self.rx_continuous {
Expand Down Expand Up @@ -352,19 +380,21 @@ where
.process_irq(
self.radio_mode,
self.rx_continuous,
TargetIrqState::Done,
&mut self.delay,
None,
Some(&mut cad_activity_detected),
)
.await
{
Ok(()) => Ok(cad_activity_detected),
Ok(TargetIrqState::Done) => Ok(cad_activity_detected),
Err(err) => {
self.radio_kind.ensure_ready(self.radio_mode).await?;
self.radio_kind.set_standby().await?;
self.radio_mode = RadioMode::Standby;
Err(err)
}
Ok(_) => unreachable!(),
}
}

Expand Down
21 changes: 20 additions & 1 deletion src/mod_traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,24 @@ pub trait InterfaceVariant {
async fn disable_rf_switch(&mut self) -> Result<(), RadioError>;
}

/// Specifies an IRQ processing state to run the loop to
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum TargetIrqState {
/// Runs the loop until after the preamble has been received
PreambleReceived,
/// Runs the loop until the operation is fully complete
Done,
}

/// An actual operation state, including some details where necessary
#[derive(Clone, Copy)]
pub enum IrqState {
/// Preamble has been received
PreambleReceived,
/// The RX operation is complete
RxDone(u8, PacketStatus),
}

/// Functions implemented for a specific kind of LoRa chip, called internally by the outward facing
/// LoRa physical layer API
pub trait RadioKind {
Expand Down Expand Up @@ -122,10 +140,11 @@ pub trait RadioKind {
&mut self,
radio_mode: RadioMode,
rx_continuous: bool,
target_rx_state: TargetIrqState,
delay: &mut impl DelayUs,
polling_timeout_in_ms: Option<u32>,
cad_activity_detected: Option<&mut bool>,
) -> Result<(), RadioError>;
) -> Result<TargetIrqState, RadioError>;
/// Set the LoRa chip into the TxContinuousWave mode
async fn set_tx_continuous_wave_mode(&mut self) -> Result<(), RadioError>;
}
Expand Down
15 changes: 11 additions & 4 deletions src/sx1261_2/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use embedded_hal_async::spi::*;
use radio_kind_params::*;

use crate::mod_params::*;
use crate::mod_traits::TargetIrqState;
use crate::{InterfaceVariant, RadioKind, SpiInterface};

// Syncwords for public and private networks
Expand Down Expand Up @@ -822,10 +823,11 @@ where
&mut self,
radio_mode: RadioMode,
rx_continuous: bool,
target_rx_state: TargetIrqState,
delay: &mut impl DelayUs,
polling_timeout_in_ms: Option<u32>,
cad_activity_detected: Option<&mut bool>,
) -> Result<(), RadioError> {
) -> Result<TargetIrqState, RadioError> {
let mut iteration_guard: u32 = 0;
if polling_timeout_in_ms.is_some() {
iteration_guard = polling_timeout_in_ms.unwrap();
Expand Down Expand Up @@ -880,7 +882,7 @@ where
if radio_mode == RadioMode::Transmit {
if (irq_flags & IrqMask::TxDone.value()) == IrqMask::TxDone.value() {
debug!("TxDone in radio mode {}", radio_mode);
return Ok(());
return Ok(TargetIrqState::Done);
}
if (irq_flags & IrqMask::RxTxTimeout.value()) == IrqMask::RxTxTimeout.value() {
debug!("RxTxTimeout in radio mode {}", radio_mode);
Expand Down Expand Up @@ -927,7 +929,12 @@ where
];
self.intf.write(&[&register_and_evt_clear], false).await?;
}
return Ok(());
return Ok(TargetIrqState::Done);
}
if target_rx_state == TargetIrqState::PreambleReceived
&& (IrqMask::PreambleDetected.is_set_in(irq_flags) || IrqMask::HeaderValid.is_set_in(irq_flags))
{
return Ok(TargetIrqState::PreambleReceived);
}
if (irq_flags & IrqMask::RxTxTimeout.value()) == IrqMask::RxTxTimeout.value() {
debug!("RxTxTimeout in radio mode {}", radio_mode);
Expand All @@ -941,7 +948,7 @@ where
*(cad_activity_detected.unwrap()) =
(irq_flags & IrqMask::CADActivityDetected.value()) == IrqMask::CADActivityDetected.value();
}
return Ok(());
return Ok(TargetIrqState::Done);
}

// if an interrupt occurred for other than an error or operation completion, loop to wait again
Expand Down
4 changes: 4 additions & 0 deletions src/sx1261_2/radio_kind_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ impl IrqMask {
pub fn value(self) -> u16 {
self as u16
}

pub fn is_set_in(self, mask: u16) -> bool {
self.value() & mask == self.value()
}
}

#[derive(Clone, Copy)]
Expand Down
14 changes: 10 additions & 4 deletions src/sx1276_7_8_9/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use embedded_hal_async::spi::*;
use radio_kind_params::*;

use crate::mod_params::*;
use crate::mod_traits::TargetIrqState;
use crate::{InterfaceVariant, RadioKind, SpiInterface};

// Syncwords for public and private networks
Expand Down Expand Up @@ -538,10 +539,11 @@ where
&mut self,
radio_mode: RadioMode,
_rx_continuous: bool,
target_rx_state: TargetIrqState,
delay: &mut impl DelayUs,
polling_timeout_in_ms: Option<u32>,
cad_activity_detected: Option<&mut bool>,
) -> Result<(), RadioError> {
) -> Result<TargetIrqState, RadioError> {
let mut iteration_guard: u32 = 0;
if polling_timeout_in_ms.is_some() {
iteration_guard = polling_timeout_in_ms.unwrap();
Expand Down Expand Up @@ -578,15 +580,19 @@ where
if radio_mode == RadioMode::Transmit {
if (irq_flags & IrqMask::TxDone.value()) == IrqMask::TxDone.value() {
debug!("TxDone in radio mode {}", radio_mode);
return Ok(());
return Ok(TargetIrqState::Done);
}
} else if radio_mode == RadioMode::Receive {
if (irq_flags & IrqMask::CRCError.value()) == IrqMask::CRCError.value() {
debug!("CRCError in radio mode {}", radio_mode);
}
if (irq_flags & IrqMask::RxDone.value()) == IrqMask::RxDone.value() {
debug!("RxDone in radio mode {}", radio_mode);
return Ok(());
return Ok(TargetIrqState::Done);
}
if target_rx_state == TargetIrqState::PreambleReceived && IrqMask::HeaderValid.is_set_in(irq_flags) {
debug!("HeaderValid in radio mode {}", radio_mode);
return Ok(TargetIrqState::PreambleReceived);
}
if (irq_flags & IrqMask::RxTimeout.value()) == IrqMask::RxTimeout.value() {
debug!("RxTimeout in radio mode {}", radio_mode);
Expand All @@ -600,7 +606,7 @@ where
*(cad_activity_detected.unwrap()) =
(irq_flags & IrqMask::CADActivityDetected.value()) == IrqMask::CADActivityDetected.value();
}
return Ok(());
return Ok(TargetIrqState::Done);
}

// if an interrupt occurred for other than an error or operation completion, loop to wait again
Expand Down
4 changes: 4 additions & 0 deletions src/sx1276_7_8_9/radio_kind_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ impl IrqMask {
pub fn value(self) -> u8 {
self as u8
}

pub fn is_set_in(self, mask: u8) -> bool {
self.value() & mask == self.value()
}
}

#[derive(Clone, Copy)]
Expand Down