Skip to content

Add WiiM media player integration#148948

Merged
joostlek merged 146 commits intohome-assistant:devfrom
Linkplay2020:feature/wiim-integration
Mar 19, 2026
Merged

Add WiiM media player integration#148948
joostlek merged 146 commits intohome-assistant:devfrom
Linkplay2020:feature/wiim-integration

Conversation

@Linkplay2020
Copy link
Copy Markdown
Contributor

@Linkplay2020 Linkplay2020 commented Jul 17, 2025

Proposed change

Adds a new Home Assistant integration for WiiM devices.
This integration provides media player functionality for WiiM Streamer devices (e.g., WiiM Pro, WiiM Amp) via their UPnP/DLNA protocol and proprietary HTTP API.

Key features include:

  • Automatic discovery of WiiM devices on the network (via Zeroconf).
  • Media playback control (play, pause, stop, next track, previous track).
  • Volume control and mute toggling.
  • Retrieval of current playback status and media information.
  • Input source switching (e.g., Line-in, Bluetooth, Optical, etc.).
  • Support for auto reconnect to handle device IP changes or network interruptions.

This integration allows Home Assistant users to seamlessly integrate their WiiM devices into their smart home automations.

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

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:

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:

Copy link
Copy Markdown

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @WiimHome

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
Copy link
Copy Markdown

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.

Copy link
Copy Markdown

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Linkplay2020

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 home-assistant bot marked this pull request as draft July 18, 2025 02:47
@Linkplay2020
Copy link
Copy Markdown
Contributor Author

Linkplay2020 commented Jul 18, 2025

Proposed change

Adds a new Home Assistant integration for WiiM devices. This integration provides media player functionality for WiiM Streamer devices (e.g., WiiM Pro, WiiM Amp) via their UPnP/DLNA protocol and proprietary HTTP API.

Key features include:

  • Automatic discovery of WiiM devices on the network (via Zeroconf).
  • Media playback control (play, pause, stop, next track, previous track).
  • Volume control and mute toggling.
  • Retrieval of current playback status and media information.
  • Input source switching (e.g., Line-in, Bluetooth, Optical, etc.).
  • Support for auto reconnect to handle device IP changes or network interruptions.

This integration allows Home Assistant users to seamlessly integrate their WiiM devices into their smart home automations.

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

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:

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. https://github.com/Linkplay2020/wiim

To help with the load of incoming pull requests:

@Linkplay2020 Linkplay2020 marked this pull request as ready for review July 18, 2025 05:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 4 comments.


@callback
def _handle_sdk_av_transport_event(
self, service: UpnpService, state_variables: list[UpnpStateVariable]
Comment on lines +23 to +39
async def _async_probe_wiim_host(hass: HomeAssistant, host: str) -> WiimProbeResult:
"""Probe the given host and return WiiM device information."""
session = async_get_clientsession(hass)
location = f"http://{host}:{UPNP_PORT}/description.xml"
LOGGER.debug("Validating UPnP device at location: %s", location)
try:
probe_result = await async_probe_wiim_device(
location,
session,
host=host,
)
except TimeoutError as err:
raise CannotConnect from err

if probe_result is None:
raise CannotConnect
return probe_result
"homeassistant.components.media_source.async_browse_media",
AsyncMock(return_value=mock_browse_item),
) as mock_browse_media:
browse_result = await entity.async_browse_media(MEDIA_CONTENT_ID_ROOT)
| MediaPlayerEntityFeature.BROWSE_MEDIA
| MediaPlayerEntityFeature.PLAY_MEDIA
| MediaPlayerEntityFeature.SELECT_SOURCE
| MediaPlayerEntityFeature.GROUPING
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 1 comment.

Comment on lines +61 to +64
local_host = urlparse(base_url).hostname
if TYPE_CHECKING:
assert local_host is not None

class WiimConfigFlow(ConfigFlow, domain=DOMAIN):
"""Handle a config flow for WiiM."""

_discovered_info: WiimProbeResult | None = None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't assign it here, just make it a type annotation:

Suggested change
_discovered_info: WiimProbeResult | None = None
_discovered_info: WiimProbeResult

) -> ConfigFlowResult:
"""Handle user confirmation of discovered device."""
discovered_info = self._discovered_info
if user_input is not None and discovered_info is not None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discovered_info can't be None

Comment on lines +23 to +32
@pytest.fixture(autouse=True)
def mock_sdk_logger():
"""Mocks the LOGGER to prevent actual logging and allow assertion of calls."""
with (
patch("homeassistant.components.wiim.const.LOGGER.warning") as mock_warning,
patch("homeassistant.components.wiim.const.LOGGER.debug") as mock_debug,
patch("homeassistant.components.wiim.const.LOGGER.info") as mock_info,
patch("homeassistant.components.wiim.const.LOGGER.error") as mock_error,
):
yield mock_warning, mock_debug, mock_info, mock_error
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to patch the logger. It's not bad to log during tests. If there's something we should load from the logger, we can use caplog to capture logs

Comment on lines +35 to +38
@pytest.fixture
async def mock_hass(hass: HomeAssistant) -> HomeAssistant:
"""Return a real HomeAssistant instance for integration tests."""
return hass
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

"host": "192.168.1.100",
"udn": "uuid:test-udn-1234",
"name": "Test WiiM Device",
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use constants for the data keys and the domain

Comment on lines +48 to +52
"udn": "uuid:test-udn-1234",
"name": "Test WiiM Device",
},
title="Test WiiM Device",
source="user",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

udn and name aren't stored anymore. We also don't need source. However, udn is now set as the unique_id, so let's make sure the mock_config_entry reflects a real world config entry



@pytest.fixture
def mock_config_entry() -> ConfigEntry:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def mock_config_entry() -> ConfigEntry:
def mock_config_entry() -> MockConfigEntry:


assert result is True
mock_controller_cls.assert_called_once_with(mock_session)
assert hass.data[DATA_WIIM] == WiimData(controller=mock_controller)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't touch hass.data

Comment on lines +71 to +98
async def test_async_setup_entry_device_init_failure(
mock_hass: HomeAssistant,
mock_config_entry: ConfigEntry,
) -> None:
"""Test async_setup_entry when device initialization fails."""
mock_config_entry.add_to_hass(mock_hass)
mock_session = AsyncMock()

with (
patch(
"homeassistant.components.wiim.async_create_wiim_device",
side_effect=WiimDeviceException("Failed to initialize WiiM device"),
),
patch(
"homeassistant.components.wiim.async_get_clientsession",
return_value=mock_session,
),
patch("homeassistant.components.wiim.WiimController") as mock_controller_class,
patch(
"homeassistant.components.wiim.get_url",
return_value="http://192.168.1.10:8123",
),
):
mock_controller_instance = AsyncMock()
mock_controller_class.return_value = mock_controller_instance

with pytest.raises(ConfigEntryNotReady):
await async_setup_entry(mock_hass, mock_config_entry)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead have HA set it up, and assert the entry.state

Comment on lines +71 to +154
async def test_async_setup_entry_device_init_failure(
mock_hass: HomeAssistant,
mock_config_entry: ConfigEntry,
) -> None:
"""Test async_setup_entry when device initialization fails."""
mock_config_entry.add_to_hass(mock_hass)
mock_session = AsyncMock()

with (
patch(
"homeassistant.components.wiim.async_create_wiim_device",
side_effect=WiimDeviceException("Failed to initialize WiiM device"),
),
patch(
"homeassistant.components.wiim.async_get_clientsession",
return_value=mock_session,
),
patch("homeassistant.components.wiim.WiimController") as mock_controller_class,
patch(
"homeassistant.components.wiim.get_url",
return_value="http://192.168.1.10:8123",
),
):
mock_controller_instance = AsyncMock()
mock_controller_class.return_value = mock_controller_instance

with pytest.raises(ConfigEntryNotReady):
await async_setup_entry(mock_hass, mock_config_entry)


async def test_async_setup_entry_request_exception(
mock_hass: HomeAssistant,
mock_config_entry: ConfigEntry,
) -> None:
"""Test async_setup_entry when HTTP API request fails."""
mock_config_entry.add_to_hass(mock_hass)
mock_session = AsyncMock()

with (
patch(
"homeassistant.components.wiim.async_create_wiim_device",
side_effect=WiimRequestException("HTTP failure"),
),
patch(
"homeassistant.components.wiim.async_get_clientsession",
return_value=mock_session,
),
patch("homeassistant.components.wiim.WiimController") as mock_controller_class,
patch(
"homeassistant.components.wiim.get_url",
return_value="http://192.168.1.10:8123",
),
):
mock_controller_instance = AsyncMock()
mock_controller_class.return_value = mock_controller_instance

with pytest.raises(ConfigEntryNotReady):
await async_setup_entry(mock_hass, mock_config_entry)


async def test_async_setup_entry_no_url_available(
mock_hass: HomeAssistant,
mock_config_entry: ConfigEntry,
) -> None:
"""Test async_setup_entry raises when Home Assistant URL is unavailable."""
mock_config_entry.add_to_hass(mock_hass)
mock_session = AsyncMock()

with (
patch(
"homeassistant.components.wiim.async_get_clientsession",
return_value=mock_session,
),
patch("homeassistant.components.wiim.WiimController") as mock_controller_class,
patch(
"homeassistant.components.wiim.get_url",
side_effect=NoURLAvailableError,
),
):
mock_controller_instance = AsyncMock()
mock_controller_class.return_value = mock_controller_instance

with pytest.raises(ConfigEntryNotReady):
await async_setup_entry(mock_hass, mock_config_entry)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 3 tests can be parametrized

Comment on lines +163 to +203
mock_session = AsyncMock()
hass.config_entries.async_forward_entry_setups = AsyncMock(return_value=True)

unload_callbacks: list[object] = []

def _capture_callback(cb):
unload_callbacks.append(cb)

mock_config_entry.async_on_unload = MagicMock(side_effect=_capture_callback)

with (
patch("homeassistant.components.wiim.WiimController") as mock_controller_cls,
patch(
"homeassistant.components.wiim.async_create_wiim_device"
) as mock_create_wiim_device,
patch(
"homeassistant.components.wiim.async_get_clientsession",
return_value=mock_session,
),
patch(
"homeassistant.components.wiim.get_url",
return_value="http://192.168.1.10:8123",
),
):
mock_controller = MagicMock()
mock_controller.add_device = AsyncMock()
mock_controller.remove_device = AsyncMock()
mock_controller_cls.return_value = mock_controller

mock_device = MagicMock()
mock_device.udn = "test-udn"
mock_device.name = "Test Device"
mock_device.disconnect = AsyncMock()
mock_create_wiim_device.return_value = mock_device

result = await async_setup_entry(hass, mock_config_entry)

assert result is True
assert len(unload_callbacks) == 2

await unload_callbacks[1]()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't patch internals, ask HA to setup the integration and to unload it. We can then assert on the mocks if the right methods have been called

Comment on lines +209 to +250
async def test_async_unload_entry_success(
mock_hass: HomeAssistant,
mock_config_entry: ConfigEntry,
mock_wiim_device: WiimDevice,
mock_wiim_controller: WiimController,
) -> None:
"""Test successful unloading of a config entry."""
mock_config_entry.runtime_data = mock_wiim_device

mock_hass.data[DATA_WIIM] = WiimData(
controller=mock_wiim_controller,
entity_id_to_udn_map={"media_player.test": "uuid:123"},
)

mock_hass.config_entries.async_unload_platforms = AsyncMock(return_value=True)

with patch.object(
mock_hass.config_entries, "async_loaded_entries", return_value=[]
) as mock_loaded_entries:
result = await async_unload_entry(mock_hass, mock_config_entry)

assert result is True

mock_loaded_entries.assert_called_once_with(DOMAIN)

assert DATA_WIIM not in mock_hass.data

mock_hass.config_entries.async_unload_platforms.assert_awaited_once_with(
mock_config_entry, PLATFORMS
)


async def test_async_unload_entry_returns_false(
mock_hass: HomeAssistant,
mock_config_entry: ConfigEntry,
) -> None:
"""Test async_unload_entry returns False when platform unload fails."""
mock_hass.config_entries.async_unload_platforms = AsyncMock(return_value=False)

result = await async_unload_entry(mock_hass, mock_config_entry)

assert result is False
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use internals

Comment on lines +47 to +57
def _set_wiim_data(
hass: HomeAssistant,
*,
controller: MagicMock | None = None,
entity_id_to_udn_map: dict[str, str] | None = None,
) -> None:
"""Populate the typed WiiM domain data for a test Home Assistant instance."""
hass.data[DATA_WIIM] = WiimData(
controller=controller or MagicMock(),
entity_id_to_udn_map=entity_id_to_udn_map or {},
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't touch internals. Instead the controller seems to be a library component, so that would be patched out, meaning we can interact with it and change its behaviour

Comment on lines +67 to +69
mock_config_entry.runtime_data = mock_wiim_device
_set_wiim_data(mock_hass)
await async_setup_entry(mock_hass, mock_config_entry, mock_add_entities)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry.runtime_data is also considered an internal. Instead we should have HA setup the integration

Comment on lines +71 to +111
mock_add_entities.assert_called_once()
entities = mock_add_entities.call_args[0][0]
assert len(entities) == 1
entity = entities[0]
assert isinstance(entity, WiimMediaPlayerEntity)
assert entity._device is mock_wiim_device


async def test_media_player_attributes(
mock_wiim_media_player_entity: WiimMediaPlayerEntity, mock_wiim_device: WiimDevice
) -> None:
"""Test media player entity attributes."""
entity = mock_wiim_media_player_entity
entity._device = mock_wiim_device
entity._attr_name = mock_wiim_device.name # type: ignore[assignment]
entity._attr_unique_id = mock_wiim_device.udn
entity._attr_device_info = {
"identifiers": {(DOMAIN, mock_wiim_device.udn)},
"name": mock_wiim_device.name,
}
entity._transport_capabilities = None
entity._attr_device_class = MediaPlayerDeviceClass.SPEAKER
entity._attr_state = MediaPlayerState.IDLE
entity._attr_volume_level = mock_wiim_device.volume / 100
entity._attr_is_volume_muted = mock_wiim_device.is_muted
entity._attr_repeat = RepeatMode.OFF
entity._attr_source = InputMode.LINE_IN.display_name # type: ignore[attr-defined]

assert entity.unique_id == mock_wiim_device.udn
assert entity.name == mock_wiim_device.name
assert entity.device_info is not None
assert entity.device_info["identifiers"] == {(DOMAIN, mock_wiim_device.udn)}
assert entity.device_info["name"] == mock_wiim_device.name
assert entity.supported_features == SUPPORT_WIIM_BASE
assert entity.device_class == MediaPlayerDeviceClass.SPEAKER

assert entity.state == MediaPlayerState.IDLE
assert entity.volume_level == mock_wiim_device.volume / 100
assert entity.is_volume_muted == mock_wiim_device.is_muted
assert entity.repeat == RepeatMode.OFF
assert entity.source == InputMode.LINE_IN.display_name # type: ignore[attr-defined]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, replace this with a snapshot test and run pytest ./tests/components/wiim --snapshot-update as that will fixate the entity registry entry and the state in a .ambr file which also should be committed

Comment on lines +114 to +152
async def test_media_player_update_ha_state_from_sdk_cache(
mock_wiim_media_player_entity: WiimMediaPlayerEntity,
mock_wiim_device: WiimDevice,
mock_hass: HomeAssistant,
) -> None:
"""Test _update_ha_state_from_sdk_cache schedules HA state update."""
entity = mock_wiim_media_player_entity
_set_wiim_data(mock_hass)

entity.hass = mock_hass

mock_wiim_device.playing_status = PlayingStatus.PLAYING
mock_wiim_device.volume = 60 # 0.6
mock_wiim_device.current_media = WiimMediaMetadata(title="New Song")
mock_wiim_device.available = True # type: ignore[misc]

entity._device = mock_wiim_device
entity.entity_id = "media_player.test_device"
entity._attr_group_members = ["media_player.test_device", "media_player.follower1"]

with (
patch.object(entity, "schedule_update_ha_state", new=MagicMock()),
patch.object(entity, "async_write_ha_state", new=MagicMock()),
patch.object(
entity.hass.data[DATA_WIIM].controller,
"get_group_snapshot",
new=MagicMock(
return_value=MagicMock(
role=WiimGroupRole.LEADER,
member_udns=(mock_wiim_device.udn,),
)
),
),
):
entity._update_ha_state_from_sdk_cache()

assert entity.state == MediaPlayerState.PLAYING
assert entity.volume_level == 0.6
assert entity.media_title == "New Song"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead this should be triggered naturally

Comment on lines +155 to +177
async def test_media_player_update_ha_state_from_sdk_cache_unavailable(
mock_wiim_media_player_entity: WiimMediaPlayerEntity,
mock_wiim_device: WiimDevice,
mock_hass: HomeAssistant,
) -> None:
"""Test unavailable devices clear state and write HA state."""
entity = mock_wiim_media_player_entity
_set_wiim_data(mock_hass)
entity.hass = mock_hass
entity._device = mock_wiim_device
entity._attr_media_title = "Old"
entity._attr_source = "Old Source"
entity._transport_capabilities = WiimTransportCapabilities(can_next=True)
mock_wiim_device.available = False # type: ignore[misc]

with patch.object(entity, "async_write_ha_state", new=MagicMock()) as mock_write:
entity._update_ha_state_from_sdk_cache()

assert entity.state is None
assert entity.media_title is None
assert entity.source is None
assert entity._transport_capabilities is None
mock_write.assert_called_once()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem. If it's unavailable, we should modify the mock to behave like it's unavailable, and then assert the behavior we expect when this is the case

Comment on lines +237 to +284
async def test_media_player_async_added_to_hass_refreshes_supported_features(
mock_wiim_media_player_entity: WiimMediaPlayerEntity,
mock_hass: HomeAssistant,
) -> None:
"""Test entity setup refreshes supported features before initial state write."""
entity = mock_wiim_media_player_entity
entity.hass = mock_hass
entity.entity_id = "media_player.test_device"
_set_wiim_data(mock_hass)

with (
patch(
"homeassistant.helpers.entity.Entity.async_added_to_hass",
new=AsyncMock(),
),
patch.object(
entity, "_from_device_update_supported_features", new_callable=AsyncMock
) as mock_refresh_supported_features,
patch.object(
entity, "_update_ha_state_from_sdk_cache", new=MagicMock()
) as mock_update_state,
patch.object(
entity, "async_write_ha_state", new=MagicMock()
) as mock_write_state,
):
await entity.async_added_to_hass()

assert (
entity._device.general_event_callback
== entity._handle_sdk_general_device_update
)
assert (
entity._device.av_transport_event_callback
== entity._handle_sdk_av_transport_event
)
assert (
entity._device.rendering_control_event_callback
== entity._handle_sdk_refresh_event
)
assert entity._device.play_queue_event_callback == entity._handle_sdk_refresh_event
assert (
entity._wiim_data.entity_id_to_udn_map[entity.entity_id] == entity._device.udn
)
mock_refresh_supported_features.assert_awaited_once_with(write_state=False)
mock_update_state.assert_called_once_with(
write_state=False, update_supported_features=False
)
mock_write_state.assert_not_called()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would already be tested with the snapshot test

Comment on lines +471 to +481
event = Event(
"entity_registry_updated",
{
"action": "update",
"old_entity_id": "media_player.old",
"entity_id": "media_player.new",
},
)

with patch.object(Entity, "_async_registry_updated", new=MagicMock()):
entity._async_registry_updated(event)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead we should ask the entity registry to change the entity_id

@davidanthoff
Copy link
Copy Markdown

I'm still confused why the strategy here is to go with this PR instead of just moving the excellent https://github.com/mjcumming/wiim into core? The latter works for way more devices, is actually stable and ready etc?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 4 comments.

Comment on lines +594 to +606
elif media_type in {MediaType.MUSIC, MEDIA_TYPE_WIIM_LIBRARY}:
if not media_id.isdigit():
raise ServiceValidationError(f"Invalid preset ID: {media_id}")

preset_number = int(media_id)
await target_device.play_preset(preset_number)
self._attr_media_content_id = f"wiim_preset_{preset_number}"
self._attr_media_content_type = MediaType.PLAYLIST
self._attr_state = MediaPlayerState.PLAYING
elif media_type == MediaType.TRACK:
if not media_id.isdigit():
raise ServiceValidationError(
f"Invalid media_id: {media_id}. Expected a valid track index."
Comment on lines +620 to +621
await self._device.async_set_loop_mode(
self._device.build_loop_mode(WiimRepeatMode(repeat), self._attr_shuffle)
Comment on lines +625 to +634
async def async_set_shuffle(self, shuffle: bool) -> None:
"""Enable/disable shuffle mode."""
repeat = self._attr_repeat or WiimRepeatMode.OFF
await self._device.async_set_loop_mode(
self._device.build_loop_mode(
WiimRepeatMode(repeat),
shuffle,
)
)

@media_player_exception_wrap
async def async_select_source(self, source: str) -> None:
"""Select input mode."""
await self._device.async_set_play_mode(source)
@mjcumming
Copy link
Copy Markdown

mjcumming commented Mar 17, 2026

For visibility, there is already an existing Home Assistant WiiM integration here:

https://github.com/mjcumming/wiim

It is already in active use, with about 700 installs, and the repo currently has 88 GitHub stars.

That integration already covers a fairly broad set of WiiM/LinkPlay-specific behavior, including:

  • multiroom/group handling
  • EQ support, including dynamic preset handling
  • firmware update support
  • source and output mode handling
  • TTS / announce behavior
  • device- and firmware-specific capability detection and compatibility fixes

I think it is important that reviewers and maintainers have visibility into the fact that there is already a fairly mature, actively maintained implementation with a real user base, so any decision about a separate core implementation can be made with that context in mind.

@balloob-travel
Copy link
Copy Markdown
Contributor

@mjcumming we are aware and thanks for your work on it.

It is always better to have a built-in integration in Home Assistant over a custom one that lives in HACS. Built-in allows Home Assistant to automatically discover the integration and offer set-up to the user. Although today it will not be as good as the custom one, we hope that over time we can match all the functions.

New integrations always start with the bare minimum, to make it easier to review (as you can see from this PR, even minimal functionality is a lot of review work!).

We don't allow contributors to contribute work from other people, which is why this integration has been built from scratch. I agree that time could have probably been saved if the efforts were combined. It would be great if we can collaborate on the built-in integration in the future.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 5 comments.

self._device.current_track_duration = 0
self._attr_media_position_updated_at = None
self._attr_media_duration = None
self._attr_media_position = None
Comment on lines +57 to +61
async def test_state_machine_updates_from_device_callbacks(
hass: HomeAssistant,
mock_wiim_device,
init_wiim_media_player,
) -> None:
Comment on lines +94 to +96
async def test_user_flow_already_configured(
hass: HomeAssistant, mock_config_entry
) -> None:
Comment on lines +168 to +170
async def test_zeroconf_flow_already_configured(
hass: HomeAssistant, mock_config_entry
) -> None:
Comment on lines +81 to +86
LOGGER.info(
"WiiM device %s (UDN: %s) linked to HASS. Name: '%s', HTTP: %s, UPnP Location: %s",
entry.entry_id,
wiim_device.udn,
wiim_device.name,
host,
@mjcumming
Copy link
Copy Markdown

mjcumming commented Mar 20, 2026

I think the goal makes sense on paper: one integration path that works well for LinkPlay‑based gear (including WiiM) is better UX than a maze of partially overlapping options.

The tension is what "one integration" should sit on technically. The LinkPlay code that landed in core (migrated from the older user integration) is in a different era than how HA does things today—e.g. async patterns, structure, and ongoing maintenance burden. So "just extend that forever" is not obviously cheaper than "replace the transport/library layer with something actively maintained," if we are honest about long‑term fit.

pywiim was not written as a "WiiM‑only" fork of one OEM stack; it is aimed at the broader LinkPlay ecosystem (many brands/devices) with device‑specific features where the API exposes them (WiiM‑only capabilities where applicable). It has also seen a lot of real‑world use outside core (custom integration install counts, diverse hardware), which is the kind of production proof you would want before trusting a shared library.

If the preference is still another official Python package or a WiiM‑branded integration path, that is a product choice—but then it is a bit contradictory to also argue for one core component without a plan to retire or modernize the legacy LinkPlay foundation. Leaving users with "old core LinkPlay + new efforts side by side" risks the same fragmentation Home Assistant already has in other domains (e.g. multiple GPIO / device families where users have to guess which integration is actually maintained).

So the constructive path, in my view: either invest in bringing one modern, well‑tested shared library into the story the core integration is built on—including willingness to replace what is outdated—or accept that several paths will exist and focus on clear documentation so users know which one is recommended and supported. Happy to collaborate on technical alignment either way.

@balloob-travel
Copy link
Copy Markdown
Contributor

balloob-travel commented Mar 20, 2026 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.