search

kk7ds / pynx584

Python library and server for talking to NX584/NX8E interfaces
GNU General Public License v3.0
29 stars 26 forks source link

Purpose of zone names in config file #66

Closed chrodriguez closed 2 years ago

chrodriguez commented 2 years ago

Hello, I'm new in alarm systems, and my experience configuring my oldie nx-8 alarm was very painful. Finaly I've got it working, and integrated with my hassio. The first problem after having a working scenario, is that I've found some odd behavior when configured names were automatically changed.

I'm wondering why pynx584 provided configuration file allows users to set zone names? I'm asking this, because it seems that every call to get_zone_name (message 0x23) will be responded with a 0x03 message. Then when a 0x03h message is received, a configured zone with some name will be updated in memory and also written to configuration file: https://github.com/kk7ds/pynx584/blob/master/nx584/controller.py#L353

The problem with this, is with zone 1, when watchdog expires and a get_zone_name(1) is called, and consequently zone 1 configured name updated. The problem is that even its configuration is persisted, so configured names are lost.

Do you think giving an option to disable name updates can be helpful? I think I'm going to implement a PR with this feature, but I would like to ask you before this, because I may be using this program incorrectly. If my contribution is wellcome, I'll try to do my best.

Thanks in advance

kk7ds commented 2 years ago

The reason the names are in the config file is that not all NX8 systems can program zone names, like if they only have the LED (vs LCD) keypads. I originally had those and thus had no names in my system. Once I swapped out my keypads, I added the functionality, which is why it's maybe not very coherent anymore. Regardless, I want the ones on my panel to be the source of truth (but not everyone may feel that way of course).

The polling of zone 1 was a quick one-off I added to fix a stale connection problem I was having, but it wasn't really the best choice. I've got a local change yet to be pushed to change that to a system status request. Sounds like that would be of interest to you, but that you also need some way of saying "treat the config file as the source of truth and do not update it" right?

Assuming the latter, I think maybe a flag in [config] to disable polling for names entirely would be appropriate. I can do it, but if you want to that's fine as well.

chrodriguez commented 2 years ago

I can try doing it. You have done a great work and this new feature is not so critical. I will create a PR when finish it and feel free to comment on it.

In a few days I'll be working on this. Thanks for your clear explanation