Closed garage door opener opening when told to close !

[jdtsmith]

I recently implemented an MQTT garage door opener using custom apply (#225). I have been surprised to discover my garage door open in the morning the last few days. I tracked this down to my "Good Night" scene which turns off many things and closes the garage door. I discovered that in (some, mostly complex) scenes, when the garage is already closed and the scene specifies closing it, Homekit will in fact call SetTargetDoorState and "close again" (aka open!) it, despite knowing that it was closed. Then I discovered that even Siri, when asked to close, will open a closed garage! I had thought preventing this was a basic feature of Homekit. I never had this behavior in my old plugin based on GPIO's.

I looked closely at that plugin code and discovered a bit of defensive programming that I bet explains it:

// If the target state is equal to current state, do nothing.
if (asDoorState(state) == curState) {
    callback();
    return;
}

So I'd urge including something like this to deal with a serious security issue. I.e., if the door state is already at what is requested, just leave it there.

I tried out a heavy-handed fix at this line and it fixed this security issue temporarily, but obviously you would need to apply to all accessories with get/set, and it may cause confusion if people had been relying on this (wrong, IMO) behavior. Here's my cheap fix:

            if (!state.hasOwnProperty(property) || state[property]!=value) {
            state[property] = value;
            let mqttVal = values[value];
            if (mqttVal !== undefined) {
                            mqttPublish(setTopic, property, mqttVal);
            }
            raiseEvent( property );
            }

[jdtsmith]

OK, I've altered my apply functions to use the new state variable and to explicitly prevent "closing" an already closed door:

    {
      "accessory": "mqttthing",
      "type": "garageDoorOpener",
      "name": "ShellyGarage",
      "logMqtt": true,
      "topics": {
    "setTargetDoorState": {
      "topic": "shellies/shelly-garage/relay/0/command",
      "apply": "if(state.global.status!=message) {state.global.isMoving=message; setTimeout(()=>{state.global.isMoving=false},13e3); return 'on'} else {return null}"
    },
    "getTargetDoorState": {
      "topic": "shellies/shelly-garage/input/0",
      "apply": "if (state.global.isMoving) {return state.global.isMoving} else {return message==1?'C':'O'}"
    },    
    "getCurrentDoorState": {
      "topic": "shellies/shelly-garage/input/0",
      "apply": "closed=message==1; state.global.status=closed?'C':'O'; if(closed) return 'C'; if (state.global.isMoving) return state.global.isMoving.toLowerCase(); return 'O';"
    }
      }
    },

For any device which can't be specifically commanded to alter state, but instead just toggles, such a protection should be included.

[arachnetech]

Thanks, @jdtsmith. I think it's better for codecs/apply functions to trap this behaviour as yours now does than to try to do it centrally in mqttthing. Mqttthing is already passing on Homekit's intent - to open or close. There may be a situation where you want to send a close to a door which Homekit thinks is already closed (e.g. after a restart given that Mqttthing doesn't store the state).

[jdtsmith]

Yeah it occurred to me that sometimes this behavior might be useful somewhere. I was actually surprised Homekit itself doesn't trap and squelch these. The real issue is devices that have only toggle-based control. It's no harm to hit the close button if something is already closed, if all that button does is close (I have a screen like this)! Kind of a limitation of dumb garage doors. BTW, I used state to good effect. It wasn't entirely clear why global wouldn't be the default state object. It's not global in the sense of going between accessories is it? Just topics within one accessory?

[jdtsmith]

One other small point: undefined and null are separately used for apply(/codec's?) to signal to MQTTThing not to proceed with this value in any way, for get vs. set topics. Is that intentional? Would using one value make more sense? Should probably be documented either way what to return to "cancel" a given update/set.

[arachnetech]

Yes, I think we should use undefined. I’ll leave apply functions supporting null too for compatibility.

I must do a release - it’s been a busy few days at work...

[uspino2]

@jdtsmith your config works great for a single reed setting. But I can't seem to find a way to make use of my second Shelly, used as a sensor to detect "fully open" through magnetic reed. I'd add one thing to the rationale I mentioned yesterday:

• When sensor 1 is closed and sensor 2 is open, opening sensor 1 sends HomeKit opening until sensor 2 is closed.
• When sensor 2 is closed and sensor 1 is open, opening sensor 2 sends HomeKit closing until sensor 1 is closed.

And that would be:

• When both sensors are open for X number of seconds (X being a little over the opening/closing time), door is stopped.

That would give us pretty much a perfect, solid system with just two Shelly 1 ($20 total)!

Any help would be much appreciated.

[jdtsmith]

One issue is that I believe MQTTThing only expects one topic to report on door state, not two (as you'd have with 2 Shelley's). It would be ugly, but you could use the other sensor's topic for an unused item like getLockCurrentState, and in that one's apply, only set an appropriate global variable (like state.global.isOpened=true) and just return '?' each time. Then in getTargetDoorState (on your other sensor) set state.global.isClosed and apply your logic above. There would be an asymmetry in initiating external opens/closes then though.

Or you could ask @arachnetech to add the possibility for multiple topics to apply for a single item like getTargetDoorState. I.e. so you could specify an array of topic strings, or an array of {topic:...,apply:...} type things, which would mean "any of these topics can report on this".

[uspino2]

After lots of trials I ended up using iomax implementation. It's very smart and simple.

  "topics": {
                "statusSet":    "MQTT TOPIC TO SET THE DOOR OPENER",
                "openGet":      "OPTIONAL: MQTT TOPIC TO GET THE DOOR OPEN STATUS",
                "openValue":    "OPTIONAL VALUE THAT MEANS OPEN (DEFAULT true)",
                "closedGet":    "OPTIONAL: MQTT TOPIC TO GET THE DOOR CLOSED STATUS",
                "closedValue":  "OPTIONAL VALUE THAT MEANS CLOSED (DEFAULT true)",
                "openStatusCmdTopic": "OPTIONAL: MQTT TOPIC TO ASK ABOUT THE OPEN STATUS",
                "openStatusCmd": "OPTIONAL: THE OPEN STATUS COMMAND ( DEFAULT "")",
                "closeStatusCmdTopic": "OPTIONAL: MQTT TOPIC TO ASK ABOUT THE CLOSED STATUS",
                "closeStatusCmd": "OPTIONAL THE CLOSED STATUS COMMAND (DEFAULT "")",
            },

In my case, just like this:

        {
            "accessory": "mqttgaragedoor",
            "name": "MQTT",
            "topics": {
                "statusSet": "shellies/shelly1-garage/relay/0/command",
                "closedGet": "shellies/shelly1-garage/input/0",
                "closedValue": "1",
                "openGet": "shellies/shelly2-garage/input/0",
                "openValue": "1"
            },
            "doorRunInSeconds": "10"
        },

It links two different topics to two different sensors (two different Shellies in my case) and it works exactly the way it is supposed to work:

• When reed in sensor 1 (fully closed) opens, Gate goes to the opening state until reed in sensor 2 (fully open) gets closed, and only then Gate appears as open.
• When reed in sensor 2 (fully open) opens, Gate goes to the closing state until reed in sensor 1 (fully closed) gets closed, and only then Gate appears as closed.

On top of that, he implemented a door opening time, so you if your gate takes let's say 22 seconds to open or close, the plug in will show stop (obstruction) when one of the two sensors is not activated in that amount of time. As I said, solid, simple and smart. Also you can specify the value of the sensor topic so you won't need extra mqtt rules.

I wonder if @arachnetech can take a look at this sort of implementation in mqttthings, which is definitely a better overall solution.

[jdtsmith]

It’s a good solution. You could emulate almost all of that with an apply (or the new “codec” system), except at the end of a timeout you can’t perform a status update.

[arachnetech]

Codecs do allow timer-driven updates, but currently you have to hang on to the 'delivery' function. I'll fix this gap before I document them, hopefully very soon.

[ceraz68]

OK, I've altered my apply functions to use the new state variable and to explicitly prevent "closing" an already closed door:

    {
      "accessory": "mqttthing",
      "type": "garageDoorOpener",
      "name": "ShellyGarage",
      "logMqtt": true,
      "topics": {
  "setTargetDoorState": {
    "topic": "shellies/shelly-garage/relay/0/command",
    "apply": "if(state.global.status!=message) {state.global.isMoving=message; setTimeout(()=>{state.global.isMoving=false},13e3); return 'on'} else {return null}"
  },
  "getTargetDoorState": {
    "topic": "shellies/shelly-garage/input/0",
    "apply": "if (state.global.isMoving) {return state.global.isMoving} else {return message==1?'C':'O'}"
  },    
  "getCurrentDoorState": {
    "topic": "shellies/shelly-garage/input/0",
    "apply": "closed=message==1; state.global.status=closed?'C':'O'; if(closed) return 'C'; if (state.global.isMoving) return state.global.isMoving.toLowerCase(); return 'O';"
  }
      }
    },

For any device which can't be specifically commanded to alter state, but instead just toggles, such a protection should be included.

Hi jdtsmith , I really like your post as I came across the same problem. Asking Siri to close an already closed door actually opens it. It makes sense to beef up the logic in mqttthing rather than Siri shortcuts or tasmota rules.

I have a single reed switch reporting on topic 'tele/svgaragerelay/SENSOR' as is connected to a sonoff SV running Tasmota. The sonoff SV on-board relay acts a dry contact for the motor when 'ON' is published to 'cmnd/svgaragerelay/POWER'. My reed switch is closed when the door is really closed and reports 'ON' via Switch2 of the sonoff (e.g. tele/svgaragerelay/SENSOR = {"Time":"2020-09-19T15:44:41","Switch2":"ON"}) otherwise it reports 'OFF'. The sonoff relay dry contact is time limited to emulate a push button, so like your case it doesn't know if the door is 'opening', 'closing' or 'stopped'.

Your use of the apply function is above my level, would you be explain to a novice where the global.status variable is declared, maybe walk through your apply commands in plain English?

Thanks a lot ;-)

My setup is below and would like to make it more robust: "topics": { "getCurrentDoorState": { "topic": "tele/svgaragerelay/SENSOR", "apply": "return JSON.parse(message).Switch2;" }, "getTargetDoorState": { "topic": "tele/svgaragerelay/SENSOR", "apply": "return JSON.parse(message).Switch2;" }, "setTargetDoorState": "cmnd/svgaragerelay/POWER" }, "doorCurrentValues": ["OFF", "ON", "Opening", "Closing", "Stopped"], "doorTargetValues": ["ON", "ON"],

[jdtsmith]

Your use of the apply function is above my level, would you be explain to a novice where the global.status variable is declared, maybe walk through your apply commands in plain English?

state is passed into your code block by MQTTThing, just like message. state.global is preserved between all calls and among different topics, so I use that. What my apply code above does:

"setTargetDoorState": {
      "topic": "shellies/shelly-garage/relay/0/command",
      "apply": "if(state.global.status!=message) {state.global.isMoving=message; setTimeout(()=>{state.global.isMoving=false},13e3); return 'on'} else {return null}"
    },

This code is the callback that happens when setting the door state externally (like from Siri). It does the following:

1. Checks if the new desired state (closed or open) actually differs from the current status. Returns null if not (to stop the command in its tracks). This is what prevents "closing again opens" misbehavior.
2. If it is indeed a new state, sets a global isMoving flag to that state ('C' or 'O') so we know we are moving that way,
3. Sets a timer running (for 13 seconds in my case, about how long it takes my door to move).
4. After the timer completes, the global isMoving flag is turned back off.
5. If we are all set for the move, return the MQTT command that makes the shelly move (here 'on', aka turn on the shelly; note that I used the Shelly 1's interface to configure it to "turn off after 1 second" so "on" acts like a temporary button press).

    "getTargetDoorState": {
      "topic": "shellies/shelly-garage/input/0",
      "apply": "if (state.global.isMoving) {return state.global.isMoving} else {return message==1?'C':'O'}"
    },

This is my reed switch input reporting. It is wired to trip when the door is fully closed, reporting from the Shelly via MQTT (with a "1" when the magnetic switch closes). There's no way for the garage opener device to remember and respond as to what its intended target state is. So if we are still within the 13sec timer, just report that our target state is what we started that timer moving towards ('C' for closed and 'O' for open, saved above in isMoving). If we are no longer moving, report that our target door state is what we now know the door's new actual state to be (from the reed switch). BTW, Home app gets very confused if target state and current state are inconsistent, so this is important.

    "getCurrentDoorState": {
      "topic": "shellies/shelly-garage/input/0",
      "apply": "closed=message==1; state.global.status=closed?'C':'O'; if(closed) return 'C'; if (state.global.isMoving) return state.global.isMoving.toLowerCase(); return 'O';"
    }

This callback also runs for the exact same MQTT topic (sensor switch changing state on the Shelly). It first checks if the switch indicates closed or open, and then saves that status as a global variable (status='C' or 'O'; to answer your question this is where state.global.status is effectively "declared"). If the sensor switch knows the door is closed, that's definitive: return 'C'. If the reed switch isn't tripped, and we know we are still moving, we could be opening or closing. Luckily we are keeping track of that in the isMoving global flag, so if that's set, just return the lowercase 'c' or 'o' to indicate we are closing or opening. Otherwise, we'll assume we are open and return 'O'.

Obviously this could all go wrong if you interrupt the garage and turn it around using your car remote for example. But even then, once it trips the reed switch that will definitely set it closed (and it will tell Home that's what it's target was too, so everything will be consistent).

HTH

[ceraz68]

state is passed into your code block by MQTTThing, just like message. state.global is preserved between all calls and among different topics, so I use that. What my apply code above does:

Thanks a lot for this. I've now understood the state.global in the wiki!

I'll study your code .... two things are putting me off: (1) in the setTargetDoorState, state.global.status is compared to the Mqtt message which contains on (or off after 1s), whereas state.global.status is set to 'C' or 'O'; (2) state.global.isMoving is treated as a boolean in getTargetDoorState whereas it gets a value in setTargetState.

Cheers

[jdtsmith]

I'll study your code .... two things are putting me off: (1) in the setTargetDoorState, state.global.status is compared to the Mqtt message which contains on (or off after 1s), whereas state.global.status is set to 'C' or 'O'; (2) state.global.isMoving is treated as a boolean in getTargetDoorState whereas it gets a value in setTargetState.

1. In setTargetDoorState, the message is coming from Homekit via MQTTThing, i.e. telling us to (O)pen or (C)lose. We compare that to our status, which is also 'O' or 'C', as set in getCurrentDoorState, and only move if needed.
2. Both 'C' and 'O' are true according to JS, and when treated as a Boolean, just mean "moving in some direction". (BTW a good trick is to open your browser's JS console to check stuff like that). We need to save which direction we're moving so we can report it later as getTargetDoorState. We also have to remember that the door might be closed/opened out from under us using the button in the garage, such that setTargetDoorState will never have a chance to save isMoving. Home gets very unhappy if target and actual door state disagree, so we must ensure they do. Since manual open/close will always trip the reed switch once, we can track those pretty well too (without any opening/closing timeout).

[ceraz68]

Fantastic, thanks for that, the penny dropped :-)

Very smart and neat well done!

I started integrating the without the isMoving parameter. It revealed a funny case as the garagedoor takes longer to open/close than the period of the telemetry associated to the reed switch. So I had a case from open to closed state whereby the HomeApp indicated "closing", the garagedoor hadn't yet closed but an mqtt from the reed arrived (claiming it's open), then HK indicated "Open", then "Closed" (mqtt message saying the reed is closed). So some fine tuning for me.

I think I'll install a second reed to confirm the open state or maybe a smartly placed movement sensor.

Hard to think of the use-case and real advantage, I was thinking of HK vs. manual operations: what about replacing 'if(closed) return 'C';' by 'if(closed) state.global.isMoving=false; return 'C'}; in the getCurrentDoorState.

Thanks for sharing and explaining, so again well done!

[jdtsmith]

Fantastic, thanks for that, the penny dropped :-)

Very smart and neat well done!

I started integrating the without the isMoving parameter. It revealed a funny case as the garagedoor takes longer to open/close than the period of the telemetry associated to the reed switch. So I had a case from open to closed state whereby the HomeApp indicated "closing", the garagedoor hadn't yet closed but an mqtt from the reed arrived (claiming it's open), then HK indicated "Open", then "Closed" (mqtt message saying the reed is closed). So some fine tuning for me.

I think I'll install a second reed to confirm the open state or maybe a smartly placed movement sensor.

Hard to think of the use-case and real advantage, I was thinking of HK vs. manual operations: what about replacing 'if(closed) return 'C';' by 'if(closed) state.global.isMoving=false; return 'C'}; in the getCurrentDoorState.

Are both to those supposed to be in the if? Right now only the state setting is.

With both inside, this would be fine as well, though is_moving times out on its own soon. If you have two reed switches it might make sense to do this (for both switches going to 1, since you know you're not moving anymore).

[ceraz68]

Indeed missing bracket in if(closed) {state.global.isMoving=false; return 'C'}

I decided to work on removing uncertainty so just installed a second reed switch to confirm the door is open and searching the best solution. For the moment I'm preferring the rules offered in tasmota to simplify the code. If I were to do it in Mqttthing I'd have to store the state of each reed and parse MQTT message content for the two switches to detect switch changes. Switch#state in Tasmota already does that work. For the moment I'm preferring the rules offered in tasmota to simplify the code.

Simple rules are as below pending testing (I'll post when fully tested): Switch#2 switches to 1 [Door has closed] => Stop timer, Publish C to stat/normstahl/STATE Switch#2 switches to 0 [Door is opening] => Start 20s timer, Publish o to stat/normstahl/STATE Switch#3 switches to 1 [Door is open] => Stop timer, Publish O to stat/normstahl/STATE Switch#3 switches to 0 [Door is closing] => Start 20s timer, Publish c to stat/normstahl/STATE Timer expires [Assumed Door is stopped] => Publish S to stat/normstahl/STATE

Next Mqqtthing getCurrentDoorState can simply connect to stat/normstahl/STATE Mqqtthing getTargetDoorState can connect to stat/normstahl/STATE (if o => target =O, if c => target=C - but I think HK does this automatically). Mqqtthing setTargetDoorState just publishes to the relay power topic

Now I need to think how best to handle when the door stops and moves again with a change of direction hence target and what that means for getTargetDoorState. There could even be multiple stops. An Mqttthing apply function inspired by your looks like the best option to try and manage this.

[jdtsmith]

Sounds like a much better solution if you have two switches and access to the firmware! If you device is 'c' but then the 'O' reed switch trips, just tell HomeKit your target was actually 'O'. You could also implement a timeout as I did as a failsafe. Good luck.

[ceraz68]

I'm getting close to completion, need to think of any more cases that would break the logic.

I decided to create two topics for the garage door (current ['O', 'C', 'o', 'c', 'S'] and target ['O', 'C']) as some did others in other threads. I had tried to use a single topic but could get the getTargetDoorState to replace 'o' by 'O' or 'c' by 'C' and discovered more control with two topics. This creates a 'smart' door whereby it knows where it is supposed to go, its current state is known and a timeout to detect if it has stopped somewhere along the way (obstruction or deliberate user intervention).

On the mqtthing side of things I just needed to: a) stop Siri or HK from opening an already opened door or closing and already open door. Simple apply routines on setTargetDoorState. b) If Siri or HK decide to open/close a moving door, this is acceptable. This will lead to a stopped situation which is reported by the door. Home app will display an appropriate icon (there are two icons depending on the target state, I force the one with a closed door with the OPEN caption, it is enlightened).

For recovery from a stopped situation, in theory the direction of movement inverses but there could be multiple stops, triggered by HK or manual use. So I gave up trying to report the true movement direction and update the current/target state. So there is an element of uncertainty but HK sorts itself once either reed switch closes.

If you device is 'c' but then the 'O' reed switch trips, just tell HomeKit your target was actually 'O'.

Once the 'O' reed closes, the garage sends 'O' to both current and target topics, so HK is in sync.

            "topics": {
                "getTargetDoorState": "stat/normstahl/target",
                "getCurrentDoorState": {
                    "topic": "stat/normstahl/current",
                    "apply": "if(message=='C'||'O') doorPosition=message; return message;"
                },
                "setTargetDoorState": {
                    "topic": "cmnd/svgaragerelay/POWER",
                    "apply": "if (doorPosition==message) return 'OFF'; return 'ON'"
                }
            },

in setTargetDoorState I returned 'OFF' instead of null so I can see the accessory apply is functioning (logMqtt or mqtt client).

In tasmota I made to rules and publish Mqtt retained messages (publish2) or HK will show a spinning wheel on restart:

Rule1 sets the topics and logic for when either reed is closed (stop timer1). Rule1 on System#Boot do backlog var1 stat/normstahl/current; var2 stat/normstahl/target endon on switch2#state=1 do backlog ruletimer1 0; publish2 %var2% C; publish2 %var1% C endon on switch3#state=1 do backlog ruletimer1 0; publish2 %var2% O; publish2 %var1% O endon

Rule2 is for when the door is moving (start timer1) or stopped (timer1 expires)

Rule2 on switch2#state=0 do backlog ruletimer1 20; publish2 %var2% O; publish2 %var1% o endon on switch3#state=0 do backlog ruletimer1 20; publish2 %var2% C; publish2 %var1% c endon on Rules#timer=1 do backlog publish2 %var2% C; publish2 %var1% S endon

I have a custom plugin that detects when the garage door is open for too long and triggers (via mqtt) and audio alert on a speaker. So that plugin can be killed and replaced by another timer in Rule1/Rule2 that triggers the same event :-)

Thanks again for your explanations or won't have made it !

[jdtsmith]

Looks very fancy compared to my setup! Having the ability to "feed" MQTTThing just what it expects makes the control side much, much simpler.

                "apply": "if(message=='C'||'O') doorPosition=message; return message;"

Oops, that will always be true:

> message=false
< false
> if(message=='C' || 'O') console.log("True?")
[Log] True?

I definitely recommend trying all of your logic step by step in a JS console to catch hard-to-spot errors like this. Also keep in mind a variable like doorPosition won't be saved between topic callbacks (use state.global.doorPosition or similar).

One other tip: to "cancel" a request to move, you can return the bare word undefined in setTargetDoorState and MQTTThing won't even bother contacting your device. null also works but is deprecated.

I think yours will be a common use scenario (Tasmoto + MQTTThing 2-sensor garage door) so once you get it fully working, writing it up in the Tested Config Wiki might be nice.

[jdtsmith]

Codecs do allow timer-driven updates, but currently you have to hang on to the 'delivery' function. I'll fix this gap before I document them, hopefully very soon.

@arachnetech Did timer-based codec updates ever get documented? Is the output async function passed in what enables this? Are there any limits to how long output-based callback can be delayed?

BTW @ceraz68, the jsonCodec simplified "inline" codec looks ideal for your application (vs. apply). In particular:

By default, the JSON codec only publishes properties which have updated. To collect all published properties for each published topic, set "retain": true.

[ceraz68]

@jdtsmith thanks a mil'. Was working fine but will now double check and amend (thought I'd cracked it, Argh!). I published the example on the wiki but will fix it the getCurrentDoorState. Still struggle with these things in JS. Once stable I'll look at the "inline" codec

[ceraz68]

Silly me, should have remembered one of your key explanations:

Both 'C' and 'O' are true according to JS,

... and respect OR syntax. Fixed to: "apply": "if(message=='C' || message == 'O') state.global.doorPosition=message; return message;"

Figured how to use one topic reporting the current state. With one topic sending the 5 values C,O,c, o, S we need to handle c,o,S for the getTargetDoorState. Setting 'S' to 'O' or 'C' will change the look of the Home app icon

            "getTargetDoorState": {
                "topic": "stat/normstahl/current",
                "apply": "if(message == 'o') message='O'; if(message == 'c' || message=='S') message='C'; return message;"
            },

With two topics (one current and one target) nothing special to do with getTargetDoorState

            "getTargetDoorState": "stat/normstahl/target",

I'll build a second scenario for the wiki.