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.

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

Requesting MTU #407

Draft
wants to merge 6 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ tokio-stream = { version = "0.1.16", features = ["sync"] }

[target.'cfg(target_os = "linux")'.dependencies]
dbus = "0.9.7"
bluez-async = "0.7.2"
bluez-async = "0.8.0"

[target.'cfg(target_os = "android")'.dependencies]
jni = "0.19.0"
Expand Down
6 changes: 6 additions & 0 deletions src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ pub use self::bdaddr::{BDAddr, ParseBDAddrError};

use crate::platform::PeripheralId;

/// The default MTU size for a peripheral.
pub const DEFAULT_MTU_SIZE: u16 = 23;

#[cfg_attr(
feature = "serde",
derive(Serialize, Deserialize),
Expand Down Expand Up @@ -230,6 +233,9 @@ pub trait Peripheral: Send + Sync + Clone + Debug {
/// Returns the MAC address of the peripheral.
fn address(&self) -> BDAddr;

/// Returns the currently negotiated mtu size
fn mtu(&self) -> u16;

/// Returns the set of properties associated with the peripheral. These may be updated over time
/// as additional advertising reports are received.
async fn properties(&self) -> Result<Option<PeripheralProperties>>;
Expand Down
11 changes: 11 additions & 0 deletions src/bluez/peripheral.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,17 @@ impl api::Peripheral for Peripheral {
self.mac_address
}

fn mtu(&self) -> u16 {
let services = self.services.lock().unwrap();
for (_, service) in services.iter() {
for (_, characteristic) in service.characteristics.iter() {
return characteristic.info.mtu.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be different for different characteristics?

Copy link
Author

Choose a reason for hiding this comment

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

In Bluetooth 5.2 and forward, yes. The reason they implemented it as a per-characteristic MTU is because of the enhanced ATT (EATT) bearer feature.

They mention it here: bluez/bluez#199
I mention it here: #246 (comment)

The majority of devices on the market are going to be using the unenhanced version (single MTU per connection), so I don't understand why bluez didn't add support for that first.

In any case, the implementation was based on how SimpleBle did it: https://github.com/OpenBluetoothToolbox/SimpleBLE/blob/614a5af35cdd22ba08318cb776795ecd2da8c389/simpleble/src/backends/linux/PeripheralBase.cpp#L57

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, seems reasonable given the API limitations.

}
}

api::DEFAULT_MTU_SIZE
}

async fn properties(&self) -> Result<Option<PeripheralProperties>> {
let device_info = self.device_info().await?;
Ok(Some(PeripheralProperties {
Expand Down
32 changes: 31 additions & 1 deletion src/winrtble/ble/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,37 @@ use windows::{
BluetoothCacheMode, BluetoothConnectionStatus, BluetoothLEDevice,
GenericAttributeProfile::{
GattCharacteristic, GattCommunicationStatus, GattDescriptor, GattDeviceService,
GattDeviceServicesResult,
GattDeviceServicesResult, GattSession,
},
},
Foundation::{EventRegistrationToken, TypedEventHandler},
};

pub type ConnectedEventHandler = Box<dyn Fn(bool) + Send>;
pub type MaxPduSizeChangedEventHandler = Box<dyn Fn(u16) + Send>;

pub struct BLEDevice {
device: BluetoothLEDevice,
gatt_session: GattSession,
connection_token: EventRegistrationToken,
pdu_change_token: EventRegistrationToken,
services: Vec<GattDeviceService>,
}

impl BLEDevice {
pub async fn new(
address: BDAddr,
connection_status_changed: ConnectedEventHandler,
max_pdu_size_changed: MaxPduSizeChangedEventHandler,
) -> Result<Self> {
let async_op = BluetoothLEDevice::FromBluetoothAddressAsync(address.into())
.map_err(|_| Error::DeviceNotFound)?;
let device = async_op.await.map_err(|_| Error::DeviceNotFound)?;

let async_op = GattSession::FromDeviceIdAsync(&device.BluetoothDeviceId()?)
.map_err(|_| Error::DeviceNotFound)?;
let gatt_session = async_op.await.map_err(|_| Error::DeviceNotFound)?;

let connection_status_handler =
TypedEventHandler::new(move |sender: &Option<BluetoothLEDevice>, _| {
if let Some(sender) = sender {
Expand All @@ -57,9 +66,23 @@ impl BLEDevice {
.ConnectionStatusChanged(&connection_status_handler)
.map_err(|_| Error::Other("Could not add connection status handler".into()))?;

max_pdu_size_changed(gatt_session.MaxPduSize().unwrap());
let max_pdu_size_changed_handler =
TypedEventHandler::new(move |sender: &Option<GattSession>, _| {
if let Some(sender) = sender {
max_pdu_size_changed(sender.MaxPduSize().unwrap());
}
Ok(())
});
let pdu_change_token = gatt_session
.MaxPduSizeChanged(&max_pdu_size_changed_handler)
.map_err(|_| Error::Other("Could not add max pdu size changed handler".into()))?;

Ok(BLEDevice {
device,
gatt_session,
connection_token,
pdu_change_token,
services: vec![],
})
}
Expand Down Expand Up @@ -167,6 +190,13 @@ impl BLEDevice {

impl Drop for BLEDevice {
fn drop(&mut self) {
let result = self
.gatt_session
.RemoveMaxPduSizeChanged(self.pdu_change_token);
if let Err(err) = result {
debug!("Drop: remove_max_pdu_size_changed {:?}", err);
}

let result = self
.device
.RemoveConnectionStatusChanged(self.connection_token);
Expand Down
34 changes: 28 additions & 6 deletions src/winrtble/peripheral.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use std::{
convert::TryInto,
fmt::{self, Debug, Display, Formatter},
pin::Pin,
sync::atomic::{AtomicBool, Ordering},
sync::atomic::{AtomicBool, AtomicU16, Ordering},
sync::{Arc, RwLock},
};
use tokio::sync::broadcast;
Expand Down Expand Up @@ -70,6 +70,7 @@ struct Shared {
device: tokio::sync::Mutex<Option<BLEDevice>>,
adapter: Weak<AdapterManager<Peripheral>>,
address: BDAddr,
mtu: AtomicU16,
connected: AtomicBool,
ble_services: DashMap<Uuid, BLEService>,
notifications_channel: broadcast::Sender<ValueNotification>,
Expand All @@ -93,6 +94,7 @@ impl Peripheral {
adapter,
device: tokio::sync::Mutex::new(None),
address,
mtu: AtomicU16::new(api::DEFAULT_MTU_SIZE),
connected: AtomicBool::new(false),
ble_services: DashMap::new(),
notifications_channel: broadcast_sender,
Expand Down Expand Up @@ -341,6 +343,11 @@ impl ApiPeripheral for Peripheral {
self.shared.address
}

/// Returns the currently negotiated mtu size
fn mtu(&self) -> u16 {
self.shared.mtu.load(Ordering::Relaxed)
}

/// Returns the set of properties associated with the peripheral. These may be updated over time
/// as additional advertising reports are received.
async fn properties(&self) -> Result<Option<PeripheralProperties>> {
Expand All @@ -364,12 +371,12 @@ impl ApiPeripheral for Peripheral {
/// Ok there has been successful connection. Note that peripherals allow only one connection at
/// a time. Operations that attempt to communicate with a device will fail until it is connected.
async fn connect(&self) -> Result<()> {
let shared_clone = Arc::downgrade(&self.shared);
let adapter_clone = self.shared.adapter.clone();
let address = self.shared.address;
let device = BLEDevice::new(
self.shared.address,
Box::new(move |is_connected| {

let connection_status_changed = Box::new({
let shared_clone = Arc::downgrade(&self.shared);
move |is_connected| {
if let Some(shared) = shared_clone.upgrade() {
shared.connected.store(is_connected, Ordering::Relaxed);
}
Expand All @@ -379,7 +386,22 @@ impl ApiPeripheral for Peripheral {
adapter.emit(CentralEvent::DeviceDisconnected(address.into()));
}
}
}),
}
});

let max_pdu_size_changed = Box::new({
let shared_clone = Arc::downgrade(&self.shared);
move |mtu| {
if let Some(shared) = shared_clone.upgrade() {
shared.mtu.store(mtu, Ordering::Relaxed);
}
}
});

let device = BLEDevice::new(
self.shared.address,
connection_status_changed,
max_pdu_size_changed,
)
.await?;

Expand Down
Loading