Tom-Hirschberger / MMM-GPIO-Notifications

Magic Mirror² Module which sends custom notifications based on GPIO events
MIT License
11 stars 3 forks source link

Different notifications on different pin states of the same pin #10

Closed MTJoker closed 1 year ago

MTJoker commented 1 year ago

Hi, I recently switched to this module from MMM-PIR-Sensor and use it according to your tutorial with a HC-SR501 PIR sensor. Works great so far! The only issue is, that I propagate the USER_PRESENCE status via MQTT into my home automation system to graphically show in my UI that there is movement . But USER_PRESENCE never changes to false again, as the module only triggers on the high state of the pin. Is it possible to send a different notifications, if the same pin changes to low state? I tried this: '24': { gpio_state: 1, gpio_debounce: 0, delay: 10000, notifications: [ { notification: 'USER_PRESENCE', payload: true }, ] }, '24': { gpio_state: 0, gpio_debounce: 0, delay: 10000, notifications: [ { notification: 'USER_PRESENCE', payload: false }, ] } }

But this does not work... Is there another way to do it or is it possible to add support for my use-case?

Tom-Hirschberger commented 1 year ago

Hi,

the module did not support sending notification for both states of a pin. BUT As i see it as a absolutely reasonable request I spend the last two hours implementing this feature and released version 0.0.8 a couple of seconds ago.

Instead of configuring gpio_state and notifications for a pin the two arrays notifications_low and notifications_high can be configured now. In your case it will look something like:

        {
            module: "MMM-GPIO-Notifications",
            config: {
                24: {
                    delay: 10000,
                    gpio_debounce: 0,
                    notifications_high: [
                        {
                            notification: "USER_PRESENCE",
                            payload: true
                        }
                    ],
                    notifications_low: [
                        {
                            notification: "USER_PRESENCE",
                            payload: false
                        }
                    ]
                }
            }
        },
MTJoker commented 1 year ago

Wow. That was fast. Thank you very much, it works like expected and resolves my "problem".

[18.01.2023 11:47.38.384] [LOG]   MMM-GPIO-Notifications: Watched pin: 24 triggered with value 1!
[18.01.2023 11:47.38.385] [LOG]   Length of toSendNotifications: 1
[18.01.2023 11:47.38.386] [LOG]   MMM-GPIO-Notifications: Sending notifications of pin 24...
[18.01.2023 11:47.38.387] [LOG]   MMM-GPIO-Notifications: Sending notification:
[18.01.2023 11:47.38.388] [LOG]   {"notification":"USER_PRESENCE","payload":true}
[18.01.2023 11:47.38.787] [LOG]   MMM-Screen-Powersave-Notification: Resetted screen timeout to 600 seconds!
[18.01.2023 11:47.38.791] [LOG]   [MQTT bridge] NOTI->MQTT. Topic: iotdevices/mm_bathroom/presence, payload: "ON"
[18.01.2023 11:47.44.080] [LOG]   MMM-GPIO-Notifications: Watched pin: 24 triggered with value 0!
[18.01.2023 11:47.44.080] [LOG]   Length of toSendNotifications: 1
[18.01.2023 11:47.44.081] [LOG]   MMM-GPIO-Notifications: Sending notifications of pin 24...
[18.01.2023 11:47.44.081] [LOG]   MMM-GPIO-Notifications: Sending notification:
[18.01.2023 11:47.44.082] [LOG]   {"notification":"USER_PRESENCE","payload":false}
[18.01.2023 11:47.44.096] [LOG]   [MQTT bridge] NOTI->MQTT. Topic: iotdevices/mm_bathroom/presence, payload: "OFF"

I have two things that might by worth considering in future version (just as a suggestion):

  1. I think the "new" syntax is much better to understand and simpler for the user. All previous use-cases can be covered also with just the new syntax (just add only notifications_high or only notifications_low to the pin). You would also not have to think about which value to put for gpio_state (1/0? true/false?). So maybe you could consider dropping the "old" syntax with gpio_state and notifications in future releases. It would be a breaking change regarding the config, but the code would get simpler too :-)
  2. I am unsure about the delay option. It is for the pin, but not for both states individually. So if the pin goes high, notifications for high state are sent, if it goes low within the delay, notifications for low state are not sent. My first intention would be, that I add the delay because I do not want the same notifications within that time, but the notifications for the other state would be OK. But that might be different for other users :-) I now just removed the delay and it works fine for me.

As said, these are only some thoughts by me. (Just noticed that MMM-MQTTbridge that I use was also written by you. Nice ;-) And thanks again.)

Tom-Hirschberger commented 1 year ago

Hi,

your are welcome.

You are right the new syntax is more readable than the old one. I added a deprecation warning to the module if the old syntax is used and changed the examples in the Readme in the development branch.

The module was one my first ones and has some very ugly legacy stuff in it. Currently the delay option is more or less redundant to the debounce option. Debounce is handled by the gpio libraray, delay by the module. I will need to add some new data structures to support different delays for high and low state. I will add a enhancement ticket and will try to add it with the next release.

I just took over the ownership of the MMM-MQTTbridge module of sergge1 cause i use it in all of my setups and sergge1 has no more time to maintain it. So more credits to him than to me at the moment ;-)