[homekit] fix for battery charging state (#12959)

* fix for battery charging state

Signed-off-by: Eugen Freiter <freiter@gmx.de>

bundles/org.openhab.io.homekit/README.md

@@ -473,6 +473,7 @@ For example, ceiling fans often include lighting functionality. Such fans can be
 - two separate HomeKit accessories - fan **and** light. 
   
   iOS home app would show them as **two tiles** that can be controlled directly from home screen.
+  
   ![ios_fan_and_light_home_screen.png](doc/ios_fan_and_light_home_screen.png)
   
 - one complex accessory - fan **with** light. 
@@ -517,12 +518,17 @@ Group           FanWithLight        "Fan with Light"                           {
 
 ![ui_fan_with_light_primary.png](doc/ui_fan_with_light_primary.png)
 
+Similarly, you can create a sensor with battery
+
+![ui_sensor_with_battery.png](doc/ui_sensor_with_battery.png)
+
 However, home app does not support changing of tiles for already added accessory. 
 If you want to change the tile after the accessory was added, you need either to rename the group, if you use textual item configuration, or to delete and to create a new group with a different name, if you use UI for configuration.
 
 You can combine more than two accessories as well as accessories linked to different physical devices. 
 You can also do unusually combinations, e.g. you can combine temperature sensor with blinds and light. 
 It will be represented by home app as follows
+
 ![ios_complex_accessory_detail_screen.png](doc/ios_complex_accessory_detail_screen.png)
 
 

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitCharacteristicType.java

@@ -153,4 +153,9 @@ public enum HomekitCharacteristicType {
     public static Optional<HomekitCharacteristicType> valueOfTag(String tag) {
         return Optional.ofNullable(TAG_MAP.get(tag));
     }
+
+    @Override
+    public String toString() {
+        return tag;
+    }
 }

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/HomekitTaggedItem.java

@@ -282,7 +282,7 @@ public class HomekitTaggedItem {
     }
 
     public String toString() {
-        return "Item:" + proxyItem.getItem() + "  HomeKit type:" + homekitAccessoryType + " HomeKit characteristic:"
+        return "Item:" + proxyItem.getItem() + "  HomeKit type: '" + homekitAccessoryType.getTag()
-                + homekitCharacteristicType;
+                + "' characteristic: '" + homekitCharacteristicType.getTag() + "'";
     }
 }

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitAccessoryFactory.java

@@ -141,8 +141,21 @@ public class HomekitAccessoryFactory {
         }
     };
 
+    private static List<HomekitCharacteristicType> getRequiredCharacteristics(HomekitTaggedItem taggedItem) {
+        if (taggedItem.getAccessoryType() == BATTERY) {
+            final String isChargeable = taggedItem.getConfiguration(HomekitBatteryImpl.BATTERY_TYPE, "false");
+            if ("true".equalsIgnoreCase(isChargeable) || "yes".equalsIgnoreCase(isChargeable)) {
+                final List<HomekitCharacteristicType> characteristics = new ArrayList<>();
+                characteristics.addAll(Arrays.asList(MANDATORY_CHARACTERISTICS.get(taggedItem.getAccessoryType())));
+                characteristics.add(BATTERY_CHARGING_STATE);
+                return characteristics;
+            }
+        }
+        return Arrays.asList(MANDATORY_CHARACTERISTICS.get(taggedItem.getAccessoryType()));
+    }
+
     /**
-     * creates HomeKit accessory for a openhab item.
+     * creates HomeKit accessory for an openhab item.
      *
      * @param taggedItem openhab item tagged as HomeKit item
      * @param metadataRegistry openhab metadata registry required to get item meta information
@@ -158,12 +171,12 @@ public class HomekitAccessoryFactory {
             HomekitAccessoryUpdater updater, HomekitSettings settings) throws HomekitException {
         final HomekitAccessoryType accessoryType = taggedItem.getAccessoryType();
         logger.trace("Constructing {} of accessory type {}", taggedItem.getName(), accessoryType.getTag());
-        final List<HomekitTaggedItem> requiredCharacteristics = getMandatoryCharacteristics(taggedItem,
+        final List<HomekitTaggedItem> foundCharacteristics = getMandatoryCharacteristicsFromItem(taggedItem,
                 metadataRegistry);
-        final HomekitCharacteristicType[] mandatoryCharacteristics = MANDATORY_CHARACTERISTICS.get(accessoryType);
+        final List<HomekitCharacteristicType> mandatoryCharacteristics = getRequiredCharacteristics(taggedItem);
-        if ((mandatoryCharacteristics != null) && (requiredCharacteristics.size() < mandatoryCharacteristics.length)) {
+        if (foundCharacteristics.size() < mandatoryCharacteristics.size()) {
             logger.warn("Accessory of type {} must have following characteristics {}. Found only {}",
-                    accessoryType.getTag(), mandatoryCharacteristics, requiredCharacteristics);
+                    accessoryType.getTag(), mandatoryCharacteristics, foundCharacteristics);
             throw new HomekitException("Missing mandatory characteristics");
         }
         AbstractHomekitAccessoryImpl accessoryImpl;
@@ -171,10 +184,9 @@ public class HomekitAccessoryFactory {
             final @Nullable Class<? extends AbstractHomekitAccessoryImpl> accessoryImplClass = SERVICE_IMPL_MAP
                     .get(accessoryType);
             if (accessoryImplClass != null) {
-                accessoryImpl = accessoryImplClass
+                accessoryImpl = accessoryImplClass.getConstructor(HomekitTaggedItem.class, List.class,
-                        .getConstructor(HomekitTaggedItem.class, List.class, HomekitAccessoryUpdater.class,
+                        HomekitAccessoryUpdater.class, HomekitSettings.class)
-                                HomekitSettings.class)
+                        .newInstance(taggedItem, foundCharacteristics, updater, settings);
-                        .newInstance(taggedItem, requiredCharacteristics, updater, settings);
                 addOptionalCharacteristics(taggedItem, accessoryImpl, metadataRegistry);
                 return accessoryImpl;
             } else {
@@ -255,7 +267,7 @@ public class HomekitAccessoryFactory {
      * @param metadataRegistry meta data registry
      * @return list of mandatory
      */
-    private static List<HomekitTaggedItem> getMandatoryCharacteristics(HomekitTaggedItem taggedItem,
+    private static List<HomekitTaggedItem> getMandatoryCharacteristicsFromItem(HomekitTaggedItem taggedItem,
             MetadataRegistry metadataRegistry) {
         List<HomekitTaggedItem> collectedCharacteristics = new ArrayList<>();
         if (taggedItem.isGroup()) {
@@ -265,8 +277,7 @@ public class HomekitAccessoryFactory {
         } else {
             addMandatoryCharacteristics(taggedItem, collectedCharacteristics, taggedItem.getItem(), metadataRegistry);
         }
-        logger.trace("Mandatory characteristics for item {} characteristics {}", taggedItem.getName(),
+        logger.trace("Mandatory characteristics: {}", collectedCharacteristics);
-                collectedCharacteristics);
         return collectedCharacteristics;
     }
 
@@ -285,9 +296,8 @@ public class HomekitAccessoryFactory {
     private static void addMandatoryCharacteristics(HomekitTaggedItem mainItem, List<HomekitTaggedItem> characteristics,
             Item item, MetadataRegistry metadataRegistry) {
         // get list of mandatory characteristics
-        HomekitCharacteristicType[] mandatoryCharacteristics = MANDATORY_CHARACTERISTICS
+        List<HomekitCharacteristicType> mandatoryCharacteristics = getRequiredCharacteristics(mainItem);
-                .get(mainItem.getAccessoryType());
+        if (mandatoryCharacteristics.isEmpty()) {
-        if ((mandatoryCharacteristics == null) || (mandatoryCharacteristics.length == 0)) {
             // no mandatory characteristics linked to accessory type of mainItem. we are done
             return;
         }
@@ -299,13 +309,12 @@ public class HomekitAccessoryFactory {
         for (Entry<HomekitAccessoryType, HomekitCharacteristicType> accessory : getAccessoryTypes(item,
                 metadataRegistry)) {
             // if the item has only accessory tag, e.g. TemperatureSensor,
-            // the we will link all mandatory characteristic to this item,
+            // then we will link all mandatory characteristic to this item,
             // e.g. we will link CurrentTemperature in case of TemperatureSensor.
             if (isRootAccessory(accessory)) {
-                Arrays.stream(mandatoryCharacteristics)
+                mandatoryCharacteristics.forEach(c -> characteristics.add(new HomekitTaggedItem(itemProxy,
-                        .forEach(c -> characteristics.add(new HomekitTaggedItem(itemProxy, accessory.getKey(), c,
+                        accessory.getKey(), c, mainItem.isGroup() ? (GroupItem) mainItem.getItem() : null,
-                                mainItem.isGroup() ? (GroupItem) mainItem.getItem() : null,
+                        HomekitAccessoryFactory.getItemConfiguration(item, metadataRegistry))));
-                                HomekitAccessoryFactory.getItemConfiguration(item, metadataRegistry))));
             } else {
                 // item has characteristic tag on it, so, adding it as that characteristic.
 
@@ -313,7 +322,7 @@ public class HomekitAccessoryFactory {
 
                 // check whether it is a mandatory characteristic. optional will be added later by another method.
                 if (belongsToType(mainItem.getAccessoryType(), accessory)
-                        && isMandatoryCharacteristic(mainItem.getAccessoryType(), characteristic)) {
+                        && isMandatoryCharacteristic(mainItem, characteristic)) {
                     characteristics.add(new HomekitTaggedItem(itemProxy, accessory.getKey(), characteristic,
                             mainItem.isGroup() ? (GroupItem) mainItem.getItem() : null,
                             HomekitAccessoryFactory.getItemConfiguration(item, metadataRegistry)));
@@ -373,30 +382,28 @@ public class HomekitAccessoryFactory {
             GroupItem groupItem = (GroupItem) taggedItem.getItem();
             groupItem.getMembers().forEach(item -> getAccessoryTypes(item, metadataRegistry).stream()
                     .filter(c -> !isRootAccessory(c)).filter(c -> belongsToType(taggedItem.getAccessoryType(), c))
-                    .filter(c -> !isMandatoryCharacteristic(taggedItem.getAccessoryType(), c.getValue()))
+                    .filter(c -> !isMandatoryCharacteristic(taggedItem, c.getValue()))
                     .forEach(characteristic -> characteristicItems.put(characteristic.getValue(), (GenericItem) item)));
         } else {
             getAccessoryTypes(taggedItem.getItem(), metadataRegistry).stream().filter(c -> !isRootAccessory(c))
-                    .filter(c -> !isMandatoryCharacteristic(taggedItem.getAccessoryType(), c.getValue()))
+                    .filter(c -> !isMandatoryCharacteristic(taggedItem, c.getValue()))
                     .forEach(characteristic -> characteristicItems.put(characteristic.getValue(),
                             (GenericItem) taggedItem.getItem()));
         }
-        logger.trace("Optional characteristics for item {} characteristics {}", taggedItem.getName(),
+        logger.trace("Optional characteristics for item {}: {}", taggedItem.getName(), characteristicItems.values());
-                characteristicItems);
         return Collections.unmodifiableMap(characteristicItems);
     }
 
     /**
      * return true is characteristic is a mandatory characteristic for the accessory.
      *
-     * @param accessory accessory
+     * @param item item
      * @param characteristic characteristic
      * @return true if characteristic is mandatory, false if not mandatory
      */
-    private static boolean isMandatoryCharacteristic(HomekitAccessoryType accessory,
+    private static boolean isMandatoryCharacteristic(HomekitTaggedItem item, HomekitCharacteristicType characteristic) {
-            HomekitCharacteristicType characteristic) {
+        return MANDATORY_CHARACTERISTICS.containsKey(item.getAccessoryType())
-        return MANDATORY_CHARACTERISTICS.containsKey(accessory)
+                && getRequiredCharacteristics(item).contains(characteristic);
-                && Arrays.asList(MANDATORY_CHARACTERISTICS.get(accessory)).contains(characteristic);
     }
 
     /**

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitBatteryImpl.java

@@ -37,7 +37,7 @@ import io.github.hapjava.services.impl.BatteryService;
  * @author Eugen Freiter - Initial contribution
  */
 public class HomekitBatteryImpl extends AbstractHomekitAccessoryImpl implements BatteryAccessory {
-    private static final String BATTERY_TYPE = "chargeable";
+    public static final String BATTERY_TYPE = "chargeable";
 
     private final BooleanItemReader lowBatteryReader;
     private BooleanItemReader chargingBatteryReader;
@@ -87,7 +87,9 @@ public class HomekitBatteryImpl extends AbstractHomekitAccessoryImpl implements
 
     @Override
     public void subscribeBatteryChargingState(final HomekitCharacteristicChangeCallback callback) {
-        subscribe(BATTERY_CHARGING_STATE, callback);
+        if (isChargeable) {
+            subscribe(BATTERY_CHARGING_STATE, callback);
+        }
     }
 
     @Override
@@ -102,6 +104,8 @@ public class HomekitBatteryImpl extends AbstractHomekitAccessoryImpl implements
 
     @Override
     public void unsubscribeBatteryChargingState() {
-        unsubscribe(BATTERY_CHARGING_STATE);
+        if (isChargeable) {
+            unsubscribe(BATTERY_CHARGING_STATE);
+        }
     }
 }

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitCharacteristicFactory.java

@@ -184,13 +184,14 @@ public class HomekitCharacteristicFactory {
     public static Characteristic createCharacteristic(HomekitTaggedItem item, HomekitAccessoryUpdater updater)
             throws HomekitException {
         final @Nullable HomekitCharacteristicType type = item.getCharacteristicType();
-        logger.trace("CreateCharacteristic, type {} item {}", type, item);
+        logger.trace("Create characteristic {}", item);
         if (optional.containsKey(type)) {
             return optional.get(type).apply(item, updater);
         }
         logger.warn("Unsupported optional characteristic. Accessory type {}, characteristic type {}",
-                item.getAccessoryType(), type);
+                item.getAccessoryType(), type.getTag());
-        throw new HomekitException("Unsupported optional characteristic. Characteristic type \"" + type + "\"");
+        throw new HomekitException(
+                "Unsupported optional characteristic. Characteristic type \"" + type.getTag() + "\"");
     }
 
     // METHODS TO CREATE SINGLE CHARACTERISTIC FROM OH ITEM

bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/IncompleteAccessoryException.java

@@ -24,7 +24,7 @@ public class IncompleteAccessoryException extends Exception {
     private static final long serialVersionUID = 8595808359805444177L;
 
     public IncompleteAccessoryException(HomekitCharacteristicType missingType) {
-        super(String.format("Missing accessory type %s", missingType.getTag()));
+        super(String.format("Missing accessory characteristic %s", missingType.getTag()));
     }
 
     public IncompleteAccessoryException(String message) {