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

Bluetooth: gatt: allow disabling subscription enforcement #48929

Merged

Conversation

jori-nordic
Copy link
Contributor

This introduces a new option that allows the user to disable the
subscription checking when notifying or indicating.

Some users might have use-cases where they would like to send notifications
or indications without the peer having to go through the subscription
process, as that is allowed by the Bluetooth specification.

Signed-off-by: Jonathan Rico jonathan.rico@nordicsemi.no

This introduces a new option that allows the user to disable the
subscription checking when notifying or indicating.

Some users might have use-cases where they would like to send notifications
or indications without the peer having to go through the subscription
process, as that is allowed by the Bluetooth specification.

Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
@Thalley
Copy link
Collaborator

Thalley commented Aug 11, 2022

This introduces a new option that allows the user to disable the subscription checking when notifying or indicating.

Some users might have use-cases where they would like to send notifications or indications without the peer having to go through the subscription process, as that is allowed by the Bluetooth specification.

Can you provide an example of such a use case?

@jori-nordic
Copy link
Contributor Author

This introduces a new option that allows the user to disable the subscription checking when notifying or indicating.
Some users might have use-cases where they would like to send notifications or indications without the peer having to go through the subscription process, as that is allowed by the Bluetooth specification.

Can you provide an example of such a use case?

Not something I've seen in the wild. I don't remember the discussion exactly but I think it was so a low-mem device didn't need to perform discovery and subscription. Maybe it was with @alwa-nordic , did you suggest that?
Ah I think I found it, it was @Vudentz suggesting we leave the behavior as-is: #46136 (comment)

@Thalley
Copy link
Collaborator

Thalley commented Aug 11, 2022

This introduces a new option that allows the user to disable the subscription checking when notifying or indicating.
Some users might have use-cases where they would like to send notifications or indications without the peer having to go through the subscription process, as that is allowed by the Bluetooth specification.

Can you provide an example of such a use case?

Not something I've seen in the wild. I don't remember the discussion exactly but I think it was so a low-mem device didn't need to perform discovery and subscription. Maybe it was with @alwa-nordic , did you suggest that? Ah I think I found it, it was @Vudentz suggesting we leave the behavior as-is: #46136 (comment)

Discovery cannot really be ignored; the notification will always just provide a handle, and without knowing what the handle refers to, this will most likely be impossible to use (unless the GATT server only has a single known notifiable characteristic).

If the handle is known, subscribing to it should be very easy (and does not require more than a single write).

I'll be happy to approve if we can come up with an actual usecase :)

@jori-nordic
Copy link
Contributor Author

Yeah I don't actually have one :) . Hence closing the PR. If anyone needs that option, GH has linked to the original PR introducing the check so people can easily find it.

@alwa-nordic
Copy link
Collaborator

There is no concrete use case for now, so we can wait until someone requests this.

The reasoning for why we want to allow this is that, in principle, the spec does not specify that CCC is the only mechanism for solicitation of notifications/indications. And I can for example easily imagine an alternative CCC where the client writes the desired notification frequency.

@petejohanson
Copy link
Contributor

I believe I have a real world valid use case.

ZMK uses BAS, which works great against latest macOS and Linux, but Windows seems to simply read the value on pairing and then never updates.

Some users have used low level tools to force a subscription to the GATT characteristic, and then Windows properly updates the UI with the updated levels.

I don't have a Windows host to test, but everything points to Windows never subscribing to the notifications, but happily receiving them to update the peripheral battery level if it gets the notification.

@jori-nordic
Copy link
Contributor Author

alright then, reopening.

@jori-nordic jori-nordic reopened this Aug 17, 2022
@Thalley
Copy link
Collaborator

Thalley commented Aug 17, 2022

alright then, reopening.

It is still TBD if this will actual fix the issue, since it is only a theory so far.

@petejohanson
Copy link
Contributor

Yeah, I've asked a couple folks to test this patch + Kconfig setting to see if it helps.

@petejohanson
Copy link
Contributor

Ok, have had a user confirm by simply manually checking out that CCC check (since the patch here doesn't cleanly apply against our fork of Zephyr 3.0, and confirmed that Windows now properly updates the battery level when it changes.

So yes, Windows doesn't bother with the CCC subscription, just happily waits to receive notifications on that characteristic of BAS.

@Thalley
Copy link
Collaborator

Thalley commented Aug 17, 2022

Ok, have had a user confirm by simply manually checking out that CCC check (since the patch here doesn't cleanly apply against our fork of Zephyr 3.0, and confirmed that Windows now properly updates the battery level when it changes.

So yes, Windows doesn't bother with the CCC subscription, just happily waits to receive notifications on that characteristic of BAS.

Interesting. I guess that does state a proper case for this PR (although it's clearly odd behavior from Windows...).

@joelspadin
Copy link

This is probably the tool @petejohanson mentioned earlier: https://github.com/joelspadin/BleBatterySubscribe

It does nothing but watch for new BLE devices, look for a battery level characteristic, and write GattClientCharacteristicConfigurationDescriptorValue.Notify to it. Then the battery level starts updating correctly, which seems to be a pretty good indication that Windows doesn't do that normally. I don't have any other standard BLE devices readily available to check their behavior with or without my hacky background service though.

@Thalley
Copy link
Collaborator

Thalley commented Aug 18, 2022

This is probably the tool @petejohanson mentioned earlier: https://github.com/joelspadin/BleBatterySubscribe

It does nothing but watch for new BLE devices, look for a battery level characteristic, and write GattClientCharacteristicConfigurationDescriptorValue.Notify to it. Then the battery level starts updating correctly, which seems to be a pretty good indication that Windows doesn't do that normally. I don't have any other standard BLE devices readily available to check their behavior with or without my hacky background service though.

I wonder if the Microsoft Windows people are aware of this issue. Should be a quick fix, and since most BLE devices are battery powered, it's surprising that this isn't the case for them.

@joelspadin @petejohanson have we seen this with other OSes too? MacOS, Android, iOS?

@petejohanson
Copy link
Contributor

I wonder if the Microsoft Windows people are aware of this issue. Should be a quick fix, and since most BLE devices are battery powered, it's surprising that this isn't the case for them.

@joelspadin @petejohanson have we seen this with other OSes too? MacOS, Android, iOS?

No, only Windows so far has behaved this way.

If someone knows the right place to report this to MSFT, probably would be a good idea. I don't have much confidence in that being an efficient process to fix though.

@Thalley
Copy link
Collaborator

Thalley commented Aug 18, 2022

I wonder if the Microsoft Windows people are aware of this issue. Should be a quick fix, and since most BLE devices are battery powered, it's surprising that this isn't the case for them.
@joelspadin @petejohanson have we seen this with other OSes too? MacOS, Android, iOS?

No, only Windows so far has behaved this way.

Hmm, from this post (https://answers.microsoft.com/en-us/windows/forum/all/fixed-bluetooth-headset-and-battery-indicator/75bf5d5c-0341-4575-9b45-a7a3cdb32cad) it seems to be an issue only if you are using the headset with A2DP, and possibly should work with HFP. Which profile are you using?

If someone knows the right place to report this to MSFT, probably would be a good idea. I don't have much confidence in that being an efficient process to fix though.

Bug reports should, AFAIK, be filed via the Microsoft Feedback Hub app (https://support.microsoft.com/en-us/windows/send-feedback-to-microsoft-with-the-feedback-hub-app-f59187f8-8739-22d6-ba93-f66612949332)

@carlescufi carlescufi merged commit fae5dd3 into zephyrproject-rtos:main Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants