Skip to content

Conversation

@bouwew
Copy link
Contributor

@bouwew bouwew commented Dec 23, 2025

Also, replace all async_setup_component() use.

Summary by CodeRabbit

  • Behavior Change

    • Update interval defaults now depend on device type: power devices use a faster 10s interval, others use 60s; options flow reflects this selection.
  • Tests

    • Added tests validating default update intervals (Adam: 60s, P1: 10s) using simulated time and updated tests to initialize via config entries.
  • Documentation

    • Changelog updated to note the new tests and interval verification.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 23, 2025

Walkthrough

Replaces per-device DEFAULT_SCAN_INTERVAL mapping with fixed DEFAULT_UPDATE_INTERVAL (60s) and P1_UPDATE_INTERVAL (10s); coordinator constructor no longer accepts an update_interval parameter and selects intervals based on device type; config flow/options use the new constants; tests updated to set up via config entries and two tests added validating Adam (60s) and P1 (10s) coordinator update timing.

Changes

Cohort / File(s) Summary
Constants
custom_components/plugwise/const.py
Removed DEFAULT_SCAN_INTERVAL dict; added DEFAULT_UPDATE_INTERVAL = 60s and P1_UPDATE_INTERVAL = 10s.
Coordinator
custom_components/plugwise/coordinator.py
Removed update_interval parameter from PlugwiseDataUpdateCoordinator.__init__; coordinator now defaults to DEFAULT_UPDATE_INTERVAL and switches to P1_UPDATE_INTERVAL for power/P1 devices on connect.
Config flow / Options
custom_components/plugwise/config_flow.py
Replaced usage of DEFAULT_SCAN_INTERVAL with DEFAULT_UPDATE_INTERVAL and P1_UPDATE_INTERVAL; options schema/selection logic updated to choose P1 interval for power devices.
Tests
tests/components/plugwise/test_init.py
Tests switched from async_setup_component() to mock_config_entry.add_to_hass() + hass.config_entries.async_setup(); added test_update_interval_adam() and test_update_interval_p1() that use freezer to assert 60s and 10s coordinator update timing respectively; other tests updated for config_entries setup.
Changelog
CHANGELOG.md
Added "Ongoing" entry referencing the core PR / downstream change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Test/Hass as Hass (test)
  participant ConfigEntry as ConfigEntry
  participant Coordinator as PlugwiseDataUpdateCoordinator
  participant Smile as SmileDevice
  participant Scheduler as HA Update Scheduler

  rect rgba(200,230,255,0.4)
  Note right of Hass: Test creates mock config entry\nand calls hass.config_entries.async_setup()
  Hass->>ConfigEntry: async_setup(entry)
  ConfigEntry->>Coordinator: instantiate(entry)
  end

  rect rgba(230,255,220,0.35)
  Note right of Coordinator: Coordinator selects interval\nDEFAULT_UPDATE_INTERVAL (60s)\nor P1_UPDATE_INTERVAL (10s) for power/P1
  Coordinator->>Smile: connect() / identify device type
  Smile-->>Coordinator: type = "power" or "adam"/other
  Coordinator->>Scheduler: schedule periodic updates (interval chosen)
  end

  rect rgba(255,240,220,0.35)
  Note right of Scheduler: Freezer in tests advances time\nand triggers coordinator refresh()
  Scheduler->>Coordinator: call _async_update_data()
  Coordinator->>Smile: fetch data
  Smile-->>Coordinator: data
  Coordinator->>Hass: update entities/state
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement, downstreaming, core-dev-compat

Suggested reviewers

  • CoMPaTech

Poem

🐰 Hopping through code with a carrot in hand,
Intervals fixed so the updates all land.
Adam waits sixty, P1 counts to ten,
Freezer ticks, coordinator hums — then again. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Implement Core PR #159626' is vague and does not clearly convey what changes were made; it only references an external PR number without describing the actual modifications. Use a more descriptive title that summarizes the main changes, such as 'Replace DEFAULT_SCAN_INTERVAL with update interval constants and add test coverage' or 'Refactor update intervals and add test cases for Adam and P1 devices'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch test-update-interval

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64e5a25 and d0cda1a.

📒 Files selected for processing (2)
  • tests/components/plugwise/snapshots/test_select.ambr
  • tests/components/plugwise/test_init.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T16:26:34.700Z
Learnt from: bouwew
Repo: plugwise/plugwise-beta PR: 906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.

Applied to files:

  • tests/components/plugwise/test_init.py
🧬 Code graph analysis (1)
tests/components/plugwise/test_init.py (1)
tests/components/plugwise/conftest.py (3)
  • mock_config_entry (81-95)
  • mock_smile_adam_heat_cool (185-208)
  • mock_smile_p1 (295-317)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (2)
tests/components/plugwise/snapshots/test_select.ambr (1)

144-144: LGTM! Appropriate reclassification of schedule entities.

The snapshot correctly reflects the reclassification of thermostat schedule select entities from EntityCategory.CONFIG to None. This distinguishes user-facing operational settings (schedules) from gateway-level configuration settings, improving the UX by making schedules more prominent in the UI.

Also applies to: 266-266, 389-389, 454-454, 519-519, 584-584

tests/components/plugwise/test_init.py (1)

17-17: LGTM! Necessary import for new tests.

The addition of DEFAULT_SCAN_INTERVAL to the imports is required for the new update interval tests.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/components/plugwise/test_init.py (1)

33-33: Remove the unused import.

The async_setup_component import is no longer needed since all tests now use hass.config_entries.async_setup(mock_config_entry.entry_id) instead. The pre-commit hook flagged this import as unused.

🔎 Proposed fix
-from homeassistant.setup import async_setup_component
♻️ Duplicate comments (3)
tests/components/plugwise/test_init.py (3)

399-400: Use safe dictionary access instead of direct indexing.

Lines 399-400 use unsafe dictionary indexing on DEFAULT_SCAN_INTERVAL[mock_smile_adam_heat_cool.smile.type] without a fallback. If smile.type isn't in the dictionary, this will raise a KeyError. Consider using .get() with a default value to match the pattern used in the coordinator.

🔎 Proposed fix
-    assert DEFAULT_SCAN_INTERVAL[mock_smile_adam_heat_cool.smile.type] == timedelta(seconds=60)
-    freezer.tick(DEFAULT_SCAN_INTERVAL[mock_smile_adam_heat_cool.smile.type])
+    interval = DEFAULT_SCAN_INTERVAL.get(mock_smile_adam_heat_cool.smile.type, timedelta(seconds=60))
+    assert interval == timedelta(seconds=60)
+    freezer.tick(interval)

417-417: Fix the docstring copy-paste error.

The docstring says "Test Adam update interval" but this test is for the P1 gateway, not Adam.

🔎 Proposed fix
-    """Test Adam update interval."""
+    """Test P1 update interval."""

425-426: Use safe dictionary access instead of direct indexing.

Lines 425-426 use unsafe dictionary indexing on DEFAULT_SCAN_INTERVAL[mock_smile_p1.smile.type] without a fallback. If smile.type isn't in the dictionary, this will raise a KeyError. Consider using .get() with a default value to match the pattern used in the coordinator.

🔎 Proposed fix
-    assert DEFAULT_SCAN_INTERVAL[mock_smile_p1.smile.type] == timedelta(seconds=10)
-    freezer.tick(DEFAULT_SCAN_INTERVAL[mock_smile_p1.smile.type])
+    interval = DEFAULT_SCAN_INTERVAL.get(mock_smile_p1.smile.type, timedelta(seconds=10))
+    assert interval == timedelta(seconds=10)
+    freezer.tick(interval)
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0cda1a and 6421361.

📒 Files selected for processing (1)
  • tests/components/plugwise/test_init.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T16:26:34.700Z
Learnt from: bouwew
Repo: plugwise/plugwise-beta PR: 906
File: tests/components/plugwise/test_binary_sensor.py:9-16
Timestamp: 2025-08-23T16:26:34.700Z
Learning: The Plugwise integration test files import utilities like `snapshot_platform`, `MockConfigEntry`, and `async_fire_time_changed` from `tests.common`, which is from the Home Assistant core repository at https://github.com/home-assistant/core/blob/master/tests/common.py, not from local test files.

Applied to files:

  • tests/components/plugwise/test_init.py
🧬 Code graph analysis (1)
tests/components/plugwise/test_init.py (1)
tests/components/plugwise/conftest.py (3)
  • mock_config_entry (81-95)
  • mock_smile_adam_heat_cool (185-208)
  • mock_smile_p1 (295-317)
🪛 GitHub Actions: Test PR against HA-core
tests/components/plugwise/test_init.py

[error] 1-1: Pre-commit hook 'ruff' reported code changes and ultimately failed with exit code 1. 1 error fixed, 0 remaining. The hook modified the file (e.g., removal of an unused import: 'from homeassistant.setup import async_setup_component').

🔇 Additional comments (3)
tests/components/plugwise/test_init.py (3)

17-17: LGTM!

The import of DEFAULT_SCAN_INTERVAL is correctly added and used in the new test cases.


159-159: LGTM!

The migration from async_setup_component to hass.config_entries.async_setup(mock_config_entry.entry_id) is the correct approach for testing config entry-based integrations. This pattern is consistently applied across all affected tests.

Also applies to: 284-284, 364-364


383-405: Well-structured test for update interval verification.

The test correctly:

  • Sets up the config entry and verifies the LOADED state
  • Checks the initial update call count
  • Uses freezer.tick() to advance time by the expected interval
  • Triggers the coordinator update with async_fire_time_changed
  • Verifies the second update call

@bouwew bouwew force-pushed the test-update-interval branch 2 times, most recently from ed0e836 to f56ef94 Compare December 23, 2025 11:13
@bouwew bouwew marked this pull request as ready for review December 23, 2025 11:43
@bouwew bouwew requested a review from a team as a code owner December 23, 2025 11:43
@bouwew bouwew force-pushed the test-update-interval branch from 1e4d7fe to 5b2f882 Compare December 26, 2025 09:21
@bouwew bouwew force-pushed the test-update-interval branch from 18f8f6b to 77d9835 Compare January 1, 2026 18:25
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 1, 2026

@bouwew bouwew changed the title Add testcases for update_interval Implement Core PR #159626 Jan 1, 2026
@bouwew bouwew requested a review from CoMPaTech January 1, 2026 18:34
@bouwew bouwew merged commit cb49d5f into main Jan 2, 2026
13 checks passed
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