Add support for sending notifications to a specific device. |
|||
Issue descriptionCurrently BlueZ only supports the ability to send notifications to all devices connected to a GATT server. Change this to be able to send notifications to a specific device. We also need to add the ability to specify if we want to send a notification or an indication.
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/29d3f15b5d1e6e99da160f46e29231c68e5f3938 commit 29d3f15b5d1e6e99da160f46e29231c68e5f3938 Author: rkc <rkc@chromium.org> Date: Mon Jun 13 17:42:50 2016 Add support in Chrome to send notifications to a specific device. This CL adds the Chrome side support for sending notications to a specific device as well as specifying whether an indiciation or notification should be used. Reviews requested: xiyuan@ - general review scheib@ - owner's review for the tests R=scheib@chromium.org, xiyuan@chromium.org BUG= 617745 Review-Url: https://codereview.chromium.org/2039773005 Cr-Commit-Position: refs/heads/master@{#399475} [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/bluez/bluetooth_adapter_bluez.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/bluez/bluetooth_adapter_bluez.h [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.h [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/bluetooth_gatt_application_service_provider_unittest.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider.h [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.h [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/test/bluetooth_test.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/test/bluetooth_test.h [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/test/bluetooth_test_bluez.cc [modify] https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938/device/bluetooth/test/bluetooth_test_bluez.h
,
Jun 15 2016
Im a little confused by these changes, is this supporting the idea that unsolicited notifications are a good behavior? Because otherwise I see no point in doing this since the remote shall program the CCC which the stack takes care of storing so it shall be able to identify what peers shall be notified. Furthermore this does requires changes to D-Bus interface which should perhaps be discussed first before introducing such a change.
,
Jun 15 2016
Unsolicited notifications are unrelated to this. This bug only seeks to add support for sending a notification to only 'one' of many devices that may have requested notifications. The CLs till now only add the capability to 'specify' the target device (which we couldn't before). A DBus change is required and we'll be starting a discussion with BlueZ authors separately to explain what we need and come up with an API. The purpose behind this API is this, Gatt Server S <--- hosts Characteristic C Device A connects to S --> requests start notifications by writing to the CCCD Device B connects to S --> requests start notifications by writing to the CCCD Device C connects to S --> requests start notifications by writing to the CCCD Gatt Server S wants to send a notification *only* to B, not to A or C, we want to be able to do that. Currently the DBus API (or till now even the Chrome API) did not allow this. This is the capability we're looking to add.
,
Jun 15 2016
(moving to M54, since there are no plans on working further on this for M53)
,
Jun 15 2016
Is this going to be possible on other platforms? If not please make sure it's clearly documented in the Cross Platform API.
,
Jun 15 2016
AFAICT, it is. On Android the API is pretty clear. Core Bluetooth has this line, "When you call this method to send updated characteristic values to subscribed centrals, you can specify which centrals you want to update in the last parameter." in their docs. Windows doesn't have a GATT server API yet. I presumed that we only document when a cross platform API 'cannot' be implemented on a particular platform. Otherwise every single API would have an annotation of which platforms it works on, wouldn't it?
,
Jun 15 2016
Actually I don't think it is correct to have different policies per device when they have CCCD configured, there are only 3 values for CCCD, notify, indicate and disable. I believe this was actually done on purpose so the stack could implement this in a generic way, and there are qualifications tests for this behavior, otherwise every single profile would need to implement a scheme with their own values.
,
Jun 15 2016
This has nothing to do with whether or not the CCCD has been set to allow notifications/indications. This is only to specify *which* of all the devices that have requested notifications should this be sent to. This is similar to the Android's notifyCharacteristicChanged API ( boolean notifyCharacteristicChanged (BluetoothDevice device, BluetoothGattCharacteristic characteristic, boolean confirm) ). It allows us to specify which device the notification should be sent to. If the device has not solicited notifications, the notification won't be sent (and/or we'll return an error).
,
Jun 16 2016
Luiz, after thinking about this a bit, I think I agree with you. We shouldn't decide what devices are sent the notification based on user input. The BT spec itself doesn't make it very clear on what the expected behavior is (it says that we 'enable' notifications for that device, not that they are required) but thinking about the fact that if a Characteristic Value is updated for one device, it is updated for all devices, so all subscribed devices really should get the notification. I'll revert this CL, marking this bug as WontFix.
,
Jun 16 2016
Which is not what CCCD was intended for, it is a client configuration which shall be followed, so this API allows applications to misbehave and deny client notifications/indicates.
*Perhaps you are still thinking that Im talking about unsolicited notification, which Im not and please don't be use Android's API as the state of the art, just the fact one has to iterate into a list of devices configured with a CCCD, which you state can fail, it sounds like a terrible API, one which will lead application to write error prone code like the following:
...
foreach connected device:
// Some sort of server side policy ignoring CCCD state? If CCCD is checked here then it has to be managed by the application
if device should receive a notification:
notifyCharacteristicChanged(device)
When I think in terms of D-Bus it makes even less sense, most likely we would need a list of device objects, since round trips are expensive and we want to notify devices in parallel, to notify to which can fail given the application has no idea of the state of CCCD since it managed by bluetoothd and as the values are propagated using PropertiesChanged signal there is no reply either.
,
Jun 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b06678e016f208aa77857a82325180ef3c75705 commit 7b06678e016f208aa77857a82325180ef3c75705 Author: rkc <rkc@chromium.org> Date: Thu Jun 16 01:18:47 2016 Revert of Add support in Chrome to send notifications to a specific device. (patchset #3 id:40001 of https://codereview.chromium.org/2039773005/ ) Reason for revert: Original bug is not WontFix. Original issue's description: > Add support in Chrome to send notifications to a specific device. > > This CL adds the Chrome side support for sending notications to a specific > device as well as specifying whether an indiciation or notification should > be used. > > Reviews requested: > > xiyuan@ - general review > scheib@ - owner's review for the tests > > R=scheib@chromium.org, xiyuan@chromium.org > BUG= 617745 > > Committed: https://crrev.com/29d3f15b5d1e6e99da160f46e29231c68e5f3938 > Cr-Commit-Position: refs/heads/master@{#399475} TBR=scheib@chromium.org,xiyuan@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 617745 Review-Url: https://codereview.chromium.org/2076433002 Cr-Commit-Position: refs/heads/master@{#400068} [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/bluetooth_local_gatt_characteristic_unittest.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/bluez/bluetooth_adapter_bluez.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/bluez/bluetooth_adapter_bluez.h [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/bluez/bluetooth_local_gatt_characteristic_bluez.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/bluetooth_gatt_application_service_provider.h [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/bluetooth_gatt_application_service_provider_unittest.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider.h [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/bluetooth_gatt_characteristic_service_provider_impl.h [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/dbus/fake_bluetooth_gatt_characteristic_service_provider.h [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/test/bluetooth_test.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/test/bluetooth_test.h [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/test/bluetooth_test_bluez.cc [modify] https://crrev.com/7b06678e016f208aa77857a82325180ef3c75705/device/bluetooth/test/bluetooth_test_bluez.h
,
Jun 16 2016
Luiz, I agree with you :) Which is why I reverted my change and marked this issue as WontFix. |
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Jun 13 2016