[hdpowerview] Return capabilities if capabilitiesOverride is not defined (#13031)

* [hdpowerview] bug fix capabilitiesOverride
* [hdpowerview] adopt reviewer requests
* [hdpowerview] suppress log warning on hub v1
* [hdpowerview] adopt reviwer suggestions

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/database/ShadeCapabilitiesDatabase.java

@@ -333,20 +333,37 @@ public class ShadeCapabilitiesDatabase {
     }
 
     /**
-     * Return a Capabilities class instance that corresponds to the given 'typeId' parameter. If the 'typeId' parameter
-     * is a valid type in the database, and it has a 'capabilitiesOverride' value, then an instance of the respective
-     * overridden Capabilities class is returned. Otherwise if the 'capabilitiesId' parameter is for a valid
-     * capabilities entry in the database, then that respective Capabilities class instance is returned. Otherwise a
-     * blank Capabilities class instance is returned.
+     * Return a Capabilities class instance that corresponds to the given 'typeId' parameter.
+     * <p>
+     * <ul>
+     * <li>If the 'typeId' parameter is a valid type in the database, and it has a 'capabilitiesOverride' value, then an
+     * instance of the respective overridden Capabilities class is returned.
+     * <li>Otherwise if the 'capabilitiesId' parameter is for a valid capabilities entry in the database, then that
+     * respective Capabilities class instance is returned.
+     * <li>Otherwise if the type is a valid type in the database, then its 'capabilities' instance is returned.
+     * <li>Otherwise a default Capabilities '0' class instance is returned.
+     * </ul>
+     * <p>
      *
      * @param typeId the target shade type Id (to check if it has a 'capabilitiesOverride' value).
      * @param capabilitiesId the target capabilities value (when type Id does not have a 'capabilitiesOverride').
      * @return corresponding Capabilities class instance.
      */
     public Capabilities getCapabilities(int typeId, @Nullable Integer capabilitiesId) {
-        int targetCapabilities = TYPE_DATABASE.getOrDefault(typeId, new Type()).getCapabilitiesOverride();
+        Type type = TYPE_DATABASE.getOrDefault(typeId, new Type());
+        // first try capabilitiesOverride for type Id
+        int targetCapabilities = type.getCapabilitiesOverride();
+        // then try capabilitiesId
+        if (targetCapabilities < 0 && capabilitiesId != null && isCapabilitiesInDatabase(capabilitiesId.intValue())) {
+            targetCapabilities = capabilitiesId.intValue();
+        }
+        // then try capabilities for typeId
         if (targetCapabilities < 0) {
-            targetCapabilities = capabilitiesId != null ? capabilitiesId.intValue() : -1;
+            targetCapabilities = type.getCapabilities();
+        }
+        // fallback to default capabilities 0 (so at least something may work..)
+        if (targetCapabilities < 0) {
+            targetCapabilities = 0;
         }
         return getCapabilities(targetCapabilities);
     }

bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewShadeHandler.java

@@ -320,7 +320,7 @@ public class HDPowerViewShadeHandler extends AbstractHubbedThingHandler {
         }
 
         if (propChanged && db.isCapabilitiesInDatabase(capabilitiesVal) && db.isTypeInDatabase(type)
-                && (capabilitiesVal != db.getType(type).getCapabilities())) {
+                && (capabilitiesVal != db.getType(type).getCapabilities()) && (shadeData.capabilities != null)) {
             db.logCapabilitiesMismatch(type, capabilitiesVal);
         }
     }

bundles/org.openhab.binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/ShadePositionTest.java

@@ -403,4 +403,58 @@ public class ShadePositionTest {
         assertShadePosition(test.getState(capabilities, SECONDARY_POSITION), UnDefType.UNDEF);
         assertShadePosition(test.getState(capabilities, VANE_TILT_POSITION), 88);
     }
+
+    /**
+     * Test the getCapabilities functionality.
+     */
+    @Test
+    public void testGetCapabilities() {
+        Capabilities caps;
+        /*
+         * - type not in database
+         * - null external capabilities
+         * => return default (0)
+         */
+        caps = db.getCapabilities(0, null);
+        assertEquals(0, caps.getValue());
+        /*
+         * - type not in database
+         * - valid external capabilities (1)
+         * => return external capabilities (1)
+         */
+        caps = db.getCapabilities(0, 1);
+        assertEquals(1, caps.getValue());
+        /*
+         * - type not in database
+         * - external capabilities not in database (99)
+         * => return default (0)
+         */
+        caps = db.getCapabilities(0, 99);
+        assertEquals(0, caps.getValue());
+        /*
+         * - type 62 in database
+         * - inherent capabilities (2)
+         * - null external capabilities
+         * => return inherent capabilities (2)
+         */
+        caps = db.getCapabilities(62, null);
+        assertEquals(2, caps.getValue());
+        /*
+         * - type 62 in database
+         * - inherent capabilities (2)
+         * - non matching external capabilities (1)
+         * => return external capabilities (1)
+         */
+        caps = db.getCapabilities(62, 1);
+        assertEquals(1, caps.getValue());
+        /*
+         * - type 44 in database
+         * - inherent capabilities (0)
+         * - with capabilitiesOverride (1)
+         * - non matching external capabilities (2)
+         * => return capabilitiesOverride (1)
+         */
+        caps = db.getCapabilities(44, 2);
+        assertEquals(1, caps.getValue());
+    }
 }