Fix for wrong command being sent or no command sent in certain circumstances

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [4.4.15] - 2023-07-27
+### Fixed
+ - Fixes the Fanv2 'On' Characteristic warning. (Thanks @dnicolson) #639
+
 ## [4.4.14] - 2023-07-26
 ### Added
  - Adding support for 520d device (#632)

accessories/fan.js

@@ -1,12 +1,24 @@
 const ServiceManagerTypes = require('../helpers/serviceManagerTypes');
-const SwitchAccessory = require('./switch');
+const BroadlinkRMAccessory = require('./accessory');
 const catchDelayCancelError = require('../helpers/catchDelayCancelError');
 const delayForDuration = require('../helpers/delayForDuration');
+const ping = require('../helpers/ping');
+const arp = require('../helpers/arp')
+
+class FanAccessory extends BroadlinkRMAccessory {
+  constructor(log, config = {}, serviceManagerType) {
+    super(log, config, serviceManagerType);
+
+    if (!config.isUnitTest) {this.checkPing(ping);}
+  }
 
-class FanAccessory extends SwitchAccessory {
   setDefaults() {
-    super.setDefaults();
     let { config, state } = this;
+    config.pingFrequency = config.pingFrequency || 1;
+    config.pingGrace = config.pingGrace || 10;
+
+    config.offDuration = config.offDuration || 60;
+    config.onDuration = config.onDuration || 60;
 
     // Defaults
     config.showSwingMode = config.hideSwingMode === true || config.showSwingMode === false ? false : true;
@@ -47,17 +59,79 @@ class FanAccessory extends SwitchAccessory {
       this.autoOnTimeoutPromise = null;
     }
 
+    if (this.pingGraceTimeout) {
+      this.pingGraceTimeout.cancel();
+      this.pingGraceTimeout = null;
+    }
+
     if (this.serviceManager.getCharacteristic(Characteristic.Active) === undefined) {
       this.serviceManager.setCharacteristic(Characteristic.Active, false);
     }
   }
 
   checkAutoOnOff() {
     this.reset();
+    this.checkPingGrace();
     this.checkAutoOn();
     this.checkAutoOff();
   }
 
+  checkPing(ping) {
+    const { config } = this;
+    let { pingIPAddress, pingFrequency, pingUseArp } = config;
+
+    if (!pingIPAddress) {return;}
+
+    // Setup Ping/Arp-based State
+    if(!pingUseArp) {ping(pingIPAddress, pingFrequency, this.pingCallback.bind(this))}
+    else {arp(pingIPAddress, pingFrequency, this.pingCallback.bind(this))}
+  }
+
+  pingCallback(active) {
+    const { config, state, serviceManager } = this;
+
+    if (this.stateChangeInProgress){
+      return;
+    }
+
+    if (config.pingIPAddressStateOnly) {
+      state.switchState = active ? true : false;
+      serviceManager.refreshCharacteristicUI(Characteristic.Active);
+
+      return;
+    }
+
+    const value = active ? true : false;
+    serviceManager.setCharacteristic(Characteristic.Active, value);
+  }
+
+  //async setSwitchState(hexData) {
+  //  const { data, host, log, name, logLevel } = this;
+
+  //  this.stateChangeInProgress = true;
+  //  this.reset();
+
+  //  if (hexData) {await this.performSend(hexData);}
+
+  //  this.checkAutoOnOff();
+  //}
+
+  async checkPingGrace () {
+    await catchDelayCancelError(async () => {
+      const { config, log, name, state, serviceManager } = this;
+
+      let { pingGrace } = config;
+
+      if (pingGrace) {
+
+        this.pingGraceTimeoutPromise = delayForDuration(pingGrace);
+        await this.pingGraceTimeoutPromise;
+
+        this.stateChangeInProgress = false;
+      }
+    });
+  }
+
   async checkAutoOff() {
     await catchDelayCancelError(async () => {
       const { config, log, logLevel, name, state, serviceManager } = this;
@@ -106,7 +180,9 @@ class FanAccessory extends SwitchAccessory {
       serviceManager.setCharacteristic(Characteristic.RotationSpeed, state.fanSpeed);
     }
 
-    super.setSwitchState(hexData, previousValue);
+    this.reset();
+
+    if (hexData) {await this.performSend(hexData);}
   }
 
   async setFanSpeed(hexData) {

accessories/windowCovering.js

@@ -64,44 +64,89 @@ class WindowCoveringAccessory extends BroadlinkRMAccessory {
       const closeCompletely = await this.checkOpenOrCloseCompletely();
       if (closeCompletely) {return;}
 
-      log(`${name} setTargetPosition: (currentPosition: ${state.currentPosition})`);
+      log(`${name} setTargetPosition: (set new position)`);
 
       // Determine if we're opening or closing
       let difference = state.targetPosition - state.currentPosition;
 
-      state.opening = (difference > 0);
-      if (!state.opening) {difference = -1 * difference;}
-
-      hexData = state.opening ? open : close
-
+      if (difference > 0) {
+        state.positionState = Characteristic.PositionState.INCREASING
+        hexData = open
+      } else if (difference < 0) {
+        state.positionState = Characteristic.PositionState.DECREASING
+        hexData = close
+      } else {
+        state.positionState = Characteristic.PositionState.STOPPED
+        hexData = stop
+      }
+
       // Perform the actual open/close asynchronously i.e. without await so that HomeKit status can be updated
       this.openOrClose({ hexData, previousValue });
     });
   }
 
+  getUpToDatePosition (state) {  
+    let currentValue = state.currentPosition || 0;
+
+    if (state.positionState == Characteristic.PositionState.INCREASING) {currentValue++;}
+    if (state.positionState == Characteristic.PositionState.DECREASING) {currentValue--;}
+
+    if (currentValue < 0) {
+      currentValue = 0
+    } else if (currentValue > 100) {
+      currentValue = 100
+    }
+
+    return currentValue;
+  }
+
   async openOrClose ({ hexData, previousValue }) {
     await catchDelayCancelError(async () => {
       let { config, data, host, name, log, state, logLevel, serviceManager } = this;
       let { totalDurationOpen, totalDurationClose } = config;
       const { stop } = data;
 
-      const newPositionState = state.opening ? Characteristic.PositionState.INCREASING : Characteristic.PositionState.DECREASING;
-      serviceManager.setCharacteristic(Characteristic.PositionState, newPositionState);
-
-      log(`${name} setTargetPosition: currently ${state.currentPosition}%, moving to ${state.targetPosition}%`);
-
-      await this.performSend(hexData);
+      serviceManager.setCharacteristic(Characteristic.PositionState, state.positionState);
 
       let difference = state.targetPosition - state.currentPosition
-      if (!state.opening) {difference = -1 * difference;}
-
-      const fullOpenCloseTime = state.opening ? totalDurationOpen : totalDurationClose;
-      const durationPerPercentage = fullOpenCloseTime / 100;
-      const totalTime = durationPerPercentage * difference;
+      let positionStateDescription = null;
+      let fullOpenCloseTime = null
+
+      if (state.positionState == Characteristic.PositionState.INCREASING) {
+        positionStateDescription = 'opening';
+        fullOpenCloseTime = totalDurationOpen;
+      } else if (state.positionState == Characteristic.PositionState.DECREASING) {
+        positionStateDescription = 'closing';
+        fullOpenCloseTime = totalDurationClose;
+        difference = -1 * difference;
+      } else {
+        positionStateDescription = 'stopped';
+        fullOpenCloseTime = 0;
+      }
+
+      const totalTime = Math.abs(difference / 100 * fullOpenCloseTime);
 
-      log(`${name} setTargetPosition: ${totalTime}s (${fullOpenCloseTime} / 100 * ${difference}) until auto-stop`);
+      log(`${name} setTargetPosition: position change ${state.currentPosition}% -> ${state.targetPosition}% (${positionStateDescription})`);
+      log(`${name} setTargetPosition: ${+totalTime.toFixed(2)}s ((${Math.abs(difference)} / 100) * ${fullOpenCloseTime}) until auto-stop`);
 
-      this.startUpdatingCurrentPositionAtIntervals();
+      await this.performSend(hexData);
+
+      if (state.positionState != Characteristic.PositionState.STOPPED) {
+        // immediately update position to reflect that there's already some change in the position (even though its fractional,
+        // we have to add 1 whole %), we then skip incrementing the position within startUpdatingCurrentPositionAtIntervals
+        // if this is a first iteration, this way at time 0 the position delta is already 1, and so is the position at time 1
+        // and we do not overshoot the actual position.
+
+        // NOTE: ideally send+position update should be an "atomic" operation and the position should change by some
+        //       fractional value (e.g. 0.00001) but that requires significant changes to the code base.
+
+        const currentValue = this.getUpToDatePosition(state)
+        serviceManager.setCharacteristic(Characteristic.CurrentPosition, currentValue);
+
+        this.startUpdatingCurrentPositionAtIntervals(true, name, log);
+      } else {
+        this.startUpdatingCurrentPositionAtIntervals(false, name, log);
+      }
 
       this.autoStopPromise = delayForDuration(totalTime);
       await this.autoStopPromise;
@@ -157,42 +202,61 @@ class WindowCoveringAccessory extends BroadlinkRMAccessory {
 
     return false;
   }
-
+  
   // Determine how long it should take to increase/decrease a single %
-  determineOpenCloseDurationPerPercent ({ opening, totalDurationOpen, totalDurationClose  }) {
-    assert.isBoolean(opening);
+  determineOpenCloseDurationPerPercent ({ positionState, totalDurationOpen, totalDurationClose  }) {
     assert.isNumber(totalDurationOpen);
     assert.isNumber(totalDurationClose);
     assert.isAbove(totalDurationOpen, 0);
     assert.isAbove(totalDurationClose, 0);
 
-    const fullOpenCloseTime = opening ? totalDurationOpen : totalDurationClose;
+    let fullOpenCloseTime = null
+    if (positionState == Characteristic.PositionState.INCREASING) {
+      fullOpenCloseTime = totalDurationOpen;
+    } else if (positionState == Characteristic.PositionState.DECREASING) {
+      fullOpenCloseTime = totalDurationClose;
+    } else {
+      fullOpenCloseTime = 0;
+    }
+
     const durationPerPercentage = fullOpenCloseTime / 100;
 
     return durationPerPercentage;
   }
 
-  async startUpdatingCurrentPositionAtIntervals () {
+  async startUpdatingCurrentPositionAtIntervals (isFirst, name, log) {
     catchDelayCancelError(async () => {
       const { config, serviceManager, state } = this;
       const { totalDurationOpen, totalDurationClose } = config;
 
-      const durationPerPercentage = this.determineOpenCloseDurationPerPercent({ opening: state.opening, totalDurationOpen, totalDurationClose })
+      const durationPerPercentage = this.determineOpenCloseDurationPerPercent({ positionState: state.positionState, totalDurationOpen, totalDurationClose });
 
       // Wait for a single % to increase/decrease
       this.updateCurrentPositionPromise = delayForDuration(durationPerPercentage)
       await this.updateCurrentPositionPromise
 
       // Set the new currentPosition
-      let currentValue = state.currentPosition || 0;
+      let positionStateDescription = null;
+
+      if (state.positionState == Characteristic.PositionState.INCREASING) {
+        positionStateDescription = 'opening';
+      } else if (state.positionState == Characteristic.PositionState.DECREASING) {
+        positionStateDescription = 'closing';
+      } else {
+        positionStateDescription = 'stopped';
+      }
 
-      if (state.opening) {currentValue++;}
-      if (!state.opening) {currentValue--;}
+      if (!isFirst) {               
+        const currentValue = this.getUpToDatePosition(state)
+        serviceManager.setCharacteristic(Characteristic.CurrentPosition, currentValue);
 
-      serviceManager.setCharacteristic(Characteristic.CurrentPosition, currentValue);
+        log(`${name} setTargetPosition: updated position to ${currentValue} (${positionStateDescription})`);
+      }
 
       // Let's go again
-      this.startUpdatingCurrentPositionAtIntervals();
+      if (state.positionState != Characteristic.PositionState.STOPPED) {
+        this.startUpdatingCurrentPositionAtIntervals(false, name, log);
+      }
     });
   }