Skip to content

Commit

Permalink
[actions & triggers] Remove arg typ checks
Browse files Browse the repository at this point in the history
Because:
* they cause trouble
* the addon now logs a stacktrace on IllegalArgumentException

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
  • Loading branch information
florian-h05 committed Jan 3, 2023
1 parent c8c4535 commit 9975507
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 107 deletions.
9 changes: 5 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

## to be released

| Type | Namespace | Description | Reference | Breaking |
|-------------|------------|----------------------------------------------------------------------------|--------------------------------------------------------|----------|
| Enhancement | `items` | Refactor `metadata` & `itemchannllink` APIs & add additional functionality | [#212](https://github.com/openhab/openhab-js/pull/212) | **Yes** |
| Enhancement | `Quantity` | Rename comparison methods | [#211](https://github.com/openhab/openhab-js/pull/211) | **Yes** |
| Type | Namespace | Description | Reference | Breaking |
|-------------|------------|---------------------------------------------------------------------------------------------------|--------------------------------------------------------|----------|
| Enhancement | `items` | Refactor `metadata` & `itemchannllink` APIs & add additional functionality | [#212](https://github.com/openhab/openhab-js/pull/212) | **Yes** |
| Enhancement | `Quantity` | Rename comparison methods | [#211](https://github.com/openhab/openhab-js/pull/211) | **Yes** |
| Bugfix | `triggers` | Remove arg type checks as they cause trouble & addon now logs stack on `IllegalArgumentException` | | No |

Also see the [Release Milestone](https://github.com/openhab/openhab-js/milestone/11).

Expand Down
3 changes: 0 additions & 3 deletions actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
/** @typedef {import('@js-joda/core').ZonedDateTime} time.ZonedDateTime */

const typeOfArguments = require('./typeOfArguments');
const osgi = require('./osgi');
// See https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/defaultscope/ScriptThingActionsImpl.java
const { actions } = require('@runtime/Defaults');
Expand Down Expand Up @@ -324,7 +323,6 @@ class Transformation {
* @returns {string} the transformed value or the original one, if there was no service registered for the given type or a transformation exception occurred
*/
static transform (type, fn, value) {
typeOfArguments([type, fn, value], ['string', 'string', 'string']);
return JavaTransformation.transform(type, fn, value).toString();
}

Expand All @@ -338,7 +336,6 @@ class Transformation {
* @throws Java {@link https://www.openhab.org/javadoc/latest/org/openhab/core/transform/TransformationException.html TransformationException}
*/
static transformRaw (type, fn, value) {
typeOfArguments([type, fn, value], ['string', 'string', 'string']);
// Wrap exception to enable JS stack traces
try {
return JavaTransformation.transformRaw(type, fn, value).toString();
Expand Down
14 changes: 0 additions & 14 deletions test/actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,6 @@ describe('actions.js', () => {
const value = 'ON';

describe('transform', () => {
it('throws TypeError when a wrong argument is passed.', () => {
expect(() => Transformation.transform({}, fn, value)).toThrow(TypeError);
expect(() => Transformation.transform(type, {}, value)).toThrow(TypeError);
expect(() => Transformation.transform(type, fn, { value })).toThrow(TypeError);
expect(JavaTransformation.transform).not.toHaveBeenCalled();
});

it('delegates to Java Transformation.', () => {
Transformation.transform(type, fn, value);

Expand All @@ -88,13 +81,6 @@ describe('actions.js', () => {
});

describe('transformRaw', () => {
it('throws TypeError when a wrong argument is passed.', () => {
expect(() => Transformation.transformRaw({}, fn, value)).toThrow(TypeError);
expect(() => Transformation.transformRaw(type, {}, value)).toThrow(TypeError);
expect(() => Transformation.transformRaw(type, fn, { value })).toThrow(TypeError);
expect(JavaTransformation.transformRaw).not.toHaveBeenCalled();
});

it('delegates to Java Transformation.', () => {
Transformation.transformRaw(type, fn, value);

Expand Down
10 changes: 6 additions & 4 deletions test/triggers.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,12 @@ describe('triggers.js', () => {
});

describe('DateTimeTrigger', () => {
it('creates trigger.', () => {
const itemName = 'itemName';
const timeOnly = true;
const triggerName = 'triggerName';
const itemName = 'itemName';
const timeOnly = true;
const triggerName = 'triggerName';
const item = new Item(itemName);

it.each([[itemName], [item]])('creates trigger from %s.', (groupOrName) => {
const trigger = DateTimeTrigger(itemName, timeOnly, triggerName);

expect(trigger).not.toBe(undefined);
Expand Down
131 changes: 53 additions & 78 deletions triggers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
* @namespace triggers
*/

const typeOfArguments = require('./typeOfArguments');
const utils = require('./utils');
/**
* @type {Item}
Expand Down Expand Up @@ -51,13 +50,11 @@ const _createTrigger = function (typeString, name, config) {
* @param {string} [triggerName] the optional name of the trigger to create
*
*/
const ChannelEventTrigger = (channel, event, triggerName) => {
typeOfArguments([channel, event, triggerName], ['string', 'string', 'string|undefined']);
return _createTrigger('core.ChannelEventTrigger', triggerName, {
const ChannelEventTrigger = (channel, event, triggerName) =>
_createTrigger('core.ChannelEventTrigger', triggerName, {
channelUID: channel,
event: event
});
};

/**
* Creates a trigger that fires upon an Item changing state.
Expand All @@ -74,14 +71,12 @@ const ChannelEventTrigger = (channel, event, triggerName) => {
* @param {string} [newState] the new state of the Item
* @param {string} [triggerName] the optional name of the trigger to create
*/
const ItemStateChangeTrigger = (itemOrName, oldState, newState, triggerName) => {
typeOfArguments([itemOrName, oldState, newState, triggerName], ['string|Item', 'string|undefined', 'string|undefined', 'string|undefined']);
return _createTrigger('core.ItemStateChangeTrigger', triggerName, {
const ItemStateChangeTrigger = (itemOrName, oldState, newState, triggerName) =>
_createTrigger('core.ItemStateChangeTrigger', triggerName, {
itemName: (itemOrName instanceof Item) ? itemOrName.name : itemOrName,
state: newState,
previousState: oldState
});
};

/**
* Creates a trigger that fires upon an Item receiving a state update. Note that the Item does not need to change state.
Expand All @@ -95,13 +90,11 @@ const ItemStateChangeTrigger = (itemOrName, oldState, newState, triggerName) =>
* @param {string} [state] the new state of the Item
* @param {string} [triggerName] the optional name of the trigger to create
*/
const ItemStateUpdateTrigger = (itemOrName, state, triggerName) => {
typeOfArguments([itemOrName, state, triggerName], ['string|Item', 'string|undefined', 'string|undefined']);
return _createTrigger('core.ItemStateUpdateTrigger', triggerName, {
const ItemStateUpdateTrigger = (itemOrName, state, triggerName) =>
_createTrigger('core.ItemStateUpdateTrigger', triggerName, {
itemName: (itemOrName instanceof Item) ? itemOrName.name : itemOrName,
state: state
});
};

/**
* Creates a trigger that fires upon an Item receiving a command. Note that the Item does not need to change state.
Expand All @@ -115,13 +108,11 @@ const ItemStateUpdateTrigger = (itemOrName, state, triggerName) => {
* @param {string} [command] the command received
* @param {string} [triggerName] the optional name of the trigger to create
*/
const ItemCommandTrigger = (itemOrName, command, triggerName) => {
typeOfArguments([itemOrName, command, triggerName], ['string|Item', 'string|undefined', 'string|undefined']);
return _createTrigger('core.ItemCommandTrigger', triggerName, {
const ItemCommandTrigger = (itemOrName, command, triggerName) =>
_createTrigger('core.ItemCommandTrigger', triggerName, {
itemName: (itemOrName instanceof Item) ? itemOrName.name : itemOrName,
command: command
});
};

/**
* Creates a trigger that fires upon a member of a group changing state. Note that group Item does not need to change state.
Expand All @@ -135,14 +126,12 @@ const ItemCommandTrigger = (itemOrName, command, triggerName) => {
* @param {string} [newState] the new state of the group
* @param {string} [triggerName] the optional name of the trigger to create
*/
const GroupStateChangeTrigger = (groupOrName, oldState, newState, triggerName) => {
typeOfArguments([groupOrName, oldState, newState, triggerName], ['string|Item', 'string|undefined', 'string|undefined', 'string|undefined']);
return _createTrigger('core.GroupStateChangeTrigger', triggerName, {
const GroupStateChangeTrigger = (groupOrName, oldState, newState, triggerName) =>
_createTrigger('core.GroupStateChangeTrigger', triggerName, {
groupName: (groupOrName instanceof Item) ? groupOrName.name : groupOrName,
state: newState,
previousState: oldState
});
};

/**
* Creates a trigger that fires upon a member of a group receiving a state update. Note that group Item does not need to change state.
Expand All @@ -155,13 +144,11 @@ const GroupStateChangeTrigger = (groupOrName, oldState, newState, triggerName) =
* @param {string} [state] the new state of the group
* @param {string} [triggerName] the optional name of the trigger to create
*/
const GroupStateUpdateTrigger = (groupOrName, state, triggerName) => {
typeOfArguments([groupOrName, state, triggerName], ['string|Item', 'string|undefined', 'string|undefined']);
return _createTrigger('core.GroupStateUpdateTrigger', triggerName, {
const GroupStateUpdateTrigger = (groupOrName, state, triggerName) =>
_createTrigger('core.GroupStateUpdateTrigger', triggerName, {
groupName: (groupOrName instanceof Item) ? groupOrName.name : groupOrName,
state: state
});
};

/**
* Creates a trigger that fires upon a member of a group receiving a command. Note that the group Item does not need to change state.
Expand All @@ -174,13 +161,11 @@ const GroupStateUpdateTrigger = (groupOrName, state, triggerName) => {
* @param {string} [command] the command received
* @param {string} [triggerName] the optional name of the trigger to create
*/
const GroupCommandTrigger = (groupOrName, command, triggerName) => {
typeOfArguments([groupOrName, command, triggerName], ['string|Item', 'string|undefined', 'string|undefined']);
return _createTrigger('core.GroupCommandTrigger', triggerName, {
const GroupCommandTrigger = (groupOrName, command, triggerName) =>
_createTrigger('core.GroupCommandTrigger', triggerName, {
groupName: (groupOrName instanceof Item) ? groupOrName.name : groupOrName,
command: command
});
};

/**
* Creates a trigger that fires upon a Thing status updating.
Expand All @@ -193,13 +178,11 @@ const GroupCommandTrigger = (groupOrName, command, triggerName) => {
* @param {string} [status] the optional status to monitor for
* @param {string} [triggerName] the optional name of the trigger to create
*/
const ThingStatusUpdateTrigger = (thingUID, status, triggerName) => {
typeOfArguments([thingUID, status, triggerName], ['string', 'string|undefined', 'string|undefined']);
return _createTrigger('core.ThingStatusUpdateTrigger', triggerName, {
const ThingStatusUpdateTrigger = (thingUID, status, triggerName) =>
_createTrigger('core.ThingStatusUpdateTrigger', triggerName, {
thingUID: thingUID,
status: status
});
};

/**
* Creates a trigger that fires upon a Thing status changing.
Expand All @@ -213,14 +196,12 @@ const ThingStatusUpdateTrigger = (thingUID, status, triggerName) => {
* @param {string} [previousStatus] the optional previous state to monitor from
* @param {string} [triggerName] the optional name of the trigger to create
*/
const ThingStatusChangeTrigger = (thingUID, status, previousStatus, triggerName) => {
typeOfArguments([thingUID, status, previousStatus, triggerName], ['string', 'string|undefined', 'string|undefined', 'string|undefined']);
return _createTrigger('core.ThingStatusChangeTrigger', triggerName, {
const ThingStatusChangeTrigger = (thingUID, status, previousStatus, triggerName) =>
_createTrigger('core.ThingStatusChangeTrigger', triggerName, {
thingUID: thingUID,
status: status,
previousStatus: previousStatus
});
};

/**
* Creates a trigger that fires if a given start level is reached by the system
Expand All @@ -236,12 +217,10 @@ const ThingStatusChangeTrigger = (thingUID, status, previousStatus, triggerName)
* @param {string|number} startlevel the system start level to be triggered on
* @param {string} [triggerName] the optional name of the trigger to create
*/
const SystemStartlevelTrigger = (startlevel, triggerName) => {
typeOfArguments([startlevel, triggerName], ['string|number', 'string|undefined']);
return _createTrigger('core.SystemStartlevelTrigger', triggerName, {
const SystemStartlevelTrigger = (startlevel, triggerName) =>
_createTrigger('core.SystemStartlevelTrigger', triggerName, {
startlevel: startlevel.toString()
});
};

/**
* Creates a trigger that fires on a cron schedule. The supplied cron expression defines when the trigger will fire.
Expand All @@ -253,12 +232,10 @@ const SystemStartlevelTrigger = (startlevel, triggerName) => {
* @param {string} expression the cron expression defining the triggering schedule
* @param {string} [triggerName] the optional name of the trigger to create
*/
const GenericCronTrigger = (expression, triggerName) => {
typeOfArguments([expression, triggerName], ['string', 'string|undefined']);
return _createTrigger('timer.GenericCronTrigger', triggerName, {
const GenericCronTrigger = (expression, triggerName) =>
_createTrigger('timer.GenericCronTrigger', triggerName, {
cronExpression: expression
});
};

/**
* Creates a trigger that fires daily at a specific time. The supplied time defines when the trigger will fire.
Expand All @@ -270,12 +247,10 @@ const GenericCronTrigger = (expression, triggerName) => {
* @param {string} time the time expression defining the triggering schedule
* @param {string} [triggerName] the optional name of the trigger to create
*/
const TimeOfDayTrigger = (time, triggerName) => {
typeOfArguments([time, triggerName], ['string', 'string|undefined']);
return _createTrigger('timer.TimeOfDayTrigger', triggerName, {
const TimeOfDayTrigger = (time, triggerName) =>
_createTrigger('timer.TimeOfDayTrigger', triggerName, {
time: time
});
};

/**
* Creates a trigger that fires at a (optional) date and time specified in an DateTime Item.
Expand All @@ -284,17 +259,15 @@ const TimeOfDayTrigger = (time, triggerName) => {
* DateTimeTrigger('MyDateTimeItem');
*
* @memberof triggers
* @param {string} itemName the name of the DateTime Item
* @param {Item|string} itemOrName the {@link Item} or the name of the Item to monitor for change
* @param {boolean} [timeOnly=false] Specifies whether only the time of the Item should be compared or the date and time.
* @param {string} [triggerName] the optional name of the trigger to create
*/
const DateTimeTrigger = (itemName, timeOnly = false, triggerName) => {
typeOfArguments([itemName, timeOnly, triggerName], ['string', 'boolean|undefined', 'string|undefined']);
return _createTrigger('timer.DateTimeTrigger', triggerName, {
itemName: itemName,
const DateTimeTrigger = (itemOrName, timeOnly = false, triggerName) =>
_createTrigger('timer.DateTimeTrigger', triggerName, {
itemName: (itemOrName instanceof Item) ? itemOrName.name : itemOrName,
timeOnly: timeOnly
});
};

/**
* Creates a trigger for the {@link https://openhab.org/addons/automation/pwm/ Pulse Width Modulation (PWM) Automation} add-on.
Expand All @@ -318,13 +291,14 @@ const DateTimeTrigger = (itemName, timeOnly = false, triggerName) => {
* @param {number} [deadManSwitch] output will be switched off, when the duty cycle is not updated within this time (in ms)
* @param {string} [triggerName] the optional name of the trigger to create
*/
const PWMTrigger = (dutycycleItem, interval, minDutyCycle, maxDutyCycle, deadManSwitch, triggerName) => _createTrigger('pwm.trigger', triggerName, {
dutycycleItem: dutycycleItem,
interval: interval,
minDutyCycle: minDutyCycle,
maxDutyCycle: maxDutyCycle,
deadManSwitch: deadManSwitch
});
const PWMTrigger = (dutycycleItem, interval, minDutyCycle, maxDutyCycle, deadManSwitch, triggerName) =>
_createTrigger('pwm.trigger', triggerName, {
dutycycleItem: dutycycleItem,
interval: interval,
minDutyCycle: minDutyCycle,
maxDutyCycle: maxDutyCycle,
deadManSwitch: deadManSwitch
});

/**
* Creates a trigger for the {@link https://www.openhab.org/addons/automation/pidcontroller/ PID Controller Automation} add-on.
Expand Down Expand Up @@ -359,22 +333,23 @@ const PWMTrigger = (dutycycleItem, interval, minDutyCycle, maxDutyCycle, deadMan
* @param {string} [errorInspectorItem] name of the debug Item for the current regulation difference (error)
* @param {string} [triggerName] the optional name of the trigger to create
*/
const PIDTrigger = (inputItem, setpointItem, kp = 1, ki = 1, kd = 1, kdTimeConstant = 1, loopTime = 1000, commandItem, integralMinValue, integralMaxValue, pInspectorItem, iInspectorItem, dInspectorItem, errorInspectorItem, triggerName) => _createTrigger('pidcontroller.trigger', triggerName, {
input: inputItem,
setpoint: setpointItem,
kp: kp,
ki: ki,
kd: kd,
kdTimeConstant: kdTimeConstant,
loopTime: loopTime,
commandItem: commandItem,
integralMinValue: integralMinValue,
integralMaxValue: integralMaxValue,
pInspector: pInspectorItem,
iInspector: iInspectorItem,
dInspector: dInspectorItem,
eInspector: errorInspectorItem
});
const PIDTrigger = (inputItem, setpointItem, kp = 1, ki = 1, kd = 1, kdTimeConstant = 1, loopTime = 1000, commandItem, integralMinValue, integralMaxValue, pInspectorItem, iInspectorItem, dInspectorItem, errorInspectorItem, triggerName) =>
_createTrigger('pidcontroller.trigger', triggerName, {
input: inputItem,
setpoint: setpointItem,
kp: kp,
ki: ki,
kd: kd,
kdTimeConstant: kdTimeConstant,
loopTime: loopTime,
commandItem: commandItem,
integralMinValue: integralMinValue,
integralMaxValue: integralMaxValue,
pInspector: pInspectorItem,
iInspector: iInspectorItem,
dInspector: dInspectorItem,
eInspector: errorInspectorItem
});

/* not yet tested
ItemStateCondition: (itemName, state, triggerName) => createTrigger("core.ItemStateCondition", triggerName, {
Expand Down
Loading

0 comments on commit 9975507

Please sign in to comment.