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

Missing basic SDP attributes definitions #618

Open
Slion opened this issue Jul 6, 2024 · 7 comments
Open

Missing basic SDP attributes definitions #618

Slion opened this issue Jul 6, 2024 · 7 comments

Comments

@Slion
Copy link

Slion commented Jul 6, 2024

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.

  • ServiceName (AttributeID 0x0100)
  • ServiceDescription (AttributeID 0x0101)
  • ProviderName (AttributeID 0x0102)

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
Copy link
Member

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
Copy link
Author

Slion commented Jul 11, 2024

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
Copy link
Author

Slion commented Jul 11, 2024

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
Copy link
Author

Slion commented Jul 11, 2024

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
Copy link
Author

Slion commented Oct 15, 2024

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
Copy link
Member

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants