See How to write a rule template. Once the code works you’ve done the hard part. The rest is really mostly just formatting and tedium.
I’ve been pushing here and there to make the experience better but it’s slow going.
At a high level, make the rule a managed rule. Add the “configDescriptions” section for all the parameters you want the end user to be able to set. See [Do not install] Rule Template Parameters Experiments and Examples for how to define parameters of various types (e.g. an Item, Thing, Channel, boolean, etc.). In the actual rule, you’ll add {{ param }}
where “param” is the name of the parameter. At instantiation time those will be replaced with what the user selected/entered in the config.
All this checking is probably unnecessary. The openhab
library comes with the add-on. If this code is going to be able to run at all in the first place, openhab
is going to be there. So this can all safely be reduced to
const { items, rules, log, triggers, actions } = require('openhab');
As mentioned before, when/if this becomes a template, these will be populated from the parameters.
Note, a trick that has served me well in my rule templates is to assign all the parameters to constants/variables at the top of the scripts. That way you can update the template easily by just replacing everything below the definition of the variables without needing to remember where something needs to be replaced with a {{ param }}
.
When moving to a rule template you’d want to store these in the cache. If you manage to merge all the rules into one rule you can use the private cache. If multiple rules you’ll want to use the shared cache.
If you use the cache, you should use openHAB timers instead of JS timeouts as then when the rule gets reloaded, the existing timers will be cancelled automatically for you instead of becoming orphaned and continuing to run in the background.
I always get a little weary when I see locks in code like this. I’m not saying it’s not appropriate here but it does raise flags.
It might be better to utilize a ReentrantLock but I’m not sure that’s possible in JS Scripting given it’s single threaded access requirements.
Because a failure to release the lock can have devastating impacts on these rules, you should always make sure the lock is released by using try/catch/finally and release the lock in the finally. NOTE: I don’t know if this is true for JS Scripting but in Rules DSL, not all errors that occur are catchable meaning even then you might end up with a lock that is forever locked and can never be unlocked until a restart.
In general, it’s better to rely on the fact that only one instance of a given rule can run at a time. That will mean all this locking stuff will come to you for free. But that requires merging the rules into one rule.
The openhab_rules_tools library has a built in countdown timer. That can free up a lot of code here. openHAB Rules Tools Announcements OHRT is available through openHABian or via npm
.
I think all of this logic could be managed through Debounce [4.0.0.0;4.9.9.9] and a Group. You’d debounce the motion sensors and have them a member of a Group with a count aggregation function. You can check if Group’s state > 1 to trigger the alarm.
Be careful here. If you have a Thing that never comes online your system start levels will stall out at level 60. You wouldn’t want your alarm system to be disabled just because a Light is broken.
When pondering whether to combine these rules into one rule, you can distinguish between the various events. For example, my debounce rule template:
if(this.event === undefined) {
init();
}
else {
switch(event.type) {
case 'GroupItemStateChangedEvent':
case 'ItemStateEvent':
case 'ItemStateChangedEvent':
case 'ItemCommandEvent':
debounce();
break;
default:
init();
}
}
In that case if it’s an Item event I run the rule. If it’s not an item event, I just validate the configuration of the Items and the rule.
This should never happen. The rule should only be triggered when it’s newly loaded and the start level has passed. If the rule is newly loaded it means there is nothing left over from the previous rule. If by chance alarmSoundTimeoutId
isn’t null
it means that one of your other rules somehow manged to run before this system started rule and that means your whole situation just got a lot more complicated.
If might be a better approach to have a rule trigger at startlevel 40 that disables the rules that should run and then only after this rule runs reenable them. That would prevent the situation sited above in almost all cases (not all). There’s a rule template for that too. Delay Start [4.0.0.0;4.9.9.9]
In a managed rule, it’s not running in a function so return
doesn’t work. You’d have to use process.exit(1)
.
The state of an Item will never be null
and never be undefined
. They will be NULL
or UNDEF
. They are not the same. This is a bug. Use
if(triggeringItem.isUnintialized || alarmStateItem.isUninitialized)
Over all I see a good deal of duplicated code. Consider applying Design Pattern: How to Structure a Rule so some of these to avoid a lot. For example, when you have a case where you have a lot of sending of broadcasts, you should only have one line that actually sends the broadcast. The rest of the code just creates the message.
I bring up the rule templates and library not to encourage you to use them (though that would be good too) but as another example of a way to achieve the same goal.