tsightler / ring-mqtt

Ring devices to MQTT Bridge
MIT License
600 stars 106 forks source link

HA 2024.6 - Can't change Ring mode - requires code #846

Closed Scope666 closed 5 months ago

Scope666 commented 5 months ago

Describe the Bug

Can't change Ring Mode (Disarmed, Home or Away) for cameras

Steps to Reproduce

Try to change mode without owning a Ring Alarm system.

Expected Behavior

Works successfully without throwing error.

Log Output

2024-06-06 07:47:42.309 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [23151872931536] Arming requires a code but none was given for alarm_control_panel.branchburg_mode

This was working fine in 2024.5 and before.

Screenshots

Screenshot_1

Config File

{
    "mqtt_url": "mqtt://192.168.1.200:1883",
    "mqtt_options": "",
    "livestream_user": "",
    "livestream_pass": "",
    "disarm_code": "",
    "enable_cameras": true,
    "enable_modes": true,
    "enable_panic": false,
    "hass_topic": "homeassistant/status",
    "ring_topic": "ring",
    "location_ids": [
        ""
    ]
}

Install Type

Docker - Unraid

Version

v5.6.4

Operating System

Unraid

Architecture

x86_64

Machine Details

12th Gen Intel® Core™ i7-12700 @ 2079 MHz

bit-bat commented 5 months ago

This change is also impacting setups with Ring alarm. I can no longer arm using Home Assistant, although disarming works.

Relevant info: https://developers.home-assistant.io/blog/2024/05/22/alarm_control_panel_validation/

tsightler commented 5 months ago

Personally, I'm considering this a Home Assistant issue. For me it makes no sense that Home Assistant enforces a code requirement for the arm service when no code is actually configured and, even more strangely, it seems they don't require it for a call to disarm even though this also defaults to true.

Yes, it could be fixed in ring-mqtt by explicitly setting code_arm_required = false even when no code is configured, and I will probably do that eventually, but from my perspective HA pushed out a breaking change with no significant notice so, if they don't care about reliability for their users, well, I don't either.

Current workaround is to simply modify the automation to include a code, even though one is required, the code can be anything. Alternatively, you can configure a disarm code in ring-mqtt which, quite counterintuitively, will make it so a code is not required for arming.

Scope666 commented 5 months ago

Can confirm, setting a code, even though it's not needed in the app, fixes the issue.

NOTE: I don't own the Ring Alarm system, I'm just using this to change modes for the cameras. I use these presets for different combos... all cams on, backyard off, and all off.

image

tsightler commented 5 months ago

@Scope666 Just FYI, you have "disarm_code" in the configuration twice (line 6 and 12), that makes the behavior somewhat undefined. It would be better to modify the existing property vs duplicating the property with a second value.

Also note that setting this means that disarming will require a code.

Scope666 commented 5 months ago

Oh crap, you're right ... I'll fix it right now. Thanks!!!

I'm ok with the code required on a full disarm, I RARELY do that.... if anything it's probably good to keep the rest of the family out of there. ;-)

broyuken commented 5 months ago

Is there a current workaround that doesn’t need a code to arm or disarm? It seems like I need to choose if a code is required to arm or disarm even through a card.

tsightler commented 5 months ago

Is there a current workaround that doesn’t need a code to arm or disarm? It seems like I need to choose if a code is required to arm or disarm even through a card.

Downgrade to prior HA version via backup is one option for a workaround.

bkowalk commented 5 months ago

I'd love if we could turn on that code_arm_required = false option to get this working again for the moment! Definitely understand the frustration with HA, just bummed all of my homekit alarm integrations are failing now. What do you think of that for a (hopefully) temporary fix until HA changes their minds?

Thanks for all the work on this by the way! Such an amazingly useful tool.

broyuken commented 5 months ago

I rolled back to 2024.5.5 as 2024.6 was underwhelming feature wise and having my alarm work is more important than being on the latest version.

I totally understand why you're annoyed with HA by breaking this without any kind of heads up, pretty crappy thing to do. I'll be on 2024.5.5 until you get around to pushing out a patch. If I could patch it myself I would make a PR, but I'm kind of useless in that department unfortunately.

For anyone else interested in downgrading, log into your HA via SSH and type

ha core upgrade --version 2024.5.5
tsightler commented 5 months ago

Of course I will push out a version that addresses this eventually, even if HA doesn't, but I'm travelling for work this week and just don't have the time to do it, nor do I have my personal laptop or access to my standard dev/test setup to go through the normal development process until I get back home.. I will try to sneak 30 minutes to fix it this weekend and push out a new version, but, realistically, HA broke it in a stupid way, and, IMO, they should fix it. It makes zero sense to enforce a requirement for a code if no code is set.

Also, for automations, all you have to do is modify the automation to include a code (it's a checkbox in the UI or a single line in YAML), any random number will work since the backend doesn't care, it's the frontend that has decided to enforce a code even if one is not set, which it, quite interestingly, doesn't do for disarming, only for arming, just freaking illogical.

broyuken commented 5 months ago

As this breaks my card on my dashboard as well, I'll just stay rolled back for now. No rush, get to it when you can.

broyuken commented 5 months ago

Has anyone upgraded and checked if https://github.com/home-assistant/core/pull/118961 reverts the change for ring? I can't arm at the moment, too many kids running in and out :)

tsightler commented 5 months ago

Turns out this is impacting alarms exposed via other external integrations as well, such as those from HomeKit, so it's not just an issue with the MQTT alarm integration. They also apparently missed a bunch of other alarm integrations as well (see PR above). Just an incredibly poorly thought out change in general. Oh well, par for the course for HA, although at least it's only a couple of times a year these days, vs 4-5 years ago when they would break things in nearly every release. I still don't understand why they don't value stability more, but oh well.

I've pushed an update to the dev branch which should address this, at least, it seems to work for me. If anyone could temporarily test this branch by setting the branch option to "dev" and restarting the addon/container and provide some additional confirmation, I'll push out a production fix tomorrow. The dev branch currently has no other major changes, other than minor dependency updates, so it should be safe.

Scope666 commented 5 months ago

Has anyone upgraded and checked if home-assistant/core#118961 reverts the change for ring? I can't arm at the moment, too many kids running in and out :)

JUST tested, it's not fixed in 2024.6.1

image
TheLoudSteve commented 5 months ago

Has anyone upgraded and checked if home-assistant/core#118961 reverts the change for ring? I can't arm at the moment, too many kids running in and out :)

JUST tested, it's not fixed in 2024.6.1

image

I can confirm the same.

tsightler commented 5 months ago

I'm not sure why people keep reporting on HA version not working, that's up to HA people to fix, I can't do anything with that.

I'd ask to please keep comments to things related to this project and I'm looking for people to test the dev branch of ring-mqtt, as requested above.

On Fri, Jun 7, 2024, 4:59 PM TheLoudSteve @.***> wrote:

Has anyone upgraded and checked if home-assistant/core#118961 https://github.com/home-assistant/core/pull/118961 reverts the change for ring? I can't arm at the moment, too many kids running in and out :)

JUST tested, it's not fixed in 2024.6.1 [image: image] https://private-user-images.githubusercontent.com/31927547/337784401-c986c2ca-b8f3-45b1-b1f1-5833b1853c12.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTc3OTQyMjAsIm5iZiI6MTcxNzc5MzkyMCwicGF0aCI6Ii8zMTkyNzU0Ny8zMzc3ODQ0MDEtYzk4NmMyY2EtYjhmMy00NWIxLWIxZjEtNTgzM2IxODUzYzEyLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA2MDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNjA3VDIwNTg0MFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTNhZTk2YmI2MDM1M2UyZGEzZTc4ZGU5MTIwMDBjM2VlNmRmZDE0MzJkZTE5YzBhOGZiMWJlY2JkYjQ4YTg3MDImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.Dbuqg1H3AYZz6Qz_3WrqSaY_tMhB63GYW52UCAEBWgg

I can confirm the same.

— Reply to this email directly, view it on GitHub https://github.com/tsightler/ring-mqtt/issues/846#issuecomment-2155534378, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEHV7G4MSUBCFZVWP2JZI33ZGINMTAVCNFSM6AAAAABI4SN6DSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVGUZTIMZXHA . You are receiving this because you commented.Message ID: @.***>

broyuken commented 5 months ago

I’m not blaming you for it not being fixed in 2024.6.1. I just noticed in the changelog that there were some changes to the alarm control panel that may have fixed this issue so you wouldn’t have to do anything.

broyuken commented 5 months ago

Tested with dev on 2024.6.1 and confirm it works with both a lovelace card and an automation!

tsightler commented 5 months ago

@broyuken Yes, sorry, I knew your weren't blaming me, but i guess from my perspective there was no expectation that 2024.6.1 would fix it since there were no patches pushed to HA MQTT alarm control panel component. It came across way harsher in that message which I quickly typed on my phone than what I intended! Sorry about that.

Thanks for testing a confirming it works, I'll get v5.6.5 pushed out sometime tomorrow so at least it will work in the meantime.

broyuken commented 5 months ago

All good, this is what made me think that it could have been fixed.

Fix Alarm control panel not require code in several integrations (@gjohansson-ST - #118961)

TheLoudSteve commented 5 months ago

Just updated my docker container and can confirm the alarm code requirement is resolved. Thank you so much!

tsightler commented 5 months ago

Version 5.6.5 of both the docker and addon versions have been released and includes a workaround for this new HA behavior.