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(macos): reading descriptor value #391

Merged
merged 3 commits into from
Jul 23, 2024
Merged

fix(macos): reading descriptor value #391

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca1ab83
fix: reading descriptor value
kovapatrik Jul 15, 2024
35b1d3d
fix: incorrect delegate
kovapatrik Jul 15, 2024
a090173
fix: use class type instead of matching UUIDs
kovapatrik Jul 15, 2024
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
21 changes: 18 additions & 3 deletions src/corebluetooth/central_delegate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ declare_class!(
}
}

#[method(peripheral:didUpdateNotificationStateForCharacteristic:error:)]
#[method(peripheral:didWriteValueForCharacteristic:error:)]
fn delegate_peripheral_didwritevalueforcharacteristic_error(
&self,
peripheral: &CBPeripheral,
Expand All @@ -622,7 +622,7 @@ declare_class!(
}
}

#[method(peripheral:didWriteValueForCharacteristic:error:)]
#[method(peripheral:didUpdateNotificationStateForCharacteristic:error:)]
fn delegate_peripheral_didupdatenotificationstateforcharacteristic_error(
&self,
peripheral: &CBPeripheral,
Expand Down Expand Up @@ -685,7 +685,7 @@ declare_class!(
service_uuid: cbuuid_to_uuid(unsafe { &service.UUID() }),
characteristic_uuid: cbuuid_to_uuid(unsafe { &characteristic.UUID() }),
descriptor_uuid: cbuuid_to_uuid(unsafe { &descriptor.UUID() }),
data: get_characteristic_value(&characteristic),
data: get_descriptor_value(&descriptor),
});
// Notify BluetoothGATTCharacteristic::read_value that read was successful.
}
Expand Down Expand Up @@ -749,6 +749,21 @@ fn get_characteristic_value(characteristic: &CBCharacteristic) -> Vec<u8> {
v.unwrap_or_default()
}

fn get_descriptor_value(descriptor: &CBDescriptor) -> Vec<u8> {
trace!("Getting data!");
let v = unsafe { descriptor.value() }.map(|value| unsafe {
match descriptor.UUID().UUIDString().to_string().as_str() {
"2901" => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's special about "2901" here? Why can't this work for other descriptor UUIDs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are predefined UUIDs for descriptors for which we know the type for (in case of 2901, it is NSString). I think it's possible to create custom descriptors, but without knowing what the underlying type if for that it's not possible to get the value (AFAIK)

The predefined descriptor UUIDs are in this pdf, in section 3.7:

https://www.bluetooth.com/wp-content/uploads/Files/Specification/HTML/Assigned_Numbers/out/en/Assigned_Numbers.pdf?v=1720988510534

Copy link
Contributor Author

@kovapatrik kovapatrik Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Forgot to mention the root problem: descriptor.value() is a Retained<AnyObject>. It must be cast to a type and it will throw an error if you try to cast it to an invalid type. So in this case if I would try to cast the value to for example NSData instead of NSString, it will throw an error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

From what I can see in the Apple documentation, it looks like the value will either be an NSString, NSData, NSNumber or UInt16. Can you use AnyObject::class to check which type it is, then cast and convert it accordingly? This should be more robust and general than special-casing each UUID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to trace back to the superclass of the class because in my case I have the same descriptor within 5 different characteristic and in one characteristic the type was NSTaggedPointerString, in the other one it was __NSCFString. Both are subclasses of NSString and can be safely cast to it. This way it's a bit hacky to get the value of NSNumber. Do you have any suggestions how should I get the class of a number this way? Also, should we panic if there are no matching class type?

let d: Retained<NSString> = Retained::cast(value);
d.to_string().into_bytes()
}
_ => vec![],
}
});
trace!("BluetoothGATTDescriptor::get_value -> {:?}", v);
v.unwrap_or_default()
}

fn peripheral_debug(peripheral: &CBPeripheral) -> String {
let uuid = unsafe { peripheral.identifier() }.UUIDString();
if let Some(name) = unsafe { peripheral.name() } {
Expand Down
5 changes: 2 additions & 3 deletions src/corebluetooth/internal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,15 +1009,14 @@ impl CoreBluetoothInternal {
if let Some(service) = peripheral.services.get_mut(&service_uuid) {
if let Some(characteristic) = service.characteristics.get_mut(&characteristic_uuid)
{
if let Some(_descriptor) = characteristic.descriptors.get_mut(&descriptor_uuid)
{
if let Some(descriptor) = characteristic.descriptors.get_mut(&descriptor_uuid) {
trace!("Got read event!");

let mut data_clone = Vec::new();
for byte in data.iter() {
data_clone.push(*byte);
}
let state = characteristic.read_future_state.pop_back().unwrap();
let state = descriptor.read_future_state.pop_back().unwrap();
state
.lock()
.unwrap()
Expand Down
Loading