From 54815f03cacd6fa595c2f0a5b64a09d4df9ad73f Mon Sep 17 00:00:00 2001 From: Orne Brocaar Date: Tue, 24 Jan 2023 09:46:31 +0000 Subject: [PATCH] Reset channels to default on AdrAckReq uplink. As part of the ADR back-off, the device will eventually revert to the default channel-plan but there is no explicit signalling if this happened. This might cause some small overhead in case the device did not (yet) revert to the default channel-mask, but it avoids scenarios where the device and NS are out-of-sync. --- chirpstack/src/test/class_a_test.rs | 3 ++- chirpstack/src/uplink/data.rs | 32 ++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/chirpstack/src/test/class_a_test.rs b/chirpstack/src/test/class_a_test.rs index d7253cca..28e79605 100644 --- a/chirpstack/src/test/class_a_test.rs +++ b/chirpstack/src/test/class_a_test.rs @@ -4001,7 +4001,7 @@ async fn test_lorawan_10_adr() { device_queue_items: vec![], before_func: None, after_func: None, - device_session: Some(ds.clone()), + device_session: Some(ds_7chan.clone()), // we want to see the NS to reset channels tx_info: tx_info.clone(), rx_info: rx_info.clone(), phy_payload: lrwn::PhyPayload { @@ -4027,6 +4027,7 @@ async fn test_lorawan_10_adr() { assert: vec![ assert::f_cnt_up(dev.dev_eui.clone(), 11), assert::n_f_cnt_down(dev.dev_eui.clone(), 5), + assert::enabled_uplink_channel_indices(dev.dev_eui.clone(), vec![0, 1, 2]), assert::downlink_phy_payloads(vec![ lrwn::PhyPayload { mhdr: lrwn::MHDR { diff --git a/chirpstack/src/uplink/data.rs b/chirpstack/src/uplink/data.rs index 81c710e5..47fcd83c 100644 --- a/chirpstack/src/uplink/data.rs +++ b/chirpstack/src/uplink/data.rs @@ -16,7 +16,7 @@ use crate::storage::{ application, device, device_gateway, device_profile, device_queue, device_session, fields, metrics, tenant, }; -use crate::{codec, config, downlink, framelog, integration, maccommand, metalog}; +use crate::{codec, config, downlink, framelog, integration, maccommand, metalog, region}; use chirpstack_api::{api, common, integration as integration_pb, internal, meta}; use lrwn::AES128Key; @@ -97,6 +97,7 @@ impl Data { ctx.set_uplink_data_rate().await?; ctx.set_enabled_class().await?; ctx.log_uplink_meta().await?; + ctx.reset_channels_on_adr_ack_req()?; ctx.handle_mac_commands().await?; if !ctx._is_roaming() { ctx.save_device_gateway_rx_info().await?; @@ -570,6 +571,35 @@ impl Data { Ok(()) } + // This is needed as in case the device sets the ADRAckReq bit, we do not know if the device + // has reset its channels / channel-mask or not, as there is no explicit signalling in case + // this happens. This way, we make sure that the channels are always in sync, although it could + // lead to a small bit of overhead (e.g. re-sending the channels / channel-mask even if the + // device did not reset these). + fn reset_channels_on_adr_ack_req(&mut self) -> Result<()> { + trace!("Reset channels on adr ack req"); + + if let lrwn::Payload::MACPayload(pl) = &self.uplink_frame_set.phy_payload.payload { + if pl.fhdr.f_ctrl.adr_ack_req { + let region_conf = region::get(&self.uplink_frame_set.region_config_id)?; + let mut ds = self.device_session.as_mut().unwrap(); + + // We reset the device-session enabled_uplink_channel_indices and + // extra_uplink_channels. On the downlink path, the mac-command handling will + // detect that the device is out-of-sync with the NS configuration and will send + // mac-commands to re-sync. + ds.enabled_uplink_channel_indices = region_conf + .get_default_uplink_channel_indices() + .iter() + .map(|i| *i as u32) + .collect(); + ds.extra_uplink_channels = HashMap::new(); + } + } + + Ok(()) + } + async fn handle_mac_commands(&mut self) -> Result<()> { trace!("Handling uplink mac-commands");