Anamico / node-red-contrib-alarm

Nodes to build your own home alarm system. Designed to work easily with (but does not require) homekit.
MIT License
24 stars 8 forks source link

Make sending current state on registration configurable #4

Closed Vermatic closed 5 years ago

Vermatic commented 5 years ago

I am currently using your nodes to build a simple alarm system and I have attached some handling to the State Changed node.

The problem for me is that for each Deploy the current state is emitted.

I don't see a way to to distinguish between the message that is emitted after Deploy and that is emitted after a real state change.

So in my setup this will always trigger the whole processing chain. In my case this also involves sending an e-mail, so for each Deploy an e-mail is sent. I'd like to avoid that.

I suppose that emitting the current state was added on purpose for some reason. Can you please explain the reasoning?

Maybe my setup is incorrect as well and it should or can be used differently.

In function AnamicoAlarmPanel() in panel.js there is the following comment: // also emit current state on registration (after delay of 100 msec?):

Afterwards the current state is emitted unconditionally.

Would it be possible to add a switch to the configuration node so that it is possible to enable/disable this behaviour?

macinspak commented 5 years ago

Thanks for the feedback and would like to hear more about your deployment.

I believe the emit on deploy was to sync state with any other flow/accessory or homekit device after being offline for a period of time (I can't exactly remember). Perhaps an option would be to make it configurable or to include a tag or attribute to allow it to be filtered out?

Let me think about the best way forward and get back to you. Any other suggestions or weighting to the argument? Happy to publish a quick mod to solve this for you in the next few days.

Vermatic commented 5 years ago

This is my first encounter with Node Red, so please take any of my suggestions with a grain of salt.

I am replacing my pure HomeMatic CCU2 installation with HomeMatic CCU2 and ioBroker (for access to the CCU2) + NodeRed (for the control) running on a Raspberry Pi.

Inputs to node-red-contrib-alarm are the HomeMatic motion sensors and door/window sensors. Attached to the outputs are switchable power outlets and sending e-mails. No HomeKit involved whatsoever.

So I don't have the need to sync any state with an external device, because all the logic is within Node Red.

Yes, making it configurable would be a good idea. Maybe the default can be coupled to if HomeKit protocol is used or not and disabled by default if no HomeKit is used.

Adding a tag for the initial configuration would work as well, this can then be filtered out easily - which is currently not possible.

macinspak commented 5 years ago

OK, did both as they were easy.

So the payload on the initial state message on deployment will have the extra field: msg.initialState = true.

And added a "send initial state" boolean checkbox to the config of the stateChanged node to allow you to selectively send that message on deployment. It defaults to true.

I guess I could have added it to the panel as well, and maybe to the alarm node, but this works for now, let me know how you go. It will be in release 0.9.9

Vermatic commented 5 years ago

Thank you very much. I updated to 0.9.9 and it works as expected. If the option is not selected, the initial state is not sent. If the option is selected, the initial state is sent and the extra field is included.