davesmeghead / visonic

Visonic Custom Component for integration with Home Assistant
Apache License 2.0
84 stars 16 forks source link

Sensor Entities not updating #118

Closed XxTopKillerzZ closed 1 week ago

XxTopKillerzZ commented 2 weeks ago

Hello,

I am trying to configure my 2 home panels.

I am using PowerMax Pro (2 diferent firmware versions because one auto enrolls and other stays standard mode) Arm/Disarm and all panel info (including events with condition 3, zone update) work but the entities for each door/motion sensor do not update. If I reload the integration they get updated for the current state (opened/closed) but they do not update again. Last update also does not change. I did change entity id's and names and some of the icons, can it be the reason?

Any tips for troubleshooting this?

Thank you in advance

davesmeghead commented 2 weeks ago

Hi,

Does it work when you only include one panel in to HA? Try each panel alone in HA, is it one of the panels that causes the problem? Make sure that you give each panel a different panel number when you set it up

When you say you've changed Entity IDs what do you mean?
Changing icons should be OK.

Set the logger to debug as per the wiki here and upload the Home Assistant Log file in this issue and I'll take a look.

D

XxTopKillerzZ commented 2 weeks ago

Hi,

I am not sure but I can test. Last week the entities where not updating aswell for some time, when I noticed it, I reloaded the integration and it all came back to normal. I only setup second panel yesterday.

This is what I ended up with:

Panel 1:

Panel 2 (Added yesterday):

This is an example of the things I have changed in a sensor: Capture

If it maters:

Interior Alarm -> Panel 1 Exterior Alarm -> Panel 2 Panel 2 has unused zones: 7,8,9,10,13,14,15,16,17,19 Panel 2 Z28 is not a sensor, its a reapeater

Here is the log file: visonic_log.txt

davesmeghead commented 2 weeks ago

Hi,

Changing the names and icons in Home Assistant like that is OK, it will not affect the Integration at all.

I've also had a quick look at the log file, can you please delete panel 2 and when you set it up this time tick Auto Enroll as it's a Powermax Pro Part, it should auto enroll. Does that make any difference? If not then I'll need a new log file please :) D

XxTopKillerzZ commented 2 weeks ago

I can try but panel 1 is the one with the most errors. Entities in panel 2 mostly work fine, appart from some that do not update but I think thats a range problem with the sensors.

The control panels where bought at different times and have diferrent firmware versions but look the same. Both have Auto Enroll off, Panel 1 is in powerlink mode and Panel 2 in standard plus. I did not enroll via INSTALLER MODE → 7. ENROLL PWRLNK → 01: INSTALL because option 7. does not exist

There's also this entries in the logs:

2024-07-09 12:30:42.722 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry Panel 1 for binary_sensor Traceback (most recent call last): File "/usr/src/homeassistant/homeassistant/config_entries.py", line 586, in async_setup result = await component.async_setup_entry(hass, self) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/components/binary_sensor/__init__.py", line 231, in async_setup_entry return await component.async_setup_entry(entry) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 184, in async_setup_entry raise ValueError( ValueError: Config entry Panel 1 (a62a4e211eb3863b3fef0ce4165222c2) for visonic.binary_sensor has already been setup!

davesmeghead commented 2 weeks ago

Your panels are reporting what they are as follows: Panel 1: [handle_msgtype3C] PanelType=2 : PowerMax Pro , Model=27 Powermaster False Panel 2: [handle_msgtype3C] PanelType=4 : PowerMax Pro Part , Model=87 Powermaster False

These are extracted from your log file

Even if they physically look the same, Panel 2 is reporting as a PowerMax Pro Part with a later Model number than I have myself (71). Panel 2 will auto enroll, that is why there are no panel settings to manually enroll.

Now on to the main issue as I've taken a bit more of a look at your log file. I can see that you're using the Master release of the Integration, I last updated this several months ago. The issue is that the Home Assistant development team have continued to develop the core and frontend software, and as part of this they have made the "rules" for all Integrations (including Custom Integrations like mine) much stricter, but also changed the core functionality in small ways. As such, although the Master release still "mostly" works there are a number of flaws in it and you've uncovered one of those flaws, as you also spotted "... has already been setup".
Over the past 9 or 10 months I have been working on a Dev release that is available on Github but is not on HACS. This has had many updates including updates to keep it in line with the HA updates.

You have 3 ways forward, depending on how long you want to wait:

The decision is yours, if you do decide to download the Dev release and upzip it then delete the original "visonic" directory first. And if you want help in doing this then just keep this thread open and I'll help where I can. It all depends on how confident you are with doing this.

XxTopKillerzZ commented 2 weeks ago

Its really strange then. Never understood how firmware versions in these panels work but panel2 is older, has an older firmware and somehow has a newer model... We had to change panel1 for a newer one with new firmware because of some PGM error, anyway.

I will remove both integrations. I will remove entries in core.entity_registry to make sure everything reset. I will install dev version and test again.

I did not know master version was much behind dev. I actually read the issue about deprecations. Is there any updated wiki or old one is somehow similar? I have automations based on visonic_alarm_panel_state_update but I can update them easily.

I will post the updates once I'm finished

XxTopKillerzZ commented 2 weeks ago

The error is still present in the dev version for me

I removed visonic integrations and every piece of old data from .storage. I removed it from hacs and installed dev files myself. I then added integration(I think I found a little error, made pull request). Arm/Disarm is working but I get the same error. Sensor updates one time but then is stuck open. Alarm detects opening and closing.

I will attach a trimmed logfile home-assistant.log

davesmeghead commented 2 weeks ago

Hi,

The wiki still represents the current Master release but I'm in the process of updating it ready for when I release the Dev as the Master. I had already spotted the str problem that you had to fix, I'd updated it in my local copy but not pushed it to github as I've also done some major changes, but thanks anyway just in case other people use the Dev release. I'm not really sure what else to suggest for the problem that you see, as far as I can tell from the log file the Integration is responding to the messages correctly from the panel. Is it just a problem when you first start the Integration or does it keep happening? I'd be interested in the log file after what you uploaded i.e. when does the zone10 report open again. D

XxTopKillerzZ commented 2 weeks ago

Upon further investigation, I found that magnet sensors are indeed updating the entities in home assistant but they have the same timer logic as the motion sensors. They go from closed to open and stay open for 2-3 minutes and them they get updates, not when I close the window but later.

One thing I noticed is that the integrations knows about the sensor opening and closing but the event triggered is wrong. They have condition 2 instead of the old condition 1 for zoneupdates, maybe thats intentional for the new version.

This was the event trigerred when I opened the windows: Screenshot from 2024-07-11 20-19-53

And this was the event when I closed: Screenshot from 2024-07-11 20-19-45

Edit

I tried to open the window and let it be open for more than 2 minutes before closing. After closing the entity changed to closed immediately.

I then tried to open another window, let it be open for around 1 minute and then close. It had some delay on changing in Home Assistant. The total open time reported by Home Assistant was 2 minutes. So the delay was 2 minutes minus the time it stayed open.

One thing I also noticed is the attribute called "Zone open" In Zone10, it was Yes/No synced to the real status of the window. On Zone11, it's always reporting "No".

The sensors are the same magnetic sensors, MCT-302. They are both detected as Magnetic and Perimeter and their models are correct in the integration.

davesmeghead commented 2 weeks ago

Without a log file I can't really help much, the event images aren't enough for me to provide any help. The only observation I can make is that the timestamp on the 2 images means they are only 7 seconds apart. is this correct. What have you set as the "Motion off delay time"? as it seems to be very small. Try around 20 or 30 seconds. But this is why I need a log file please. I assume that this is Panel 2, the PowerMax Pro Part. Can you make sure that you disable Panel 1 so the log file just represents a single panel, that makes it easier when I look at the log file.

Also, the events have been reworked and I'm testing them now, so yes they have changed in the Dev release you're trying and changed again in my local copy. I'm currently comparing my local operation with my local copy of the wiki so I get the documentation correct.

This is how it is supposed to work for Magnet Sensors, you need to ignore the events and look at the state of the sensor in developer tools, like this image

Let's start with the main state off and Zone Open as No (as per the sensor picture above) Let's also assume that the Motion off delay time is set to 20 seconds.

If the sensor is opened i.e. triggered:

If the sensor is closed before the "Motion off delay time" expires

If the sensor is opened again before the "Motion off delay time" expires

If nothing changes in 20 seconds:

If you stood there and opened and closed the window within every 20 seconds then the entity state would stay on all the time as you are re-triggering it within the delay time.

XxTopKillerzZ commented 2 weeks ago

In all the tests using dev version I'm only setting up panel 1. I did not change the numbering so it is the same panel 1 as before. Model PowerMax Pro Part is Panel 1 Model PowerMax Pro is Panel 2

My motion of is set to 120s as it is the default.

I confirm that my sensors have the same attributes as these one.

Does motion off delay also apply to magnetic sensors? Shouldn't the entity of a magnetic sensor return to closed right after the window is closed? Because the integration is receiving information that zone closed the second I close the window.

They way I see it is that introducing a delay to magnetic sensors makes them update slower than necessary. For PIR I understand that we have to deal with the timer.

Could we have an option off for example 2 timers one "motion off delay" and one "magnetic closed delay" and if "magnetic closed delay" is set to 0 the timer is ignored?

davesmeghead commented 2 weeks ago

I'm confused, does this mean that you have it working now with the dev release? You confirmed that your sensors have the same attributes but nothing else, and then you went straight in to adding a new config setting.

I think you've missed the point of the "Zone Open" attribute which you can easily make a new Entity from in Home Assistant. This attribute represents the actual open/closed state for magnetic sensors. So this represent a 0 second "magnet closed delay" and I think that is all is necessary. So I'm not convinced that we need a separate timeout for magnetic sensors. The integration currently presents the "live" state using this attribute and also a timeout on the trigger that prevents automations in HA being retriggered too quickly.

EDIT: Just in case you're not familiar with templates in HA please look here Specifically, see the delay_on and delay_off settings for a binary sensor.

EDIT2: I went through this a few years ago. Most users are happy with the fact that it works and they can configure their alarm in HA. After that, the single timeout works for 99% of users. For the 1% who want to tailor their setup they also want different timeouts on the different magnet sensors and not be limited to a single timeout (as per your pull request). So the best way is to let the 1% of users create individual Entities from HA templates for each magnet sensor they want to do it for, each with their own timeouts. Templates in HA are relatively straightforward, and binary_sensors can be configured within the UI now. Most users in that 1% can fairly easily create these templates, people such as yourself for example. In fact, create a blueprint first and then create the Entities from that by having config variables in the blueprint.

XxTopKillerzZ commented 2 weeks ago

It is now working. Master version was having problems. Dev version was working but I taught it was not working due to the motion off delay, which I tough would not be applied to door/window sensors.

Its obviously up to you to decide and I'm not requesting features if I am not willing to contribute to them like I did.

Also the functionality stays the same with 2 different settings for people that want to the same delay. Also in home assistant the entity says Open/Closed so it should be correlated with the real status of the door.

I can easily do the templates has I do with other attributes for the alarm (ready,trouble,alarm memory, etc..) but I think having it simple enough would be more beneficial since it was an easy addition that could be documented in the settings list. Also the "Motion Off Delay" setting would really be about motion and not doors like the naming suggests, which was partially the think that make me think that magnet sensors did not follow a motion setting I think since it is not clear in the documentation how the delay works it should be explained. Also "Motion Off Delay" relates to only motion.

davesmeghead commented 2 weeks ago

Hi,

I'm really pleased that the dev version works, or at least does what it's designed to do :)

I've put some thought in to it today and I have a question for you.

There are many other sensor types than motion and magnet, do you propose to have individual settings for these too? In your proposed update the "Magnet closed delay time" applies to everything other than motion/camera sensors. What about flood, smoke, temperature and sound sensors e.g. MCT-427 smoke sensor? Do you think the second delay time is equally applicable to these sensors? Can it be made to have more of a generic description maybe.

I agree that the current documentation is not clear and that it's logical for motion/camera but not for other sensors.

I see a few options

I do not favour the last option but if you agree that your option can be made more generic that would be better i.e. "all sensors except motion/camera".

Rgds Dave

EDIT: I've just noticed in your code that your 2nd setting is only for Magnet sensors and all other sensors use the original motion, surely this is wrong with your logic, what do you think? These are all the sensor types

class AlSensorType(AlEnum):
    IGNORED = AlIntEnum(-2)
    UNKNOWN = AlIntEnum(-1)
    MOTION = AlIntEnum(0)
    MAGNET = AlIntEnum(1)
    CAMERA = AlIntEnum(2)
    WIRED = AlIntEnum(3)
    SMOKE = AlIntEnum(4)
    FLOOD = AlIntEnum(5)
    GAS = AlIntEnum(6)
    VIBRATION = AlIntEnum(7)
    SHOCK = AlIntEnum(8)
    TEMPERATURE = AlIntEnum(9)
    SOUND = AlIntEnum(10)
XxTopKillerzZ commented 2 weeks ago

I think option one is valid

Option two would remove the ability for people to have the delay, and since its already implemented in the code I think you should not discard the option to have it.

last options seem like a lot of options so I do not agree too.

I think we could group types of sensors for this matter. I have not tested the smoke/fire/water sensors and how do they report to the alarm, you have more knowledge than me. Maybe emergency sensors could have one delay specially because they are the ones that are so rare to accour and normally in an emergency situation so having a larger delay to return to normal does not feel bad, the important is when they trigger. Motion/Cameras makes sense because its the delay they have built in the sensors logic. Doors/Windows should have one.

In my implementation yes I only apply the new delay to magnet sensors. I do not now how some type of sensors work in the integration so I did not want to mess with it.

To sum up I think if its documented in the wiki what delay applies to each sensors its good. Then maybe 3 groups. Emergency, Motion, Magnet.

its up to what you think its the best, I can change my code to fit it

davesmeghead commented 2 weeks ago

Hi, All these sensors are binary, including "Temperature" which is really a heat sensor with a trigger threshold to detect a fire. I think that some of them also give the actual values (like the actual temperature) but I can't decode these messages yet as I don't have all these sensors to try them.

Within HA as long as the device class is set properly then they get the proper frontend. https://www.home-assistant.io/integrations/binary_sensor/#device-class

Overall I think you might be right and have 3 types.

Sensors are either Event or State based:

For Wired and Magnet this is the code I use to differentiate them in the frontend but then the user can override them in the UI anyway to set them correctly.

                if stype == AlSensorType.MAGNET:
                    return BinarySensorDeviceClass.WINDOW
                if stype == AlSensorType.WIRED:
                    return BinarySensorDeviceClass.DOOR

So what about these 3 as the config settings to the user: "Motion and Camera" "Magnet and Wired" "Emergency"

If you want to change your code accordingly and test it as best you can then I'll accept the pull request. I've already made a few changes to the config settings in my local copy that I'm already testing so I'll merge yours in here locally and test them all in combination.

I'm also in the process of updating the wiki page locally so you've no need to worry about the changes there, they'll come through as well.

Rgds Dave

XxTopKillerzZ commented 2 weeks ago

I will work it. I will also try to make them easy to read in the code by not having ie “motion off delay” and “zone closed delay” Maybe “TriggerOffDelayDoor”, idk. If you have some good naming scheme do tell me