[bluetooth.generic] Enable BLE notification for linked channels (open…

…hab#10122)

* [bluetooth] Add BluetoothDevice.isNotifying()
* [bluetooth] Improve Characteristic properties support
* [bluez] Improve Characteristic properties support
* [bluetooth] Add BluetoothDevice.canNotify()
* [bluez] Also catch DBusExecutionException on read value
* [bluetooth.generic] Activate notifications for linked channels where characteristics are able to notify
* [bluez] Adjust javadoc
* [bluegiga] Add BluetoothDevice.isNotifying() support
* [bluegiga] Fix notification enabled check
* [bluetooth] move canNotify() to Characteristic
* [bluegiga] rename notificationEnabled to notifying
* [bluetooth.generic] use handlerToChannels to subscribe to notifications
* [bluetooth.generic] implement TODOs of canRead()/canWrite()
* [bluetooth.generic] optimize ChannelUID
* [bluetooth.generic] use channelUids for link check

Signed-off-by: Peter Rosenberg <prosenb.dev@gmail.com>

bundles/org.openhab.binding.bluetooth.bluegiga/src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothCharacteristic.java

@@ -27,7 +27,7 @@
  */
 public class BlueGigaBluetoothCharacteristic extends BluetoothCharacteristic {
 
- private boolean notificationEnabled;
+ private boolean notifying;
 
  public BlueGigaBluetoothCharacteristic(int handle) {
  super(null, handle);
@@ -45,11 +45,11 @@ public void setUUID(UUID uuid) {
  this.uuid = uuid;
  }
 
- public boolean isNotificationEnabled() {
- return notificationEnabled;
+ public boolean isNotifying() {
+ return notifying;
  }
 
- public void setNotificationEnabled(boolean enable) {
- this.notificationEnabled = enable;
+ public void setNotifying(boolean enable) {
+ this.notifying = enable;
  }
 }

bundles/org.openhab.binding.bluetooth.bluegiga/src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothDevice.java

@@ -196,7 +196,7 @@ public boolean enableNotifications(BluetoothCharacteristic characteristic) {
  }
 
  BlueGigaBluetoothCharacteristic ch = (BlueGigaBluetoothCharacteristic) characteristic;
- if (ch.isNotificationEnabled()) {
+ if (ch.isNotifying()) {
  return true;
  }
 
@@ -241,12 +241,12 @@ public boolean enableNotifications(BluetoothCharacteristic characteristic) {
  @Override
  public boolean disableNotifications(BluetoothCharacteristic characteristic) {
  if (connection == -1) {
- logger.debug("Cannot enable notifications, device not connected {}", this);
+ logger.debug("Cannot disable notifications, device not connected {}", this);
  return false;
  }
 
  BlueGigaBluetoothCharacteristic ch = (BlueGigaBluetoothCharacteristic) characteristic;
- if (ch.isNotificationEnabled()) {
+ if (!ch.isNotifying()) {
  return true;
  }
 
@@ -288,6 +288,12 @@ public boolean disableNotifications(BluetoothCharacteristic characteristic) {
  return true;
  }
 
+ @Override
+ public boolean isNotifying(BluetoothCharacteristic characteristic) {
+ BlueGigaBluetoothCharacteristic ch = (BlueGigaBluetoothCharacteristic) characteristic;
+ return ch.isNotifying();
+ }
+
  @Override
  public boolean enableNotifications(BluetoothDescriptor descriptor) {
  // TODO will be implemented in a followup PR
@@ -613,7 +619,7 @@ private void handleProcedureCompletedEvent(BlueGigaProcedureCompletedEvent event
  if (!success) {
  logger.debug("write to descriptor failed");
  }
- ((BlueGigaBluetoothCharacteristic) procedureCharacteristic).setNotificationEnabled(success);
+ ((BlueGigaBluetoothCharacteristic) procedureCharacteristic).setNotifying(success);
  procedureProgress = BlueGigaProcedure.NONE;
  procedureCharacteristic = null;
  break;
@@ -622,7 +628,7 @@ private void handleProcedureCompletedEvent(BlueGigaProcedureCompletedEvent event
  if (!success) {
  logger.debug("write to descriptor failed");
  }
- ((BlueGigaBluetoothCharacteristic) procedureCharacteristic).setNotificationEnabled(!success);
+ ((BlueGigaBluetoothCharacteristic) procedureCharacteristic).setNotifying(!success);
  procedureProgress = BlueGigaProcedure.NONE;
  procedureCharacteristic = null;
  break;
@@ -656,7 +662,7 @@ private void handleDisconnectedEvent(BlueGigaDisconnectedEvent event) {
  }
 
  for (BlueGigaBluetoothCharacteristic ch : handleToCharacteristic.values()) {
- ch.setNotificationEnabled(false);
+ ch.setNotifying(false);
  }
 
  cancelTimer(procedureTimer);

bundles/org.openhab.binding.bluetooth.bluez/src/main/java/org/openhab/binding/bluetooth/bluez/internal/BlueZBluetoothDevice.java

@@ -57,6 +57,7 @@
  *
  * @author Kai Kreuzer - Initial contribution and API
  * @author Benjamin Lafois - Replaced tinyB with bluezDbus
+ * @author Peter Rosenberg - Improve notifications and properties support
  *
  */
 @NonNullByDefault
@@ -399,6 +400,7 @@ public boolean discoverServices() {
  for (BluetoothGattCharacteristic dBusBlueZCharacteristic : dBusBlueZService.getGattCharacteristics()) {
  BluetoothCharacteristic characteristic = new BluetoothCharacteristic(
  UUID.fromString(dBusBlueZCharacteristic.getUuid()), 0);
+ convertCharacteristicProperties(dBusBlueZCharacteristic, characteristic);
 
  for (BluetoothGattDescriptor dBusBlueZDescriptor : dBusBlueZCharacteristic.getGattDescriptors()) {
  BluetoothDescriptor descriptor = new BluetoothDescriptor(characteristic,
@@ -414,6 +416,42 @@ public boolean discoverServices() {
  return true;
  }
 
+ /**
+ * Convert the flags of BluetoothGattCharacteristic to the int bitset used by BluetoothCharacteristic.
+ *
+ * @param dBusBlueZCharacteristic source characteristic to read the flags from
+ * @param characteristic destination characteristic to write to properties to
+ */
+ private void convertCharacteristicProperties(BluetoothGattCharacteristic dBusBlueZCharacteristic,
+ BluetoothCharacteristic characteristic) {
+ int properties = 0;
+
+ for (String property : dBusBlueZCharacteristic.getFlags()) {
+ switch (property) {
+ case "broadcast":
+ properties |= BluetoothCharacteristic.PROPERTY_BROADCAST;
+ break;
+ case "read":
+ properties |= BluetoothCharacteristic.PROPERTY_READ;
+ break;
+ case "write-without-response":
+ properties |= BluetoothCharacteristic.PROPERTY_WRITE_NO_RESPONSE;
+ break;
+ case "write":
+ properties |= BluetoothCharacteristic.PROPERTY_WRITE;
+ break;
+ case "notify":
+ properties |= BluetoothCharacteristic.PROPERTY_NOTIFY;
+ break;
+ case "indicate":
+ properties |= BluetoothCharacteristic.PROPERTY_INDICATE;
+ break;
+ }
+ }
+
+ characteristic.setProperties(properties);
+ }
+
  @Override
  public boolean readCharacteristic(BluetoothCharacteristic characteristic) {
  BluetoothGattCharacteristic c = getDBusBlueZCharacteristicByUUID(characteristic.getUuid().toString());
@@ -428,7 +466,8 @@ public boolean readCharacteristic(BluetoothCharacteristic characteristic) {
  characteristic.setValue(value);
  notifyListeners(BluetoothEventType.CHARACTERISTIC_READ_COMPLETE, characteristic,
  BluetoothCompletionStatus.SUCCESS);
- } catch (DBusException e) {
+ } catch (DBusException | DBusExecutionException e) {
+ // DBusExecutionException is thrown if the value cannot be read
  logger.debug("Exception occurred when trying to read characteristic '{}': {}", characteristic.getUuid(),
  e.getMessage());
  notifyListeners(BluetoothEventType.CHARACTERISTIC_READ_COMPLETE, characteristic,
@@ -438,6 +477,18 @@ public boolean readCharacteristic(BluetoothCharacteristic characteristic) {
  return true;
  }
 
+ @Override
+ public boolean isNotifying(BluetoothCharacteristic characteristic) {
+ BluetoothGattCharacteristic c = getDBusBlueZCharacteristicByUUID(characteristic.getUuid().toString());
+ if (c != null) {
+ Boolean isNotifying = c.isNotifying();
+ return Objects.requireNonNullElse(isNotifying, false);
+ } else {
+ logger.warn("Characteristic '{}' is missing on device '{}'.", characteristic.getUuid(), address);
+ return false;
+ }
+ }
+
  @Override
  public boolean disableNotifications(BluetoothCharacteristic characteristic) {
  BluetoothGattCharacteristic c = getDBusBlueZCharacteristicByUUID(characteristic.getUuid().toString());

bundles/org.openhab.binding.bluetooth.generic/src/main/java/org/openhab/binding/bluetooth/generic/internal/GenericBluetoothHandler.java

@@ -58,6 +58,7 @@
  * channels based off of a bluetooth device's GATT characteristics.
  *
  * @author Connor Petty - Initial contribution
+ * @author Peter Rosenberg - Use notifications
  *
  */
 @NonNullByDefault
@@ -68,6 +69,7 @@ public class GenericBluetoothHandler extends ConnectedBluetoothHandler {
  private final Map<ChannelUID, CharacteristicHandler> channelHandlers = new ConcurrentHashMap<>();
  private final BluetoothGattParser gattParser = BluetoothGattParserFactory.getDefault();
  private final CharacteristicChannelTypeProvider channelTypeProvider;
+ private final Map<CharacteristicHandler, List<ChannelUID>> handlerToChannels = new ConcurrentHashMap<>();
 
  private @Nullable ScheduledFuture<?> readCharacteristicJob = null;
 
@@ -84,20 +86,35 @@ public void initialize() {
  readCharacteristicJob = scheduler.scheduleWithFixedDelay(() -> {
  if (device.getConnectionState() == ConnectionState.CONNECTED) {
  if (resolved) {
- for (CharacteristicHandler charHandler : charHandlers.values()) {
- if (charHandler.canRead()) {
+ handlerToChannels.forEach((charHandler, channelUids) -> {
+ // Only read the value manually if notification is not on.
+ // Also read it the first time before we activate notifications below.
+ if (!device.isNotifying(charHandler.characteristic) && charHandler.canRead()) {
  device.readCharacteristic(charHandler.characteristic);
  try {
  // TODO the ideal solution would be to use locks/conditions and timeouts
- // between this code and `onCharacteristicReadComplete` but
+ // Kbetween this code and `onCharacteristicReadComplete` but
  // that would overcomplicate the code a bit and I plan
  // on implementing a better more generalized solution later
  Thread.sleep(50);
  } catch (InterruptedException e) {
  return;
  }
  }
- }
+ if (charHandler.characteristic.canNotify()) {
+ // Enabled/Disable notifications dependent on if the channel is linked.
+ // TODO check why isLinked() is true for not linked channels
+ if (channelUids.stream().anyMatch(this::isLinked)) {
+ if (!device.isNotifying(charHandler.characteristic)) {
+ device.enableNotifications(charHandler.characteristic);
+ }
+ } else {
+ if (device.isNotifying(charHandler.characteristic)) {
+ device.disableNotifications(charHandler.characteristic);
+ }
+ }
+ }
+ });
  } else {
  // if we are connected and still haven't been able to resolve the services, try disconnecting and
  // then connecting again
@@ -117,6 +134,7 @@ public void dispose() {
 
  charHandlers.clear();
  channelHandlers.clear();
+ handlerToChannels.clear();
  }
 
  @Override
@@ -161,9 +179,11 @@ private void updateThingChannels() {
  logger.trace("{} processing characteristic {}", address, characteristic.getUuid());
  CharacteristicHandler handler = getCharacteristicHandler(characteristic);
  List<Channel> chans = handler.buildChannels();
- for (Channel channel : chans) {
- channelHandlers.put(channel.getUID(), handler);
+ List<ChannelUID> chanUids = chans.stream().map(Channel::getUID).collect(Collectors.toList());
+ for (ChannelUID channel : chanUids) {
+ channelHandlers.put(channel, handler);
  }
+ handlerToChannels.put(handler, chanUids);
  return chans.stream();
  })//
  .collect(Collectors.toList());
@@ -341,17 +361,15 @@ public boolean canRead() {
  if (gattParser.isKnownCharacteristic(charUUID)) {
  return gattParser.isValidForRead(charUUID);
  }
- // TODO: need to evaluate this from characteristic properties, but such properties aren't support yet
- return true;
+ return characteristic.canRead();
  }
 
  public boolean canWrite() {
  String charUUID = getCharacteristicUUID();
  if (gattParser.isKnownCharacteristic(charUUID)) {
  return gattParser.isValidForWrite(charUUID);
  }
- // TODO: need to evaluate this from characteristic properties, but such properties aren't support yet
- return true;
+ return characteristic.canWrite();
  }
 
  private boolean isAdvanced() {

bundles/org.openhab.binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothCharacteristic.java

@@ -31,6 +31,7 @@
  *
  * @author Chris Jackson - Initial contribution
  * @author Kai Kreuzer - Cleaned up code
+ * @author Peter Rosenberg - Improve properties support
  */
 public class BluetoothCharacteristic {
  public static final int FORMAT_UINT8 = 0x11;
@@ -142,6 +143,16 @@ public int getInstanceId() {
  return instance;
  }
 
+ /**
+ * Set the raw properties. The individual properties are represented as bits inside
+ * of this int value.
+ *
+ * @param properties of this Characteristic
+ */
+ public void setProperties(int properties) {
+ this.properties = properties;
+ }
+
  /**
  * Returns the properties of this characteristic.
  *
@@ -152,6 +163,46 @@ public int getProperties() {
  return properties;
  }
 
+ /**
+ * Returns if the given characteristics property is enabled or not.
+ *
+ * @param property one of the Constants BluetoothCharacteristic.PROPERTY_XX
+ * @return true if this characteristic has the given property enabled, false if properties not set or
+ * the given property is not enabled.
+ */
+ public boolean hasPropertyEnabled(int property) {
+ return (properties & property) != 0;
+ }
+
+ /**
+ * Returns if notifications can be enabled on this characteristic.
+ *
+ * @return true if notifications can be enabled, false if notifications are not supported, characteristic is missing
+ * on device or notifications are not supported.
+ */
+ public boolean canNotify() {
+ return hasPropertyEnabled(BluetoothCharacteristic.PROPERTY_NOTIFY);
+ }
+
+ /**
+ * Returns if the value can be read on this characteristic.
+ *
+ * @return true if the value can be read, false otherwise.
+ */
+ public boolean canRead() {
+ return hasPropertyEnabled(BluetoothCharacteristic.PROPERTY_READ);
+ }
+
+ /**
+ * Returns if the value can be written on this characteristic.
+ *
+ * @return true if the value can be written with of without a response, false otherwise.
+ */
+ public boolean canWrite() {
+ return hasPropertyEnabled(BluetoothCharacteristic.PROPERTY_WRITE)
+ || hasPropertyEnabled(BluetoothCharacteristic.PROPERTY_WRITE_NO_RESPONSE);
+ }
+
  /**
  * Returns the permissions for this characteristic.
  */

bundles/org.openhab.binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothDevice.java

@@ -28,6 +28,7 @@
  * @author Chris Jackson - Initial contribution
  * @author Kai Kreuzer - Refactored class to use Integer instead of int, fixed bugs, diverse improvements
  * @author Connor Petty - Made most of the methods abstract
+ * @author Peter Rosenberg - Improve notifications
  */
 @NonNullByDefault
 public abstract class BluetoothDevice {
@@ -249,6 +250,15 @@ public BluetoothAdapter getAdapter() {
  */
  public abstract boolean writeCharacteristic(BluetoothCharacteristic characteristic);
 
+ /**
+ * Returns if notification is enabled for the given characteristic.
+ *
+ * @param characteristic the {@link BluetoothCharacteristic} to check if notifications are enabled.
+ * @return true if notification is enabled, false if notification is disabled, characteristic is missing on device
+ * or notifications are not supported.
+ */
+ public abstract boolean isNotifying(BluetoothCharacteristic characteristic);
+
  /**
  * Enables notifications for a characteristic. Only a single read or write operation can be requested at once.
  * Attempting to perform an operation when one is already in progress will result in subsequent calls returning

bundles/org.openhab.binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/DelegateBluetoothDevice.java

@@ -112,6 +112,12 @@ public boolean writeCharacteristic(BluetoothCharacteristic characteristic) {
  return delegate != null && delegate.writeCharacteristic(characteristic);
  }
 
+ @Override
+ public boolean isNotifying(BluetoothCharacteristic characteristic) {
+ BluetoothDevice delegate = getDelegate();
+ return delegate != null ? delegate.isNotifying(characteristic) : false;
+ }
+
  @Override
  public boolean enableNotifications(BluetoothCharacteristic characteristic) {
  BluetoothDevice delegate = getDelegate();