rajlaud / pysqueezebox

Python interface to Logitech Media Server for Home Assistant Squeezebox integration
Apache License 2.0
15 stars 5 forks source link

Add modelname to Player class #8

Closed peteS-UK closed 2 months ago

peteS-UK commented 2 months ago

Small PR to add modelname to the Player class. The players status query returns the modelname, so I thought we might as well expose this in the Player class so that we can include it in the media_player device.

peteS-UK commented 2 months ago

Thanks Raj. When you create I guess 0.8.2, I'll make a pr for the media_player.py to use this.

From: Raj Laud @.> Sent: 06 September 2024 13:15 To: rajlaud/pysqueezebox @.> Cc: peteS-UK @.>; Mention @.> Subject: Re: [rajlaud/pysqueezebox] Add modelname to Player class (PR #8)

@rajlaud approved this pull request.

Makes sense to me. Thanks @peteS-UKhttps://github.com/peteS-UK .

- Reply to this email directly, view it on GitHubhttps://github.com/rajlaud/pysqueezebox/pull/8#pullrequestreview-2286126814, or unsubscribehttps://github.com/notifications/unsubscribe-auth/API7QENBZVW4TG37AEOR4MLZVGMFXAVCNFSM6AAAAABNYJPAOGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOBWGEZDMOBRGQ. You are receiving this because you were mentioned.Message ID: @.**@.>>

peteS-UK commented 1 month ago

Hi @rajlaud . Great work on all the tests - they don't make that stuff simple do they! Now that things are mostly settled again (I know Phil is still working on the update entity), I wondered if you could create a 0.8.2 so I can test and then PR the model name change. Thanks

rajlaud commented 1 month ago

Yes, almost there. It is going to be version 0.9.0 and also include alarm clock support (I got a PR on that right around the same time).

peteS-UK commented 1 month ago

Yes, almost there. It is going to be version 0.9.0 and also include alarm clock support (I got a PR on that right around the same time).

Thanks Raj

peteS-UK commented 1 month ago

Hi @rajlaud . Thanks for doing 0.9.2. Change to media_player is just in SqueezeBoxEntity init

    def __init__(self, player: Player, server: Server) -> None:
        """Initialize the SqueezeBox device."""
        self._player = player
        self._query_result: bool | dict = {}
        self._remove_dispatcher: Callable | None = None
        self._attr_unique_id = format_mac(player.player_id)
        if player.model == "SqueezeLite":
            _manufacturer = "https://github.com/ralph-irving/squeezelite"
        elif (
            "Squeezebox" in player.model
            or "Transporter" in player.model
            or "Slim" in player.model
        ):
            _manufacturer = "Logitech"
        elif "Transporter" in player.model:
            _manufacturer = "Logitech"
        elif "SqueezePlay" in player.model:
            _manufacturer = "https://github.com/ralph-irving/squeezeplay"
        else:
            _manufacturer = ""

        self._attr_device_info = DeviceInfo(
            identifiers={(DOMAIN, self._attr_unique_id)},
            name=player.name,
            connections={(CONNECTION_NETWORK_MAC, self._attr_unique_id)},
            via_device=(DOMAIN, server.uuid),
            model=player.model,
            manufacturer=_manufacturer,
        )

and then you see

image

but it needs a change to the requirements in manifest, which I think only the code owner can make. Could you maybe submit the PR, or will you be submitted a PR just to change the manifest.json perhaps - which seemed to be what they were asking for, and then I'll submit the PR for the model?

rajlaud commented 1 month ago

Yep, I opened the manifest PR here: https://github.com/home-assistant/core/pull/126347

peteS-UK commented 1 month ago

Perfect. Sorry, didn't see that one. Thanks very much. Once that one's merged, I'll put in the PR for the model and manufacturer.

peteS-UK commented 1 month ago

OK PR is https://github.com/home-assistant/core/pull/126435