[sonos] Discovery of unsupported models without exception (#12609)

* [sonos] Discovery of unsupported models without exception
* Unit tests added for method extractModelName
* Replace extractModelName by buildThingTypeIdFromModelName

Transform the found model name into a valid thing type ID when trying
to match a thing type

This could avoid an IllegalArgumentException thrown by the discovery
service when new models will be introduced on the market.

Should be compatible with Bubble UPnP Server which modifies the model
name

A message is now logged for unsupported Sonos models.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

diff --git a/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/SonosXMLParser.java b/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/SonosXMLParser.java
index 25f296c75955f6785e0805f2cfc6de4998f2ca82..6a321376dde1e98682f096e17f9d625bc9b31efd 100644 (file)
--- a/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/SonosXMLParser.java
+++ b/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/SonosXMLParser.java
@@ -1014,21 +1014,41 @@ public class SonosXMLParser {
     }
 
     /**
-     * The model name provided by upnp is formated like in the example form "Sonos PLAY:1" or "Sonos PLAYBAR"
+     * Build a valid thing type ID from the model name provided by UPnP
      *
-     * @param sonosModelName Sonos model name provided via upnp device
-     * @return the extracted players model name without column (:) character used for ThingType creation
+     * @param sonosModelName Sonos model name provided via UPnP device
+     * @return a valid thing type ID that can then be used for ThingType creation
      */
-    public static String extractModelName(String sonosModelName) {
-        String ret = sonosModelName;
-        Matcher matcher = Pattern.compile("\\s(.*)").matcher(ret);
+    public static String buildThingTypeIdFromModelName(String sonosModelName) {
+        // For Ikea SYMFONISK models, the model name now starts with "SYMFONISK" with recent firmwares
+        if (sonosModelName.toUpperCase().contains("SYMFONISK")) {
+            return "SYMFONISK";
+        }
+        String id = sonosModelName;
+        // Remove until the first space (in practice, it removes the leading "Sonos " from the model name)
+        Matcher matcher = Pattern.compile("\\s(.*)").matcher(id);
         if (matcher.find()) {
-            ret = matcher.group(1);
+            id = matcher.group(1);
+            // Remove a potential ending text surrounded with parenthesis
+            matcher = Pattern.compile("(.*)\\s\\(.*\\)").matcher(id);
+            if (matcher.find()) {
+                id = matcher.group(1);
+            }
         }
-        if (ret.contains(":")) {
-            ret = ret.replace(":", "");
+        // Finally remove unexpected characters in a thing type ID
+        id = id.replaceAll("[^a-zA-Z0-9_]", "");
+        // ZP80 is translated to CONNECT and ZP100 to CONNECTAMP
+        switch (id) {
+            case "ZP80":
+                id = "CONNECT";
+                break;
+            case "ZP100":
+                id = "CONNECTAMP";
+                break;
+            default:
+                break;
         }
-        return ret;
+        return id;
     }
 
     public static String compileMetadataString(SonosEntry entry) {

diff --git a/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/discovery/ZonePlayerDiscoveryParticipant.java b/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/discovery/ZonePlayerDiscoveryParticipant.java
index 805bded37b1f58dc89300cae6ef8537a2c252cd9..a497d09305a2c1897d397b63224865d2438014ff 100644 (file)
--- a/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/discovery/ZonePlayerDiscoveryParticipant.java
+++ b/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/discovery/ZonePlayerDiscoveryParticipant.java
@@ -80,45 +80,25 @@ public class ZonePlayerDiscoveryParticipant implements UpnpDiscoveryParticipant
     public @Nullable ThingUID getThingUID(RemoteDevice device) {
         if (device.getDetails().getManufacturerDetails().getManufacturer() != null) {
             if (device.getDetails().getManufacturerDetails().getManufacturer().toUpperCase().contains("SONOS")) {
-                boolean ignored = false;
-                String modelName = getModelName(device);
-                switch (modelName) {
-                    case "ZP80":
-                        modelName = "CONNECT";
-                        break;
-                    case "ZP100":
-                        modelName = "CONNECTAMP";
-                        break;
-                    case "One SL":
-                        modelName = "OneSL";
-                        break;
-                    case "Arc SL":
-                        modelName = "ArcSL";
-                        break;
-                    case "Roam SL":
-                        modelName = "RoamSL";
-                        break;
-                    case "Sub":
-                        // The Sonos Sub is ignored
-                        ignored = true;
-                        break;
-                    default:
-                        break;
-                }
-                if (!ignored) {
-                    ThingTypeUID thingUID = new ThingTypeUID(SonosBindingConstants.BINDING_ID, modelName);
-                    if (!SonosBindingConstants.SUPPORTED_KNOWN_THING_TYPES_UIDS.contains(thingUID)) {
+                String id = SonosXMLParser
+                        .buildThingTypeIdFromModelName(device.getDetails().getModelDetails().getModelName());
+                if (!"Sub".equalsIgnoreCase(id)) {
+                    ThingTypeUID thingTypeUID = new ThingTypeUID(SonosBindingConstants.BINDING_ID, id);
+                    if (!SonosBindingConstants.SUPPORTED_KNOWN_THING_TYPES_UIDS.contains(thingTypeUID)) {
                         // Try with the model name all in uppercase
-                        thingUID = new ThingTypeUID(SonosBindingConstants.BINDING_ID, modelName.toUpperCase());
+                        thingTypeUID = new ThingTypeUID(SonosBindingConstants.BINDING_ID, id.toUpperCase());
                         // In case a new "unknown" Sonos player is discovered a generic ThingTypeUID will be used
-                        if (!SonosBindingConstants.SUPPORTED_KNOWN_THING_TYPES_UIDS.contains(thingUID)) {
-                            thingUID = SonosBindingConstants.ZONEPLAYER_THING_TYPE_UID;
+                        if (!SonosBindingConstants.SUPPORTED_KNOWN_THING_TYPES_UIDS.contains(thingTypeUID)) {
+                            thingTypeUID = SonosBindingConstants.ZONEPLAYER_THING_TYPE_UID;
+                            logger.warn(
+                                    "'{}' is not yet a supported model, thing type '{}' is considered as default; please open an issue",
+                                    device.getDetails().getModelDetails().getModelName(), thingTypeUID);
                         }
                     }
 
-                    logger.debug("Discovered a Sonos '{}' thing with UDN '{}'", thingUID,
+                    logger.debug("Discovered a Sonos '{}' thing with UDN '{}'", thingTypeUID,
                             device.getIdentity().getUdn().getIdentifierString());
-                    return new ThingUID(thingUID, device.getIdentity().getUdn().getIdentifierString());
+                    return new ThingUID(thingTypeUID, device.getIdentity().getUdn().getIdentifierString());
                 }
             }
         }
@@ -126,13 +106,6 @@ public class ZonePlayerDiscoveryParticipant implements UpnpDiscoveryParticipant
         return null;
     }
 
-    private String getModelName(RemoteDevice device) {
-        // For Ikea SYMFONISK models, the model name now starts with "SYMFONISK" with recent firmwares
-        // We can no more use extractModelName as it deletes the first word ("Sonos" for all other devices)
-        return device.getDetails().getModelDetails().getModelName().toUpperCase().contains("SYMFONISK") ? "SYMFONISK"
-                : SonosXMLParser.extractModelName(device.getDetails().getModelDetails().getModelName());
-    }
-
     private @Nullable String getSonosRoomName(RemoteDevice device) {
         return SonosXMLParser.getRoomName(device.getIdentity().getDescriptorURL().toString());
     }

diff --git a/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/handler/ZonePlayerHandler.java b/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/handler/ZonePlayerHandler.java
index ae41c812dcbd13b92879a1bcf4b187b3b7cc41b9..e0784cce5b372e2d57f815164bb49f4d89b6dc97 100644 (file)
--- a/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/handler/ZonePlayerHandler.java
+++ b/bundles/org.openhab.binding.sonos/src/main/java/org/openhab/binding/sonos/internal/handler/ZonePlayerHandler.java
@@ -3292,7 +3292,8 @@ public class ZonePlayerHandler extends BaseThingHandler implements UpnpIOPartici
         URL descriptor = service.getDescriptorURL(this);
         if (descriptor != null) {
             String sonosModelDescription = SonosXMLParser.parseModelDescription(descriptor);
-            return sonosModelDescription == null ? null : SonosXMLParser.extractModelName(sonosModelDescription);
+            return sonosModelDescription == null ? null
+                    : SonosXMLParser.buildThingTypeIdFromModelName(sonosModelDescription);
         } else {
             return null;
         }

diff --git a/bundles/org.openhab.binding.sonos/src/test/java/org/openhab/binding/sonos/internal/SonosXMLParserTest.java b/bundles/org.openhab.binding.sonos/src/test/java/org/openhab/binding/sonos/internal/SonosXMLParserTest.java
new file mode 100644 (file)
index 0000000..0dca901
--- /dev/null
+++ b/bundles/org.openhab.binding.sonos/src/test/java/org/openhab/binding/sonos/internal/SonosXMLParserTest.java
@@ -0,0 +1,63 @@
+/**
+ * Copyright (c) 2010-2022 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.sonos.internal;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.junit.jupiter.api.Test;
+
+/**
+ *
+ * @author Laurent Garnier - Initial contribution
+ */
+@NonNullByDefault
+public class SonosXMLParserTest {
+
+    @Test
+    public void buildThingTypeIdFromModelWithoutSpace() {
+        assertEquals("Move", SonosXMLParser.buildThingTypeIdFromModelName("Sonos Move"));
+    }
+
+    @Test
+    public void buildThingTypeIdFromModelWithSpace() {
+        assertEquals("RoamSL", SonosXMLParser.buildThingTypeIdFromModelName("Sonos Roam SL"));
+    }
+
+    @Test
+    public void buildThingTypeIdFromModelWithColon() {
+        assertEquals("PLAY5", SonosXMLParser.buildThingTypeIdFromModelName("Sonos PLAY:5"));
+    }
+
+    @Test
+    public void buildThingTypeIdFromSymfoniskModel() {
+        assertEquals("SYMFONISK", SonosXMLParser.buildThingTypeIdFromModelName("SYMFONISK Table lamp"));
+        assertEquals("SYMFONISK", SonosXMLParser.buildThingTypeIdFromModelName("Symfonisk Table lamp"));
+        assertEquals("SYMFONISK", SonosXMLParser.buildThingTypeIdFromModelName("Sonos Symfonisk"));
+    }
+
+    @Test
+    public void buildThingTypeIdFromZP80Model() {
+        assertEquals("CONNECT", SonosXMLParser.buildThingTypeIdFromModelName("Sonos ZP80"));
+    }
+
+    @Test
+    public void buildThingTypeIdFromZP100Model() {
+        assertEquals("CONNECTAMP", SonosXMLParser.buildThingTypeIdFromModelName("Sonos ZP100"));
+    }
+
+    @Test
+    public void buildThingTypeIdFromModelWithAdditionalTextInParenthesis() {
+        assertEquals("OneSL", SonosXMLParser.buildThingTypeIdFromModelName("Sonos One SL (OpenHome)"));
+    }
+}