search

XaF / qolsysgw

Qolsys IQ Panel 2+ gateway to an Home Assistant Alarm Control Panel
MIT License
131 stars 13 forks source link

Unique_id collisions on 1.3.0 for sensors added via IQ Hardwire #108

Closed idle-mind closed 1 year ago

idle-mind commented 1 year ago

Before submitting this form

Description of the bug

It appears the changes in #95 causes the collisions in unique_id for all sensors added via hardwire module. https://qolsys.com/iq-hardwire-powerg/

The sensor id on the panel is shared for all sensors added via the hardwire translator. Ex:
hardwire translator sensor id = 8061 front door =8061 garage door = 8061

This causes the front door and garage door to have the same unique_id and HA ignoring (auto discovery ) of sensors after the first one it sees.

Falling back to 1.2.0 resolves the issue of collisions of unique_id

Happy to provide any additional information you may need.

Expected behavior

Sensor unique_id should be unique

DEBUG logs

2023-05-26 22:38:34.464956 WARNING qolsys_panel: sensor of unknown type: {'id': '460-6068', 'type': 'TakeoverModule', 'name': 'Hardwire Translator', 'group': 'takeovermodule', 'status': 'Closed', 'state': '0', 'zone_id': 1, 'zone_physical_type': 13, 'zone_alarm_type': 0, 'zone_type': 18, 'partition_id': 0}

Additional context

No response

XaF commented 1 year ago

That's interesting! Do you see any other information for those sensors that would allow distinguishing them? I would have thought the panel would have sent different ids... I could add the type in the unique id but that would also collision if two translated sensors are of the same type. Any way to identify from those sensors which is the parent?

idle-mind commented 1 year ago

Let me dig into the data coming from the panel for those sensors. I will also try to grab a picture of my panel and its ids.

I think more people will have this same issue as the hardwire modules are popular for re-using existing wired sensors with the IQ panel.

jsb5151 commented 1 year ago

Same issue here. I am getting the following errors with my PowerG hardwire translator (PG9WLSHW8) from the AppDaemon log:

2023-06-21 12:08:30.196204 WARNING qolsys_panel: sensor of unknown type: {'id': '460-1234', 'type': 'TakeoverModule', 'name': 'Hardwire Translator', 'group': 'takeovermodule', 'status': 'Closed', 'state': '0', 'zone_id': 10, 'zone_physical_type': 13, 'zone_alarm_type': 0, 'zone_type': 18, 'partition_id': 0}

2023-06-21 12:08:30.804595 WARNING qolsys_panel: sensor of unknown type: {'id': '460-1234', 'type': 'TakeoverModule', 'name': 'Hardwire Translator', 'group': 'takeovermodule', 'status': 'Closed', 'state': '0', 'zone_id': 10, 'zone_physical_type': 13, 'zone_alarm_type': 0, 'zone_type': 18, 'partition_id': 0}

If I recall correctly, the Takeover Module itself has the ability to report a panel tamper situation (that one I know for sure), and possibly other things such as low battery, AC loss, sounder supervision error, etc but I haven't tested all of them.

As for Home Assistant, same story there - all sensors that are under that HW translator break discovery:

2023-06-21 12:08:31.126 INFO (MainThread) [homeassistant.components.mqtt.discovery] Found new component: binary_sensor garage_keypad

2023-06-21 12:08:31.128 ERROR (MainThread) [homeassistant.components.binary_sensor] Platform mqtt does not generate unique IDs. ID qolsys_panel_s460_1234 already exists - ignoring binary_sensor.access_control_trouble

2023-06-21 12:08:31.169 INFO (MainThread) [homeassistant.components.mqtt.discovery] Found new component: binary_sensor garage_door 2023-06-21 12:08:31.174 ERROR (MainThread) [homeassistant.components.binary_sensor] Platform mqtt does not generate unique IDs. ID qolsys_panel_s460_1234 already exists - ignoring binary_sensor.access_control_trouble ...and repeats itself for all zones on the HW translator including my wired keypad (HS2LCD). For your reference, the "Access Control Trouble" zone is wired to my keypad's built-in alarm input so it's also considered a hardwired zone.

That being said, the hardwire translator/takeover module may have more than 8 zones under the same PowerG ID since it supports expanders, keypads and other wired peripherals.

jsb5151 commented 1 year ago

BTW, I think we need to look at #97 and/or #96 for HW translators, but keep in mind this also happens with sensors that have 2 or more reporting loops. For example, the PG9945 has a built-in reed switch but also has an AUX input. Both sensors are reported with the same unique_id which breaks things unfortunately.

I think the best Unique ID would be the zone id (zone number). Thoughts?

XaF commented 1 year ago

zone_id was used before but is not stable through installations (i.e. same sensors on a new panel would create whole new sensors or shuffle them around).

I could probably add a parameter to add the zone id to the sensor id, but I would prefer something that can be considered stable across installations!

jsb5151 commented 1 year ago

I'm curious, zone_id usually doesn't change, unless I'm mistaken... in which scenario would this happen?

Unless the installer/admin changes sensors in the panel, they should always keep the same number. I had this issue at some point where I changed a sensor and it renumbered a zone; I just ended up deleting the discovery topic which fixed the issue.

I think this method would be a lesser evil as the new unique_id method is breaking any device that has multiple sensors - I have 10 dead zones just on the hardwire translator, and 4 on two PG9945s. I have to either modify the discovery topics myself or manually add them to HA, or I'd have to revert to roopesh's app which I was using up until last week.

XaF commented 1 year ago

That's the thing: trying to reach stability across reinstalls, or switching of the panel (had to change my panel following an issue and had a bad time initially with some automations not doing what they should).

I'd be inclined to offer a config parameter to template the unique ids, so that it would be easier to make it work in cases where there are conflicts, but also in cases where, without translator, unique id would be sufficient!

jsb5151 commented 1 year ago

I put some thought into this and took another look at the Control4 driver, and IMO, unique_ids should NOT be based on the sensor id (xxx-yyyy) as it is not a unique value in Qolsys systems. The only unique value is the zone_id parameter. This is how the Control4 integration builds its table.

In any case, a panel swap would most likely end up requiring a reconfiguration anyway. This is the lesser of two evils versus breaking support for hardwire translators and multiple-zone sensors, which are a very common occurrence.

XaF commented 1 year ago

I disagree with your statement of a panel most likely requiring reconfiguration anyway, since you could switch your panel while keeping your sensors and they would thus already be at the right place, which means that if the unique ids are stable, you wouldn't have to touch your home assistant configuration at all, for any of your automations.

As I mentioned, I'm not willing to change the default sensor id format. I'm willing, however, to either (1) provide a template for the sensor id format so you can decide on the format you wish to use, or (2) as an alternative, of adding the zone_id only for specific dedup cases requiring it (such as when using a translator). (1) is pretty easy and I already have a working version, (2) might require a bit more refactor but might be nicer.

XaF commented 1 year ago

Would you be able to provide me the INFO message response with all your sensors? I'm curious about the order in which their appear in that message. I also wanna see what the different "subsensors" look like.

jsb5151 commented 1 year ago

See attached file. Disregard the weird zone names as this is a lab/testing system.

Relevant sensor ids are as follows:

qolsys-test.txt

efaden commented 1 year ago

Having the same issue. Just updated to main from an older version (1.2 I think). All my loops vanished except for the first one from my hardwire takeover module.

Update: Reading through it does seem like a template for unique ID would solve it. Personally I'd use the zone_id on mine because I have a bunch of sensors that have more than one loop/zone assigned to them. E.g. the hardwire takeover that currently only shows a single loop because of the collision. I haven't really looked through the code, but in version 1.3 all the individual loops showed up, but now only the 1st. Either way though making it a template would solve the issues.

XaF commented 1 year ago

So reading your INFO message, seems that the sensors related to the translator are ALWAYS produced after the translator itself. What I am having in mind of doing:

That way, it would keep things stable across reinstall in most cases, except for the handling of the translators. I'll start looking at how complicated it would be to handle.