Merge pull request #319 from michikrug/state-errors

openhab · Mar 13, 2022 · 6fc2ec5 · 6fc2ec5
2 parents e025b7d + 6458a88
commit 6fc2ec5
Show file tree

Hide file tree

Showing 24 changed files with 436 additions and 252 deletions.
diff --git a/docs/USAGE.md b/docs/USAGE.md
diff --git a/functions/commands/armdisarm.js b/functions/commands/armdisarm.js
@@ -60,37 +60,10 @@ class ArmDisarm extends DefaultCommand {
     return true;
   }
 
-  static validateStateChange(params, item, device) {
-    let isCurrentlyArmed;
-    let currentLevel;
-
-    if (this.getDeviceType(device) === 'SecuritySystem') {
-      const members = SecuritySystem.getMembers(item);
-      isCurrentlyArmed =
-        (SecuritySystem.armedMemberName in members && members[SecuritySystem.armedMemberName].state) ===
-        (this.isInverted(device) ? 'OFF' : 'ON');
-      currentLevel =
-        (SecuritySystem.armLevelMemberName in members && members[SecuritySystem.armLevelMemberName].state) || undefined;
-    } else {
-      isCurrentlyArmed = item.state === (this.isInverted(device) ? 'OFF' : 'ON');
-    }
-
-    if (params.armLevel && this.getDeviceType(device) === 'SecuritySystem') {
-      if (params.arm && isCurrentlyArmed && params.armLevel === currentLevel) {
-        throw { errorCode: 'alreadyInState' };
-      }
-      return true;
-    }
-
-    if (params.arm && isCurrentlyArmed) {
-      throw { errorCode: 'alreadyArmed' };
+  static checkCurrentState(target, state, params) {
+    if (target === state) {
+      throw { errorCode: params.armLevel ? 'alreadyInState' : params.arm ? 'alreadyArmed' : 'alreadyDisarmed' };
     }
-
-    if (!params.arm && !isCurrentlyArmed) {
-      throw { errorCode: 'alreadyDisarmed' };
-    }
-
-    return true;
   }
 
   static validateUpdate(params, item, device) {

diff --git a/functions/commands/default.js b/functions/commands/default.js
@@ -25,13 +25,23 @@ class DefaultCommand {
   }
 
   /**
-   * Is the requested new state valid
-   * @param {object} params Requested change params
-   * @param {object} item Current state of item
-   * @returns {boolean} true if state change is valid otherwise throws error
+   * Is the requested new state change valid?
+   * @param {string} target Requested target state
+   * @param {string} state Current state of item
+   * @param {object} params Parameters of the command
+   * @returns {void} returns if current state is different otherwise throws error
    */
-  static validateStateChange(params, item, device) {
-    return true;
+  static checkCurrentState(target, state, params) {
+    if (target === state) {
+      throw { errorCode: 'alreadyInState' };
+    }
+  }
+
+  /**
+   * @param {object} item
+   */
+  static getNormalizedState(item) {
+    return item.type.startsWith('Number:') ? item.state.split(' ')[0] : item.state;
   }
 
   static get requiresUpdateValidation() {
@@ -94,7 +104,7 @@ class DefaultCommand {
    * @param {object} device
    */
   static isInverted(device) {
-    return device.customData && device.customData.inverted === true;
+    return !!(device.customData && device.customData.inverted);
   }
 
   /**
@@ -202,7 +212,6 @@ class DefaultCommand {
    * @param {object} challenge
    */
   static execute(apiHandler, devices, params, challenge) {
-    // console.log(`openhabGoogleAssistant - ${this.type}: ${JSON.stringify({ devices: devices, params: params })}`);
     const commandsResponse = [];
     const promises = devices.map((device) => {
       const authPinResponse = this.handleAuthPin(device, challenge, params);
@@ -217,14 +226,25 @@ class DefaultCommand {
         (device.customData.ackNeeded || device.customData.tfaAck) &&
         !(challenge && challenge.ack);
 
-      let getItemPromise = Promise.resolve({ name: device.id });
-      if (this.requiresItem(device) || ackWithState || this.requiresUpdateValidation) {
+      const shouldCheckState = device.customData && device.customData.checkState;
+
+      let getItemPromise = Promise.resolve({ name: device.id, state: null, members: [] });
+      if (this.requiresItem(device) || ackWithState || shouldCheckState) {
         getItemPromise = apiHandler.getItem(device.id);
       }
 
       return getItemPromise
         .then((item) => {
-          this.validateStateChange(params, item, device);
+          const targetItem = this.getItemName(item, device, params);
+          const targetValue = this.convertParamsToValue(params, item, device);
+          if (shouldCheckState) {
+            let currentState = this.getNormalizedState(item);
+            if (targetItem !== device.id && item.members && item.members.length) {
+              const member = item.members.find((m) => m.name === targetItem);
+              currentState = member ? this.getNormalizedState(member) : currentState;
+            }
+            this.checkCurrentState(targetValue, currentState, params);
+          }
 
           const responseStates = this.getResponseStates(params, item, device);
           if (Object.keys(responseStates).length) {
@@ -237,8 +257,6 @@ class DefaultCommand {
             return;
           }
 
-          const targetItem = this.getItemName(item, device, params);
-          const targetValue = this.convertParamsToValue(params, item, device);
           let sendCommandPromise = Promise.resolve();
           if (typeof targetItem === 'string' && typeof targetValue === 'string') {
             sendCommandPromise = apiHandler.sendCommand(targetItem, targetValue);

diff --git a/functions/commands/lockunlock.js b/functions/commands/lockunlock.js
@@ -25,6 +25,12 @@ class LockUnlock extends DefaultCommand {
       isLocked: params.lock
     };
   }
+
+  static checkCurrentState(target, state, params) {
+    if (target === state) {
+      throw { errorCode: params.lock ? 'alreadyLocked' : 'alreadyUnlocked' };
+    }
+  }
 }
 
 module.exports = LockUnlock;
diff --git a/functions/commands/onoff.js b/functions/commands/onoff.js
@@ -50,6 +50,12 @@ class OnOff extends DefaultCommand {
       on: params.on
     };
   }
+
+  static checkCurrentState(target, state, params) {
+    if (target === state) {
+      throw { errorCode: params.on ? 'alreadyOn' : 'alreadyOff' };
+    }
+  }
 }
 
 module.exports = OnOff;
diff --git a/functions/commands/openclose.js b/functions/commands/openclose.js
@@ -19,7 +19,7 @@ class OpenClose extends DefaultCommand {
       openPercent = 100 - openPercent;
     }
     if (itemType === 'Rollershutter') {
-      return openPercent === 0 ? 'DOWN' : openPercent === 100 ? 'UP' : (100 - openPercent).toString();
+      return (100 - openPercent).toString();
     }
     if (itemType === 'Switch') {
       return openPercent === 0 ? 'OFF' : 'ON';
@@ -32,6 +32,12 @@ class OpenClose extends DefaultCommand {
       openPercent: params.openPercent
     };
   }
+
+  static checkCurrentState(target, state, params) {
+    if (target === state) {
+      throw { errorCode: params.openPercent === 0 ? 'alreadyClosed' : 'alreadyOpen' };
+    }
+  }
 }
 
 module.exports = OpenClose;
diff --git a/functions/commands/setvolume.js b/functions/commands/setvolume.js
@@ -34,6 +34,14 @@ class SetVolume extends DefaultCommand {
       currentVolume: params.volumeLevel
     };
   }
+
+  static checkCurrentState(target, state) {
+    if (target === state) {
+      throw {
+        errorCode: state === '100' ? 'volumeAlreadyMax' : state === '0' ? 'volumeAlreadyMin' : 'alreadyInState'
+      };
+    }
+  }
 }
 
 module.exports = SetVolume;
diff --git a/functions/commands/startstop.js b/functions/commands/startstop.js
@@ -26,6 +26,12 @@ class StartStop extends DefaultCommand {
       isPaused: !params.start
     };
   }
+
+  static checkCurrentState(target, state, params) {
+    if (target === state) {
+      throw { errorCode: params.start ? 'alreadyStarted' : 'alreadyStopped' };
+    }
+  }
 }
 
 module.exports = StartStop;
diff --git a/functions/commands/volumerelative.js b/functions/commands/volumerelative.js
@@ -44,6 +44,14 @@ class VolumeRelative extends DefaultCommand {
       currentVolume: parseInt(this.convertParamsToValue(params, item, device))
     };
   }
+
+  static checkCurrentState(target, state) {
+    if (target === state) {
+      throw {
+        errorCode: state === '100' ? 'volumeAlreadyMax' : state === '0' ? 'volumeAlreadyMin' : 'alreadyInState'
+      };
+    }
+  }
 }
 
 module.exports = VolumeRelative;
diff --git a/functions/devices/charger.js b/functions/devices/charger.js
@@ -17,7 +17,7 @@ class Charger extends DefaultDevice {
     const config = this.getConfig(item);
     const members = this.getMembers(item);
     const attributes = {
-      isRechargeable: config.isRechargeable === true,
+      isRechargeable: !!config.isRechargeable,
       queryOnlyEnergyStorage: !('chargerCharging' in members)
     };
     return attributes;

diff --git a/functions/devices/default.js b/functions/devices/default.js
@@ -91,6 +91,9 @@ class DefaultDevice {
     if (config.inverted === true) {
       metadata.customData.inverted = true;
     }
+    if (config.checkState === true) {
+      metadata.customData.checkState = true;
+    }
     if (config.ackNeeded === true || config.tfaAck === true) {
       metadata.customData.ackNeeded = true;
     }

diff --git a/tests/commands/armdisarm.test.js b/tests/commands/armdisarm.test.js
@@ -112,128 +112,30 @@ describe('ArmDisarm Command', () => {
     expect(Command.bypassPin({ customData: { pinOnDisarmOnly: true } }, { armLevel: 'L1' })).toBe(true);
   });
 
-  describe('validateStateChange', () => {
-    test('validateStateChange SimpleSecuritySystem', () => {
-      expect.assertions(6);
-
-      const item = { type: 'Switch', state: 'ON' };
-
-      expect(Command.validateStateChange({ arm: false }, item, {})).toBe(true);
-      try {
-        Command.validateStateChange({ arm: true }, item, {});
-      } catch (e) {
-        expect(e.errorCode).toBe('alreadyArmed');
-      }
-
-      item.state = 'OFF';
-      expect(Command.validateStateChange({ arm: true }, item, {})).toBe(true);
-      try {
-        Command.validateStateChange({ arm: false }, item, {});
-      } catch (e) {
-        expect(e.errorCode).toBe('alreadyDisarmed');
-      }
-
-      item.state = 'ON';
-      expect(
-        Command.validateStateChange({ arm: true }, item, {
-          customData: { inverted: true }
-        })
-      ).toBe(true);
-      item.state = 'OFF';
-      try {
-        Command.validateStateChange({ arm: true }, item, {
-          customData: { inverted: true }
-        });
-      } catch (e) {
-        expect(e.errorCode).toBe('alreadyArmed');
-      }
-    });
-
-    describe('validateStateChange SecuritySystem', () => {
-      const item = {
-        members: [
-          {
-            state: 'ON',
-            metadata: { ga: { value: SecuritySystem.armedMemberName } }
-          },
-          {
-            state: 'L0',
-            metadata: { ga: { value: SecuritySystem.armLevelMemberName } }
-          }
-        ]
-      };
-
-      test('arming without level', () => {
-        expect.assertions(2);
-        item.members[0].state = 'ON';
-        try {
-          Command.validateStateChange({ arm: true }, item, { customData: { deviceType: 'SecuritySystem' } });
-        } catch (e) {
-          expect(e.errorCode).toBe('alreadyArmed');
-        }
-
-        item.members[0].state = 'OFF';
-        expect(Command.validateStateChange({ arm: true }, item, { customData: { deviceType: 'SecuritySystem' } })).toBe(
-          true
-        );
-      });
-
-      test('arming without level inverted', () => {
-        expect.assertions(2);
-        item.members[0].state = 'OFF';
-        try {
-          Command.validateStateChange({ arm: true }, item, {
-            customData: { deviceType: 'SecuritySystem', inverted: true }
-          });
-        } catch (e) {
-          expect(e.errorCode).toBe('alreadyArmed');
-        }
-
-        item.members[0].state = 'ON';
-        expect(
-          Command.validateStateChange({ arm: true }, item, {
-            customData: { deviceType: 'SecuritySystem', inverted: true }
-          })
-        ).toBe(true);
-      });
-
-      test('arming with level', () => {
-        expect.assertions(2);
-        item.members[0].state = 'OFF';
-        item.members[1].state = 'L1';
-        expect(
-          Command.validateStateChange({ arm: true, armLevel: 'L2' }, item, {
-            customData: { deviceType: 'SecuritySystem' }
-          })
-        ).toBe(true);
-
-        item.members[0].state = 'ON';
-        try {
-          Command.validateStateChange({ arm: true, armLevel: 'L1' }, item, {
-            customData: { deviceType: 'SecuritySystem' }
-          });
-        } catch (e) {
-          expect(e.errorCode).toBe('alreadyInState');
-        }
-      });
-
-      test('disarming', () => {
-        expect.assertions(2);
-        item.members[0].state = 'ON';
-        expect(
-          Command.validateStateChange({ arm: false }, item, { customData: { deviceType: 'SecuritySystem' } })
-        ).toBe(true);
-
-        item.members[0].state = 'OFF';
-        try {
-          Command.validateStateChange({ arm: false }, item, {
-            customData: { deviceType: 'SecuritySystem' }
-          });
-        } catch (e) {
-          expect(e.errorCode).toBe('alreadyDisarmed');
-        }
-      });
-    });
+  test('checkCurrentState', () => {
+    expect.assertions(6);
+
+    expect(Command.checkCurrentState('ON', 'OFF', { arm: true })).toBeUndefined();
+    try {
+      Command.checkCurrentState('ON', 'ON', { arm: true });
+    } catch (e) {
+      expect(e.errorCode).toBe('alreadyArmed');
+    }
+
+    expect(Command.checkCurrentState('OFF', 'ON', { arm: false })).toBeUndefined();
+    try {
+      Command.checkCurrentState('OFF', 'OFF', { arm: false });
+    } catch (e) {
+      expect(e.errorCode).toBe('alreadyDisarmed');
+    }
+
+    expect(Command.checkCurrentState('L2', 'L1', { arm: true, armLevel: 'L2' })).toBeUndefined();
+
+    try {
+      Command.checkCurrentState('L2', 'L2', { arm: true, armLevel: 'L2' });
+    } catch (e) {
+      expect(e.errorCode).toBe('alreadyInState');
+    }
   });
 
   describe('validateUpdate', () => {