From d26f70e9ac340a7e74bfd62f09e1a76da2d8222c Mon Sep 17 00:00:00 2001 From: Alex Moon Date: Mon, 25 Mar 2024 16:01:07 -0400 Subject: [PATCH 1/2] Make data length updates optional No longer initiate the data length update procedure immediately after connection. Instead, provide a `data_length_update` method on `Connection` to allow the application to choose whether and with what parameters to perform the data length update. --- nrf-softdevice/src/ble/central.rs | 7 +--- nrf-softdevice/src/ble/connection.rs | 57 ++++++++++++++++++++++++++++ nrf-softdevice/src/ble/gap.rs | 27 ++----------- nrf-softdevice/src/ble/peripheral.rs | 7 +--- 4 files changed, 62 insertions(+), 36 deletions(-) diff --git a/nrf-softdevice/src/ble/central.rs b/nrf-softdevice/src/ble/central.rs index f516b6f..8a13305 100644 --- a/nrf-softdevice/src/ble/central.rs +++ b/nrf-softdevice/src/ble/central.rs @@ -98,12 +98,7 @@ where debug!("connected role={:?} peer_addr={:?}", role, peer_address); match new_conn(conn_handle, role, peer_address, conn_params) { - Ok(conn) => { - #[cfg(any(feature = "s113", feature = "s132", feature = "s140"))] - crate::ble::gap::do_data_length_update(conn_handle, ptr::null()); - - Ok(conn) - } + Ok(conn) => Ok(conn), Err(_) => { raw::sd_ble_gap_disconnect( conn_handle, diff --git a/nrf-softdevice/src/ble/connection.rs b/nrf-softdevice/src/ble/connection.rs index 96be422..3ab6c81 100644 --- a/nrf-softdevice/src/ble/connection.rs +++ b/nrf-softdevice/src/ble/connection.rs @@ -63,6 +63,25 @@ impl From for IgnoreSlaveLatencyError { } } +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[cfg_attr(feature = "defmt", derive(defmt::Format))] +pub enum DataLengthUpdateError { + Disconnected, + Raw(RawError), +} + +impl From for DataLengthUpdateError { + fn from(_err: DisconnectedError) -> Self { + Self::Disconnected + } +} + +impl From for DataLengthUpdateError { + fn from(err: RawError) -> Self { + Self::Raw(err) + } +} + pub enum PhyUpdateError { Disconnected, Raw(RawError), @@ -543,6 +562,44 @@ impl Connection { ConnectionIter(0) } + /// Initiate a Data Length Update procedure. + /// + /// Note that this just initiates the data length update, it does not wait for completion. + /// Immediately after return, the active data length will still be the old one, and after some time they + /// should change to the new ones. + #[cfg(any(feature = "s113", feature = "s132", feature = "s140"))] + pub fn data_length_update( + &mut self, + params: Option<&raw::ble_gap_data_length_params_t>, + ) -> Result { + let conn_handle = self.with_state(|state| state.check_connected())?; + + let params = params.map(core::ptr::from_ref).unwrap_or(core::ptr::null()); + let mut dl_limitation = unsafe { core::mem::zeroed() }; + let ret = unsafe { raw::sd_ble_gap_data_length_update(conn_handle, params, &mut dl_limitation) }; + + if let Err(err) = RawError::convert(ret) { + warn!("sd_ble_gap_data_length_update err {:?}", err); + return Err(err.into()); + } + + if dl_limitation.tx_payload_limited_octets != 0 || dl_limitation.rx_payload_limited_octets != 0 { + warn!( + "The requested TX/RX packet length is too long by {:?}/{:?} octets.", + dl_limitation.tx_payload_limited_octets, dl_limitation.rx_payload_limited_octets + ); + } + + if dl_limitation.tx_rx_time_limited_us != 0 { + warn!( + "The requested combination of TX and RX packet lengths is too long by {:?} us", + dl_limitation.tx_rx_time_limited_us + ); + } + + Ok(dl_limitation) + } + /// Send a request to the connected device to change the PHY. /// /// Note that this just initiates the PHY change, it does not wait for completion. diff --git a/nrf-softdevice/src/ble/gap.rs b/nrf-softdevice/src/ble/gap.rs index 5dddfe7..cec25a0 100644 --- a/nrf-softdevice/src/ble/gap.rs +++ b/nrf-softdevice/src/ble/gap.rs @@ -133,7 +133,9 @@ pub(crate) unsafe fn on_evt(ble_evt: *const raw::ble_evt_t) { ); let conn_handle = gap_evt.conn_handle; - do_data_length_update(conn_handle, core::ptr::null()); + if let Some(mut conn) = Connection::from_handle(conn_handle) { + let _ = conn.data_length_update(None); + } } #[cfg(any(feature = "s113", feature = "s132", feature = "s140"))] raw::BLE_GAP_EVTS_BLE_GAP_EVT_DATA_LENGTH_UPDATE => { @@ -374,29 +376,6 @@ pub(crate) unsafe fn on_evt(ble_evt: *const raw::ble_evt_t) { } } -#[cfg(any(feature = "s113", feature = "s132", feature = "s140"))] -pub(crate) unsafe fn do_data_length_update(conn_handle: u16, params: *const raw::ble_gap_data_length_params_t) { - let mut dl_limitation = core::mem::zeroed(); - let ret = raw::sd_ble_gap_data_length_update(conn_handle, params, &mut dl_limitation); - if let Err(_err) = RawError::convert(ret) { - warn!("sd_ble_gap_data_length_update err {:?}", _err); - - if dl_limitation.tx_payload_limited_octets != 0 || dl_limitation.rx_payload_limited_octets != 0 { - warn!( - "The requested TX/RX packet length is too long by {:?}/{:?} octets.", - dl_limitation.tx_payload_limited_octets, dl_limitation.rx_payload_limited_octets - ); - } - - if dl_limitation.tx_rx_time_limited_us != 0 { - warn!( - "The requested combination of TX and RX packet lengths is too long by {:?} us", - dl_limitation.tx_rx_time_limited_us - ); - } - } -} - pub fn set_device_identities_list( sd: &Softdevice, id_keys: &[IdentityKey], diff --git a/nrf-softdevice/src/ble/peripheral.rs b/nrf-softdevice/src/ble/peripheral.rs index 8811ac8..4acfbda 100644 --- a/nrf-softdevice/src/ble/peripheral.rs +++ b/nrf-softdevice/src/ble/peripheral.rs @@ -370,12 +370,7 @@ where debug!("connected role={:?} peer_addr={:?}", role, peer_address); match f(conn_handle, role, peer_address, conn_params) { - Ok(conn) => { - #[cfg(any(feature = "s113", feature = "s132", feature = "s140"))] - gap::do_data_length_update(conn_handle, ptr::null()); - - Ok(conn) - } + Ok(conn) => Ok(conn), Err(_) => { raw::sd_ble_gap_disconnect( conn_handle, From c70989612be75651bdd51e87d3ef8045010dc6e0 Mon Sep 17 00:00:00 2001 From: Alex Moon Date: Mon, 25 Mar 2024 16:39:48 -0400 Subject: [PATCH 2/2] Return `ble_gap_data_length_limitation` for those error variants for which it is valid. --- nrf-softdevice/src/ble/connection.rs | 50 +++++++++++++++------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/nrf-softdevice/src/ble/connection.rs b/nrf-softdevice/src/ble/connection.rs index 3ab6c81..f373409 100644 --- a/nrf-softdevice/src/ble/connection.rs +++ b/nrf-softdevice/src/ble/connection.rs @@ -63,25 +63,22 @@ impl From for IgnoreSlaveLatencyError { } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -#[cfg_attr(feature = "defmt", derive(defmt::Format))] +#[cfg(any(feature = "s113", feature = "s132", feature = "s140"))] +#[derive(Debug, Clone, Copy)] pub enum DataLengthUpdateError { Disconnected, + NotSupported(raw::ble_gap_data_length_limitation_t), + Resources(raw::ble_gap_data_length_limitation_t), Raw(RawError), } +#[cfg(any(feature = "s113", feature = "s132", feature = "s140"))] impl From for DataLengthUpdateError { fn from(_err: DisconnectedError) -> Self { Self::Disconnected } } -impl From for DataLengthUpdateError { - fn from(err: RawError) -> Self { - Self::Raw(err) - } -} - pub enum PhyUpdateError { Disconnected, Raw(RawError), @@ -571,7 +568,7 @@ impl Connection { pub fn data_length_update( &mut self, params: Option<&raw::ble_gap_data_length_params_t>, - ) -> Result { + ) -> Result<(), DataLengthUpdateError> { let conn_handle = self.with_state(|state| state.check_connected())?; let params = params.map(core::ptr::from_ref).unwrap_or(core::ptr::null()); @@ -580,24 +577,31 @@ impl Connection { if let Err(err) = RawError::convert(ret) { warn!("sd_ble_gap_data_length_update err {:?}", err); - return Err(err.into()); - } - if dl_limitation.tx_payload_limited_octets != 0 || dl_limitation.rx_payload_limited_octets != 0 { - warn!( - "The requested TX/RX packet length is too long by {:?}/{:?} octets.", - dl_limitation.tx_payload_limited_octets, dl_limitation.rx_payload_limited_octets - ); - } + if dl_limitation.tx_payload_limited_octets != 0 || dl_limitation.rx_payload_limited_octets != 0 { + warn!( + "The requested TX/RX packet length is too long by {:?}/{:?} octets.", + dl_limitation.tx_payload_limited_octets, dl_limitation.rx_payload_limited_octets + ); + } - if dl_limitation.tx_rx_time_limited_us != 0 { - warn!( - "The requested combination of TX and RX packet lengths is too long by {:?} us", - dl_limitation.tx_rx_time_limited_us - ); + if dl_limitation.tx_rx_time_limited_us != 0 { + warn!( + "The requested combination of TX and RX packet lengths is too long by {:?} us", + dl_limitation.tx_rx_time_limited_us + ); + } + + let err = match err { + RawError::NotSupported => DataLengthUpdateError::NotSupported(dl_limitation), + RawError::Resources => DataLengthUpdateError::Resources(dl_limitation), + err => DataLengthUpdateError::Raw(err), + }; + + return Err(err); } - Ok(dl_limitation) + Ok(()) } /// Send a request to the connected device to change the PHY.