[boschshc] Cache mDNS-based bridge discovery results

bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/BoschHttpClient.java

@@ -56,6 +56,16 @@ public class BoschHttpClient extends HttpClient {
 
     private final Logger logger = LoggerFactory.getLogger(getClass());
 
+    /**
+     * Default number of seconds for HTTP request timeouts
+     */
+    public static final long DEFAULT_TIMEOUT_SECONDS = 10;
+
+    /**
+     * The time unit used for default HTTP request timeouts
+     */
+    public static final TimeUnit DEFAULT_TIMEOUT_UNIT = TimeUnit.SECONDS;
+
     private final String ipAddress;
     private final String systemPassword;
 
@@ -299,7 +309,7 @@ public Request createRequest(String url, HttpMethod method, @Nullable Object con
 
         Request request = this.newRequest(url).method(method).header("Content-Type", "application/json")
                 .header("api-version", "3.2") // see https://github.com/BoschSmartHome/bosch-shc-api-docs/issues/80
-                .timeout(10, TimeUnit.SECONDS); // Set default timeout
+                .timeout(DEFAULT_TIMEOUT_SECONDS, DEFAULT_TIMEOUT_UNIT); // Set default timeout
 
         if (content != null) {
             final String body;

bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/discovery/BridgeDiscoveryParticipant.java

@@ -31,6 +31,7 @@
 import org.openhab.binding.boschshc.internal.devices.BoschSHCBindingConstants;
 import org.openhab.binding.boschshc.internal.devices.bridge.BoschHttpClient;
 import org.openhab.binding.boschshc.internal.devices.bridge.dto.PublicInformation;
+import org.openhab.core.cache.ExpiringCacheMap;
 import org.openhab.core.config.discovery.DiscoveryResult;
 import org.openhab.core.config.discovery.DiscoveryResultBuilder;
 import org.openhab.core.config.discovery.mdns.MDNSDiscoveryParticipant;
@@ -46,23 +47,32 @@
 import com.google.gson.Gson;
 
 /**
- * The {@link BridgeDiscoveryParticipant} is responsible discovering the
- * Bosch Smart Home Controller as a Bridge with the mDNS services.
+ * The {@link BridgeDiscoveryParticipant} is responsible discovering the Bosch
+ * Smart Home Controller as a Bridge with the mDNS services.
  *
  * @author Gerd Zanker - Initial contribution
+ * @author David Pace - Discovery result caching
  */
 @NonNullByDefault
 @Component(configurationPid = "discovery.boschsmarthomebridge")
 public class BridgeDiscoveryParticipant implements MDNSDiscoveryParticipant {
-    private static final long TTL_SECONDS = Duration.ofHours(1).toSeconds();
+    private static final String NAME_PREFIX_BOSCH_SHC = "Bosch SHC";
+    private static final Duration TTL_DURATION = Duration.ofMinutes(10);
+    private static final long TTL_SECONDS = TTL_DURATION.toSeconds();
+
     public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(BoschSHCBindingConstants.THING_TYPE_SHC);
 
     private final Logger logger = LoggerFactory.getLogger(BridgeDiscoveryParticipant.class);
     private final HttpClient httpClient;
     private final Gson gson = new Gson();
 
-    /// SHC Bridge Information, read via public REST API if bridge is detected. Otherwise, strings are empty.
-    private PublicInformation bridgeInformation = new PublicInformation();
+    /**
+     * Cache for bridge discovery results. Uses the IP address of mDNS events as
+     * key. If the value is <code>null</code>, no Bosch SHC controller could be
+     * identified at the corresponding IP address.
+     */
+    private ExpiringCacheMap<String, @Nullable PublicInformation> discoveryResultCache = new ExpiringCacheMap<>(
+            TTL_DURATION);
 
     @Activate
     public BridgeDiscoveryParticipant(@Reference HttpClientFactory httpClientFactory) {
@@ -89,73 +99,147 @@ public String getServiceType() {
         return "_http._tcp.local.";
     }
 
+    /**
+     * This method is frequently called by the mDNS discovery framework in different
+     * threads with individual service info instances.
+     * <p>
+     * Different service info objects can refer to the same Bosch SHC controller,
+     * e.g. the controller may be reachable via a <code>192.168.*.*</code> IP and an
+     * IP in the <code>169.254.*.*</code> range. The response from the controller
+     * contains the actual resolved IP address.
+     * <p>
+     * We ignore mDNS events if they do not contain any IP addresses or if the name
+     * property does not start with <code>Bosch SHC</code>.
+     */
     @Override
     public @Nullable DiscoveryResult createResult(ServiceInfo serviceInfo) {
-        logger.trace("Bridge Discovery started for {}", serviceInfo);
+        if (logger.isTraceEnabled()) {
+            logger.trace("Bridge discovery invoked with mDNS service info {}", serviceInfo);
+        }
+
+        String name = serviceInfo.getName();
+        if (name == null || !name.startsWith(NAME_PREFIX_BOSCH_SHC)) {
+            if (logger.isTraceEnabled()) {
+                logger.trace("Ignoring mDNS service event because name '{}' does not start with '{}')", name,
+                        NAME_PREFIX_BOSCH_SHC);
+            }
+            return null;
+        }
+
+        @Nullable
+        String ipAddress = getFirstIPAddress(serviceInfo);
+        if (ipAddress == null || ipAddress.isBlank()) {
+            return null;
+        }
+
+        PublicInformation publicInformation = getOrComputePublicInformation(ipAddress);
+        if (publicInformation == null) {
+            return null;
+        }
 
         @Nullable
         final ThingUID uid = getThingUID(serviceInfo);
         if (uid == null) {
             return null;
         }
 
-        logger.trace("Discovered Bosch Smart Home Controller at {}", bridgeInformation.shcIpAddress);
-
         return DiscoveryResultBuilder.create(uid)
-                .withLabel("Bosch Smart Home Controller (" + bridgeInformation.shcIpAddress + ")")
-                .withProperty("ipAddress", bridgeInformation.shcIpAddress)
-                .withProperty("shcGeneration", bridgeInformation.shcGeneration)
-                .withProperty("apiVersions", bridgeInformation.apiVersions).withTTL(TTL_SECONDS).build();
+                .withLabel("Bosch Smart Home Controller (" + publicInformation.shcIpAddress + ")")
+                .withProperty("ipAddress", publicInformation.shcIpAddress)
+                .withProperty("shcGeneration", publicInformation.shcGeneration)
+                .withProperty("apiVersions", publicInformation.apiVersions).withTTL(TTL_SECONDS).build();
+    }
+
+    private @Nullable String getFirstIPAddress(ServiceInfo serviceInfo) {
+        String[] hostAddresses = serviceInfo.getHostAddresses();
+        if (hostAddresses != null && hostAddresses.length > 0 && !hostAddresses[0].isEmpty()) {
+            return hostAddresses[0];
+        }
+
+        return null;
+    }
+
+    /**
+     * Provides a cached discovery result if available, or performs an actual
+     * communication attempt to the device with the given IP address.
+     * <p>
+     * This method is synchronized because multiple threads try to access discovery
+     * results concurrently. We only want one thread to "win" and to invoke the
+     * actual HTTP communication.
+     * 
+     * @param ipAddress IP address to contact if no cached result is available
+     * @return the {@link PublicInformation} of the Bosch Smart Home Controller or
+     *         <code>null</code> if the device with the given IP address could not
+     *         be identified as Bosch Smart Home Controller
+     */
+    protected synchronized @Nullable PublicInformation getOrComputePublicInformation(String ipAddress) {
+        return discoveryResultCache.putIfAbsentAndGet(ipAddress, () -> {
+            logger.trace("No cached bridge discovery result available for IP {}, trying to contact SHC", ipAddress);
+            return discoverBridge(ipAddress);
+        });
     }
 
     @Override
     public @Nullable ThingUID getThingUID(ServiceInfo serviceInfo) {
-        String ipAddress = discoverBridge(serviceInfo).shcIpAddress;
-        if (!ipAddress.isBlank()) {
-            return new ThingUID(BoschSHCBindingConstants.THING_TYPE_SHC, ipAddress.replace('.', '-'));
+        String ipAddress = getFirstIPAddress(serviceInfo);
+        if (ipAddress != null) {
+            @Nullable
+            PublicInformation publicInformation = getOrComputePublicInformation(ipAddress);
+            if (publicInformation != null) {
+                String resolvedIpAddress = publicInformation.shcIpAddress;
+                return new ThingUID(BoschSHCBindingConstants.THING_TYPE_SHC, resolvedIpAddress.replace('.', '-'));
+            }
         }
         return null;
     }
 
-    protected PublicInformation discoverBridge(ServiceInfo serviceInfo) {
-        logger.trace("Discovering serviceInfo {}", serviceInfo);
-
-        if (serviceInfo.getHostAddresses() != null && serviceInfo.getHostAddresses().length > 0
-                && !serviceInfo.getHostAddresses()[0].isEmpty()) {
-            String address = serviceInfo.getHostAddresses()[0];
-            logger.trace("Discovering InetAddress {}", address);
-            // store all information for later access
-            bridgeInformation = getPublicInformationFromPossibleBridgeAddress(address);
+    protected @Nullable PublicInformation discoverBridge(String ipAddress) {
+        logger.debug("Attempting to contact Bosch Smart Home Controller at IP {}", ipAddress);
+        PublicInformation bridgeInformation = getPublicInformationFromPossibleBridgeAddress(ipAddress);
+        if (bridgeInformation != null && bridgeInformation.shcIpAddress != null
+                && !bridgeInformation.shcIpAddress.isBlank()) {
+            return bridgeInformation;
         }
 
-        return bridgeInformation;
+        return null;
     }
 
-    protected PublicInformation getPublicInformationFromPossibleBridgeAddress(String ipAddress) {
+    /**
+     * Attempts to send a HTTP request to the given IP address in order to determine
+     * if the device is a Bosch Smart Home Controller.
+     * 
+     * @param ipAddress the IP address of the potential Bosch Smart Home Controller
+     * @return a {@link PublicInformation} object if the bridge was successfully
+     *         contacted or <code>null</code> if the communication failed
+     */
+    protected @Nullable PublicInformation getPublicInformationFromPossibleBridgeAddress(String ipAddress) {
         String url = BoschHttpClient.getPublicInformationUrl(ipAddress);
-        logger.trace("Discovering ipAddress {}", url);
+        logger.trace("Requesting SHC information via URL {}", url);
         try {
             httpClient.start();
-            ContentResponse contentResponse = httpClient.newRequest(url).method(HttpMethod.GET).send();
+            ContentResponse contentResponse = httpClient.newRequest(url).method(HttpMethod.GET)
+                    .timeout(BoschHttpClient.DEFAULT_TIMEOUT_SECONDS, BoschHttpClient.DEFAULT_TIMEOUT_UNIT).send();
+
             // check HTTP status code
             if (!HttpStatus.getCode(contentResponse.getStatus()).isSuccess()) {
-                logger.debug("Discovering failed with status code: {}", contentResponse.getStatus());
-                return new PublicInformation();
+                logger.debug("Discovery failed with status code {}: {}", contentResponse.getStatus(),
+                        contentResponse.getContentAsString());
+                return null;
             }
             // get content from response
             String content = contentResponse.getContentAsString();
-            logger.trace("Discovered SHC - public info {}", content);
+            logger.debug("Discovered SHC at IP {}, public info: {}", ipAddress, content);
             PublicInformation bridgeInfo = gson.fromJson(content, PublicInformation.class);
-            if (bridgeInfo.shcIpAddress != null) {
+            if (bridgeInfo != null && bridgeInfo.shcIpAddress != null && !bridgeInfo.shcIpAddress.isBlank()) {
                 return bridgeInfo;
             }
         } catch (TimeoutException | ExecutionException e) {
-            logger.debug("Discovering failed with exception {}", e.getMessage());
+            logger.debug("Discovery could not reach SHC at IP {}: {}", ipAddress, e.getMessage());
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();
         } catch (Exception e) {
-            logger.debug("Discovering failed during http client request {}", e.getMessage());
+            logger.warn("Discovery failed during HTTP client request: {}", e.getMessage(), e);
         }
-        return new PublicInformation();
+        return null;
     }
 }

bundles/org.openhab.binding.boschshc/src/main/resources/OH-INF/addon/addon.xml

@@ -8,4 +8,22 @@
 	<description>This is the binding for Bosch Smart Home.</description>
 	<connection>local</connection>
 
+	<discovery-methods>
+		<discovery-method>
+			<service-type>mdns</service-type>
+			<discovery-parameters>
+				<discovery-parameter>
+					<name>mdnsServiceType</name>
+					<value>_http._tcp.local.</value>
+				</discovery-parameter>
+			</discovery-parameters>
+			<match-properties>
+				<match-property>
+					<name>name</name>
+					<regex>Bosch SHC.*</regex>
+				</match-property>
+			</match-properties>
+		</discovery-method>
+	</discovery-methods>
+
 </addon:addon>