Fix priority endpoint field names to match Resideo's live API schema - #165
Conversation
LyricPriority/CurrentPriority/LyricRoom/LyricAccessory were reading
JSON keys that don't match what the /devices/thermostats/{id}/priority
endpoint actually returns. Captured directly from a live Resideo
account (T9-T10 thermostat with paired RCHTSENSOR accessories):
currentPriority -> priority
status -> priorityStatus (top-level priority status)
roomName -> name (per room)
roomAvgTemp -> avgTemperature (per room)
roomAvgHumidity -> avgHumidity (per room)
type -> sensorType (per accessory)
excludeTemp -> excludeTemperature (per accessory)
Because current_priority previously read a nonexistent key, it
silently defaulted to an empty object instead of raising - so every
consumer got an empty room list with no error at all. This is what
made room sensor/priority data appear to work (no exceptions) while
actually returning nothing for any real thermostat.
Updated the RESPONSE_JSON_PRIORITY test fixture and its assertions in
tests/objects/test_priority.py to match the corrected schema; the
previous fixture encoded the same wrong keys the code was reading, so
tests passed without ever catching the mismatch against a real API
response.
Not changed: the currentPriority key sent by update_priority() (the
POST body for setting room priority) is a separate, unverified code
path - no live capture of that request/response yet to confirm it's
also wrong.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resideo's API returns "scheduleSubtype" (lowercase t) for both device and location responses, not "scheduleSubType". Found while auditing device.py for the same class of field-name mismatch fixed in the priority endpoint (timmo001#165). Not currently consumed anywhere in Home Assistant's lyric integration, so this was never visibly broken, but the fixtures encoded the same wrong key the code was reading, so the existing tests passed without ever catching it against a real response.
joostlek pushed back on shipping xfail-marked entities directly (vacation_hold, room_motion), consistent with abmantis's separate "keep as draft until the dependency lands" stance on the Priority Status sensor PR. Rather than argue two reviewers down independently, split scope: this PR now ships only device_pairing_enabled, which works today against the currently-pinned aiolyric. vacation_hold and room_motion move to a held-back branch (binary-sensor-vacation-hold-room-motion) to resume once timmo001/aiolyric#165 and the vacation-hold-key fix release. - Remove ACCESSORY_BINARY_SENSORS/LyricAccessoryBinarySensor and the vacation_hold DEVICE_BINARY_SENSORS entry from binary_sensor.py, and their strings.json entries - dead code with only one entity left. - Drop the now-unused priority.json fixture and the rooms_dict/ priorities_dict setup in mock_lyric_api; nothing left needs room data. - Replace the three hand-written entity tests (one xfail, two working) with a single snapshot_platform test, and drop the direct value_fn/ suitable_fn unit tests - joostlek's other point on this PR. With only one entity and a suitable_fn that's always True, there's nothing left for those to usefully cover beyond what the snapshot already asserts.
|
@timmo001 — when you get a chance, could you take a look at this one? I've been working through T9/T10 room-sensor support in Home Assistant and ended up tracing several issues back into aiolyric. This PR (#165) is the important one: the priority endpoint currently parses several field names differently from what the live Resideo API actually returns, causing the room/priority data to silently come back empty. I've captured the live API response, updated the fixture/tests to match it, and deliberately left the unverified POST path alone rather than guessing. This is also now an upstream blocker for my related Home Assistant Core work, particularly home-assistant/core#177022. The HA-side changes can fix when/how the priority endpoint is called, but the room data still can't populate correctly until the parsing fix here is adopted and an aiolyric release containing it is available. There is some additional work I need to do on the Core PRs based on subsequent review/changes there, but I'd prefer not to keep reworking the downstream implementation until I know the required upstream library fix has a path forward. While auditing the surrounding aiolyric objects I found a few other small live-API/schema discrepancies and opened #166–#170 separately to keep each change focused. I'm happy to restructure, combine, squash, or otherwise adjust these PRs however you'd prefer. Mainly wanted to make sure #165 in particular hadn't fallen off your radar, since it's currently holding up the downstream HA work. Thanks! |
* Fix ScheduleType.schedule_sub_type reading the wrong JSON key Resideo's API returns "scheduleSubtype" (lowercase t) for both device and location responses, not "scheduleSubType". Found while auditing device.py for the same class of field-name mismatch fixed in the priority endpoint (#165). Not currently consumed anywhere in Home Assistant's lyric integration, so this was never visibly broken, but the fixtures encoded the same wrong key the code was reading, so the existing tests passed without ever catching it against a real response. * Support both scheduleSubType and scheduleSubtype keys Resideo's docs specify scheduleSubType, but the live API sends scheduleSubtype (lowercase t). Check the documented key first and fall back to the observed one so neither response shape reads back None. Addresses review feedback on #166. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Summary
LyricPriority/CurrentPriority/LyricRoom/LyricAccessoryread JSON keys that don't matchwhat
GET /devices/thermostats/{id}/priorityactually returns from a live Resideo account today.Because
current_priorityreads a key that doesn't exist, it silently falls back to{}insteadof raising — so every consumer gets an empty room list with no error, no exception, nothing
logged. This makes the bug very easy to miss: the HTTP call succeeds (200), the debug log even
shows a full, correct payload, and the parsed objects are just quietly empty.
Captured directly from a live T9/T10 thermostat with three paired RCHTSENSOR room accessories:
LyricPriority.current_prioritycurrentPrioritypriorityLyricPriority.statusstatuspriorityStatusLyricRoom.room_nameroomNamenameLyricRoom.room_avg_temproomAvgTempavgTemperatureLyricRoom.room_avg_humidityroomAvgHumidityavgHumidityLyricAccessory.typetypesensorTypeLyricAccessory.exclude_tempexcludeTempexcludeTemperature(Example payload and full JSON in the file — see
pr-aiolyric-priority.md.)Changes
aiolyric/objects/priority.py: updated the 7 properties above to read the correct keys.tests/__init__.py: updatedRESPONSE_JSON_PRIORITYto the corrected schema. The previousfixture encoded the same wrong keys the code was reading, so
test_priority.pypassed withoutever catching the mismatch against a real response.
tests/objects/test_priority.py: updated assertions to match.pytest tests/objects/passes (3/3). Note:tests/test_client.pyandtests/test_init.pyfailin my environment on an unrelated
aioresponses/aiohttpversion incompatibility — reproduces onmastertoo, before this change, so it's a local environment issue, not something this PRintroduces.
What I deliberately did NOT change
update_priority()(thePOSTbody for setting room priority) still sends{"currentPriority": {...}}. I don't have a captured live request/response for the write path toconfirm whether it has the same mismatch, so I left it alone rather than guess.
Context
Found while fixing a related bug in Home Assistant's
lyricintegration, which was silentlyskipping the priority fetch entirely for some thermostats: home-assistant/core#177022. Once
merged and released to PyPI, that PR would need a version bump in
homeassistant/components/lyric/manifest.jsonto pick it up.