Add Favorites support to Media Browser for Squeezebox Integration

[peteS-UK]

trafficstars

Breaking change

Proposed change

Type of change

• [ ] Dependency upgrade
• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New integration (thank you!)
• [x] 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

• [x] The code change is tested and works locally.
• [x] Local tests pass. Your PR cannot be merged unless tests pass
• [x] There is no commented out code in this PR.
• [x] I have followed the development checklist
• [x] I have followed the perfect PR recommendations
• [x] 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.

To help with the load of incoming pull requests:

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

[home-assistant[bot]]

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.

[home-assistant[bot]]

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

Learn more about our pull request process.

[peteS-UK]

Thanks @joostlek Changes for CI made and CI now passing

[peteS-UK]

I have run these changes on my dev HA system against two LMS systems my dev and prod does what it sys on the tin no issues

Thanks for checking it @pssc . What's your thinking on the icons on the library main page - I changed them to use the normal icons from LMS for Artists, Albums, Playlists etc., rather than the default HA ones - since these are LMS Albums etc. - plus it matches the Fav icon I used. I think matching the icons to LMS here makes more sense than matching to the standard ones in HA - plus it means you get an icon in List view which you don't otherwise. What's your thinking?

[pssc]

I have run these changes on my dev HA system against two LMS systems my dev and prod does what it sys on the tin no issues

Thanks for checking it @pssc . What's your thinking on the icons on the library main page - I changed them to use the normal icons from LMS for Artists, Albums, Playlists etc., rather than the default HA ones - since these are LMS Albums etc. - plus it matches the Fav icon I used. I think matching the icons to LMS here makes more sense than matching to the standard ones in HA - plus it means you get an icon in List view which you don't otherwise. What's your thinking?

Yes I think it works. Never notice they didn't work in list view, but your right but then in all the other views there are no icons...

[peteS-UK]

@pssc I made some minor tweaks to library_payload - just to minimise the code duplication between favorites and not favorites categories. Nothing functional but just realised I was duping more than needed which might make maint or moving to the library at some stage more complex than needed.

[peteS-UK]

@pssc Hi Phil. I took another look at hierarchies today and figured out how to make it work - so this should now have support for folders, folders within folders etc.. Could you retest it please - seems to work fine on my side, but I don't actually use folders normally so might be missing something. I've pushed the PR back to draft while you take a look.

[pssc]

@pssc Hi Phil. I took another look at hierarchies today and figured out how to make it work - so this should now have support for folders, folders within folders etc.. Could you retest it please - seems to work fine on my side, but I don't actually use folders normally so might be missing something. I've pushed the PR back to draft while you take a look.

I don't use folders either but the latest changes haven't broken anything either... I might run up some test cases on the dev lms server tomorrow if I have time.

[peteS-UK]

@pssc Hi Phil. I took another look at hierarchies today and figured out how to make it work - so this should now have support for folders, folders within folders etc.. Could you retest it please - seems to work fine on my side, but I don't actually use folders normally so might be missing something. I've pushed the PR back to draft while you take a look.

I don't use folders either but the latest changes haven't broken anything either... I might run up some test cases on the dev lms server tomorrow if I have time.

Thanks - I'll move this back to Review now

[pssc]

I have thrown a few folders and albums and it it all works, sort of expected albums to be browsable like in LMS?

[peteS-UK]

I have thrown a few folders and albums and it it all works, sort of expected albums to be browsable like in LMS?

Hmm.. Not something I use, but I see what you mean that you can expand an album if it's in favs - so long as it's a local album. Most of my stuff is qobuz these days, and that's not expandable. Tricky bit is that the info you get back from an album in favorites says that it doesn't have items

  - id: 09b0635a.24
    name: 'Anthology: Through the Years'
    type: audio
    image: music/07c4d53a/cover.png
    isaudio: 1
    hasitems: 0

I think what it must do is look at the url and decide it's an album and therefore expandable. If I add the url into the query, I get

  - id: 33e7df65.24
    name: 'Anthology: Through the Years'
    type: audio
    image: music/07c4d53a/cover.png
    url: >-
      db:album.title=Anthology%3A%20Through%20the%20Years&contributor.name=Tom%20Petty
    isaudio: 1
    hasitems: 0

I'll take a look later and see if I can figure it out. I'd have thought and album favorite should have hasitems=1, but it doesn't work that way.

[pssc]

I have thrown a few folders and albums and it it all works, sort of expected albums to be browsable like in LMS?

Hmm.. Not something I use, but I see what you mean that you can expand an album if it's in favs - so long as it's a local album. Most of my stuff is qobuz these days, and that's not expandable. Tricky bit is that the info you get back from an album in favorites says that it doesn't have items

  - id: 09b0635a.24
    name: 'Anthology: Through the Years'
    type: audio
    image: music/07c4d53a/cover.png
    isaudio: 1
    hasitems: 0

I think what it must do is look at the url and decide it's an album and therefore expandable. If I add the url into the query, I get

  - id: 33e7df65.24
    name: 'Anthology: Through the Years'
    type: audio
    image: music/07c4d53a/cover.png
    url: >-
      db:album.title=Anthology%3A%20Through%20the%20Years&contributor.name=Tom%20Petty
    isaudio: 1
    hasitems: 0

I'll take a look later and see if I can figure it out. I'd have thought and album favorite should have hasitems=1, but it doesn't work that way.

Its a nice to have, I had only used single tracks up to now tbh, oh and dynamic playlists but those work

[peteS-UK]

Its a nice to have, I had only used single tracks up to now tbh, oh and dynamic playlists but those work

No, sadly not doable I think. The problem is that when we're browsing favorites, I have the favorite_id. If I want to expand an album, I need to know the album_id, but the call to get the favorites doesn't give me an album_id. I can't see a way to get from the favorite_id to the album_id with what's in the api. Playing the whole album (or playlist etc.) works fine because we're playing it via its favorite_id, not it's album_id or playlist_id.

As you say, it would have been nice, but not going to work as far as I can see.

[peteS-UK]

Hi @rajlaud. Any chance you could review this one - it replaces PR https://github.com/home-assistant/core/pull/117626 where the approach I'd taken using sources for favorites wasn't liked. This now uses the media browser, which gives us icon support and support for folders as added bonuses.

[peteS-UK]

Hi @peteS-UK thank you for your work on this. I like the approach of using the media browser. I'm going to update pysqueezebox to natively support favorites so we can call those methods rather than hitting the API directly using aync_query (see notes in code review).

Stand by and I'll be back with an update version of pysqueezebox shortly.

Hi Raj

Excellent. I tried to structure the changes I made in the same way as the library calls work, so hopefully it should be fairly straight forwards. I've been chatting to Michael H to see about expanding an album favorite to tracks. As I mentioned to Phil above, there's no single way to do it, but we can probably do it in a few steps (take the db:album url, get the title, text search for that, then filter that on contributor name if it exists to get an album object), so I'll take a look at that later as well.

[peteS-UK]

Hi @rajlaud . OK, I just added the ability to browse into a local album which is saved in favorites, as per @pssc comment previously. I think that's it now, so if you can add this all to the library, that'll be great I think.

[pssc]

Hi @rajlaud . OK, I just added the ability to browse into a local album which is saved in favorites, as per @pssc comment previously. I think that's it now, so if you can add this all to the library, that'll be great I think.

@peteS-UK you might need to do the PR for that one ;-)

[peteS-UK]

Hi @rajlaud . OK, I just added the ability to browse into a local album which is saved in favorites, as per @pssc comment previously. I think that's it now, so if you can add this all to the library, that'll be great I think.

@peteS-UK you might need to do the PR for that one ;-)

@pssc , I've pushed the change onto this PR. Once @rajlaud has made the library changes, I can take a look at doing a PR there if he doesn't get a chance to add this.

[pssc]

@peteS-UK testing some more with the browse of the album returned its not using the same library view as the player so for example I usually have ignore dups and present lossless only

[peteS-UK]

@peteS-UK testing some more with the browse of the album returned its not using the same library view as the player so for example I usually have ignore dups and present lossless only

Is it the same view as you get if you browse to the same album using the album category in media browser?

[rajlaud]

Almost there on the library and updates to this PR to use those calls.

I need to think more about the icons you added; may take them out for now. If we directly link to images on the LMS server that will not work when accessing home assistant from a computer that doesn't have access to your LMS server. We proxy images using track_id to address this, but it wouldn't be safe to proxy arbitrary internal URLs.

[peteS-UK]

Almost there on the library and updates to this PR to use those calls.

I need to think more about the icons you added; may take them out for now. If we directly link to images on the LMS server that will not work when accessing home assistant from a computer that doesn't have access to your LMS server. We proxy images using track_id to address this, but it wouldn't be safe to proxy arbitrary internal URLs.

Hmm - hadn't thought about the fact it was the client getting the icons rather than the addin, but it's just a url of course, so makes perfect sense. The reason I started looking for icons was that there doesn't seem to be a standard icon for favorites - the sonos one for example just uses the sonos logo from the HA website, so was looking for somehting meaningful to the LMS community. Also needed one for folders which I also couldn't see.

[rajlaud]

Ok, I have a version working here: https://github.com/rajlaud/home-assistant/tree/peteS-Squeezebox-MediaBrowser

I can't figure out how to open a PR to your branch, @peteS-UK, but you should be able to pull these changes into it.

[peteS-UK]

Hi @rajlaud . OK, thanks. I'll pull it over tomorrow and give it a test. I'll put this PR on hold while we test and make the changes to the PR.

Edit : I just pulled a version and it seems to work fine :-) thanks @rajlaud . I'll try it more thoroughly tomorrow and then update the PR.

[peteS-UK]

Hi @rajlaud . Found a small bug in your library. Some album favorites have a contributor.name, but some don't. Sometimes, you'll just get a url like

db:album.title=Anthology%3A%20Through%20the%20Years

rather than

db:album.title=Anthology%3A%20Through%20the%20Years&contributor.name=Tom%20Petty

So, in my _get_album_id, I did

    _album_seach_string = unquote(url)[15:].split("&contributor.name=")
    _album_title = _album_seach_string[0]
    _album_contributor = (
        _album_seach_string[1] if len(_album_seach_string) > 1 else None
    )

to get the contributor_name or set it to None and then

    if _album_contributor is None or _album_result["count"] == 1:
        _album_id = _album_result["albums_loop"][0]["id"]
    else:
        for _album in _album_result["albums_loop"]:
            if _album["artist"] == _album_contributor:
                _album_id = _album["id"]
                break
    return str(_album_id)

That seems to work fine, but with yours, I get a Browse error - unknown media Album/None.

I'm out today, but I'll try and take a look at the library later if I get a chance.

Edit Yes, I guess your conversion

        albums = await self.async_get_category("albums")
        for album in albums:
            if album["title"] == album_title:
                if album_contributor:
                    if album["artist"] == album_contributor:
                        return album["id"]
                else:
                    continue

doesn't cover the condition where we have a title match, but condition is none, and is you're else:continue at the wrong indent. Perhaps we just need

        albums = await self.async_get_category("albums")
        for album in albums:
            if album["title"] == album_title:
                if album_contributor:
                    if album["artist"] == album_contributor:
                        return album["id"]
                else:
                   return album["id"]
            else:
               continue

[pssc]

Yeah in my local collection contributor.name inst normally present

On Sat, 24 Aug 2024 at 12:29, peteS-UK @.***> wrote:

Hi @rajlaud https://github.com/rajlaud . Found a small bug in your library. Some album favorites have a contributor.name, but some don't. Sometimes, you'll just get a url like

db:album.title=Anthology%3A%20Through%20the%20Years

rather than

db:album.title=Anthology%3A%20Through%20the%20Years&contributor.name=Tom%20Petty

So, in my _get_album_id, I did

_album_seach_string = unquote(url)[15:].split("&contributor.name=")
_album_title = _album_seach_string[0]
_album_contributor = (
    _album_seach_string[1] if len(_album_seach_string) > 1 else None
)

to get the contributor_name or set it to None and then

if _album_contributor is None or _album_result["count"] == 1:
    _album_id = _album_result["albums_loop"][0]["id"]
else:
    for _album in _album_result["albums_loop"]:
        if _album["artist"] == _album_contributor:
            _album_id = _album["id"]
            break
return str(_album_id)

That seems to work fine, but with yours, I get a Browse error - unknown media Album/None.

I'm out today, but I'll try and take a look at the library later if I get a chance.

— Reply to this email directly, view it on GitHub https://github.com/home-assistant/core/pull/123953#issuecomment-2308361339, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANORYJEHVYOC7PZCW724X3ZTBVB3AVCNFSM6AAAAABMRAYHEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBYGM3DCMZTHE . You are receiving this because you were mentioned.Message ID: @.***>

-- I Can Resist Everything Except Temptation -- Oscar Wilde

[peteS-UK]

Yeah in my local collection contributor.name inst normally present

I think it's related to the version you added the favourites under. Michael said contributor.name was added to LMS versions a year or two ago.

[rajlaud]

Hi @rajlaud . Found a small bug in your library. Some album favorites have a contributor.name, but some don't. Sometimes, you'll just get a url like

db:album.title=Anthology%3A%20Through%20the%20Years

rather than

db:album.title=Anthology%3A%20Through%20the%20Years&contributor.name=Tom%20Petty

So, in my _get_album_id, I did

    _album_seach_string = unquote(url)[15:].split("&contributor.name=")
    _album_title = _album_seach_string[0]
    _album_contributor = (
        _album_seach_string[1] if len(_album_seach_string) > 1 else None
    )

to get the contributor_name or set it to None and then

    if _album_contributor is None or _album_result["count"] == 1:
        _album_id = _album_result["albums_loop"][0]["id"]
    else:
        for _album in _album_result["albums_loop"]:
            if _album["artist"] == _album_contributor:
                _album_id = _album["id"]
                break
    return str(_album_id)

That seems to work fine, but with yours, I get a Browse error - unknown media Album/None.

I'm out today, but I'll try and take a look at the library later if I get a chance.

Edit Yes, I guess your conversion

        albums = await self.async_get_category("albums")
        for album in albums:
            if album["title"] == album_title:
                if album_contributor:
                    if album["artist"] == album_contributor:
                        return album["id"]
                else:
                    continue

doesn't cover the condition where we have a title match, but condition is none, and is you're else:continue at the wrong indent. Perhaps we just need

        albums = await self.async_get_category("albums")
        for album in albums:
            if album["title"] == album_title:
                if album_contributor:
                    if album["artist"] == album_contributor:
                        return album["id"]
                else:
                   return album["id"]
            else:
               continue

Got it. Will fix and cut a new version.

[peteS-UK]

Got it. Will fix and cut a new version.

Thanks @rajlaud.

[rajlaud]

Got it. Will fix and cut a new version.

Thanks @rajlaud.

Done. Just bump the version to 0.8.1 in manifest.json.