bluekitchen / btstack

Dual-mode Bluetooth stack, with small memory footprint.
http://bluekitchen-gmbh.com
Other
1.74k stars 614 forks source link

Missing basic SDP attributes definitions #618

Open Slion opened 4 months ago

Slion commented 4 months ago

For some reasons those are not defined in the assigned numbers and they are also missing from bluetooth_sdp.h Though they are define in the SDP specifications.

They look like that when fetched from the sdp_general_query.

Attribute 0x0100: type STRING (4), element len 36 , len 34, value: 'Logitech Illuminated Keyboard K810'
Attribute 0x0101: type STRING (4), element len 20 , len 18, value: 'Bluetooth Keyboard'
Attribute 0x0102: type STRING (4), element len 10 , len  8, value: 'Logitech'

We could also make them available through the APIs much like the report descriptor since we do fetch them already. That would save application to have to make another redundant SDP query.

I reckon we should be able to fetch them from hid_host_handle_sdp_client_query_result and store them either in the hid_host_connection_t structure or as external storage like the report descriptor.

mringwal commented 4 months ago

I would have thought we only query the needed attributes, but it looks like all attributes are queried currently.

What do you need the additional attributes for? They are usually not displayed in Bluetooth UIs.

Slion commented 4 months ago

I would have thought we only query the needed attributes, but it looks like all attributes are queried currently.

It did not know it was possible to query specific attributes, but yes btstack just does: sdp_client_query_uuid16(&hid_host_handle_sdp_client_query_result, (uint8_t *) connection->remote_addr, BLUETOOTH_SERVICE_CLASS_HUMAN_INTERFACE_DEVICE_SERVICE); which will return a bunch of stuff never used in btstack including the three attributes mentioned above. Maybe we could add a query for specific attributes in the examples.

Not sure what makes more sense, restrict the SDP query to specific attributes or provide access to the currently queried and discarded attributes. The SDP quey can be quite slow depending on the device your are talking to it seems. It's notably very slow with Logitech K810 and rather fast with Samsung Galaxy Tab S8 Ultra.

What do you need the additional attributes for?

To accurately describe the connected device to the user.

They are usually not displayed in Bluetooth UIs.

Which ones are usually displayed? This is an interesting point because I'm pretty sure Windows and Android displays "Logitech K810" which is not matching any of the attributes above.

Slion commented 4 months ago

Which ones are usually displayed?

That's certainly the remote name you get from that inquiry as mentioned there. I will need the full description too.

Slion commented 4 months ago

Which ones are usually displayed?

That's certainly the remote name you get from that inquiry as mentioned there. I will need the full description too.

Is there a way to get the remote name without doing an inquiry or can I somehow just do an inquiry to one device?

Slion commented 1 month ago

Having faced issues with HID report descriptors (HRD) when connecting to multiple devices in #633 I'm now of the opinion btstack should not automatically query the HRD. That business with storing all the HRDs in a single buffer is really messy especially that they get moved around in that buffer as devices are disconnecting. That means the app needs to take a copy of it anyway and so we have wasted memory.

btstack itself should not need the HRDs right? That should be something for the app to deal with really.

mringwal commented 1 month ago

Which ones are usually displayed?

That's certainly the remote name you get from that inquiry as mentioned there. I will need the full description too.

Is there a way to get the remote name without doing an inquiry or can I somehow just do an inquiry to one device?

You can use gap_remote_name_request() to query the remote name. If you're connected, this will use the connection and a single exchange -> quick.

mringwal commented 1 month ago

Having faced issues with HID report descriptors (HRD) when connecting to multiple devices in #633 I'm now of the opinion btstack should not automatically query the HRD. That business with storing all the HRDs in a single buffer is really messy especially that they get moved around in that buffer as devices are disconnecting. That means the app needs to take a copy of it anyway and so we have wasted memory.

btstack itself should not need the HRDs right? That should be something for the app to deal with really.

As the SDP query potentially returns individual bytes, we've moved the management of the HID Descriptors into the hid_host implementation. While the descriptors are moved around, there's no need to copy them as you can always access the descriptor for a device with the hid_descriptor_storage_get_descriptor_data(..) function. I don't like the memory movements either, but the other option would have been to allocate a fixed amount of memory per connection.

At the moment, hid_host.c does not use the hid descriptors itself. The hid_device.c implementation however verifies the report size before sending.