Skip to content

Commit

Permalink
[bluetooth.generic] Enable BLE notification for linked channels (#10122)
Browse files Browse the repository at this point in the history 
* [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 <[email protected]>
  • Loading branch information
@PRosenb
PRosenb authored Feb 16, 2021
1 parent 596b261 commit 7abeb97
Show file tree
Hide file tree
Showing 9 changed files with 270 additions and 22 deletions.
10 changes: 5 additions & 5 deletions ...src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothCharacteristic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
*/
public class BlueGigaBluetoothCharacteristic extends BluetoothCharacteristic {

private boolean notificationEnabled;
private boolean notifying;

public BlueGigaBluetoothCharacteristic(int handle) {
super(null, handle);
Expand All @@ -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;
}
}
18 changes: 12 additions & 6 deletions ...luegiga/src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothDevice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public boolean enableNotifications(BluetoothCharacteristic characteristic) {
}

BlueGigaBluetoothCharacteristic ch = (BlueGigaBluetoothCharacteristic) characteristic;
if (ch.isNotificationEnabled()) {
if (ch.isNotifying()) {
return true;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -656,7 +662,7 @@ private void handleDisconnectedEvent(BlueGigaDisconnectedEvent event) {
}

for (BlueGigaBluetoothCharacteristic ch : handleToCharacteristic.values()) {
ch.setNotificationEnabled(false);
ch.setNotifying(false);
}

cancelTimer(procedureTimer);
Expand Down
53 changes: 52 additions & 1 deletion ...luez/src/main/java/org/openhab/binding/bluetooth/bluez/internal/BlueZBluetoothDevice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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());
Expand All @@ -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,
Expand All @@ -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());
Expand Down
38 changes: 28 additions & 10 deletions ...src/main/java/org/openhab/binding/bluetooth/generic/internal/GenericBluetoothHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

Expand All @@ -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
Expand All @@ -117,6 +134,7 @@ public void dispose() {

charHandlers.clear();
channelHandlers.clear();
handlerToChannels.clear();
}

@Override
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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() {
Expand Down
51 changes: 51 additions & 0 deletions ...inding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothCharacteristic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
*
Expand All @@ -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.
*/
Expand Down
10 changes: 10 additions & 0 deletions ...penhab.binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothDevice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions ...inding.bluetooth/src/main/java/org/openhab/binding/bluetooth/DelegateBluetoothDevice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading

0 comments on commit 7abeb97

Please sign in to comment.