feat(plugin): stability & reliability improvements

- simplify device updates - allows updating device info (core info) via platform - implements (better) device recovery - implements better error handling for devices - streamlines code - streamlines accessory interfaces & methods - other small chores - implements fixes #3, #103, #116, #123, #135
johannrichard · Nov 28, 2020 · 70e88aa · 70e88aa
1 parent e5a28bc
commit 70e88aa
Show file tree

Hide file tree

Showing 10 changed files with 173 additions and 206 deletions.
diff --git a/package.json b/package.json
@@ -37,7 +37,6 @@
     "iot"
   ],
   "dependencies": {
-    "@the-/jitter": "^15.4.0",
     "@types/node": "^14.0.13",
     "@types/node-fetch": "^2.5.7",
     "abort-controller": "^3.0.0",

diff --git a/src/dingzAccessory.ts b/src/dingzAccessory.ts
@@ -161,11 +161,7 @@ export class DingzAccessory extends DingzDaBaseAccessory {
             // dingz has a Motion sensor -- let's create it
             this.addMotionService();
           } else {
-            this.log.info(
-              'Your dingz',
-              this.accessory.displayName,
-              'has no Motion sensor.',
-            );
+            this.log.info('This dingz has no Motion sensor.');
           }
           // dingz has a temperature sensor and an LED,
           // make these available here
@@ -269,15 +265,15 @@ export class DingzAccessory extends DingzDaBaseAccessory {
 
   // Get updated device info and update the corresponding values
   // TODO: #103, #116, #120, #123 -- fetch state for all device elements
-  private getDeviceStateUpdate() {
+  protected getDeviceStateUpdate() {
     this.getDeviceState()
       .then((state) => {
         if (typeof state !== 'undefined') {
           if (!this.isReachable) {
             // Update reachability -- obviously, we're online again
             this.isReachable = true;
             this.log.warn(
-              `Device --> ${this.device.name} (${this.device.address}) --> recovered from unreachable state`,
+              `Device --> ${this.accessory.displayName} (${this.device.address}) --> recovered from unreachable state`,
             );
           }
           // Outputs
@@ -344,19 +340,6 @@ export class DingzAccessory extends DingzDaBaseAccessory {
     callback(null, currentTemperature);
   }
 
-  /*
-   * This method is optional to implement. It is called when HomeKit ask to identify the accessory.
-   * Typical this only ever happens at the pairing process.
-   */
-  identify(): void {
-    this.log.info(
-      'Identify! -> Who am I? I am',
-      this.accessory.displayName,
-      '-> MAC:',
-      this.device.mac,
-    );
-  }
-
   private addLightSensorService() {
     // Add the LightSensor that's integrated in the dingz
     // API: /api/v1/light
@@ -376,7 +359,6 @@ export class DingzAccessory extends DingzDaBaseAccessory {
 
   private updateLightSensor(lightService: Service) {
     const intensity: number = this.dingzStates.Brightness;
-    this.log.warn(this.device.name, 'Brightness ->', intensity);
     lightService
       .getCharacteristic(this.platform.Characteristic.CurrentAmbientLightLevel)
       .updateValue(intensity);
@@ -560,14 +542,12 @@ export class DingzAccessory extends DingzDaBaseAccessory {
       (mac, action: ButtonAction, button: ButtonId | '5') => {
         if (mac === this.device.mac && button) {
           this.log.debug(
-            `Button ${button} of ${this.device.name} pressed -> ${action}, MAC: ${mac} (This: ${this.device.mac})`,
+            `Button ${button} pressed -> ${action}, MAC: ${mac} (This: ${this.device.mac})`,
           );
           if (button === '5') {
             // PUSH MOTION
             if (!(this.platform.config.motionPoller ?? true)) {
-              this.log.debug(
-                `Button ${button} of ${this.device.name} Motion -> ${action}`,
-              );
+              this.log.debug(`Button ${button} Motion -> ${action}`);
               this.log.debug('Motion Update from CALLBACK');
               this.motionService
                 ?.getCharacteristic(this.platform.Characteristic.MotionDetected)
@@ -593,7 +573,7 @@ export class DingzAccessory extends DingzDaBaseAccessory {
               )
               .updateValue(this.dingzStates.Buttons[button].state);
             this.log.info(
-              `Button ${button} of ${this.device.name} (${service?.displayName}) pressed -> ${action}`,
+              `Button ${button} (${service?.displayName}) pressed -> ${action}`,
             );
 
             // Immediately update states after button pressed
@@ -679,14 +659,7 @@ export class DingzAccessory extends DingzDaBaseAccessory {
     callback: CharacteristicGetCallback,
   ) {
     const currentState = this.dingzStates.Buttons[button].state;
-    this.log.info(
-      'Get Switch State of',
-      this.device.name,
-      '->',
-      button,
-      '-> state:',
-      currentState,
-    );
+    this.log.info('Get Switch State of ->', button, '-> state:', currentState);
     callback(null, currentState);
   }
 
@@ -696,14 +669,7 @@ export class DingzAccessory extends DingzDaBaseAccessory {
     callback: CharacteristicSetCallback,
   ) {
     this.dingzStates.Buttons[button].state = value as ButtonState;
-    this.log.info(
-      'Set Switch State of',
-      this.device.name,
-      '->',
-      button,
-      '-> state:',
-      value,
-    );
+    this.log.info('Set Switch State of ->', button, '-> state:', value);
     callback(null);
   }
 
@@ -1088,7 +1054,7 @@ export class DingzAccessory extends DingzDaBaseAccessory {
     this.services.push(this.motionService);
     // Only check for motion if we have a PIR and set the Interval
     if (this.platform.config.motionPoller ?? true) {
-      this.log.info('Motion POLLING of', this.device.name, 'enabled');
+      this.log.info('Motion POLLING enabled');
       const motionInterval: NodeJS.Timer = setInterval(() => {
         this.getDeviceMotion()
           .then((data) => {
@@ -1106,8 +1072,7 @@ export class DingzAccessory extends DingzDaBaseAccessory {
               }
             } else {
               throw new DeviceNotReachableError(
-                `Device can not be reached ->
-              ${this.device.name}-> ${this.device.address}`,
+                `Device can not be reached -> ${this.accessory.displayName}-> ${this.device.address}`,
               );
             }
           })
@@ -1386,60 +1351,36 @@ export class DingzAccessory extends DingzDaBaseAccessory {
     callback: CharacteristicSetCallback,
   ) {
     this.dingzStates.LED.on = value as boolean;
-    const state = this.dingzStates.LED;
-    const color = `${state.hue};${state.saturation};${state.value}`;
-    this.setDeviceLED({ isOn: state.on, color: color });
-    callback(null);
+    this.setDeviceLED(callback);
   }
 
   private getLEDOn(callback: CharacteristicGetCallback) {
     const isOn = this.dingzStates.LED.on;
-    callback(null, isOn);
+    callback(this.isReachable ? null : new Error(), isOn);
   }
 
   private setLEDHue(
     value: CharacteristicValue,
     callback: CharacteristicSetCallback,
   ) {
     this.dingzStates.LED.hue = value as number;
-
-    const state: DingzLEDState = this.dingzStates.LED;
-    const color = `${state.hue};${state.saturation};${state.value}`;
-    this.setDeviceLED({
-      isOn: state.on,
-      color: color,
-    });
-    callback(null);
+    this.setDeviceLED(callback);
   }
 
   private setLEDSaturation(
     value: CharacteristicValue,
     callback: CharacteristicSetCallback,
   ) {
     this.dingzStates.LED.saturation = value as number;
-
-    const state: DingzLEDState = this.dingzStates.LED;
-    const color = `${state.hue};${state.saturation};${state.value}`;
-    this.setDeviceLED({
-      isOn: state.on,
-      color: color,
-    });
-    callback(null);
+    this.setDeviceLED(callback);
   }
 
   private setLEDBrightness(
     value: CharacteristicValue,
     callback: CharacteristicSetCallback,
   ) {
     this.dingzStates.LED.value = value as number;
-
-    const state: DingzLEDState = this.dingzStates.LED;
-    const color = `${state.hue};${state.saturation};${state.value}`;
-    this.setDeviceLED({
-      isOn: state.on,
-      color: color,
-    });
-    callback(null);
+    this.setDeviceLED(callback);
   }
 
   // Get Input & Dimmer Config
@@ -1566,25 +1507,35 @@ export class DingzAccessory extends DingzDaBaseAccessory {
   // TODO: Feedback on API doc
   /**
    * Set the LED on the dingz
-   * @param isOn: on/off state
-   * @param color color to set the LED to
+   * @param callback: Characteristic callback
    */
-  private setDeviceLED({ isOn, color }: { isOn: boolean; color: string }) {
+  private setDeviceLED(callback: CharacteristicSetCallback) {
+    const state: DingzLEDState = this.dingzStates.LED;
+    const color = `${state.hue};${state.saturation};${state.value}`;
+
     const setLEDEndpoint = `${this.baseUrl}/api/v1/led/set`;
     this.request
       .post(
         setLEDEndpoint,
         qs.stringify(
           {
-            action: isOn ? 'on' : 'off',
+            action: state.on ? 'on' : 'off',
             color: color,
             mode: 'hsv', // Fixed for the time being
             ramp: 150,
           },
           { encode: false },
         ),
       )
-      .catch(this.handleRequestErrors.bind(this));
+      .catch(this.handleRequestErrors.bind(this))
+      .finally(() => {
+        // make sure we callback
+        if (!this.isReachable) {
+          callback(new Error());
+        } else {
+          callback(null);
+        }
+      });
   }
 
   // Get the current state

diff --git a/src/lib/dingzDaBaseAccessory.ts b/src/lib/dingzDaBaseAccessory.ts
@@ -37,7 +37,7 @@ export class DingzDaBaseAccessory {
     this.device = this.accessory.context.device;
     this.baseUrl = `http://${this.device.address}`;
 
-    this.log = new DingzLogger(this.device.name, platform.log);
+    this.log = new DingzLogger(this.accessory.displayName, platform.log);
     this.request = axios.create({
       baseURL: this.baseUrl,
       timeout: RETRY_TIMEOUT,
@@ -62,6 +62,11 @@ export class DingzDaBaseAccessory {
       );
     });
 
+    this.platform.eb.on(
+      PlatformEvent.REQUEST_STATE_UPDATE,
+      this.getDeviceStateUpdate.bind(this),
+    );
+
     // Register listener for updated device info (e.g. on restore with new IP)
     this.platform.eb.on(
       PlatformEvent.UPDATE_DEVICE_INFO,
@@ -71,23 +76,38 @@ export class DingzDaBaseAccessory {
             'Updated device info received -> update accessory address',
           );
 
-          // Update core info (mainly address)
-          if (this.device.address !== deviceInfo.address) {
+          // Update core info (mainly address, maybe token too)
+          if (
+            this.device.address !== deviceInfo.address ||
+            this.device.token !== deviceInfo.token
+          ) {
             this.log.info(
               'Accessory IP changed for',
-              this.device.name,
+              this.accessory.displayName,
               '-> Updating accessory from ->',
               this.device.address,
               'to',
               deviceInfo.address,
             );
+            this.accessory.displayName = this.device.name;
             this.device.address = deviceInfo.address;
+            this.device.token = deviceInfo.token;
             this.baseUrl = `http://${this.device.address}`;
 
-            // Set AccessoryInformation and Update its configuration
+            this.request.defaults = {
+              baseURL: this.baseUrl,
+              timeout: RETRY_TIMEOUT,
+              headers: { Token: this.device.token ?? '' },
+            };
+
+            // update AccessoryInformation
             this.setAccessoryInformation();
-            this.updateAccessory();
           }
+
+          // Set accessory to reachable and
+          // updateAccessory()
+          this.isReachable = true;
+          this.updateAccessory();
         }
       },
     );
@@ -108,6 +128,13 @@ export class DingzDaBaseAccessory {
     );
   }
 
+  protected getDeviceStateUpdate() {
+    this.log.debug(
+      'getDeviceStateUpdate() not implemented for',
+      this.device.accessoryClass,
+    );
+  }
+
   /**
    * Handler for request errors
    * @param e AxiosError: the error returned by this.request()
@@ -134,7 +161,7 @@ export class DingzDaBaseAccessory {
       }
     } else if (e instanceof DeviceNotReachableError) {
       this.log.error(
-        `handleRequestErrors() --> ${this.device.name} (${this.device.address})`,
+        `handleRequestErrors() --> ${this.accessory.displayName} (${this.device.address})`,
       );
       this.isReachable = false;
     } else {

diff --git a/src/lib/libs.d.ts b/src/lib/libs.d.ts
@@ -1 +1,2 @@
 declare module 'simple-color-converter';
+declare module 'is-valid-host';
diff --git a/src/myStromButtonAccessory.ts b/src/myStromButtonAccessory.ts
@@ -141,9 +141,7 @@ export class MyStromButtonAccessory extends DingzDaBaseAccessory {
         const ProgrammableSwitchEvent = this.platform.Characteristic
           .ProgrammableSwitchEvent;
         if (buttonService) {
-          this.log.debug(
-            `Button of ${this.device.name} (${this.device.mac}) pressed -> ${action}`,
-          );
+          this.log.debug(`Button (${this.device.mac}) pressed -> ${action}`);
           switch (action) {
             case ButtonAction.SINGLE_PRESS:
               buttonService
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		declare module 'simple-color-converter';
		declare module 'is-valid-host';