Skip to content

Commit

Permalink
fix(*): multiple urc subscribers (#85)
Browse files Browse the repository at this point in the history 
* Revert "Only handle URCs in one place (#84)"

This reverts commit 9aa54c8.

* Change from a single URC subscriber to two
  • Loading branch information
@MathiasKoch
MathiasKoch committed Sep 26, 2023
1 parent 9aa54c8 commit 29741f8
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 98 deletions.
125 changes: 33 additions & 92 deletions ublox-cellular/src/client.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use atat::{blocking::AtatClient, AtatUrcChannel};
use atat::{blocking::AtatClient, AtatUrcChannel, UrcSubscription};
use embassy_time::Duration;
use ublox_sockets::SocketSet;

Expand Down Expand Up @@ -37,7 +37,7 @@ use crate::{
network::{AtTx, Network},
power::PowerState,
registration::ConnectionState,
services::data::{ContextState, PROFILE_ID},
services::data::ContextState,
UbloxCellularBuffers, UbloxCellularIngress, UbloxCellularUrcChannel,
};
use ip_transport_layer::{types::HexMode, SetHexMode};
Expand All @@ -48,7 +48,7 @@ use psn::{
};

pub(crate) const URC_CAPACITY: usize = 3;
pub(crate) const URC_SUBSCRIBERS: usize = 1;
pub(crate) const URC_SUBSCRIBERS: usize = 2;

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
Expand All @@ -65,6 +65,7 @@ pub struct Device<'buf, 'sub, AtCl, AtUrcCh, Config, const N: usize, const L: us
pub(crate) config: Config,
pub(crate) network: Network<'sub, AtCl>,
urc_channel: &'buf AtUrcCh,

Check warning on line 67 in ublox-cellular/src/client.rs

View workflow job for this annotation

GitHub Actions / clippy

field `urc_channel` is never read 

warning: field `urc_channel` is never read
  --> ublox-cellular/src/client.rs:67:5
   |
64 | pub struct Device<'buf, 'sub, AtCl, AtUrcCh, Config, const N: usize, const L: usize> {
   |            ------ field in this struct
...
67 |     urc_channel: &'buf AtUrcCh,
   |     ^^^^^^^^^^^
   |
   = note: `#[warn(dead_code)]` on by default
urc_subscription: UrcSubscription<'sub, Urc, URC_CAPACITY, URC_SUBSCRIBERS>,

pub(crate) state: State,
pub(crate) power_state: PowerState,
Expand Down Expand Up @@ -115,14 +116,15 @@ where
Config: CellularConfig,
{
pub fn new(client: AtCl, urc_channel: &'buf AtUrcCh, config: Config) -> Self {
let urc_subscription = urc_channel.subscribe().unwrap();
let network_urc_subscription = urc_channel.subscribe().unwrap();
Self {
config,
network: Network::new(AtTx::new(client, urc_subscription)),
network: Network::new(AtTx::new(client, network_urc_subscription)),
state: State::Off,
power_state: PowerState::Off,
sockets: None,
urc_channel,
urc_subscription: urc_channel.subscribe().unwrap(),
}
}
}
Expand Down Expand Up @@ -525,97 +527,36 @@ where
}

fn handle_urc_internal(&mut self) -> Result<(), Error> {
let mut ctx_state = self.network.context_state;
if let Some(ref mut sockets) = self.sockets.as_deref_mut() {
let res = self
.network
.at_tx
.handle_urc(|urc| {
match urc {
Urc::SocketClosed(ip_transport_layer::urc::SocketClosed { socket }) => {
info!("[URC] SocketClosed {}", socket.0);
if let Some((_, mut sock)) =
sockets.iter_mut().find(|(handle, _)| *handle == socket)
{
sock.closed_by_remote();
}
if let Some(urc) = self.urc_subscription.try_next_message_pure() {
match urc {
Urc::SocketClosed(ip_transport_layer::urc::SocketClosed { socket }) => {
info!("[URC] SocketClosed {}", socket.0);
if let Some((_, mut sock)) =
sockets.iter_mut().find(|(handle, _)| *handle == socket)
{
sock.closed_by_remote();
}
Urc::SocketDataAvailable(
ip_transport_layer::urc::SocketDataAvailable { socket, length },
)
| Urc::SocketDataAvailableUDP(
ip_transport_layer::urc::SocketDataAvailable { socket, length },
) => {
trace!("[Socket({})] {} bytes available", socket.0, length as u16);
if let Some((_, mut sock)) =
sockets.iter_mut().find(|(handle, _)| *handle == socket)
{
sock.set_available_data(length);
}
}
Urc::NetworkDetach => {
warn!("Network Detach URC!");
}
Urc::MobileStationDetach => {
warn!("ME Detach URC!");
}
Urc::NetworkDeactivate => {
warn!("Network Deactivate URC!");
}
Urc::MobileStationDeactivate => {
warn!("ME Deactivate URC!");
}
Urc::NetworkPDNDeactivate => {
warn!("Network PDN Deactivate URC!");
}
Urc::MobileStationPDNDeactivate => {
warn!("ME PDN Deactivate URC!");
}
Urc::ExtendedPSNetworkRegistration(
psn::urc::ExtendedPSNetworkRegistration { state },
) => {
info!("[URC] ExtendedPSNetworkRegistration {:?}", state);
}
// FIXME: Currently `atat` is unable to distinguish `xREG` family of
// commands from URC's

// Urc::GPRSNetworkRegistration(reg_params) => {
// new_reg_params.replace(reg_params.into());
// }
// Urc::EPSNetworkRegistration(reg_params) => {
// new_reg_params.replace(reg_params.into());
// }
// Urc::NetworkRegistration(reg_params) => {
// new_reg_params.replace(reg_params.into());
// }
Urc::DataConnectionActivated(psn::urc::DataConnectionActivated {
result,
ip_addr: _,
}) => {
info!("[URC] DataConnectionActivated {}", result);
if result == 0 {
ctx_state = ContextState::Active;
} else {
ctx_state = ContextState::Setup;
}
}
Urc::DataConnectionDeactivated(psn::urc::DataConnectionDeactivated {
profile_id,
}) => {
info!("[URC] DataConnectionDeactivated {:?}", profile_id);
if profile_id == PROFILE_ID {
ctx_state = ContextState::Activating;
}
}
Urc::MessageWaitingIndication(_) => {
info!("[URC] MessageWaitingIndication");
}
Urc::SocketDataAvailable(ip_transport_layer::urc::SocketDataAvailable {
socket,
length,
})
| Urc::SocketDataAvailableUDP(ip_transport_layer::urc::SocketDataAvailable {
socket,
length,
}) => {
trace!("[Socket({})] {} bytes available", socket.0, length as u16);
if let Some((_, mut sock)) =
sockets.iter_mut().find(|(handle, _)| *handle == socket)
{
sock.set_available_data(length);
}
_ => {}
}
})
.map_err(Error::Network);
self.network.context_state = ctx_state;
res
_ => {}
}
}
Ok(())
} else {
Ok(())
}
Expand Down
90 changes: 84 additions & 6 deletions ublox-cellular/src/network.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<'sub, AtCl: AtatClient> AtTx<'sub, AtCl> {
})
}

pub fn handle_urc<F: FnOnce(Urc)>(&mut self, f: F) -> Result<(), Error> {
pub fn handle_urc<F: FnOnce(Urc) -> bool>(&mut self, f: F) -> Result<(), Error> {
if let Some(urc) = self.urc_subscription.try_next_message_pure() {
f(urc);
}
Expand Down Expand Up @@ -161,6 +161,7 @@ where
return Err(Error::Generic(GenericError::Timeout));
}

self.handle_urc().ok(); // Ignore errors
self.check_registration_state();
self.intervene_registration()?;
self.check_running_imsi().ok(); // Ignore errors
Expand Down Expand Up @@ -428,6 +429,83 @@ where
Ok(())
}

pub(crate) fn handle_urc(&mut self) -> Result<(), Error> {
// TODO: How to do this cleaner?
let mut ctx_state = self.context_state;
// let mut new_reg_params: Option<RegistrationParams> = None;

self.at_tx.handle_urc(|urc| {
match urc {
Urc::NetworkDetach => {
warn!("Network Detach URC!");
}
Urc::MobileStationDetach => {
warn!("ME Detach URC!");
}
Urc::NetworkDeactivate => {
warn!("Network Deactivate URC!");
}
Urc::MobileStationDeactivate => {
warn!("ME Deactivate URC!");
}
Urc::NetworkPDNDeactivate => {
warn!("Network PDN Deactivate URC!");
}
Urc::MobileStationPDNDeactivate => {
warn!("ME PDN Deactivate URC!");
}
Urc::ExtendedPSNetworkRegistration(psn::urc::ExtendedPSNetworkRegistration {
state,
}) => {
info!("[URC] ExtendedPSNetworkRegistration {:?}", state);
}
// FIXME: Currently `atat` is unable to distinguish `xREG` family of
// commands from URC's

// Urc::GPRSNetworkRegistration(reg_params) => {
// new_reg_params.replace(reg_params.into());
// }
// Urc::EPSNetworkRegistration(reg_params) => {
// new_reg_params.replace(reg_params.into());
// }
// Urc::NetworkRegistration(reg_params) => {
// new_reg_params.replace(reg_params.into());
// }
Urc::DataConnectionActivated(psn::urc::DataConnectionActivated {
result,
ip_addr: _,
}) => {
info!("[URC] DataConnectionActivated {}", result);
if result == 0 {
ctx_state = ContextState::Active;
} else {
ctx_state = ContextState::Setup;
}
}
Urc::DataConnectionDeactivated(psn::urc::DataConnectionDeactivated {
profile_id,
}) => {
info!("[URC] DataConnectionDeactivated {:?}", profile_id);
if profile_id == PROFILE_ID {
ctx_state = ContextState::Activating;
}
}
Urc::MessageWaitingIndication(_) => {
info!("[URC] MessageWaitingIndication");
}
_ => return false,
};
true
})?;

// if let Some(reg_params) = new_reg_params {
// self.status.compare_and_set(reg_params)
// }

self.context_state = ctx_state;
Ok(())
}

pub(crate) fn send_internal<A, const LEN: usize>(
&mut self,
req: &A,
Expand All @@ -436,11 +514,11 @@ where
where
A: atat::AtatCmd<LEN>,
{
// if check_urc {
// if let Err(e) = self.handle_urc() {
// error!("Failed handle URC {:?}", &e);
// }
// }
if check_urc {
if let Err(e) = self.handle_urc() {
error!("Failed handle URC {:?}", &e);
}
}

self.at_tx.send(req)
}
Expand Down
4 changes: 4 additions & 0 deletions ublox-cellular/src/services/data/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,10 @@ where
Ok(self.network.send_internal(cmd, true)?)
}

pub fn handle_urc<F: FnOnce(Urc) -> bool>(&mut self, f: F) -> Result<(), Error> {
self.network.at_tx.handle_urc(f).map_err(Error::Network)
}

fn socket_ingress_all(&mut self) -> Result<(), Error> {
if let Some(ref mut sockets) = self.sockets {
let network = &mut self.network;
Expand Down

0 comments on commit 29741f8

Please sign in to comment.