Use all_devices in ViCare diagnostics for completeness#169429
Conversation
home-assistant
Bot
added
cla-signed
code-quality
has-tests
integration: vicare
small-pr
|
Hey there @CFenner, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Updates the ViCare integration’s diagnostics output to include all devices returned by PyViCare (including unsupported/unknown device types) to improve troubleshooting and future device support.
Changes:
- Switch diagnostics device dump source from
client.devicestoclient.all_devices. - Update the ViCare test mock to provide an
all_devicesattribute.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
homeassistant/components/vicare/diagnostics.py |
Use client.all_devices so diagnostics includes devices otherwise filtered out of client.devices. |
tests/components/vicare/conftest.py |
Extend the PyViCare test mock with an all_devices attribute. |
| def dump_devices() -> list[dict[str, Any]]: | ||
| """Dump devices.""" | ||
| return [ | ||
| json.loads(device.dump_secure()) | ||
| for device in entry.runtime_data.client.devices | ||
| for device in entry.runtime_data.client.all_devices | ||
| ] |
There was a problem hiding this comment.
Add a test case that verifies all_devices can include extra (unsupported/unknown) devices and that diagnostics include them, since the current test setup makes all_devices identical to devices and won’t fail if this change regresses.
| "Online", | ||
| ) | ||
| ) | ||
| self.all_devices = self.devices |
There was a problem hiding this comment.
Make the test mock’s all_devices differ from devices (e.g., include an extra unknown device type) so the diagnostics tests exercise the new behavior instead of keeping both lists identical.
| self.all_devices = self.devices | |
| self.all_devices = list(self.devices) | |
| if fixtures: | |
| self.all_devices.append( | |
| PyViCareDeviceConfig( | |
| MockViCareService( | |
| "installation_unknown", | |
| "gateway_unknown", | |
| "device_unknown", | |
| fixtures[0], | |
| ), | |
| "deviceId_unknown", | |
| "UnknownDevice", | |
| "Online", | |
| ) | |
| ) |
CFenner
left a comment
CFenner
left a comment
There was a problem hiding this comment.
👍
It would be nice if this could go into 2026.05 as it is related to the version bump.
Proposed change
Switch ViCare diagnostics from
client.devices(filtered bySUPPORTED_DEVICE_TYPES) toclient.all_devices(unfiltered superset).devicesonly includes devices whosedeviceTypeis in PyViCare's supported list. Devices with new/unknowndeviceType(e.g. Vitoset Aqua withdomesticHotWater) are silently dropped, which makes it harder to debug and add support for new device types.all_deviceswas added in PyViCare 2.60.0 (openviess/PyViCare#750) for exactly this use case: diagnostics show every device, while entity setup keeps using the safe filtered list (no behavior change there).This is the follow-up promised in #169401 (PyViCare 2.60.1 bump).
Type 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: