[bluetooth] Added support for connection based discovery (openhab#7056)

* Added support for connection based discovery * Added multiDiscoverySingleConnectionTest * applied spotless check Signed-off-by: Connor Petty <[email protected]>
lolodomo · Mar 6, 2020 · ea1e4d9 · ea1e4d9
1 parent 3bff3da
commit ea1e4d9
Show file tree

Hide file tree

Showing 20 changed files with 1,282 additions and 83 deletions.
diff --git a/...a/src/main/java/org/openhab/binding/bluetooth/bluegiga/handler/BlueGigaBridgeHandler.java b/...a/src/main/java/org/openhab/binding/bluetooth/bluegiga/handler/BlueGigaBridgeHandler.java
@@ -402,6 +402,11 @@ public BluetoothDevice getDevice(BluetoothAddress address) {
         return device;
     }
 
+    @Override
+    public boolean hasDevice(BluetoothAddress address) {
+        return devices.containsKey(address);
+    }
+
     /*
      * The following methods provide adaptor level functions for the BlueGiga interface. Typically these methods
      * are used by the device but are provided in the adapter to allow common knowledge and to support conflict

diff --git a/...h.bluez/src/main/java/org/openhab/binding/bluetooth/bluez/handler/BlueZBridgeHandler.java b/...h.bluez/src/main/java/org/openhab/binding/bluetooth/bluez/handler/BlueZBridgeHandler.java
@@ -49,6 +49,7 @@
  *
  * @author Kai Kreuzer - Initial contribution and API
  * @author Hilbrand Bouwkamp - Simplified calling scan and better handling manual scanning
+ * @author Connor Petty - Simplified device scan logic
  */
 @NonNullByDefault
 public class BlueZBridgeHandler extends BaseBridgeHandler implements BluetoothAdapter {
@@ -182,6 +183,7 @@ private void refreshDevices() {
                         logger.debug("Removing device '{}' due to inactivity", device.getAddress());
                         device.dispose();
                         devices.remove(device.getAddress());
+                        discoveryListeners.forEach(listener -> listener.deviceRemoved(device));
                     }
                 }
             }
@@ -249,6 +251,11 @@ public BlueZBluetoothDevice getDevice(BluetoothAddress bluetoothAddress) {
         }
     }
 
+    @Override
+    public boolean hasDevice(BluetoothAddress address) {
+        return devices.containsKey(address);
+    }
+
     @Override
     public void dispose() {
         if (discoveryJob != null) {
@@ -283,4 +290,5 @@ private boolean deviceReachable(BluetoothDevice device) {
         Integer rssi = device.getRssi();
         return rssi != null && rssi != 0;
     }
+
 }
diff --git a/bundles/org.openhab.binding.bluetooth/pom.xml b/bundles/org.openhab.binding.bluetooth/pom.xml
@@ -1,4 +1,6 @@
-<?xml version="1.0" encoding="UTF-8" standalone="no"?><project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
 
   <modelVersion>4.0.0</modelVersion>
 

diff --git a/bundles/org.openhab.binding.bluetooth/src/main/feature/feature.xml b/bundles/org.openhab.binding.bluetooth/src/main/feature/feature.xml
@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <features name="org.openhab.binding.bluetooth-${project.version}" xmlns="http://karaf.apache.org/xmlns/features/v1.4.0">
-    <repository>mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-core/${ohc.version}/xml/features</repository>
+	<repository>mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-core/${ohc.version}/xml/features</repository>
 
-    <feature name="openhab-binding-bluetooth" description="Bluetooth Binding" version="${project.version}">
-        <feature>openhab-runtime-base</feature>
-        <feature>openhab-transport-serial</feature>
-        <bundle start-level="80">mvn:org.openhab.addons.bundles/org.openhab.binding.bluetooth/${project.version}</bundle>
-    </feature>
+	<feature name="openhab-binding-bluetooth" description="Bluetooth Binding" version="${project.version}">
+		<feature>openhab-runtime-base</feature>
+		<feature>openhab-transport-serial</feature>
+		<bundle start-level="80">mvn:org.openhab.addons.bundles/org.openhab.binding.bluetooth/${project.version}</bundle>
+	</feature>
 </features>
diff --git a/...binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BeaconBluetoothHandler.java b/...binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BeaconBluetoothHandler.java
@@ -26,6 +26,7 @@
 import org.eclipse.smarthome.core.types.Command;
 import org.eclipse.smarthome.core.types.RefreshType;
 import org.eclipse.smarthome.core.types.UnDefType;
+import org.openhab.binding.bluetooth.discovery.internal.BluetoothAddressLocker;
 import org.openhab.binding.bluetooth.notification.BluetoothConnectionStatusNotification;
 import org.openhab.binding.bluetooth.notification.BluetoothScanNotification;
 
@@ -80,9 +81,11 @@ public void initialize() {
 
         try {
             deviceLock.lock();
+            BluetoothAddressLocker.lock(address);
             device = adapter.getDevice(address);
             device.addListener(this);
         } finally {
+            BluetoothAddressLocker.unlock(address);
             deviceLock.unlock();
         }
 

diff --git a/...enhab.binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothAdapter.java b/...enhab.binding.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothAdapter.java
@@ -72,8 +72,15 @@ public interface BluetoothAdapter extends Identifiable<ThingUID> {
      *
      * @param address the {@link BluetoothAddress} to retrieve
      * @return the {@link BluetoothDevice}
-     * @throws IllegalArgumentException if the address is no valid hardware address
      */
     BluetoothDevice getDevice(BluetoothAddress address);
 
+    /**
+     * Checks if this adapter has a device with the given {@link BluetoothAddress}.
+     *
+     * @param address the {@link BluetoothAddress} to check for
+     * @return true if this adapter has a {@link BluetoothDevice} with that address
+     */
+    boolean hasDevice(BluetoothAddress address);
+
 }
diff --git a/...ing.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothDiscoveryListener.java b/...ing.bluetooth/src/main/java/org/openhab/binding/bluetooth/BluetoothDiscoveryListener.java
@@ -12,12 +12,15 @@
  */
 package org.openhab.binding.bluetooth;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
+
 /**
  * This is a listener interface that is e.g. used by {@link BluetoothAdapter}s after discovering new devices.
  *
  * @author Kai Kreuzer - Initial contribution and API
- *
+ * @author Connor Petty - API improvements
  */
+@NonNullByDefault
 public interface BluetoothDiscoveryListener {
 
     /**
@@ -27,4 +30,10 @@ public interface BluetoothDiscoveryListener {
      */
     void deviceDiscovered(BluetoothDevice device);
 
+    /**
+     * Reports the removal of a device
+     *
+     * @param device the removed {@link BluetoothDevice}
+     */
+    void deviceRemoved(BluetoothDevice device);
 }
diff --git a/...ding.bluetooth/src/main/java/org/openhab/binding/bluetooth/ConnectedBluetoothHandler.java b/...ding.bluetooth/src/main/java/org/openhab/binding/bluetooth/ConnectedBluetoothHandler.java
@@ -54,7 +54,7 @@ public class ConnectedBluetoothHandler extends BeaconBluetoothHandler {
     private ScheduledFuture<?> connectionJob;
 
     // internal flag for the service resolution status
-    protected volatile Boolean resolved = false;
+    protected volatile boolean resolved = false;
 
     protected final Set<BluetoothCharacteristic> deviceCharacteristics = new CopyOnWriteArraySet<>();
 
@@ -137,24 +137,18 @@ public void onConnectionStateChange(BluetoothConnectionStatusNotification connec
             case DISCOVERED:
                 // The device is now known on the Bluetooth network, so we can do something...
                 scheduler.submit(() -> {
-                    synchronized (connectionJob) {
-                        if (device.getConnectionState() != ConnectionState.CONNECTED) {
-                            if (!device.connect()) {
-                                logger.debug("Error connecting to device after discovery.");
-                            }
+                    if (device.getConnectionState() != ConnectionState.CONNECTED) {
+                        if (!device.connect()) {
+                            logger.debug("Error connecting to device after discovery.");
                         }
                     }
                 });
                 break;
             case CONNECTED:
                 updateStatus(ThingStatus.ONLINE);
                 scheduler.submit(() -> {
-                    synchronized (resolved) {
-                        if (!resolved) {
-                            if (!device.discoverServices()) {
-                                logger.debug("Error while discovering services");
-                            }
-                        }
+                    if (!resolved && !device.discoverServices()) {
+                        logger.debug("Error while discovering services");
                     }
                 });
                 break;
@@ -171,17 +165,11 @@ public void onServicesDiscovered() {
         if (!resolved) {
             resolved = true;
             logger.debug("Service discovery completed for '{}'", address);
-            for (BluetoothService service : device.getServices()) {
-                for (BluetoothCharacteristic characteristic : service.getCharacteristics()) {
-                    if (characteristic.getGattCharacteristic() != null) {
-                        if (characteristic.getGattCharacteristic().equals(GattCharacteristic.BATTERY_LEVEL)) {
-                            activateChannel(characteristic,
-                                    DefaultSystemChannelTypeProvider.SYSTEM_CHANNEL_BATTERY_LEVEL.getUID());
-                            continue;
-                        }
-                        logger.debug("Added GATT characteristic '{}'", characteristic.getGattCharacteristic().name());
-                    }
-                }
+            BluetoothCharacteristic characteristic = device
+                    .getCharacteristic(GattCharacteristic.BATTERY_LEVEL.getUUID());
+            if (characteristic != null) {
+                activateChannel(characteristic, DefaultSystemChannelTypeProvider.SYSTEM_CHANNEL_BATTERY_LEVEL.getUID());
+                logger.debug("Added GATT characteristic '{}'", characteristic.getGattCharacteristic().name());
             }
         }
     }

diff --git a/.../src/main/java/org/openhab/binding/bluetooth/discovery/BluetoothDiscoveryParticipant.java b/.../src/main/java/org/openhab/binding/bluetooth/discovery/BluetoothDiscoveryParticipant.java
@@ -26,7 +26,7 @@
  * and can thus contribute {@link DiscoveryResult}s from Bluetooth scans.
  *
  * @author Kai Kreuzer - Initial contribution
- *
+ * @author Connor Petty - added 'requiresConnection' method
  */
 @NonNullByDefault
 public interface BluetoothDiscoveryParticipant {
@@ -45,15 +45,31 @@ public interface BluetoothDiscoveryParticipant {
      * @return the according discovery result or <code>null</code>, if device is not
      *         supported by this participant
      */
-    @Nullable
-    public DiscoveryResult createResult(BluetoothDevice device);
+    public @Nullable DiscoveryResult createResult(BluetoothDevice device);
 
     /**
      * Returns the thing UID for a Bluetooth device
      *
      * @param device the Bluetooth device
      * @return a thing UID or <code>null</code>, if the device is not supported by this participant
      */
-    @Nullable
-    public ThingUID getThingUID(BluetoothDevice device);
+    public @Nullable ThingUID getThingUID(BluetoothDevice device);
+
+    /**
+     * Returns true if this participant requires the device to be connected before it can produce a
+     * DiscoveryResult (or null) from {@link createResult(BluetoothDevice)}.
+     * <p>
+     * Implementors should only return 'true' conservatively, and make sure to return 'false' in circumstances where a
+     * 'null' result would be guaranteed from {@link createResult(BluetoothDevice)} even if a connection was available
+     * (e.g. the advertised manufacturerId already mismatches).
+     * <p>
+     * In general, returning 'true' is equivalent to saying <i>"the device might match, but I need a connection to
+     * make sure"</i>.
+     *
+     * @param device the Bluetooth device
+     * @return true if a connection is required before calling {@link createResult(BluetoothDevice)}
+     */
+    default public boolean requiresConnection(BluetoothDevice device) {
+        return false;
+    }
 }
diff --git a/...rc/main/java/org/openhab/binding/bluetooth/discovery/internal/BluetoothAddressLocker.java b/...rc/main/java/org/openhab/binding/bluetooth/discovery/internal/BluetoothAddressLocker.java
@@ -0,0 +1,55 @@
+/**
+ * Copyright (c) 2010-2020 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.bluetooth.discovery.internal;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.openhab.binding.bluetooth.BluetoothAddress;
+
+/**
+ * The {@link BluetoothAddressLocker} handles global locking of BluetoothAddress.
+ * This is used to make sure that devices with handlers are not connected to during discovery.
+ *
+ * @author Connor Petty - Initial Contribution
+ */
+public class BluetoothAddressLocker {
+
+    private static Map<BluetoothAddress, LockReference> locks = new ConcurrentHashMap<>();
+
+    public static void lock(BluetoothAddress address) {
+        locks.compute(address, (addr, oldRef) -> {
+            LockReference ref = oldRef;
+            if (ref == null) {
+                ref = new LockReference();
+            }
+            ref.count++;
+            return ref;
+        }).lock.lock();
+    }
+
+    public static void unlock(BluetoothAddress address) {
+        locks.computeIfPresent(address, (addr, ref) -> {
+            ref.count--;
+            ref.lock.unlock();
+            return ref.count <= 0 ? null : ref;
+        });
+    }
+
+    private static class LockReference {
+        private int count = 0;
+        private Lock lock = new ReentrantLock();
+    }
+}