Resolve WiiM UPnP event callback host from the route to the device - #177667
Open
markfrancisonly wants to merge 4 commits into
Open
Resolve WiiM UPnP event callback host from the route to the device#177667markfrancisonly wants to merge 4 commits into
markfrancisonly wants to merge 4 commits into
Conversation
The callback host came from get_url(), which resolves to hass.config.api.local_ip when no internal URL is configured. That value comes from network.async_get_source_ip, which returns all_ipv4s[0] whenever the routed source address is not itself an enabled adapter. The first enabled adapter has no relationship to the route to any given device, and on a container host it can be an interface the device cannot reach. The GENA subscription still succeeds in that case, so nothing errors and nothing is logged, but no NOTIFY is ever delivered: the media player reports no title, artist or artwork and its transport state never updates. Resolve the address per device with async_get_local_ip against the device's own UPnP location, as dlna_dmr already does, keeping the Home Assistant URL as the fallback when the route cannot be resolved.
home-assistant
Bot
added
bugfix
cla-signed
has-tests
integration: wiim
small-pr
Contributor
|
Hey there @Linkplay2020, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Contributor
There was a problem hiding this comment.
Pull request overview
Routes WiiM UPnP callbacks through the interface that reaches each device.
Changes:
- Resolves callback IPs from device routes with URL fallback.
- Adds dependency metadata and setup tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/wiim/__init__.py |
Uses route-derived callback host. |
homeassistant/components/wiim/util.py |
Adds callback host resolution and fallback. |
homeassistant/components/wiim/manifest.json |
Declares direct UPnP dependency. |
requirements_all.txt |
Records WiiM dependency usage. |
tests/components/wiim/conftest.py |
Mocks route resolution. |
tests/components/wiim/test_init.py |
Tests route lookup and failure handling. |
async_get_local_ip returns the family that routes to the device, and AiohttpNotifyServer brackets IPv6 callback URLs itself, so the resolved address is used verbatim. Pin that so no address parsing is introduced here later.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tests/components/wiim/test_init.py:84
- Assert that the resolved IP is forwarded to
async_create_wiim_deviceaslocal_host. This currently verifies only that route lookup occurred, so it would still pass if setup ignored the result and advertised a different callback host.
mock_local_ip.assert_awaited_once_with(
"http://192.168.1.100:49152/description.xml", hass.loop
)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
The UPnP event callback host was taken from
get_url(), which resolves tohass.config.api.local_ipwhen no internal URL is configured. That value comes fromnetwork.async_get_source_ip, which returnsall_ipv4s[0]whenever the routed source address is not itself an enabled adapter. The first enabled adapter has no relationship to the route to any given device, so on a multi-homed host it can be an address the device cannot reach.The GENA subscription still succeeds in that case, so nothing errors and nothing is logged, but no NOTIFY is ever delivered: the media player reports no
media_title,media_artistorentity_picture, and its transport state never updates.This resolves the address per device with
async_get_local_ipagainst the device's own UPnP location, which is whatdlna_dmralready does with the sameasync_upnp_clientdependency:https://github.com/home-assistant/core/blob/dev/homeassistant/components/dlna_dmr/media_player.py#L376-L379
async_setup_entryalready buildsupnp_locationimmediately above, so it is a drop-in. The Home Assistant URL is kept as the fallback when the route cannot be resolved, so the existingConfigEntryNotReadybehaviour is unchanged.Measured on the affected host (3 interfaces, device reachable only via
eth2):eth1192.168.20.5(unreachable)192.168.30.5eth1,eth2192.168.30.5192.168.30.5eth1,eth0172.30.33.5(unreachable)192.168.30.5Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: