fix: Duplicate Characteristic UUID overwrite each other

This meant that if a device (like the LOOB) declares multiple characteristics on a service with the same UUID, the later is used not the first. Ideally we'd have access to them all, but since we can't tell them apart at the higher levels first wins seems better than last wins.
deviceplug · Oct 2, 2024 · 5c9f528 · 5c9f528
1 parent 0eb6188
commit 5c9f528
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 29 deletions.
diff --git a/src/bluez/peripheral.rs b/src/bluez/peripheral.rs
@@ -182,19 +182,35 @@ impl api::Peripheral for Peripheral {
         let services = self.session.get_services(&self.device).await?;
         for service in services {
             let characteristics = self.session.get_characteristics(&service.id).await?;
-            let characteristics =
-                join_all(characteristics.into_iter().map(|characteristic| async {
-                    let descriptors = self
-                        .session
-                        .get_descriptors(&characteristic.id)
-                        .await
-                        .unwrap_or(Vec::new())
-                        .into_iter()
-                        .map(|descriptor| (descriptor.uuid, descriptor))
-                        .collect();
-                    CharacteristicInternal::new(characteristic, descriptors)
-                }))
-                .await;
+            let characteristics = join_all(
+                characteristics
+                    .into_iter()
+                    .fold(
+                        // Only consider the first characteristic of each UUID
+                        // This "should" be unique, but of course it's not enforced
+                        HashMap::<Uuid, CharacteristicInfo>::new(),
+                        |mut map, characteristic| {
+                            if !map.contains_key(&characteristic.uuid) {
+                                map.insert(characteristic.uuid, characteristic);
+                            }
+                            map
+                        },
+                    )
+                    .into_iter()
+                    .map(|mapped_characteristic| async {
+                        let characteristic = mapped_characteristic.1;
+                        let descriptors = self
+                            .session
+                            .get_descriptors(&characteristic.id)
+                            .await
+                            .unwrap_or(Vec::new())
+                            .into_iter()
+                            .map(|descriptor| (descriptor.uuid, descriptor))
+                            .collect();
+                        CharacteristicInternal::new(characteristic, descriptors)
+                    }),
+            )
+            .await;
             services_internal.insert(
                 service.uuid,
                 ServiceInternal {

diff --git a/src/corebluetooth/internal.rs b/src/corebluetooth/internal.rs
@@ -217,15 +217,20 @@ impl PeripheralInternal {
         service_uuid: Uuid,
         characteristics: HashMap<Uuid, Retained<CBCharacteristic>>,
     ) {
-        let characteristics = characteristics
-            .into_iter()
-            .map(|(characteristic_uuid, characteristic)| {
-                (
+        let characteristics = characteristics.into_iter().fold(
+            // Only consider the first characteristic of each UUID
+            // This "should" be unique, but of course it's not enforced
+            HashMap::<Uuid, CBCharacteristic>::new(),
+            |mut map, (characteristic_uuid, characteristic)| {
+                if !map.contains_key(&characteristic_uuid) {
+                    map.insert(
                     characteristic_uuid,
                     CharacteristicInternal::new(characteristic),
-                )
-            })
-            .collect();
+                );
+                }
+                map
+            },
+        );
         let service = self
             .services
             .get_mut(&service_uuid)

diff --git a/src/droidplug/peripheral.rs b/src/droidplug/peripheral.rs
@@ -280,18 +280,22 @@ impl api::Peripheral for Peripheral {
                             characteristic_uuid: characteristic.get_uuid()?,
                         });
                     }
-                    characteristics.insert(Characteristic {
+                    let char = Characteristic {
                         service_uuid: service.get_uuid()?,
                         uuid: characteristic.get_uuid()?,
                         properties: characteristic.get_properties()?,
                         descriptors: descriptors.clone(),
-                    });
-                    peripheral_characteristics.push(Characteristic {
-                        service_uuid: service.get_uuid()?,
-                        uuid: characteristic.get_uuid()?,
-                        properties: characteristic.get_properties()?,
-                        descriptors: descriptors,
-                    });
+                    };
+                    // Only consider the first characteristic of each UUID
+                    // This "should" be unique, but of course it's not enforced
+                    if !characteristics
+                        .iter()
+                        .filter(|c| c.service_uuid == char.service_uuid && c.uuid == char.uuid)
+                        .any()
+                    {
+                        characteristics.insert(char.clone());
+                        peripheral_characteristics.push(char.clone());
+                    }
                 }
                 peripheral_services.push(Service {
                     uuid: service.get_uuid()?,

diff --git a/src/winrtble/peripheral.rs b/src/winrtble/peripheral.rs
@@ -44,6 +44,8 @@ use tokio::sync::broadcast;
 use uuid::Uuid;
 
 use std::sync::Weak;
+use windows::core::GUID;
+use windows::Devices::Bluetooth::GenericAttributeProfile::GattCharacteristic;
 use windows::Devices::Bluetooth::{Advertisement::*, BluetoothAddressType};
 
 #[cfg_attr(
@@ -414,7 +416,19 @@ impl ApiPeripheral for Peripheral {
                     match BLEDevice::get_characteristics(service).await {
                         Ok(characteristics) => {
                             let characteristics =
-                                characteristics.into_iter().map(|characteristic| async {
+                                characteristics.into_iter()
+                                    .fold(
+                                        // Only consider the first characteristic of each UUID
+                                        // This "should" be unique, but of course it's not enforced
+                                        HashMap::<GUID, GattCharacteristic>::new(),
+                                        |mut map, gatt_characteristic| {
+                                            let uuid = gatt_characteristic.Uuid().unwrap_or_default();
+                                            if !map.contains_key(&uuid) {
+                                                map.insert(uuid, gatt_characteristic);
+                                            }
+                                            map
+                                        },
+                                    ).into_iter().map(|(_,characteristic)| async {
                                     let c = characteristic.clone();
                                     (
                                         characteristic,