Merge pull request #50 from timcharper/tharper/fix-battery-reporting-for-devices

Add and direct implementors to use LowBatteryStatusAccessory

Author: andylintner

src/main/java/com/beowulfe/hap/accessories/BatteryAccessory.java

@@ -1,15 +1,15 @@
 package com.beowulfe.hap.accessories;
 
-import com.beowulfe.hap.HomekitCharacteristicChangeCallback;
-
 import java.util.concurrent.CompletableFuture;
 
+import com.beowulfe.hap.HomekitCharacteristicChangeCallback;
+
 /**
- * An accessory that runs on batteries. Accessories that run on batteries are able to report
- * battery level.
+ * Do not use. Devices that have battery levels should implement LowBatteryStatusAccessory.
  *
  * @author Gaston Dombiak
  */
+@Deprecated
 public interface BatteryAccessory {
 
     /**

src/main/java/com/beowulfe/hap/accessories/BatteryStatusAccessory.java

@@ -0,0 +1,34 @@
+package com.beowulfe.hap.accessories;
+
+import java.util.concurrent.CompletableFuture;
+
+import com.beowulfe.hap.HomekitCharacteristicChangeCallback;
+
+/**
+ * An accessory that runs on batteries. Accessories that run on batteries are able to report
+ * battery level.
+ *
+ * @author Tim Harper
+ */
+public interface BatteryStatusAccessory {
+
+    /**
+     * Queries if the device battery level is low; returning a value of true
+     * will cause a low-battery status to appear in Home for the device.
+     *
+     * @return a future that will contain the accessory's low battery state
+     */
+    CompletableFuture<Boolean> getLowBatteryState();
+
+    /**
+     * Subscribes to changes in the battery level.
+     *
+     * @param callback the function to call when low battery state changes.
+     */
+    void subscribeLowBatteryState(HomekitCharacteristicChangeCallback callback);
+
+    /**
+     * Unsubscribes from changes in the low battery state.
+     */
+    void unsubscribeLowBatteryState();
+}

src/main/java/com/beowulfe/hap/accessories/CarbonMonoxideSensor.java

@@ -14,7 +14,7 @@
  * <p>A carbon monoxide sensor reports whether carbon monoxide has been detected or not.</p>
  *
  * <p>Carbon monoxide sensors that run on batteries will need to implement this interface
- * and also implement {@link BatteryAccessory}.</p>
+ * and also implement {@link BatteryStatusAccessory}.</p>
  *
  * @author Gaston Dombiak
  */

src/main/java/com/beowulfe/hap/accessories/ContactSensor.java

@@ -16,7 +16,7 @@
  * that the door/window is closed.</p>
  *
  * <p>Contact sensors that run on batteries will need to implement this interface
- * and also implement {@link BatteryAccessory}.</p>
+ * and also implement {@link BatteryStatusAccessory}.</p>
  *
  * @author Gaston Dombiak
  */

src/main/java/com/beowulfe/hap/accessories/LockMechanism.java

@@ -15,7 +15,7 @@
  * {@link LockableLockMechanism}.</p>
  *
  * <p>Locks that run on batteries will need to implement this interface and also
- * implement {@link BatteryAccessory}.</p>
+ * implement {@link BatteryStatusAccessory}.</p>
  *
  * @author Andy Lintner
  */

src/main/java/com/beowulfe/hap/accessories/MotionSensor.java

@@ -13,7 +13,7 @@
  * <p>A motion sensor that reports whether motion has been detected.</p>
  *
  * <p>Motion sensors that run on batteries will need to implement this interface
- * and also implement {@link BatteryAccessory}.</p>
+ * and also implement {@link BatteryStatusAccessory}.</p>
  *
  * @author Gaston Dombiak
  */

src/main/java/com/beowulfe/hap/accessories/SmokeSensor.java

@@ -14,7 +14,7 @@
  * <p>A smoke sensor reports whether smoke has been detected or not.</p>
  *
  * <p>Smoke sensors that run on batteries will need to implement this interface
- * and also implement {@link BatteryAccessory}.</p>
+ * and also implement {@link BatteryStatusAccessory}.</p>
  *
  * @author Gaston Dombiak
  */

src/main/java/com/beowulfe/hap/impl/characteristics/common/BatteryLevelCharacteristic.java

@@ -1,21 +1,26 @@
 package com.beowulfe.hap.impl.characteristics.common;
 
-import com.beowulfe.hap.HomekitCharacteristicChangeCallback;
-import com.beowulfe.hap.characteristics.EventableCharacteristic;
-import com.beowulfe.hap.characteristics.IntegerCharacteristic;
-
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
+import com.beowulfe.hap.HomekitCharacteristicChangeCallback;
+import com.beowulfe.hap.characteristics.EventableCharacteristic;
+import com.beowulfe.hap.characteristics.IntegerCharacteristic;
+
+/**
+ * This characteristic is used by a stand-alone BatteryService, which describes
+ * a stand-alone battery device, not the battery status of a battery operated
+ * device such as a motion sensor.
+ */
 public class BatteryLevelCharacteristic extends IntegerCharacteristic implements EventableCharacteristic {
 
     private final Supplier<CompletableFuture<Integer>> getter;
     private final Consumer<HomekitCharacteristicChangeCallback> subscriber;
     private final Runnable unsubscriber;
 
     public BatteryLevelCharacteristic(Supplier<CompletableFuture<Integer>> getter,
-                                      Consumer<HomekitCharacteristicChangeCallback> subscriber, Runnable unsubscriber) {
+            Consumer<HomekitCharacteristicChangeCallback> subscriber, Runnable unsubscriber) {
         super("00000068-0000-1000-8000-0026BB765291", false, true, "Battery Level", 0, 100, "%");
         this.getter = getter;
         this.subscriber = subscriber;
@@ -29,7 +34,7 @@ protected CompletableFuture<Integer> getValue() {
 
     @Override
     protected void setValue(Integer value) throws Exception {
-        //Read Only
+        // Read Only
     }
 
     @Override

src/main/java/com/beowulfe/hap/impl/characteristics/common/LowBatteryStatusCharacteristic.java

@@ -0,0 +1,44 @@
+package com.beowulfe.hap.impl.characteristics.common;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+import com.beowulfe.hap.HomekitCharacteristicChangeCallback;
+import com.beowulfe.hap.characteristics.BooleanCharacteristic;
+import com.beowulfe.hap.characteristics.EventableCharacteristic;
+
+public class LowBatteryStatusCharacteristic extends BooleanCharacteristic implements EventableCharacteristic {
+
+    private final Supplier<CompletableFuture<Boolean>> getter;
+    private final Consumer<HomekitCharacteristicChangeCallback> subscriber;
+    private final Runnable unsubscriber;
+
+    public LowBatteryStatusCharacteristic(Supplier<CompletableFuture<Boolean>> getter,
+            Consumer<HomekitCharacteristicChangeCallback> subscriber, Runnable unsubscriber) {
+        super("00000079-0000-1000-8000-0026BB765291", false, true, "Status Low Battery");
+        this.getter = getter;
+        this.subscriber = subscriber;
+        this.unsubscriber = unsubscriber;
+    }
+
+    @Override
+    protected CompletableFuture<Boolean> getValue() {
+        return getter.get();
+    }
+
+    @Override
+    protected void setValue(Boolean value) throws Exception {
+        // Read Only
+    }
+
+    @Override
+    public void subscribe(HomekitCharacteristicChangeCallback callback) {
+        subscriber.accept(callback);
+    }
+
+    @Override
+    public void unsubscribe() {
+        unsubscriber.run();
+    }
+}

src/main/java/com/beowulfe/hap/impl/services/AbstractServiceImpl.java

@@ -1,77 +1,93 @@
 package com.beowulfe.hap.impl.services;
 
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.beowulfe.hap.HomekitAccessory;
 import com.beowulfe.hap.Service;
 import com.beowulfe.hap.accessories.BatteryAccessory;
+import com.beowulfe.hap.accessories.BatteryStatusAccessory;
 import com.beowulfe.hap.characteristics.Characteristic;
 import com.beowulfe.hap.impl.characteristics.common.BatteryLevelCharacteristic;
+import com.beowulfe.hap.impl.characteristics.common.LowBatteryStatusCharacteristic;
 import com.beowulfe.hap.impl.characteristics.common.Name;
 
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
-
 abstract class AbstractServiceImpl implements Service {
-	
-	private final String type;
-	private final List<Characteristic> characteristics = new LinkedList<>();
+    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+    private final String type;
+    private final List<Characteristic> characteristics = new LinkedList<>();
 
-	/**
-	 * This constructor has been deprecated and replaced with
-	 * {@link #AbstractServiceImpl(String, HomekitAccessory, String)}. Usages of
-	 * this constructor will need to manually configure {@link Name} characteristic
-	 * and {@link BatteryLevelCharacteristic} if needed.
-	 *
-	 * @param type unique UUID of the service. This information can be obtained from HomeKit Accessory Simulator.
-	 */
-	@Deprecated
-	public AbstractServiceImpl(String type) {
-		this(type, null, null);
-	}
+    /**
+     * This constructor has been deprecated and replaced with
+     * {@link #AbstractServiceImpl(String, HomekitAccessory, String)}. Usages of
+     * this constructor will need to manually configure {@link Name} characteristic
+     * and {@link BatteryLevelCharacteristic} if needed.
+     *
+     * @param type unique UUID of the service. This information can be obtained from HomeKit Accessory Simulator.
+     */
+    @Deprecated
+    public AbstractServiceImpl(String type) {
+        this(type, null, null);
+    }
 
-	/**
-	 * <p>Creates a new instance of this class with the specified UUID and {@link HomekitAccessory}.
-	 * Download and install <i>HomeKit Accessory Simulator</i> to discover the corresponding UUID for
-	 * the specific service.</p>
-	 *
-	 * <p>The new service will automatically add {@link Name} characteristic. If the accessory
-	 * is battery operated then it must implement {@link BatteryAccessory} and {@link BatteryLevelCharacteristic}
-	 * will be added too.</p>
-	 *
-	 * @param type unique UUID of the service. This information can be obtained from HomeKit Accessory Simulator.
-	 * @param accessory HomeKit accessory exposed as a service.
-	 * @param serviceName name of the service. This information is usually the name of the accessory.
+    /**
+     * <p>
+     * Creates a new instance of this class with the specified UUID and {@link HomekitAccessory}.
+     * Download and install <i>HomeKit Accessory Simulator</i> to discover the corresponding UUID for
+     * the specific service.
+     * </p>
+     *
+     * <p>
+     * The new service will automatically add {@link Name} characteristic. If the accessory
+     * is battery operated then it must implement {@link BatteryAccessory} and {@link BatteryLevelCharacteristic}
+     * will be added too.
+     * </p>
+     *
+     * @param type        unique UUID of the service. This information can be obtained from HomeKit Accessory Simulator.
+     * @param accessory   HomeKit accessory exposed as a service.
+     * @param serviceName name of the service. This information is usually the name of the accessory.
      */
-	public AbstractServiceImpl(String type, HomekitAccessory accessory, String serviceName) {
-		this.type = type;
+    public AbstractServiceImpl(String type, HomekitAccessory accessory, String serviceName) {
+        this.type = type;
+
+        if (accessory != null) {
+            // Add name characteristic
+            addCharacteristic(new Name(serviceName));
+
+            // If battery operated accessory then add BatteryLevelCharacteristic
+            if (accessory instanceof BatteryAccessory) {
+                logger.warn(
+                        "Accessory {} implements BatteryAccessory, which was incorrectly used to advertise battery state and is not recognized by HomeKit. "
+                                + "Battery-powered devices should report their battery status using LowBatteryStatusAccessory",
+                        accessory.getClass());
+            }
+
+            // If battery operated accessory then add LowBatteryStatusAccessory
+            if (accessory instanceof BatteryStatusAccessory) {
+                BatteryStatusAccessory batteryStatusAccessory = (BatteryStatusAccessory) accessory;
+                addCharacteristic(new LowBatteryStatusCharacteristic(batteryStatusAccessory::getLowBatteryState,
+                        batteryStatusAccessory::subscribeLowBatteryState,
+                        batteryStatusAccessory::unsubscribeLowBatteryState));
+
+            }
+        }
+    }
 
-		if (accessory != null) {
-			// Add name characteristic
-			addCharacteristic(new Name(serviceName));
+    @Override
+    public List<Characteristic> getCharacteristics() {
+        return Collections.unmodifiableList(characteristics);
+    }
 
-			// If battery operated accessory then add BatteryLevelCharacteristic
-			if (accessory instanceof BatteryAccessory) {
-				BatteryAccessory batteryAccessory = (BatteryAccessory) accessory;
-				addCharacteristic(new BatteryLevelCharacteristic(
-						batteryAccessory::getBatteryLevelState,
-						batteryAccessory::subscribeBatteryLevelState,
-						batteryAccessory::unsubscribeBatteryLevelState
-				));
-			}
-		}
-	}
+    @Override
+    public String getType() {
+        return type;
+    }
 
-	@Override
-	public List<Characteristic> getCharacteristics() {
-		return Collections.unmodifiableList(characteristics);
-	}
-	
-	@Override
-	public String getType() {
-		return type;
-	}
-	
-	protected void addCharacteristic(Characteristic characteristic) {
-		this.characteristics.add(characteristic);
-	}
+    protected void addCharacteristic(Characteristic characteristic) {
+        this.characteristics.add(characteristic);
+    }
 }