Improve UX of WindowCovering by using motor_state (#854)

* Move `this.ignoreNextUpdateIfEqualToTarget` * Stop `updateTimer` only if `doIgnoreIfEqual` is false * Add support for `motor_state` * Check for `motor_state` in `CoverCreator` * Rework check for `motor_state` in `CoverCreator` * Refactor based on feedback * Rework `scaleAndUpdateCurrentPosition` * Cosmetic fix for Sonarcloud * Update changelog * Update docs for window covering --------- Co-authored-by: Arno Moonen <[email protected]>
itavero · Jun 30, 2024 · 8374850 · 8374850
1 parent 58d037a
commit 8374850
Show file tree

Hide file tree

Showing 5 changed files with 309 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,10 @@ Since version 1.0.0, we try to follow the [Semantic Versioning](https://semver.o
 
 ## [Unreleased]
 
+### Changed
+
+- Window Covering now uses `motor_state` (if provided) to improve the user experience in the Home.app (see [#852](https://github.com/itavero/homebridge-z2m/issues/852) / [#854](https://github.com/itavero/homebridge-z2m/issues/854))
+
 ## [1.11.0-beta.5] - 2024-05-21
 
 ### Fixed

diff --git a/docs/cover.md b/docs/cover.md
@@ -7,6 +7,7 @@ The table below shows how the different features within this `exposes` entry are
 | `position` | published, set | [Current Position](https://developers.homebridge.io/#/characteristic/CurrentPosition) (for the value from MQTT),<br>[Target Position](https://developers.homebridge.io/#/characteristic/TargetPosition) (for the value set from HomeKit) | Required (unless `tilt` is present) |
 | `tilt` | published, set | [Current Horizontal Tilt Angle](https://developers.homebridge.io/#/characteristic/CurrentHorizontalTiltAngle) (for the value from MQTT),<br>[Target Horizontal Tilt Angle](https://developers.homebridge.io/#/characteristic/TargetHorizontalTiltAngle) (for the value set from HomeKit)| Optional. Will be used as _Current Position_ if `position` is not available. |
 
-The required [Position State](https://developers.homebridge.io/#/characteristic/PositionState) characteristic is set when the _Target Position_ is changed or when an `position` is received from MQTT (and the movement is assumed to be stopped).
+If the device definition also provides a `motor_state`, it will be used to update the required [Position State](https://developers.homebridge.io/#/characteristic/PositionState) characteristic.
 
-If the `position` can be _get_, the plugin will try to get frequent updates, after changing the _Target Position_. If the same position is reported twice, movement is assumed to be stopped.
+If `motor_state` is not provided, the [Position State](https://developers.homebridge.io/#/characteristic/PositionState) characteristic is set when the _Target Position_ is changed or when an `position` is received from MQTT (and the movement is assumed to be stopped).
+Additionally, if the `position` can be _get_, the plugin will try to get frequent updates, after changing the _Target Position_. If the same position is reported twice, movement is assumed to be stopped.
diff --git a/src/converters/cover.ts b/src/converters/cover.ts
@@ -27,12 +27,28 @@ export class CoverCreator implements ServiceCreator {
           exposesHasFeatures(e) &&
           !accessory.isServiceHandlerIdKnown(CoverHandler.generateIdentifier(e.endpoint))
       )
-      .forEach((e) => this.createService(e as ExposesEntryWithFeatures, accessory));
+      .forEach((e) => {
+        const motorStateExpose = exposes.find(
+          (m) =>
+            m.endpoint === e.endpoint &&
+            m.name === 'motor_state' &&
+            exposesHasEnumProperty(m) &&
+            exposesIsPublished(m) &&
+            m.values.includes(CoverHandler.MOTOR_STATE_OPENING) &&
+            m.values.includes(CoverHandler.MOTOR_STATE_CLOSING) &&
+            m.values.includes(CoverHandler.MOTOR_STATE_STOPPED)
+        );
+        this.createService(e as ExposesEntryWithFeatures, accessory, motorStateExpose as ExposesEntryWithEnumProperty);
+      });
   }
 
-  private createService(expose: ExposesEntryWithFeatures, accessory: BasicAccessory): void {
+  private createService(
+    expose: ExposesEntryWithFeatures,
+    accessory: BasicAccessory,
+    motorStateExpose: ExposesEntryWithEnumProperty | undefined
+  ): void {
     try {
-      const handler = new CoverHandler(expose, accessory);
+      const handler = new CoverHandler(expose, accessory, motorStateExpose);
       accessory.registerServiceHandler(handler);
     } catch (error) {
       accessory.log.warn(`Failed to setup cover for accessory ${accessory.displayName} from expose "${JSON.stringify(expose)}": ${error}`);
@@ -42,6 +58,9 @@ export class CoverCreator implements ServiceCreator {
 
 class CoverHandler implements ServiceHandler {
   private static readonly STATE_HOLD_POSITION = 'STOP';
+  public static readonly MOTOR_STATE_OPENING = 'opening';
+  public static readonly MOTOR_STATE_CLOSING = 'closing';
+  public static readonly MOTOR_STATE_STOPPED = 'stopped';
   private readonly positionExpose: ExposesEntryWithNumericRangeProperty;
   private readonly tiltExpose: ExposesEntryWithNumericRangeProperty | undefined;
   private readonly stateExpose: ExposesEntryWithEnumProperty | undefined;
@@ -58,12 +77,16 @@ class CoverHandler implements ServiceHandler {
   private ignoreNextUpdateIfEqualToTarget: boolean;
   private lastPositionSet = -1;
   private positionCurrent = -1;
+  private motorState: string | undefined;
+  private motorStatePrevious: string | undefined;
+  private setTargetPositionHandled: boolean;
 
   public readonly mainCharacteristics: Characteristic[] = [];
 
   constructor(
     expose: ExposesEntryWithFeatures,
-    private readonly accessory: BasicAccessory
+    private readonly accessory: BasicAccessory,
+    private readonly motorStateExpose: ExposesEntryWithEnumProperty | undefined
   ) {
     const endpoint = expose.endpoint;
     this.identifier = CoverHandler.generateIdentifier(endpoint);
@@ -151,11 +174,14 @@ class CoverHandler implements ServiceHandler {
       getOrAddCharacteristic(this.service, hap.Characteristic.HoldPosition).on('set', this.handleSetHoldPosition.bind(this));
     }
 
-    if (exposesCanBeGet(this.positionExpose)) {
+    if (exposesCanBeGet(this.positionExpose) && this.motorStateExpose === undefined) {
       this.updateTimer = new ExtendedTimer(this.requestPositionUpdate.bind(this), 4000);
     }
     this.waitingForUpdate = false;
     this.ignoreNextUpdateIfEqualToTarget = false;
+    this.setTargetPositionHandled = false;
+    this.motorState = undefined;
+    this.motorStatePrevious = undefined;
   }
 
   identifier: string;
@@ -167,12 +193,28 @@ class CoverHandler implements ServiceHandler {
     if (this.tiltExpose !== undefined && exposesCanBeGet(this.tiltExpose)) {
       keys.push(this.tiltExpose.property);
     }
+    if (this.motorStateExpose !== undefined && exposesCanBeGet(this.motorStateExpose)) {
+      keys.push(this.motorStateExpose.property);
+    }
     return keys;
   }
 
   updateState(state: Record<string, unknown>): void {
     this.monitors.forEach((m) => m.callback(state));
 
+    if (this.motorStateExpose !== undefined && this.motorStateExpose.property in state) {
+      const latestMotorState = state[this.motorStateExpose.property] as string;
+      switch (latestMotorState) {
+        case CoverHandler.MOTOR_STATE_OPENING:
+        case CoverHandler.MOTOR_STATE_CLOSING:
+          this.motorState = latestMotorState;
+          break;
+        default:
+          this.motorState = CoverHandler.MOTOR_STATE_STOPPED;
+          break;
+      }
+    }
+
     if (this.positionExpose.property in state) {
       const latestPosition = state[this.positionExpose.property] as number;
 
@@ -251,15 +293,44 @@ class CoverHandler implements ServiceHandler {
       this.current_min,
       this.current_max
     );
-
-    this.service.updateCharacteristic(hap.Characteristic.CurrentPosition, characteristicValue);
-
-    if (isStopped) {
-      // Update target position and position state
-      // This should improve the UX in the Home.app
-      this.accessory.log.debug(`${this.accessory.displayName}: cover: assume movement stopped`);
-      this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
-      this.service.updateCharacteristic(hap.Characteristic.TargetPosition, characteristicValue);
+    if (this.motorStateExpose === undefined) {
+      this.service.updateCharacteristic(hap.Characteristic.CurrentPosition, characteristicValue);
+      if (isStopped) {
+        // Update target position and position state
+        // This should improve the UX in the Home.app
+        this.accessory.log.debug(`${this.accessory.displayName}: cover: assume movement stopped`);
+        this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
+        this.service.updateCharacteristic(hap.Characteristic.TargetPosition, characteristicValue);
+      }
+    } else if (this.motorState !== this.motorStatePrevious) {
+      let newPositionState: number;
+      let newTargetPosition: number;
+      switch (this.motorState) {
+        case CoverHandler.MOTOR_STATE_CLOSING:
+          newPositionState = hap.Characteristic.PositionState.DECREASING;
+          newTargetPosition = 0;
+          this.accessory.log.debug(`${this.accessory.displayName}: cover: closing via motor_state`);
+          break;
+        case CoverHandler.MOTOR_STATE_OPENING:
+          newPositionState = hap.Characteristic.PositionState.INCREASING;
+          newTargetPosition = 100;
+          this.accessory.log.debug(`${this.accessory.displayName}: cover: opening via motor_state`);
+          break;
+        default:
+          newPositionState = hap.Characteristic.PositionState.STOPPED;
+          newTargetPosition = characteristicValue;
+          this.accessory.log.debug(`${this.accessory.displayName}: cover: stopped via motor_state`);
+          break;
+      }
+      this.service.updateCharacteristic(hap.Characteristic.PositionState, newPositionState);
+      if (newPositionState === hap.Characteristic.PositionState.STOPPED) {
+        this.service.updateCharacteristic(hap.Characteristic.CurrentPosition, characteristicValue);
+      }
+      if (newPositionState === hap.Characteristic.PositionState.STOPPED || !this.setTargetPositionHandled) {
+        this.service.updateCharacteristic(hap.Characteristic.TargetPosition, newTargetPosition);
+      }
+      this.motorStatePrevious = this.motorState;
+      this.setTargetPositionHandled = false;
     }
   }
 
@@ -276,15 +347,19 @@ class CoverHandler implements ServiceHandler {
     data[this.positionExpose.property] = target;
     this.accessory.queueDataForSetAction(data);
 
-    // Assume position state based on new target
-    if (target > this.positionCurrent) {
-      this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.INCREASING);
-    } else if (target < this.positionCurrent) {
-      this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.DECREASING);
+    if (this.motorStateExpose === undefined) {
+      // Assume position state based on new target
+      if (target > this.positionCurrent) {
+        this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.INCREASING);
+      } else if (target < this.positionCurrent) {
+        this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.DECREASING);
+      } else {
+        this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
+      }
     } else {
-      this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
+      this.service.updateCharacteristic(hap.Characteristic.TargetPosition, target);
+      this.setTargetPositionHandled = true;
     }
-
     // Store last sent position for future reference
     this.lastPositionSet = target;
 

diff --git a/test/cover.spec.ts b/test/cover.spec.ts
@@ -413,4 +413,107 @@ describe('Cover', () => {
       harness.checkHomeKitUpdate(hap.Service.WindowCovering, 'state', true, { state: 'STOP' });
     });
   });
+
+  describe('Bosch Light/shutter control unit II', () => {
+    // Shared "state"
+    let deviceExposes: ExposesEntry[] = [];
+    let harness: ServiceHandlersTestHarness;
+
+    beforeEach(() => {
+      // Only test service creation for first test case and reuse harness afterwards
+      if (deviceExposes.length === 0 && harness === undefined) {
+        // Load exposes from JSON
+        deviceExposes = loadExposesFromFile('bosch/bmct-slz.json');
+        expect(deviceExposes.length).toBeGreaterThan(0);
+        const newHarness = new ServiceHandlersTestHarness();
+
+        // Check service creation
+        const windowCovering = newHarness
+          .getOrAddHandler(hap.Service.WindowCovering)
+          .addExpectedCharacteristic('position', hap.Characteristic.CurrentPosition, false)
+          .addExpectedCharacteristic('target_position', hap.Characteristic.TargetPosition, true)
+          .addExpectedCharacteristic('position_state', hap.Characteristic.PositionState, false)
+          .addExpectedCharacteristic('state', hap.Characteristic.HoldPosition, true);
+        newHarness.prepareCreationMocks();
+
+        const positionCharacteristicMock = windowCovering.getCharacteristicMock('position');
+        if (positionCharacteristicMock !== undefined) {
+          positionCharacteristicMock.props.minValue = 0;
+          positionCharacteristicMock.props.maxValue = 100;
+        }
+
+        const targetPositionCharacteristicMock = windowCovering.getCharacteristicMock('target_position');
+        if (targetPositionCharacteristicMock !== undefined) {
+          targetPositionCharacteristicMock.props.minValue = 0;
+          targetPositionCharacteristicMock.props.maxValue = 100;
+        }
+
+        newHarness.callCreators(deviceExposes);
+
+        newHarness.checkCreationExpectations();
+        newHarness.checkHasMainCharacteristics();
+        newHarness.checkExpectedGetableKeys(['position']);
+        harness = newHarness;
+      }
+      harness?.clearMocks();
+    });
+
+    afterEach(() => {
+      verifyAllWhenMocksCalled();
+      resetAllWhenMocks();
+    });
+
+    test('Status update is handled: Position changes', () => {
+      expect(harness).toBeDefined();
+
+      // First update (previous state is unknown, so)
+      harness.checkUpdateState(
+        '{"position":100, "motor_state":"stopped"}',
+        hap.Service.WindowCovering,
+        new Map([
+          [hap.Characteristic.CurrentPosition, 100],
+          [hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED],
+          [hap.Characteristic.TargetPosition, 100],
+        ])
+      );
+      harness.clearMocks();
+    });
+
+    test('HomeKit: Hold position', () => {
+      expect(harness).toBeDefined();
+
+      harness.checkHomeKitUpdate(hap.Service.WindowCovering, 'state', true, { state: 'STOP' });
+    });
+
+    test('HomeKit: Change target position', () => {
+      expect(harness).toBeDefined();
+
+      // Ignore known stopped position
+      harness.checkUpdateStateIsIgnored('{"position":100, "motor_state":"stopped"}');
+      harness.clearMocks();
+
+      // Check changing the position to a lower value
+      harness.checkHomeKitUpdate(hap.Service.WindowCovering, 'target_position', 51, { position: 51 });
+      harness.getOrAddHandler(hap.Service.WindowCovering).checkCharacteristicUpdates(new Map([[hap.Characteristic.TargetPosition, 51]]));
+      harness.clearMocks();
+
+      harness.checkUpdateState(
+        '{"position":41, "motor_state":"closing"}',
+        hap.Service.WindowCovering,
+        new Map([[hap.Characteristic.PositionState, hap.Characteristic.PositionState.DECREASING]])
+      );
+      harness.clearMocks();
+
+      harness.checkUpdateState(
+        '{"position":51, "motor_state":"stopped"}',
+        hap.Service.WindowCovering,
+        new Map([
+          [hap.Characteristic.CurrentPosition, 51],
+          [hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED],
+          [hap.Characteristic.TargetPosition, 51],
+        ])
+      );
+      harness.clearMocks();
+    });
+  });
 });