Skip to content

Fix VacationHold.enabled reading the wrong JSON key - #170

Open
clutch2sft wants to merge 2 commits into
timmo001:masterfrom
clutch2sft:fix-vacation-hold-key
Open

Fix VacationHold.enabled reading the wrong JSON key#170
clutch2sft wants to merge 2 commits into
timmo001:masterfrom
clutch2sft:fix-vacation-hold-key

Conversation

@clutch2sft

Copy link
Copy Markdown
Contributor

Because the property silently defaulted to False on the missing key, an active
vacation hold would always report as off — no error, no exception, just wrong data.
Same failure mode as the other field-name mismatches already fixed in this library
(#165).

Changes

  • aiolyric/objects/device.py: VacationHold.enabled now reads Enabled.
  • tests/__init__.py, tests/objects/test_device.py, tests/objects/test_location.py:
    fixtures and assertions updated to match.

pytest tests/objects/ passes (3/3).

Context

Found via a Copilot review comment on a companion home-assistant/core PR
(Ocala Thermostat room-sensor work), then confirmed against a debug log already
captured from a live account rather than guessed.

@timmo001 timmo001 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The API docs use lowercase vacationHold.enabled here: https://developer.honeywellhome.com/lyric/apis/get/devices/thermostats/%7BdeviceId%7D-0. Can we support both keys? Replacing it outright means responses matching the documented schema will silently return False.

clutch2sft added a commit to clutch2sft/aiolyric that referenced this pull request Aug 10, 2026
Resideo's docs specify lowercase enabled, but a live captured payload
from a T9-T10 account shows Enabled (capital E). Check the documented
key first and fall back to the observed one so neither response shape
reads back False.

Addresses review feedback on timmo001#170.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@clutch2sft

Copy link
Copy Markdown
Contributor Author

Good catch — updated to support both keys, same approach as the fix on #166:

@Property
def enabled(self):
"""Return if enabled."""
return self.attributes.get("enabled", self.attributes.get("Enabled", False))
Checks the documented enabled key first, falls back to Enabled (the casing confirmed via the live capture used to write this fix originally). Neither a docs-compliant response nor the live one should read back False now.

Also added test_vacation_hold_enabled_key_variants covering both casings plus the missing-key default, and updated the existing assertions accordingly — all green.

clutch2sft and others added 2 commits August 10, 2026 13:18
Resideo's device response returns vacationHold.Enabled (capital E),
not vacationHold.enabled. Confirmed against a live captured payload
from a T9-T10 account: {'vacationHold': {'Enabled': False}}.

Because the property silently defaulted to False on the missing key,
an active vacation hold would always report as off with no error -
same failure mode as the other field-name mismatches already fixed
in this library.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resideo's docs specify lowercase enabled, but a live captured payload
from a T9-T10 account shows Enabled (capital E). Check the documented
key first and fall back to the observed one so neither response shape
reads back False.

Addresses review feedback on timmo001#170.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@clutch2sft
clutch2sft force-pushed the fix-vacation-hold-key branch from 2bb1b9b to 16fda6b Compare August 10, 2026 17:19
@clutch2sft
clutch2sft requested a review from timmo001 August 10, 2026 17:21
clutch2sft added a commit to clutch2sft/aiolyric that referenced this pull request Aug 10, 2026
Resideo's docs specify lowercase day/period, but a live capture from
a T9-T10 account shows the API actually sending PascalCase Day/Period
(same casing mismatch as vacationHold.enabled/Enabled, currently under
review on timmo001#170). Check the documented key first and fall back to the
observed one so neither response shape reads back None.

Found while auditing device.py for the class of field-name/casing
mismatch raised in review on timmo001#166 and timmo001#170.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants