bushvin / hass-integrations

My custom Home Assistant integrations
Apache License 2.0
64 stars 11 forks source link

New service: get search results instead of queueing them #71

Closed daniele-athome closed 3 months ago

daniele-athome commented 4 months ago

I needed the search service to return the results instead of queueing them, so I made this modification. It's obviously backwards incompatible, but I wanted to compare notes with you first.

At first, I tried to have two birds with one stone: I used SupportsResponse.OPTIONAL but I couldn't find a way to receive the ServiceCall object from HA: only service parameters were always passed to the service method. The ServiceCall object is needed to decide whether the service should return a response or not: using that, you could have a service that behaves both as an action and a function (i.e. something that returns data).

It could be a bug in HA (from https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/service.py#L942-L947):

    # If the service function is a string, we'll pass it the service call data
    if isinstance(func, str):
        data: dict | ServiceCall = remove_entity_service_fields(call)
    # If the service function is not a string, we pass the service call
    else:
        data = call

From that point on, the call: ServiceCall object is lost, I think - at least I couldn't find it anywhere else in the flow until the actual service method is called. Am I missing something here? Could it really be a HA bug?

If passing ServiceCall wouldn't be supported by HA API in any way, I would go about writing another service (like get_search_result or something), keeping backwards compatibility with the existing search service.

What do you think?

bushvin commented 4 months ago

Hey @daniele-athome , Can you provide an example on how your code is used?

daniele-athome commented 4 months ago

Sure! I use it in a HA script:

script:
  search_and_play_music:
    fields: 
      [...]
    sequence:
      - service: mopidy.search
        data:
          keyword_artist: "{{ keyword_artist }}"
          keyword_track_name: "{{ keyword_track_name }}"
          source: local
        target:
          entity_id: "{{ target }}"
        response_variable: music_tracks # service returns something and result will be put into this variable
      - if: "{{ music_tracks['media_player.music'].result|length > 0 }}"
        then:
          - service: media_player.play_media
            data:
              media_content_id: "{{ music_tracks['media_player.music'].result[0] }}"
              media_content_type: music
              enqueue: "{{ enqueue }}"
            target:
              entity_id: "{{ target }}"

target, enqueue, keyword_artist and keyword_track_name are parameters passed to the script by some other automation/intent/script/whatever.

I have some custom voice commands that need to interact with the Mopidy queue in a different way than what mopidy.search does now (that is, put the results at the end of the queue).

bushvin commented 4 months ago

That's an interesting use case, and actually fits more what I intended the service to be...

Maybe we it's worthwhile publishing it under a different name, so the original service is unaffected...

daniele-athome commented 4 months ago

Well, service responses are a relatively recent thing in HA, it probably didn't even exist when you wrote the search service.

I'll modify the PR as soon as I can.

daniele-athome commented 4 months ago

I've created a new service get_search_result: feel free to change its name to whatever you like.

I also took the liberty to refactor some code for the two search services because they had much code in common. I isolated the refactoring in a dedicated commit so pick it if it's to your liking.

bushvin commented 4 months ago

I will check it out in the morning!

daniele-athome commented 3 months ago

Closing in favour of #73.