Issue of the topic:
I have some rooms with 2 Sonoff Touch Light Switches but only one Light. When I switch one of the Switches, I want the other one to be in the same state. Therefore I wrote these rules:
import java.util.concurrent.locks.ReentrantLock
val ReentrantLock lock_AZL = new ReentrantLock()
rule "Arbeitszimmerlicht ON"
when
Item SonoffTS02 changed to ON or Item SonoffTS14 changed to ON
then
if(!lock_AZL.isLocked && Sonoff_STOP.state != ON){
lock_AZL.lock
try {
sendCommand(Sonoff_LOOP,Sonoff_LOOP.state as Number + 1 )
sendCommand(SonoffTS02,ON)
sendCommand(SonoffTS14,ON)
Thread::sleep(1000)
} finally {
lock_AZL.unlock
}
}
end
rule "Arbeitszimmerlicht OFF"
when
Item SonoffTS02 changed to OFF or Item SonoffTS14 changed to OFF
then
if(!lock_GZL.isLocked && Sonoff_STOP.state != ON){
lock_AZL.lock
try {
sendCommand(Sonoff_LOOP,Sonoff_LOOP.state as Number + 1 )
sendCommand(SonoffTS02,OFF)
sendCommand(SonoffTS14,OFF)
Thread::sleep(1000)
} finally {
lock_AZL.unlock
}
}
end
in the Logfile it looks like the rules execute 2 times even though they should not:
2018-11-09 17:42:46.000 [vent.ItemStateChangedEvent] - SonoffTS14_Verbose changed from {"POWER":"ON"} to {"POWER":"OFF"}
2018-11-09 17:42:46.060 [vent.ItemStateChangedEvent] - SonoffTS14 changed from ON to OFF
2018-11-09 17:42:46.087 [ome.event.ItemCommandEvent] - Item 'Sonoff_LOOP' received command 1
2018-11-09 17:42:46.100 [ome.event.ItemCommandEvent] - Item 'SonoffTS02' received command OFF
2018-11-09 17:42:46.110 [vent.ItemStateChangedEvent] - Sonoff_LOOP changed from 0 to 1
2018-11-09 17:42:46.114 [ome.event.ItemCommandEvent] - Item 'SonoffTS14' received command OFF
2018-11-09 17:42:46.121 [vent.ItemStateChangedEvent] - SonoffTS02 changed from ON to OFF
2018-11-09 17:42:46.243 [vent.ItemStateChangedEvent] - SonoffTS02_Verbose changed from {"POWER":"ON"} to {"POWER":"OFF"}
2018-11-09 17:42:47.127 [ome.event.ItemCommandEvent] - Item 'Sonoff_LOOP' received command 2
2018-11-09 17:42:47.146 [ome.event.ItemCommandEvent] - Item 'SonoffTS02' received command OFF
2018-11-09 17:42:47.152 [ome.event.ItemCommandEvent] - Item 'SonoffTS14' received command OFF
2018-11-09 17:42:47.160 [vent.ItemStateChangedEvent] - Sonoff_LOOP changed from 1 to 2
I already implemented the Sonoff_LOOP and Sonoff_STOP items to react to and stop loops.
rule "Toggle light"
when
Item SonoffTS02 changed or
Item SonoffTS14 changed
then
if (Light.state == OFF) Light.sendCommand(ON)
else Light.sendCommand(OFF)
end
This way you don’t have to worry about synchronised switch states. Each switch when actionned either way (ON or OFF) will toggle the light
I think the OP may have his wall switches in parallel, so there is no separate “Light” Item.
I think it can be simplified though
rule "Arbeitszimmerlicht ON"
when
Item SonoffTS02 changed to ON or
Item SonoffTS14 changed to ON
then
if (SonoffTS02.state != ON) { SonoffTS02.sendCommand(ON) }
if (SonoffTS14.state != ON) { SonoffTS14.sendCommand(ON) }
end
I have one switch hardwired to the light and the other one actually controlls the hardwired switch.
I also have 3 other people in my household who tend to press the lightswitches repeatedly which resulted in multiple endless-loops. Therefore I implemented the Loop-Counter (Sonoff_LOOP) and the STOP-Switch (Sonoff_STOP).
But they should not be necessary because the ReentrantLock should prevent the rule from firing twice (or three times in case of one room where 3 switches controll the same light)
I don’t see where you are actually setting your stop-switch
also the logic if(!lock_GZL.isLocked && Sonoff_STOP.state != ON) will fail if you never got a lock; and by fail I mean execute multiple times
The latter issue, simply not getting a lock in time, perplexed me quite a bit some a year or two ago, here is how I am structuring my rules
rule "MyRule"
when
// trule triggers go here
then
var gotLock = lock.tryLock(1000, TimeUnit.MILLISECONDS)
try {
if (gotLock) {
// do stuff here
}
else logInfo("MyRule", "re-entry lock activated")
}
finally {
if(gotLock){
lock.unlock()
}
}
end
here are some additional considerations/explanations:
if it works you can probably get rid of your thread::sleep statements
this line: var gotLock = lock.tryLock(1000, TimeUnit.MILLISECONDS) makes the system essentially behave a little nicer (the value 1000 does not seem to matter, 0 will do too, see post linked above)
doing so you will simply drop events, but I guess that is exactly what you want; just stating this for clarity
now to the real problem:
– in certain conditions your rule may die while in the try{} block and never execute the finally {} statement releasing the lock;
– when this happens, your rule will never execute the try{} block again as the lock will still be set;
– where this is becoming a real issue in my case is during start up where a rule may execute before items are defined or even the rule parser is fully up and running and I am seeing syntax errors that are not real syntax errors but some artefact during system start and Null pointer exceptions; when this happens the rule will abort and the lock will not be released.
– if you apply one of the many tricks that will run your rules only after everything else started up well (e.g, “preferred version” from here: Cleaning up the startup process / renaming rules (windows possible)), it will however work like a charm.
– Finally, be forewarned already in case you are reading the JAVA docs on concurrency guards, this whole mechanism of concurrency guard is implemented only implemented to a limited extent in the DSL; while JAVA offers here error catching and handling, this will not work well in DSL and not at all for NPE or syntax errors; (hence it is not implemented above); I tried… and others on the forum too, it is not worth the efforts as only very few errors are caught; you will need to take extra care in coding