Skip to content

Add timetables controls - #116

Open
swann05 wants to merge 66 commits into
banter240:devfrom
swann05:dev2
Open

Add timetables controls#116
swann05 wants to merge 66 commits into
banter240:devfrom
swann05:dev2

Conversation

@swann05

@swann05 swann05 commented Jul 27, 2026

Copy link
Copy Markdown

What does this PR do?

Add a timetable selector for each room and a global one
Add a refresh button for each room and a global one

Related Issue

Fixes #

Type of Change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / tooling

Affected Generation(s)

  • V2 - GW Bridge
  • V3 Classic (HomeKit)
  • Tado X (Matter)
  • All / not generation-specific

Testing

Test on my instance, works perfectly

Checklist

  • Pre-commit passes (ruff, mypy, hassfest, HACS)
  • No debug logging left in
  • Tested on real hardware or described why not applicable

banter240 and others added 30 commits April 11, 2026 10:36
Root cause: quota reset history was stored in Europe/Berlin local time.
After CET->CEST, the learned reset hour shifted by 1h in UTC (e.g.
12:30 Berlin stored as 13:30 CEST = 11:30 UTC instead of 12:30 UTC),
causing the integration to enter quota-conservation mode ~30 min early.

reset_window_tracker.py:
- record_reset: normalize history in UTC instead of Berlin tz
- _update_learned_window: compute recent list once; detect pattern break
  (two newest entries disagree on UTC hour) and reset to 1 entry so the
  tracker re-learns cleanly; remove redundant same_hour_resets filter
- _default_utc_window: new helper to convert Berlin-local defaults to
  UTC respecting the current DST offset - single source of truth
- get_expected_window: use _default_utc_window so "default" confidence
  also returns correct UTC values, not Berlin-local hour as UTC
- get_next_reset_time: use _learned_window directly (learned and
  single_observation) so the proactive poll is never scheduled from the
  Berlin-local default hour treated as UTC; fall back to _default_utc_window
- ResetWindow.__str__: convert UTC hour to Berlin local for display
- to_dict: add data_version=2, drop serialized learned_window
- load_dict: remove inline migration (handled by config entry v10);
  fix duplicate initial_target parse; always re-derive learned_window
- DATA_VERSION = 2 class constant for versioned storage migration

quota_math.py:
- is_in_reset_safe_window: compare UTC hours instead of Berlin hours;
  when no explicit hour given, derive expected UTC hour from the
  Berlin-local default respecting the current DST offset

coordinator.py:
- async_setup: reschedule reset poll after loading stored tracker so
  the proactive poll fires at the correct learned time after a restart
  (previously scheduled in __init__ with an empty tracker)

migration.py / __init__.py / config_flow.py:
- _v10 (async): one-time migration of stored reset_tracker history from
  Berlin tz to UTC for existing users
- async_migrate_entry: await coroutine migration steps
- config entry VERSION bumped 9 -> 10
## [5.4.1-dev.1](banter240/tado_hijack@v5.4.0...v5.4.1-dev.1) (2026-04-11)

### 🐛 Bug Fixes

* fix(quota): fix DST-driven reset prediction drift and CET->UTC migration

Root cause: quota reset history was stored in Europe/Berlin local time.
After CET->CEST, the learned reset hour shifted by 1h in UTC (e.g.
12:30 Berlin stored as 13:30 CEST = 11:30 UTC instead of 12:30 UTC),
causing the integration to enter quota-conservation mode ~30 min early.

reset_window_tracker.py:
- record_reset: normalize history in UTC instead of Berlin tz
- _update_learned_window: compute recent list once; detect pattern break
  (two newest entries disagree on UTC hour) and reset to 1 entry so the
  tracker re-learns cleanly; remove redundant same_hour_resets filter
- _default_utc_window: new helper to convert Berlin-local defaults to
  UTC respecting the current DST offset - single source of truth
- get_expected_window: use _default_utc_window so "default" confidence
  also returns correct UTC values, not Berlin-local hour as UTC
- get_next_reset_time: use _learned_window directly (learned and
  single_observation) so the proactive poll is never scheduled from the
  Berlin-local default hour treated as UTC; fall back to _default_utc_window
- ResetWindow.__str__: convert UTC hour to Berlin local for display
- to_dict: add data_version=2, drop serialized learned_window
- load_dict: remove inline migration (handled by config entry v10);
  fix duplicate initial_target parse; always re-derive learned_window
- DATA_VERSION = 2 class constant for versioned storage migration

quota_math.py:
- is_in_reset_safe_window: compare UTC hours instead of Berlin hours;
  when no explicit hour given, derive expected UTC hour from the
  Berlin-local default respecting the current DST offset

coordinator.py:
- async_setup: reschedule reset poll after loading stored tracker so
  the proactive poll fires at the correct learned time after a restart
  (previously scheduled in __init__ with an empty tracker)

migration.py / __init__.py / config_flow.py:
- _v10 (async): one-time migration of stored reset_tracker history from
  Berlin tz to UTC for existing users
- async_migrate_entry: await coroutine migration steps
- config entry VERSION bumped 9 -> 10

[skip ci]
…translations

Quota (fix):
- get_next_reset_time() anchored on now.replace(hour, minute) instead of
  last_history_entry + 1 day, collapsing adaptive interval to 20s minimum
  after every observed reset
- get_initial_target() cached a stale past timestamp and never refreshed,
  driving seconds_until_reset negative for all generations
- TadoX rate limit counter stayed frozen because hops.tado.com calls never
  went through the V3 handler; TadoXApi now captures ratelimit headers and
  exposes them via rate_limit_data. UnifiedDataProvider gains
  get_rate_limit_source() so the coordinator stays generation-agnostic

Redundancy (fix):
- _filter_presence and _filter_simple_attributes compared against the
  optimistic already-patched state, silently dropping every command when
  suppress_redundant_calls was enabled
- Debounce replacements overwrote the rollback reference with the optimistic
  intermediate state; preserve_rollback_state() now carries the original
  confirmed API state forward through replacements
- RESUME_SCHEDULE commands for zones already in schedule are now filtered
  when suppress_redundant_buttons is enabled

Redundancy (refactor):
- Extract _merge_keyed() in CommandMerger replacing 5 identical merge methods
- Replace 6x _filter_simple_attributes() calls with a config loop
- Add _suppress_calls/_suppress_buttons properties on TadoApiManager

i18n: fix raw key names shown as labels in Auto API Quota config section
## [5.4.1-dev.2](banter240/tado_hijack@v5.4.1-dev.1...v5.4.1-dev.2) (2026-04-14)

### 🐛 Bug Fixes

* fix: fix quota overconsumption, redundancy filter regression, and DE translations

Quota (fix):
- get_next_reset_time() anchored on now.replace(hour, minute) instead of
  last_history_entry + 1 day, collapsing adaptive interval to 20s minimum
  after every observed reset
- get_initial_target() cached a stale past timestamp and never refreshed,
  driving seconds_until_reset negative for all generations
- TadoX rate limit counter stayed frozen because hops.tado.com calls never
  went through the V3 handler; TadoXApi now captures ratelimit headers and
  exposes them via rate_limit_data. UnifiedDataProvider gains
  get_rate_limit_source() so the coordinator stays generation-agnostic

Redundancy (fix):
- _filter_presence and _filter_simple_attributes compared against the
  optimistic already-patched state, silently dropping every command when
  suppress_redundant_calls was enabled
- Debounce replacements overwrote the rollback reference with the optimistic
  intermediate state; preserve_rollback_state() now carries the original
  confirmed API state forward through replacements
- RESUME_SCHEDULE commands for zones already in schedule are now filtered
  when suppress_redundant_buttons is enabled

Redundancy (refactor):
- Extract _merge_keyed() in CommandMerger replacing 5 identical merge methods
- Replace 6x _filter_simple_attributes() calls with a config loop
- Add _suppress_calls/_suppress_buttons properties on TadoApiManager

i18n: fix raw key names shown as labels in Auto API Quota config section

[skip ci]
Prevents false duplicate-device appearance in diagnostics output when
two serials both end in the same 4 digits (e.g. both ending in "1234").
Adds per-zone operating mode sensor (schedule/off/boost/manual) for
both Tado Classic and Tado X generations, and a home-level aggregate
sensor that returns "mixed" when zones are in different modes.

No additional API calls - reads from already-fetched zone_states.
Boost detection for Classic uses the existing 25°C temperature heuristic
consistent with action_provider.py.

Co-authored-by: laurensdehoorne <laurensdehoorne@users.noreply.github.com>
…nual control (banter240#89)

Fix _resolve_device_to_zone() using zone.id which doesn't exist on
HopsRoomSnapshot (Tado X zones use room_id). Iterate zones_meta.items()
and use the dict key as zone_id instead.

Fix TadoXExecutor ignoring the termination dict from the merged overlay
data. Extract termination_type and duration_seconds and pass them to
async_set_manual_control so TIMER overlays created via set_mode duration
parameter actually expire as intended.
## [5.5.0-dev.1](banter240/tado_hijack@v5.4.1-dev.2...v5.5.0-dev.1) (2026-04-17)

### ✨ New Features

* feat(sensors): add zone_mode and home_mode sensors (banter240#86)

Adds per-zone operating mode sensor (schedule/off/boost/manual) for
both Tado Classic and Tado X generations, and a home-level aggregate
sensor that returns "mixed" when zones are in different modes.

No additional API calls - reads from already-fetched zone_states.
Boost detection for Classic uses the existing 25°C temperature heuristic
consistent with action_provider.py.

Co-authored-by: laurensdehoorne <laurensdehoorne@users.noreply.github.com>

### 🐛 Bug Fixes

* fix(diagnostics): increase serial number redaction suffix to 5 chars

Prevents false duplicate-device appearance in diagnostics output when
two serials both end in the same 4 digits (e.g. both ending in "1234").

* fix(tadox): resolve device-to-zone mapping and pass termination to manual control (banter240#89)

Fix _resolve_device_to_zone() using zone.id which doesn't exist on
HopsRoomSnapshot (Tado X zones use room_id). Iterate zones_meta.items()
and use the dict key as zone_id instead.

Fix TadoXExecutor ignoring the termination dict from the merged overlay
data. Extract termination_type and duration_seconds and pass them to
async_set_manual_control so TIMER overlays created via set_mode duration
parameter actually expire as intended.

[skip ci]
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
line 1925 add data_manager
DELETED DUPLICATE COMMENT LINE 1141
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…d reset

Replace max(1s, delay) in schedule_reset_poll with an explicit 24h fallback
when delay is invalid, preventing a tight API-quota-burning loop on stale
initial_target. Reschedule the reset poll immediately when a quota reset is
detected via regular polling so the timer stays in sync without waiting for
the next scheduled fire. Also refactor zone overlay/resume redundancy checks
into dedicated helpers for clarity.
## [5.5.0-dev.2](banter240/tado_hijack@v5.5.0-dev.1...v5.5.0-dev.2) (2026-04-21)

### 🐛 Bug Fixes

* fix(quota): prevent reset-poll runaway loop and sync timer on detected reset

Replace max(1s, delay) in schedule_reset_poll with an explicit 24h fallback
when delay is invalid, preventing a tight API-quota-burning loop on stale
initial_target. Reschedule the reset poll immediately when a quota reset is
detected via regular polling so the timer stays in sync without waiting for
the next scheduled fire. Also refactor zone overlay/resume redundancy checks
into dedicated helpers for clarity.

[skip ci]
- add _ensure_ac_setting_fields coordinator helper: fetches cached
  capabilities and auto-populates fanSpeed/Level and swing fields
  when additional_setting_fields is not explicitly provided; called
  from async_set_zone_overlay and per-zone from async_set_multiple_zone_overlays

- fix _execute_set_mode in services.py: map operation_mode to ac_mode
  (HEAT/COOL/DRY/FAN) and forward to async_set_multiple_zone_overlays;
  mode field was never set for this service path

- single-toggle swing fallback: use current state, last-seen cache,
  then "OFF" (tadoasync does not expose swings in mode capabilities)

Resolves 422 errors for AC zones in standby that were missing mode,
fanSpeed and swing fields in overlay payloads.

Closes banter240#98
## [5.5.0-dev.3](banter240/tado_hijack@v5.5.0-dev.2...v5.5.0-dev.3) (2026-04-30)

### 🐛 Bug Fixes

* fix(ac): populate mode, fanSpeed and swing for AC power-on overlays

- add _ensure_ac_setting_fields coordinator helper: fetches cached
  capabilities and auto-populates fanSpeed/Level and swing fields
  when additional_setting_fields is not explicitly provided; called
  from async_set_zone_overlay and per-zone from async_set_multiple_zone_overlays

- fix _execute_set_mode in services.py: map operation_mode to ac_mode
  (HEAT/COOL/DRY/FAN) and forward to async_set_multiple_zone_overlays;
  mode field was never set for this service path

- single-toggle swing fallback: use current state, last-seen cache,
  then "OFF" (tadoasync does not expose swings in mode capabilities)

Resolves 422 errors for AC zones in standby that were missing mode,
fanSpeed and swing fields in overlay payloads.

[skip ci]
- extract _build_ac_fan_fields, _build_ac_swing_fields and
  _build_ac_light_fields from _ensure_ac_setting_fields to
  reduce function complexity and satisfy Sourcery quality check

- _build_ac_light_fields: include light field when the mode
  capability exposes it; defaults to "OFF" if present in the
  allowed values, otherwise first listed value

Resolves 422 errors for AC zones whose capabilities require a
light field in the overlay setting (e.g. "light not in supported
light [OFF, ON]").

Refs banter240#98
swann05 and others added 26 commits June 11, 2026 19:30
5.7 DEV1 + TIMETABLE TO MAIN
Align the integration with Home Assistant's current Python 3.14 requirement.

- pyproject.toml: python = "^3.14", mypy python_version = "3.14", ruff target-version = "py314"
- .github/workflows/lint.yml: python-version 3.14
- Updated and pinned dev tooling: mypy==2.1.0 (native PEP 695 support, no more --enable-incomplete-feature), ruff==0.15.20, pre-commit==4.6.0, updated pytest stack
- .pre-commit-config.yaml: updated hook revisions, mypy hook now pins exact mypy==2.1.0 + tadoasync==0.2.2; removed --disable-error-code and --no-warn-unused-ignores
- hacs.json + pyproject: minimum homeassistant pinned to "2026.3"
- requirements.txt synced with dev dependencies
- Replaced remaining # type: ignore[method-assign] / [union-attr] etc. with cast(Any, ...) for monkey-patches (lib/patches.py) and dynamic attributes (coordinator.py + related)
- Minor ruff-driven cleanups (logging, exception syntax, etc.)

All local on ai/dev.
…d hvac_mode

In Full Cloud Mode on Tado X, TadoAirConditioning is the unified entity for all zones.

- fan_modes/swing_modes return None for GEN_X (no capabilities endpoint)
- _get_active_hvac_mode returns HEAT for GEN_X (all zones are heating)
- supported_features limited for GEN_X (no FAN/SWING)
- hvac_modes set to [OFF, HEAT, AUTO] for GEN_X (matching TadoHeating)

Prevents event loop crash during setup and incorrect 'cooling' action on heating zones.

Also improves service call diagnostics:
- Added debug logs when entering service handlers (manual_poll, set_mode, set_mode_all_zones, set_water_heater_mode).
- Warnings now include the service name (e.g. "(service: manual_poll)").
- Better messages in async_add_meter_reading for Tado X and general failures (permission/subscription hints).

See banter240#113

Fixes banter240#111
Fixes banter240#112

Incorporate work from reported issues.
## [5.7.1-dev.1](banter240/tado_hijack@v5.7.0...v5.7.1-dev.1) (2026-07-10)
* fix(full_cloud): guard Tado X in TadoAirConditioning for fan/swing and hvac_mode

In Full Cloud Mode on Tado X, TadoAirConditioning is the unified entity for all zones.

- fan_modes/swing_modes return None for GEN_X (no capabilities endpoint)
- _get_active_hvac_mode returns HEAT for GEN_X (all zones are heating)
- supported_features limited for GEN_X (no FAN/SWING)
- hvac_modes set to [OFF, HEAT, AUTO] for GEN_X (matching TadoHeating)

Prevents event loop crash during setup and incorrect 'cooling' action on heating zones.

Also improves service call diagnostics:
- Added debug logs when entering service handlers (manual_poll, set_mode, set_mode_all_zones, set_water_heater_mode).
- Warnings now include the service name (e.g. "(service: manual_poll)").
- Better messages in async_add_meter_reading for Tado X and general failures (permission/subscription hints).

See banter240#113

* chore: target Python 3.14 across the board

Align the integration with Home Assistant's current Python 3.14 requirement.

- pyproject.toml: python = "^3.14", mypy python_version = "3.14", ruff target-version = "py314"
- .github/workflows/lint.yml: python-version 3.14
- Updated and pinned dev tooling: mypy==2.1.0 (native PEP 695 support, no more --enable-incomplete-feature), ruff==0.15.20, pre-commit==4.6.0, updated pytest stack
- .pre-commit-config.yaml: updated hook revisions, mypy hook now pins exact mypy==2.1.0 + tadoasync==0.2.2; removed --disable-error-code and --no-warn-unused-ignores
- hacs.json + pyproject: minimum homeassistant pinned to "2026.3"
- requirements.txt synced with dev dependencies
- Replaced remaining # type: ignore[method-assign] / [union-attr] etc. with cast(Any, ...) for monkey-patches (lib/patches.py) and dynamic attributes (coordinator.py + related)
- Minor ruff-driven cleanups (logging, exception syntax, etc.)

All local on ai/dev.

[skip ci]
"PLR0917",  # too-many-positional-arguments — acceptable pour les factory functions
line 1294
OLD entry = await self._tado.get_active_timetable(zone_id)
NEW entry = await self.client.get_active_timetable(zone_id)

line 394
OLD self.timetable_cache: dict[int, dict] = self.data_manager.timetable_cache
NEW self.timetable_cache: dict[int, dict[str, Any]] = self.data_manager.timetable_cache
LINE 394
OLD next(iter(c.data_manager.timetable_cache.values()), {})
NEW next(iter(c.data_manager.timetable_cache.values()), cast(dict[str, Any], {}))
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@sourcery-ai

sourcery-ai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Reviewer's Guide

Implements timetable selection and refresh controls per zone and at home level, wiring them through new cached timetable state, API client methods, command merging, and executor support for batched timetable updates.

Sequence diagram for setting a zone timetable via new controls

sequenceDiagram
    actor User
    participant ZoneSelect as ZoneSelectEntity
    participant Coordinator as TadoHijackCoordinator
    participant ApiManager
    participant CommandMerger
    participant Executor as TadoV3Executor
    participant Client as TadoClient
    participant TadoApi

    User->>ZoneSelect: choose option (one_day/three_day/seven_day)
    ZoneSelect->>Coordinator: async_set_timetable(zone_id, timetable_type)
    Coordinator->>Coordinator: data_manager.timetable_cache[zone_id] = {id, type}
    Coordinator->>Coordinator: async_update_listeners()
    Coordinator->>Coordinator: _save_timetable_cache()
    Coordinator->>ApiManager: queue_command("set_timetable_zone", TadoCommand(CommandType.SET_TIMETABLE, data))

    ApiManager->>CommandMerger: add(TadoCommand)
    ApiManager->>ApiManager: _process_batch(commands)
    ApiManager->>CommandMerger: merged = result()
    ApiManager->>Executor: _execute_zone_properties(merged)

    Executor->>Executor: _safe_execute("timetable_zone", client.set_active_timetable(zone_id, timetable_id))
    Executor->>Client: set_active_timetable(zone_id, timetable_id)
    Client->>TadoApi: PUT /schedule/activeTimetable {id: timetable_id}
    TadoApi-->>Client: 200 OK
Loading

Sequence diagram for refreshing all timetables from the API

sequenceDiagram
    actor User
    participant HomeButton as HomeRefreshAllTimetablesButton
    participant Coordinator as TadoHijackCoordinator
    participant Client as TadoClient
    participant TadoApi

    User->>HomeButton: press
    HomeButton->>Coordinator: async_refresh_all_timetables()
    Coordinator->>Coordinator: [generation == GEN_CLASSIC]
    Coordinator->>Coordinator: zone_ids = [zid for zones_meta if HEATING/HOT_WATER]
    Coordinator->>Coordinator: asyncio.gather(async_refresh_timetable(zone_id))

    loop for each zone_id
        Coordinator->>Client: get_active_timetable(zone_id)
        Client->>TadoApi: GET /schedule/activeTimetable
        TadoApi-->>Client: timetable {id, type}
        Client-->>Coordinator: timetable entry
        Coordinator->>Coordinator: data_manager.timetable_cache[zone_id] = entry
        Coordinator->>Coordinator: async_update_listeners()
        Coordinator->>Coordinator: _save_timetable_cache()
    end
Loading

File-Level Changes

Change Details Files
Persist and expose timetable cache via the coordinator and data manager to support UI entities and bulk operations.
  • Add timetable_cache dict to DataManager and expose it on the coordinator after updates
  • Restore timetable_cache from Home Assistant storage during async_setup and persist changes via a new _save_timetable_cache helper
  • Notify listeners when timetable cache is changed to keep entities in sync
custom_components/tado_hijack/coordinator.py
custom_components/tado_hijack/helpers/data_manager.py
Add API- and command-layer support for getting/setting active timetables, including batched execution.
  • Extend TadoClient with get_active_timetable and set_active_timetable HTTP methods
  • Introduce CommandType.SET_TIMETABLE and merge logic in CommandMerger with last-write-wins semantics per zone
  • Have ApiManager pass merged timetable data into the v3 executor, which now executes timetable updates via the client
custom_components/tado_hijack/helpers/client.py
custom_components/tado_hijack/helpers/command_merger.py
custom_components/tado_hijack/helpers/api_manager.py
custom_components/tado_hijack/helpers/tadov3/executor.py
custom_components/tado_hijack/models.py
Expose new home- and zone-level timetable select and refresh entities, gated to GEN_CLASSIC and heating/hot water zones where applicable.
  • Extend create_home_button and create_home_select helpers to accept an is_supported_fn predicate
  • Add a home-level timetable_type_all_zones select and refresh_all_timetables button, both limited to GEN_CLASSIC
  • Add per-zone timetable_type select and refresh_timetable button entities for heating and hot-water zones in GEN_CLASSIC, backed by timetable_cache and coordinator methods
custom_components/tado_hijack/definitions.py
custom_components/tado_hijack/coordinator.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • The timetable cache is handled inconsistently: some places use self.data_manager.timetable_cache while others (e.g. value_fn for timetable_type select) check hasattr(c, "timetable_cache"); it would be more robust to consistently read/write via data_manager.timetable_cache and avoid relying on a transient timetable_cache attribute on the coordinator.
  • In the new timetable-related selectors/buttons you duplicate timetable-type strings and ID mappings in several places (e.g. hard-coded ONE_DAY/THREE_DAY/SEVEN_DAY and type_to_id), consider centralising these into a shared mapping or enum to reduce the risk of them drifting out of sync.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The timetable cache is handled inconsistently: some places use `self.data_manager.timetable_cache` while others (e.g. `value_fn` for `timetable_type` select) check `hasattr(c, "timetable_cache")`; it would be more robust to consistently read/write via `data_manager.timetable_cache` and avoid relying on a transient `timetable_cache` attribute on the coordinator.
- In the new timetable-related selectors/buttons you duplicate timetable-type strings and ID mappings in several places (e.g. hard-coded `ONE_DAY`/`THREE_DAY`/`SEVEN_DAY` and `type_to_id`), consider centralising these into a shared mapping or enum to reduce the risk of them drifting out of sync.

## Individual Comments

### Comment 1
<location path="custom_components/tado_hijack/definitions.py" line_range="1929" />
<code_context>
+    ),
+    create_zone_select(
+        key="timetable_type",
+        value_fn=lambda c, zid: (
+            c.data_manager.timetable_cache.get(zid, {}).get("type", "ONE_DAY").lower()
+            if hasattr(c, "timetable_cache")
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Zone timetable select `value_fn` mixes `hasattr` on `c.timetable_cache` with reading `c.data_manager.timetable_cache` and returns `None` fallback.

`value_fn` guards with `hasattr(c, "timetable_cache")` but actually reads `c.data_manager.timetable_cache`, so the check doesn’t protect the cache being used. It also falls back to `None`, unlike the home-level select which defaults to `"one_day"`. Please simplify this to always use `c.data_manager.timetable_cache` and apply a consistent default (e.g. `"one_day"`) when the zone entry is missing, and drop the redundant `hasattr` check.
</issue_to_address>

### Comment 2
<location path="custom_components/tado_hijack/definitions.py" line_range="1902" />
<code_context>
         optimistic_key="horizontal_swing",
         supported_generations={GEN_CLASSIC},
     ),
+    create_home_select(
+        key="timetable_type_all_zones",
+        value_fn=lambda c: (
</code_context>
<issue_to_address>
**question (bug_risk):** Home-level timetable select derives its value from the first cached zone, which may not reflect the state of all zones.

Using `next(iter(c.data_manager.timetable_cache.values()), {})` means this entity’s state is effectively taken from a single arbitrary zone. When zones differ, the reported “all zones” timetable may not match most or even any zones. Consider defining an explicit aggregation rule (e.g., require uniform timetables, use the majority timetable, or expose a mixed/indeterminate state), or making this entity write-only for bulk changes so it doesn’t present a misleading single-source-of-truth state.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

),
create_zone_select(
key="timetable_type",
value_fn=lambda c, zid: (

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Zone timetable select value_fn mixes hasattr on c.timetable_cache with reading c.data_manager.timetable_cache and returns None fallback.

value_fn guards with hasattr(c, "timetable_cache") but actually reads c.data_manager.timetable_cache, so the check doesn’t protect the cache being used. It also falls back to None, unlike the home-level select which defaults to "one_day". Please simplify this to always use c.data_manager.timetable_cache and apply a consistent default (e.g. "one_day") when the zone entry is missing, and drop the redundant hasattr check.

optimistic_key="horizontal_swing",
supported_generations={GEN_CLASSIC},
),
create_home_select(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

question (bug_risk): Home-level timetable select derives its value from the first cached zone, which may not reflect the state of all zones.

Using next(iter(c.data_manager.timetable_cache.values()), {}) means this entity’s state is effectively taken from a single arbitrary zone. When zones differ, the reported “all zones” timetable may not match most or even any zones. Consider defining an explicit aggregation rule (e.g., require uniform timetables, use the majority timetable, or expose a mixed/indeterminate state), or making this entity write-only for bulk changes so it doesn’t present a misleading single-source-of-truth state.

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.

3 participants