Koenkk / zigbee2mqtt

Zigbee 🐝 to MQTT bridge 🌉, get rid of your proprietary Zigbee bridges 🔨
https://www.zigbee2mqtt.io
GNU General Public License v3.0
11.64k stars 1.64k forks source link

[Feature request]: `no_occupancy_since` feature for Philips Hue motion sensors #11183

Closed uncle-fed closed 2 years ago

uncle-fed commented 2 years ago

Is your feature request related to a problem? Please describe

If I understand it right, the no_occupancy_since feature was introduced a while ago here: https://github.com/Koenkk/zigbee2mqtt/issues/1195 , which reads as if this was supposed to be a feature for all motion sensors.

I can't confirm it working and I also don't see it being mentioned in the documentation for the Philips Hue motion sensor https://www.zigbee2mqtt.io/devices/9290012607.html

I suppose when this feature was added the sensor was not yet supported or simply overlooked.

Describe the solution you'd like

Will it be possible to extend this very useful feature to Philips motion sensors? Maybe in some more generic way it can be done to really all present (and future) supported motion sensors, since this is purely a software feature, if I understand it right?

Koenkk commented 2 years ago

Added, please let me know if it works.

Changes will be available in the dev branch in a few hours from now. (https://www.zigbee2mqtt.io/advanced/more/switch-to-dev-branch.html)

uncle-fed commented 2 years ago

Thank you for looking into it. It does work... kinda (the property gets emitted now), but I am not sure if it's really doing it's job properly. I am assuming (and here quote your old message from the other issue) that the no_occupancy_since "message should just be send after a certain time of no motion". Well, in the case of this sensor, the message goes out X seconds after occupancy: true (it detects motion), rather than going out after detecting no motion occupancy: false.

The sensor's minimum hardware limit for occupancy timeout is just under 10 seconds, I set it to 0 anyway to keep it at possible minimum. If I set no_occupancy_since: [5, 10], then this is what I get now:

# generate movements in front of the sensor
'{"no_occupancy_since": 0, "occupancy": true}'
# keep moving for 5 seconds more. I would expect to get nothing after 5 seconds, but I get this:
'{"no_occupancy_since": 5}'
# keep moving yet another 5 seconds
'{"no_occupancy_since": 10}'
# stop moving and wait another ~10 seconds
'{"occupancy": false}'
Koenkk commented 2 years ago

This is expected, you shouldn't set the value lower than the occupancy timeout. I also think there is a bug in the current implementation; the timer should start after occupancy: false, not after occupancy: true.

uncle-fed commented 2 years ago

the timer should start after occupancy: false, not after occupancy: true

Thanks! That's precisely what I am trying to say with my report above. I would have reported this ages ago, but I only have Philips motion sensors and until yesterday they did not get this feature.

If we refer once again to the fact that when this feature was discussed originally, it was clearly stated that "message should be sent after a certain time of no motion", meaning that "no motion" starts when the occupancy: false event occurs (in real world, of course, the motion would stop earlier than the event, also "how long ago it stopped" would depend on the occupancy_timeout effective value, but we discount all of that). It is important to start counting from occupancy: false moment, which is not the case currently.

When this is implemented, it won't matter if the no_occupancy_since value is lower or greater than the occupancy_timeout. Because they will always be guaranteed to happen one after another. The no_occupancy_since timers should be cancelled by the occupancy: true event, and I would rather prefer NOT to see no_occupancy_since: 0 at all that is currently transmitted with the occupancy: true event.

uncle-fed commented 2 years ago

We should be careful changing behaviour though. I would expect that by now there is probably lots of automation that depends on this "wrong" behaviour. Perhaps differently named property should be introduced with the changed behaviour. Maybe simple no_occupancy (both in config and in payload) and a note in documentation that the old parameter is deprecated?

Koenkk commented 2 years ago

We should be careful changing behaviour though. I would expect that by now there is probably lots of automation that depends on this "wrong" behaviour. Perhaps differently named property should be introduced with the changed behaviour.

Yes we should indeed, it also depends on how you interpret the description:

Sends a message the last time occupancy was detected. When setting this for example to [10, 60] a {"no_occupancy_since": 10} will be send after 10 seconds and a {"no_occupancy_since": 60} after 60 seconds.

uncle-fed commented 2 years ago

Well, clearly the two concepts:

Sends a message the last time occupancy was detected

and

message should just be sent after a certain time of no motion

are not really the same thing.

Reading the discussion again... I think there is a use case for the current implementation, it's just that the name no_occupancy_since is actually misleading without further explanations and without certain context.

And the context was that when this feature was born, I believe, it was targeting a (rather poor and cheap) motion sensor by Xiaomi/Aqara that had hardware limitation of minimum 1 minute occupancy timeout with a possible caveat: I strongly believe from what I am reading in that discussion, that the Xiaomi sensor will emit another occupancy: true message again after 1 minute of its minimum timeout, if the motion is still there during that minute. And if so, THAT would be an absolute key difference between the Philips and Xiaomi sensors. The Philips sensor will never ever emit more than one occupancy: true message, as long as it keeps detecting motion within its occupancy timeout. While I have a very strong suspicion that Xiaomi one will send "repeating" occupancy: true messages. And if so, that would then explain the need for the current implementation of the no_occupancy_since property for the Xiaomi sensors (even if the name of the parameter is somehow misleading :-)).

The Philips Hue sensor works much more logical (at least in my opinion :-)) Once it detects motion, it will never emit more than 1 occupancy: true message no matter how long the overall motion continues for. The motion may interrupt briefly, but as long as the motion resumes within the occupancy timeout, the sensor will carry on thinking that the motion still goes on and will not send out any further messages since the initial one. If the motion stops for longer than the occupancy timeout, then the sensor will emit one and only one occupancy: false message and that would be it.

Now you may ask, if the sensor implements such behaviour quite nicely out of the box, why would anyone want an event fired "XX seconds after the motion has stopped". That's a valid question. I will try and explain using a non-imaginary situation.

Say you have a storage closet with a smart light and a motion sensor in it. You want the light to go on when somebody enters (easy) and turn off when somebody leaves (not so easy). The caveat is that a person may be inside the closet inspecting something, doing very little movement, so that the sensor cannot pick it up anymore. If you set the occupancy timeout to a few seconds only (10 seconds minimum with the Philips sensor), there is a very high chance that the lights will go off while somebody is simply staring at something, and that would be really annoying. So, the first immediate thought with this sensor: set it's occupancy timeout to a larger value... I mean how long people can spend with very little motion in the storage closet? Surprisingly, quite long, if you have limited movement space and lots of small things to browse through with your fingers. In my case here I ended up with the value close to 2 minutes, actually.

And now the most interesting part of this example. Sometimes I need to control other automation, that has nothing to do with the "light in the closet" but it has to do with human activity in the house in general. So, suddenly, the concept of "turning the lights off after 2 minutes of absence" and "was there any motion or activity in a certain room in the last 20 seconds" become two completely different problems to solve. And how can you solve this with a sensor that is set to "occupancy timeout of 2 minutes" ? Well you simply cannot, in this case. So you HAVE to set it to least common denominator, which would be back to 20 seconds then. And, in order for it not to mess the lights because the interval is now 6 times less than you'd want it, you have to implement that extra "2 minutes" logic / timers / their resets on your own, inside the home automation software (NodeRed in my case).

Having an MQTT event that simply means "xx seconds have elapsed since the occupancy: false" would basically allow to get rid of all this complexity on the HA side. Such event could be used to control the lights, while the occupancy: false event could be used for another automation, not lights related.

I thought that this feature of no_occupancy_since did precisely this and so I thought my request was an "easy win", but I guess I was wrong in the end. It does something else for a different kind of sensor logic. So my issue is not really about the same feature that the current Xiaomi motion sensor have made for them.

I suppose, given the long read above, both use cases exist and the current behaviour should not be changed. If we could have another event fired with the logic discussed above, it would be great. Everything is already there, pretty much . We just need a new name for it and slightly changed timer start / reset logic. But if I am the only one with such a need, or you see it as unnecessary bulk, I can survive with the current timers implementation of my own. Thanks!

Koenkk commented 2 years ago

I strongly believe from what I am reading in that discussion, that the Xiaomi sensor will emit another occupancy: true message again after 1 minute of its minimum timeout, if the motion is still there during that minute.

True

Actually I think we can re-use the no_occupancy_since (obviously the description will differ) (and I cannot find a better name), it will just work differently depending on the used converter:

dimmuboy commented 2 years ago

Will it be available in HA too?

uncle-fed commented 2 years ago

@Koenkk if you have time for this, it would be nice to implement this slightly different timer logic as discussed above. I.e., the countdown timer should start with the occupancy: false event (if the device is able to emit such event, like the sensor here).

Koenkk commented 2 years ago

@uncle-fed done now, please check if it works and if the description is clear (in the frontend)

Changes will be available in the dev branch in a few hours from now. (https://www.zigbee2mqtt.io/advanced/more/switch-to-dev-branch.html)

uncle-fed commented 2 years ago

@Koenkk Many thanks! Looks very good now, the behaviour is now as expected and matches the description in the frontend (which is also clearly described).

At this stage the only thing I would consider changing further would be to remove the no_occupancy_since: 0 that gets emitted simultaneously with the occupancy: false. I personally find this redundant since it will always be fired together with the occupancy: false and holds no real extra information. But then again, maybe somebody would find a use case for that, I don't know... You decide if you want to remove it or keep it.

Koenkk commented 2 years ago

At this stage the only thing I would consider changing further would be to remove the no_occupancy_since: 0

Where do you see this?

uncle-fed commented 2 years ago

In the same payload that contains occupancy: false:

payload '{"linkquality":83,"no_occupancy_since":0,"occupancy":false}

Koenkk commented 2 years ago

@uncle-fed this is added to reset the no_occupancy_since value, otherwise it would always be the last no_occupancy_since value.

uncle-fed commented 2 years ago

Sure, I understand. My point was that the zero value does not need to be in the payload at all because it does not carry any useful information for the client (but the client listening to events needs to analyze it)... but if that's complicating the implementation, it can stay in the message of course.

Koenkk commented 2 years ago

I see, we shouldn't cache the no_occupancy_since value in the first place (this is the reason this no_occupancy_since 0 is needed).

Changes will be available in the dev branch in a few hours from now. (https://www.zigbee2mqtt.io/advanced/more/switch-to-dev-branch.html)

uncle-fed commented 2 years ago

Looks fine now! No more no_occupancy_since: 0 when occupancy: false is transmitted. Thanks again!

Daniel15 commented 2 years ago

No more no_occupancy_since: 0 when occupancy: false is transmitted.

It would have been good to explicitly mention this change in the changelog... I was wondering why some of my Node-RED flows broke, but it turns out they were switching on no_occupancy_since and checking for 0

image

uncle-fed commented 2 years ago

There are two types of occupancy sensors that were discussed here: the ones that are capable of transmitting occupancy: false and the ones that only can send occupancy: true (and never false). The changes discussed here were concerning only to remove zero value for the sensors that are able to transmit occupancy: false. From what you're saying it sounds like it was removed completely. Which wasn't necessarily the intention... @Koenkk could comment further, I suppose

Daniel15 commented 2 years ago

I've got several of the Aqara/Xiaomi RTCGQ11LM sensors - not sure what behaviour they have (it's been a while since I set up all my flows) but I noticed the change when upgrading zigbee2mqtt and only found this issue because it sounded related.

uncle-fed commented 2 years ago

Those are not capable of transmitting occpiancy: false AFAIK. The removal of the zero value for no_occupancy_since should not have touched those sensors, probably... Sorry about that.

jerkovicl commented 2 years ago

Those are not capable of transmitting occpiancy: false AFAIK. The removal of the zero value for no_occupancy_since should not have touched those sensors, probably... Sorry about that.

So what value for no_occupancy_since should be used for those sensors?

Daniel15 commented 2 years ago

Those are not capable of transmitting occpiancy: false AFAIK. The removal of the zero value for no_occupancy_since should not have touched those sensors, probably... Sorry about that.

So what value for no_occupancy_since should be used for those sensors?

It's optional. Whatever you want to use. If there's no occupancy for the specified number of seconds, an event will be emitted. I use this to turn off hallway lights if there's no motion for a few minutes, without having to manually use a timer myself in Home Assistant or Node-Red

Just make sure it's more than 60 seconds since the Aqara sensor only checks occupancy every minute.

uncle-fed commented 2 years ago

The removal of the 'no_occupancy_since: 0' for the sensors that are NOT capable of transmitting 'occupancy: false' was not intended, this can be viewed as a regression that needs to be fixed. However, when 'occupancy: true' is transmitted, this basically is the same as 'no_occupancy_since: 0'

jerkovicl commented 2 years ago

@Daniel15 i have the exact case, turning off hallway lights and same sensor, so value is [0,90]? And timeout 90 also?

Daniel15 commented 2 years ago

@Daniel15 i have the exact case, turning off hallway lights and same sensor, so value is [0,90]? And timeout 90 also?

You can just do [90] and turn off the light when an MQTT event is received for the sensor where no_occupancy_since = 90. I'd share my Node-RED flow with you, but I'm currently on vacation and don't have remote access to Node-RED.

I used to do [0, 90] and turn on the light when no_occupancy_since = 0 and turn it off when no_occupancy_since = 90, but you can instead turn on the light when occupancy = true.

jerkovicl commented 2 years ago

@Daniel15 @uncle-fed thx for info

dangnguyen0906 commented 1 year ago

@Daniel15 >

can you share me the node red automation. I'm quite new and don't know how to trigger no_occupancy_since payload

Daniel15 commented 1 year ago

can you share me the node red automation. I'm quite new and don't know how to trigger no_occupancy_since payload

@dangnguyen0906 Sure!

I use it to dim one of my wall-mounted tablets when nobody is near it.

I have a device in Zigbee2MQTT called "Hallway Outside Office Motion", and its no_occupancy_since is set to [60, 240], meaning it'll send one no_occupancy_since after 60 seconds and another after 240 seconds (4 minutes).

Here's what the Node-RED flow looks like: image

Here's the JSON:

[
    {
        "id": "d013ab33ba54a861",
        "type": "mqtt in",
        "z": "2862b0b5fa361917",
        "name": "Motion sensor",
        "topic": "zigbee2mqtt/Hallway Outside Office Motion",
        "qos": "2",
        "datatype": "json",
        "broker": "e811c8eb637d26a5",
        "nl": false,
        "rap": true,
        "rh": 0,
        "inputs": 0,
        "x": 90,
        "y": 120,
        "wires": [
            [
                "8b06d40982d5e33c",
                "982e511c8ed0245b"
            ]
        ]
    },
    {
        "id": "8b06d40982d5e33c",
        "type": "switch",
        "z": "2862b0b5fa361917",
        "name": "No occupancy for 4 mins?",
        "property": "payload.no_occupancy_since",
        "propertyType": "msg",
        "rules": [
            {
                "t": "eq",
                "v": "240",
                "vt": "num"
            }
        ],
        "checkall": "true",
        "repair": false,
        "outputs": 1,
        "x": 290,
        "y": 120,
        "wires": [
            [
                "ee1704a8f1d32203"
            ]
        ]
    },
    {
        "id": "ee1704a8f1d32203",
        "type": "api-call-service",
        "z": "2862b0b5fa361917",
        "name": "Idle",
        "server": "b6f128b45d1d76f7",
        "version": 3,
        "debugenabled": false,
        "service_domain": "input_boolean",
        "service": "turn_on",
        "entityId": "input_boolean.dashboard_idle",
        "data": "",
        "dataType": "jsonata",
        "mergecontext": "",
        "mustacheAltTags": false,
        "outputProperties": [],
        "queue": "none",
        "x": 470,
        "y": 120,
        "wires": [
            []
        ]
    },
    {
        "id": "6684fad0637f6d58",
        "type": "api-call-service",
        "z": "2862b0b5fa361917",
        "name": "Not idle",
        "server": "b6f128b45d1d76f7",
        "version": 3,
        "debugenabled": false,
        "service_domain": "input_boolean",
        "service": "turn_off",
        "entityId": "input_boolean.dashboard_idle",
        "data": "",
        "dataType": "jsonata",
        "mergecontext": "",
        "mustacheAltTags": false,
        "outputProperties": [],
        "queue": "none",
        "x": 480,
        "y": 160,
        "wires": [
            []
        ]
    },
    {
        "id": "982e511c8ed0245b",
        "type": "switch",
        "z": "2862b0b5fa361917",
        "name": "Occupancy?",
        "property": "payload.occupancy",
        "propertyType": "msg",
        "rules": [
            {
                "t": "true"
            }
        ],
        "checkall": "true",
        "repair": false,
        "outputs": 1,
        "x": 250,
        "y": 160,
        "wires": [
            [
                "6684fad0637f6d58"
            ]
        ]
    },
    {
        "id": "e811c8eb637d26a5",
        "type": "mqtt-broker",
        "name": "localhost",
        "broker": "192.168.0.36",
        "port": "1883",
        "clientid": "",
        "autoConnect": true,
        "usetls": false,
        "protocolVersion": "4",
        "keepalive": "60",
        "cleansession": true,
        "birthTopic": "",
        "birthQos": "0",
        "birthPayload": "",
        "birthMsg": {},
        "closeTopic": "",
        "closeQos": "0",
        "closePayload": "",
        "closeMsg": {},
        "willTopic": "",
        "willQos": "0",
        "willPayload": "",
        "willMsg": {},
        "sessionExpiry": ""
    },
    {
        "id": "b6f128b45d1d76f7",
        "type": "server",
        "name": "Home Assistant",
        "version": 2,
        "addon": false,
        "rejectUnauthorizedCerts": true,
        "ha_boolean": "y|yes|true|on|home|open",
        "connectionDelay": true,
        "cacheJson": true,
        "heartbeat": false,
        "heartbeatInterval": "30"
    }
]

Let me know if you'd like info about the flow where this boolean is used, although it's somewhat complex as I have it configured like this:

I'm using a Fire HD 10 with WallPanel for the dashboard: https://wallpanel.xyz/