Odd behavior using ReentrantLock

  • Platform information:

    • Hardware: RPi3
    • OS: openhabian
    • Java Runtime Environment: 1.8.0_152
    • openHAB version: openHAB 2.3.0-1 Release Build
  • 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.

What am I doing wrong?

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

1 Like

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

etc.
Who cares if it runs twice :slight_smile:

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)

Not sure if this helps, but here you go:

  • 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
  • see here for for some more details: Design Pattern: Rule Latching
  • 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
3 Likes

HAH! Thank you! That worked!
Didn’t think of searching for “rule latching” :slight_smile: