I am writing my first groovy driver and need some help with implementing a mutex style lock.
The driver communicates with a Carrier thermostat using the telnet protocol. It works strictly in half duplex, ie you must not send a new request until you have received a response from the last request or the timeout has been exceeded. I think that means I need some sort of mutex lock on the sending method to stop it running in parallel threads.
It looks like I should be using "synchronized" but I'm not sure how to code that correctly. Can someone help me with a code snippet that I can wrap around my sendCommand() method?
I might start with a simpler approach: have you considered using state to store a boolean indicating whether you're waiting? Perhaps in combination with singleThreaded: true in your definition to eliminate concerns for simultaneous execution? (State is a built in map available to each driver and app usable to store data between executions. Keys can be created simply by using them, e.g., state.isLocked = true or whatever you want.)
I did try using state to store the thermostat response from parse() but that did not update reliably and certainly not while the sending command was waiting for the response in a while loop. I now send that response from parse() back to the send method in an attribute. That works but read (device.currentValue) and the write (sendEvent) are not atomic operations so do not guarantee mutual exclusion. A thread could sneak in between the read and the write.
I also tried the singleThreaded option in the definition but that means parse() will not fire until the send method has ended. Consequently I can't keep the send method alive (using pauseExecution) to check for a timeout. - it will always timeout!
What I used (on another platform, but same issue) is keep a queue of pending requests. You push each request on the queue and then process them until the queue is empty. Pushing a new can then be async, but keep the comms in sync. This with single tread should work for this scenario I think.
@MrFarmer : Many thanks for the suggestion. I am looking at synchronized because singleThreaded is too coarse and will stall the driver, let me explain.
At the moment my transmit method sends the command and then waits in a pauseExecution(1000) loop for the "SUCCESS" response to be passed back to it from parse() in an attribute. If it doesn't get a response after 5 seconds it assumes the thermostat has timed out and resends the command.
If I make this driver singleThreaded then parse() is never executed, I suspect this is because the transmit method is still alive in the pause loop and the single thread cannot invoke parse() until the transmit method has terminated. For singleThreaded to work it needs to allow parse() to be called in a separate thread from the transmit method - or allow me to periodically call parse directly.
Personally, I try to avoid blocking calls like "pauseExecution" as much as possible. Why not just send the command to the thermostat. Then, instead of 'waiting', just schedule a routine to run in 1000ms. If the parse() returns first, have it cancel the scheduled routine. If the scheduled routine runs, use the parameters you can pass to it to reattempt the send command. Obviously, build in a mechanism to make sure it does not do this forever.
@ogiewon : I use pauseExecution() to block the transmit method for 2 to 5 seconds and hopefully not use CPU time. Then when parse() gets the response from the thermostat, the transmit method is released. It error checks the response, resends if needed and updates the attributes . I guess I could use runIn and pass all the error checking data to it,
One reason: pauseExecution() doesn't cause the driver to "sleep" in the sense that state gets written. A runIn() will. Coupled with singleThreaded, I think this would make my idea above work better than it would with pauseExecution() (in fact, if you're coupling pauseExecution() with singleThreaded: true, your parse() actually wouldn't get a chance to run to let you know anything during that time -- your pause counts as running). That being said, @ogiewon's idea is probably even cleaner. Not being the one making this driver, there may some things I haven't considered in either case.
You can test the behavior of pauseExecution() vs. runIn() with a simple driver like this, BTW. Here's what I'd try:
run printStateFoo() to print state.foo (should be null to start)
run methodWithPause(), which "immediately" sets state.foo to "bar1" -- but you won't see that until after the 5-second pauseExecution() finishes, which you can test by running printStateFoo() during that pause and after
run methodWithRunIn(), which immediately sets state.foo to "bar2" (which you'll probabably never actually see), schedules a job for 5 seconds in the future with runIn(), sets state.foo to "bar3" after that call (the value you'll indeed see if you run printStateFoo() during this delay), and then eventually the scheduled job will set it to "bar4" (which you'll see if you run printStateFoo() afterwards).
@bertabcd1234 : Let me expand a little on the above. I modified your methodWithPause with code and comments to explain what I am doing with pauseExecution.
void methodWithPause() {
log.trace "--- methodWithPause() ---"
log.debug "Before pause"
retries = 0
while(retries <=2) {
//Transmit command
//Update attributeValue to "SENT"
pauseExecution(2000) // Most responses come within 2 seconds
timeout = 0
while(timeout <=3) { // If no valid response after a total of 5 seconds we will need to resend
// Get attribute value
if (attributeValue != "SENT") break //Got a response from parse
timeout++
pauseExecution(1000)
}
//Check for valid response, return if we have one
retries++
}
//Will only get here if three retries did not get a valid response
}
To get runIn working instead of using pauseExecution and while loops I presume I would have to have different runIn handlers for seconds 2 to 4 (to check for response), another for second 5 (to resend the command) and another for after 3 retries (to flag an error).
Not necessarily. I didn't do it above, but you can pass a data map to your handler method that contains whatever information you want, including what retry it is--though separate methods would also work if that's easier. See the runIn() signature: Common Methods | Hubitat Documentation. It seems like you could also just store this in information in state.
The version above with pauseExecution() will never work with singleThreaded: true as far as I can see because I don't think the event will be processed until all the pauses are done. It may work better than state for this purpose without single threading because state won't get written out until it is finished executing, while the event will likely be committed sooner, but I'm not sure an event really makes sense for this kind of data (really only things a user might want to automate).
Thought about this a lot last night and I reckon I can use runIn with multiple recursive calls. That would allow me to use singleThreaded and not block parse but I don't think that would solve the original mutex problem.
If the transmitting method ends then any other "top level" command can now start and send data out on the telnet interface before the runIn handlers have completed. That is not allowed in the Carrier protocol. Somehow I need to block the transmit method until the runIn handlers have finished.
Bump. Can anyone provide guidance on how to implement a mutex lock across different threads in Hubitat. It looks like I should be using "synchronized" but I'm not sure how to code that correctly.
