vinteo / hass-opensprinkler

OpenSprinkler Integration for Home Assistant
MIT License
204 stars 40 forks source link

Fix broken services in HA `2024.2.1` #279

Closed DylanGriffith closed 6 months ago

DylanGriffith commented 6 months ago

In Home Assistant 2024.2.1 this service is always erroring with:

Failed to call service opensprinkler/run. 'list' object has no attribute 'get'

The stacktrace for this error is below:

2024-02-10 12:46:23.684 ERROR (MainThread) [homeassistant.components.websocket_api.http.connection] [<REDACTED>] 'list' object has no attribute 'get'
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/components/websocket_api/commands.py", line 240, in handle_call_service
    response = await hass.services.async_call(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 2279, in async_call
    response_data = await coro
                    ^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/core.py", line 2316, in _execute_service
    return await target(service_call)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/opensprinkler/__init__.py", line 108, in _async_send_run_command
    await hass.helpers.service.entity_service_call(
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 843, in entity_service_call
    entity_candidates = _get_permissible_entity_candidates(
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/service.py", line 796, in _get_permissible_entity_candidates
    and (entity := entities.get(single_entity)) is not None
                   ^^^^^^^^^^^^
AttributeError: 'list' object has no attribute 'get'

After some investigation I found that the function signature for entity_service_call was changed in https://github.com/home-assistant/core/pull/106759 from Iterable[EntityPlatform] to dict[str, Entity].

It's not clear to me if this is some internal API that cannot be relied upon by custom components as it does not seem this was a widely communicated API change. But in any case it seems this change fixes the problem for me.

This new function async_get_entities was copied from a similar change in that PR except I removed the type signature because it required importing Entity.

Please let me know if there is a simpler way to fix this as I'm not adept at writing Python code.

curlydingo commented 6 months ago

Just tested, looks good to me. I think this solution will do for now, though ultimately as @EdLeckert said previously we probably need a better approach to specifying these entities for services in the first place, since the current approach is a bit confusing on the GUI and is slow to execute.

This should close #278 too.

DylanGriffith commented 6 months ago

I suppose we also need to consider version compatibility here.

This will only work with 2024.2.1 onwards. Meanwhile the previous code only works with 2024.1.2 and earlier.

Do we want to somehow build a version compatible with both?

Is there a way we can version this so that people won't be able to update this before updating home assistant? Maybe it doesn't matter which one they upgrade first because either way this is broken if they've updated HA before getting this update.

curlydingo commented 6 months ago

Is there a way we can version this so that people won't be able to update this before updating home assistant? Maybe it doesn't matter which one they upgrade first because either way this is broken if they've updated HA before getting this update.

We can update the content of hacs.json to require 2024.2.0? I've never actually used it before so might be a call for @vinteo though.

curlydingo commented 6 months ago

Also would you mind fixing the linting @DylanGriffith? Couple of blank lines is all it needs.

Thanks

sonarcloud[bot] commented 6 months ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

vinteo commented 6 months ago

We can update in the hacs.json but it's not enforced. But I don't think it is worth to maintain backwards compatibility, people just need to update

On Sun, Feb 11, 2024, 6:19 PM curlydingo @.***> wrote:

Is there a way we can version this so that people won't be able to update this before updating home assistant? Maybe it doesn't matter which one they upgrade first because either way this is broken if they've updated HA before getting this update.

We can update the content of hacs.json to require 2024.2.0? I've never actually used it before so might be a call for @vinteo https://github.com/vinteo though.

— Reply to this email directly, view it on GitHub https://github.com/vinteo/hass-opensprinkler/pull/279#issuecomment-1937457240, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGID73Z3G5MIPXZ6Q3HEVLYTBWHRAVCNFSM6AAAAABDDAZID6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZXGQ2TOMRUGA . You are receiving this because you were mentioned.Message ID: @.***>

curlydingo commented 6 months ago

Probably couldn’t hurt though? I suggest we update HACS.json and the version number and get it released soon, as I think it’s broken for everyone at the moment.

I’m obviously pretty new here so I’ll leave releases to you unless you say otherwise.

vinteo commented 6 months ago

yea sorry I mean let's do the hsac.json change.

vinteo commented 6 months ago

Thank you @DylanGriffith