Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(*): only handle URCs in one place #84

Merged
merged 1 commit into from
Sep 26, 2023
Merged

fix(*): only handle URCs in one place #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
74 changes: 65 additions & 9 deletions ublox-cellular/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
network::{AtTx, Network},
power::PowerState,
registration::ConnectionState,
services::data::ContextState,
services::data::{ContextState, PROFILE_ID},
UbloxCellularBuffers, UbloxCellularIngress, UbloxCellularUrcChannel,
};
use ip_transport_layer::{types::HexMode, SetHexMode};
Expand All @@ -64,7 +64,7 @@
pub struct Device<'buf, 'sub, AtCl, AtUrcCh, Config, const N: usize, const L: usize> {
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

pub(crate) state: State,
pub(crate) power_state: PowerState,
Expand Down Expand Up @@ -227,7 +227,7 @@
false,
)?;

#[cfg(any(feature = "lara-r6"))]

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

View workflow job for this annotation

GitHub Actions / clippy

unneeded sub `cfg` when there is only one condition 

warning: unneeded sub `cfg` when there is only one condition
   --> ublox-cellular/src/client.rs:230:15
    |
230 |         #[cfg(any(feature = "lara-r6"))]
    |               ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `feature = "lara-r6"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_minimal_cfg
    = note: `#[warn(clippy::non_minimal_cfg)]` on by default
self.network.send_internal(
&SetGpioConfiguration {
gpio_id: 42,
Expand Down Expand Up @@ -412,7 +412,7 @@
self.select_sim_card()?;

// Disable Message Waiting URCs (UMWI)
#[cfg(any(feature = "toby-r2"))]

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

View workflow job for this annotation

GitHub Actions / clippy

unneeded sub `cfg` when there is only one condition 

warning: unneeded sub `cfg` when there is only one condition
   --> ublox-cellular/src/client.rs:415:15
    |
415 |         #[cfg(any(feature = "toby-r2"))]
    |               ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `feature = "toby-r2"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#non_minimal_cfg
self.network.send_internal(
&crate::command::sms::SetMessageWaitingIndication {
mode: crate::command::sms::types::MessageWaitingMode::Disabled,
Expand Down Expand Up @@ -525,8 +525,10 @@
}

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() {
self.network
let res = self
.network
.at_tx
.handle_urc(|urc| {
match urc {
Expand All @@ -551,11 +553,69 @@
sock.set_available_data(length);
}
}
_ => return false,
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");
}
_ => {}
}
true
})
.map_err(Error::Network)
.map_err(Error::Network);
self.network.context_state = ctx_state;
res
} else {
Ok(())
}
Expand All @@ -578,8 +638,4 @@
result => result.map_err(Error::from),
}
}

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)
}
}
90 changes: 6 additions & 84 deletions ublox-cellular/src/network.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
types::OperatorSelectionMode, GetNetworkRegistrationStatus, SetOperatorSelection,
},
psn::{
self, types::PDPContextStatus, GetEPSNetworkRegistrationStatus,

Check warning on line 13 in ublox-cellular/src/network.rs

View workflow job for this annotation

GitHub Actions / clippy

unused imports: `PROFILE_ID`, `self` 

warning: unused imports: `PROFILE_ID`, `self`
  --> ublox-cellular/src/network.rs:13:13
   |
13 |             self, types::PDPContextStatus, GetEPSNetworkRegistrationStatus,
   |             ^^^^
...
20 |     services::data::{ContextState, PROFILE_ID},
   |                                    ^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default
GetGPRSNetworkRegistrationStatus, GetPDPContextState, SetPDPContextState,
},
Urc, AT,
Expand Down Expand Up @@ -118,7 +118,7 @@
})
}

pub fn handle_urc<F: FnOnce(Urc) -> bool>(&mut self, f: F) -> Result<(), Error> {
pub fn handle_urc<F: FnOnce(Urc)>(&mut self, f: F) -> Result<(), Error> {
if let Some(urc) = self.urc_subscription.try_next_message_pure() {
f(urc);
}
Expand Down Expand Up @@ -161,7 +161,6 @@
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 @@ -429,96 +428,19 @@
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,
check_urc: bool,

Check warning on line 434 in ublox-cellular/src/network.rs

View workflow job for this annotation

GitHub Actions / clippy

unused variable: `check_urc` 

warning: unused variable: `check_urc`
   --> ublox-cellular/src/network.rs:434:9
    |
434 |         check_urc: bool,
    |         ^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_check_urc`
    |
    = note: `#[warn(unused_variables)]` on by default
) -> Result<A::Response, Error>
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: 0 additions & 4 deletions ublox-cellular/src/services/data/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
command::psn::types::PDPContextStatus,
command::psn::SetPDPContextDefinition,
command::psn::SetPDPContextState,
command::Urc,

Check warning on line 22 in ublox-cellular/src/services/data/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unused import: `command::Urc` 

warning: unused import: `command::Urc`
  --> ublox-cellular/src/services/data/mod.rs:22:5
   |
22 |     command::Urc,
   |     ^^^^^^^^^^^^
command::{
ip_transport_layer::{
responses::{SocketData, UDPSocketData},
Expand Down Expand Up @@ -393,7 +393,7 @@
/// Activate context using 3GPP commands
/// Required for SARA-R4 and TOBY modules.
#[cfg(not(feature = "upsd-context-activation"))]
fn activate_context(&mut self, cid: ContextId, profile_id: ProfileId) -> nb::Result<(), Error> {

Check warning on line 396 in ublox-cellular/src/services/data/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unused variable: `profile_id` 

warning: unused variable: `profile_id`
   --> ublox-cellular/src/services/data/mod.rs:396:52
    |
396 |     fn activate_context(&mut self, cid: ContextId, profile_id: ProfileId) -> nb::Result<(), Error> {
    |                                                    ^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_profile_id`
if self.network.context_state == ContextState::Active {
return Ok(());
}
Expand Down Expand Up @@ -504,10 +504,6 @@
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
Loading