Add new attribute query_result_json to expose query_result as json in squeezebox

[peteS-UK]

…. python object

Proposed change

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes #
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Ruff (ruff format homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hi @peteS-UK

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[home-assistant]

Hey there @rajlaud, mind taking a look at this pull request as it has been labeled with an integration (squeezebox) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of squeezebox can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign squeezebox Removes the current integration label and assignees on the pull request, add the integration domain after the command.
• @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
• @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

Stale

[rajlaud]

This makes sense to me. Can you also update the docs to reflect this change?

[peteS-UK]

Of course. I'll do it now. Pete S From: Raj Laud ***@***.***> Sent: Tuesday, April 23, 2024 4:16 PM To: home-assistant/core ***@***.***> Cc: peteS-UK ***@***.***>; Mention ***@***.***> Subject: Re: [home-assistant/core] Add new attribute query_result_json to expose query_result as json (PR #115739) This makes sense to me. Can you also update the docs<https://www.home-assistant.io/integrations/squeezebox/> to reflect this change? - Reply to this email directly, view it on GitHub<#115739 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/API7QEIYZGK767KLZSHCSH3Y6Z3JJAVCNFSM6AAAAABGKW6VCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZSGY2TINRRHE>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

[rajlaud]

This will be a useful feature for scripting/automation. Thanks @peteS-UK

[peteS-UK]

Docs change PR is Documentation change for PR 115739 by peteS-UK * Pull Request #32436 * home-assistant/home-assistant.io (github.com)<home-assistant/home-assistant.io#32436> From: Raj Laud ***@***.***> Sent: Tuesday, April 23, 2024 4:16 PM To: home-assistant/core ***@***.***> Cc: peteS-UK ***@***.***>; Mention ***@***.***> Subject: Re: [home-assistant/core] Add new attribute query_result_json to expose query_result as json (PR #115739) This makes sense to me. Can you also update the docs<https://www.home-assistant.io/integrations/squeezebox/> to reflect this change? - Reply to this email directly, view it on GitHub<#115739 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/API7QEIYZGK767KLZSHCSH3Y6Z3JJAVCNFSM6AAAAABGKW6VCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZSGY2TINRRHE>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

[emontnemery]

Why is this needed? There's already a to_json template function which can be used to try parsing the existing query_result to JSON.

[MartinHjelmare]

The service can return the result as a service response if that's needed. But commands that return some state about the media player should probably just be made as sensor entities instead.

Closing this PR as it's not the correct approach.

[peteS-UK]

Hi Martin. The integration already returns the response as a service, but as a service response as an object. However, if you want to use this service with the API to allow an external app to use this response, it's much easier if the response (e.g. all of the albums on the media server) is available as json. So, the PR is to enable use of the integration with the home assistant API. If there's an existing way to do this easily, then that would be great - only a couple of lines in the integration, so would makes things simple for anyone trying to consume this through the api. For context, this service isn't really returning something about the state - it's to allow arbitrary queries to be passed to the server, and it uses the attributes to return the responses, so it's very useful for external API calls.

[MartinHjelmare]

You can read about service response here:
https://developers.home-assistant.io/docs/dev_101_services/#response-data

[rajlaud]

I'll look into changing squeezebox to use this approach. Will be a breaking change but looks like the right way to handle query results.

[peteS-UK]

I'll look into changing squeezebox to use this approach. Will be a breaking change but looks like the right way to handle query results.

Raj, I tried to avoid this in the PR to not be breaking and to follow the existing approach. How about we add a optional param to the service (returnResponse perhaps) default to false and return response only if its true, so it's not breaking.

[peteS-UK]

@rajlaud , Yes, that works fine, and is actually nicer for api since we're getting a return vs. having to poll for the attribute change. I'll send a PR tomorrow.

[peteS-UK]

Hi @rajlaud . OK, I've done a bit more digging, and there's a weird thing going on... I'd forgotten that the service call in the API is a POST, not a GET, and as such it doesn't return anything - or at least it shouldn't and when digging around, it doesn't return the returned data. However, what it does do, for some reason, is to return the entire state object of the entity if the state is updated. If the state isn't changed, it returns nothing, as I'd expect from a POST. So, for API purposes, the current integration (i.e. writing data to the attribute) actually already works perfectly and there's no need for a change. In fact, if you didn't write to the query_result attribute and just returned data, the POST call in the API doesn't return anything. So, from my perspective, I'd like to just close this one down and leave things as they are - this odd behaviour with the state object is actually just fine as a way of returning the query_result.