Skip to content

Conversation

bouwew
Copy link
Contributor

@bouwew bouwew commented Oct 16, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Improved detection and removal of orphaned/removed devices and better handling when device module data is missing (avoids stale metadata and redundant errors).
  • New Features

    • Added sample gateway (no-modules) dataset and corresponding domain model data for an Anna v4 scenario.
  • Tests

    • Added an automated test covering the Anna v4 no-modules scenario.
  • Chores

    • Version bumped to 1.8.1 and changelog updated.

Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Walkthrough

Added early-return guards when module data contents are empty to signal device removal; adjusted heater_central lookup fallback and thermostat handling. Added fixtures, test data, a test for a gateway with no modules, bumped version to 1.8.1, and added a comprehensive domain_objects XML.

Changes

Cohort / File(s) Summary
Core Device Detection Logic
plugwise/common.py, plugwise/helper.py, plugwise/legacy/helper.py
Added guards to treat empty module_data["contents"] as device removal (return empty Munch()); common.py adds secondary lookup/fallback for heater_central and returns Munch() for empty thermostat module data; helper.py skips orphan-check block and guards heater_central caller; legacy/helper.py reorders field population and removes zigbee_mac orphan path.
Version & Changelog
pyproject.toml, CHANGELOG.md
Bumped version 1.8.0 → 1.8.1 and added changelog entry about improved orphan/removed device detection.
Test Fixtures & Data
fixtures/anna_v4_no_modules/data.json, tests/data/anna/anna_v4_no_modules.json
New fixture and test data for a Plugwise Gateway (Smile Anna) that has no modules; includes device metadata and sensor sample.
Test Case
tests/test_anna.py
Added async def test_connect_anna_v4_no_modules(self) to validate the no-modules gateway scenario and entity count.
Domain Model Configuration
userdata/anna_v4_no_modules/core.domain_objects.xml
New comprehensive domain_objects XML defining templates, modules, rules, appliances, locations, and gateway/module definitions for the Anna no-modules scenario.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Helper as helper/_finder
participant Common as common._appl_heater_central_info
participant Device as module_data source
participant Caller as appliance processing

Caller->>Helper: request appliance info
Helper->>Device: lookup module_data
alt module_data.contents empty
    Note over Helper,Caller: Early return pathway (signal removal)
    Helper-->>Caller: return Munch() (removed)
else module_data.contents present
    Helper-->>Common: pass module_data (heater/thermostat)
    Common->>Device: maybe secondary lookup (heater_central)
    alt secondary lookup yields contents
        Common-->>Caller: populated info
    else
        Common-->>Caller: return Munch() (removed)
    end
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • CoMPaTech

Poem

🐰 I sniffed the modules, small and slight,
If empty shells appear at night,
I hop and mark them gone with care,
No orphan left to linger there,
Version 1.8.1 — a happy hare! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Improve detection of orphaned/removed devices" accurately reflects the main changes across the codebase. The modifications in plugwise/common.py, plugwise/helper.py, and plugwise/legacy/helper.py all implement logic to detect when devices have been removed by checking if module_data["contents"] is empty and returning signals (empty Munch objects or NONE values) to indicate removal. The title is concise, specific, and clearly conveys the primary objective without vague or generic terms. The accompanying test fixtures and changelog entry further confirm this is the intended focus of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% 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 module-deleted

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3f4e32 and 0279f99.

📒 Files selected for processing (1)
  • plugwise/helper.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugwise/helper.py

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.

@bouwew bouwew changed the title Improve Improve detection of orphaned/removed devices Oct 16, 2025
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (80a2e51) to head (0279f99).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #802   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         3400      3414   +14     
=========================================
+ Hits          3400      3414   +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bouwew bouwew marked this pull request as ready for review October 16, 2025 15:06
@bouwew bouwew requested a review from a team as a code owner October 16, 2025 15:06
@bouwew bouwew requested a review from CoMPaTech October 16, 2025 15:06
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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 811d58b and 78132d8.

📒 Files selected for processing (5)
  • plugwise/common.py (2 hunks)
  • plugwise/helper.py (1 hunks)
  • plugwise/legacy/helper.py (1 hunks)
  • pyproject.toml (1 hunks)
  • userdata/adam_anna_new_2/core.domain_objects.xml (0 hunks)
💤 Files with no reviewable changes (1)
  • userdata/adam_anna_new_2/core.domain_objects.xml
🚧 Files skipped from review as they are similar to previous changes (3)
  • plugwise/legacy/helper.py
  • pyproject.toml
  • plugwise/helper.py
🔇 Additional comments (1)
plugwise/common.py (1)

122-124: Verification confirmed: callers properly handle empty Munch() return.

Both call sites evaluate the _appliance_info_finder result using a falsy check with the walrus operator:

  • helper.py:153: if not (appl := self._appliance_info_finder(appl, appliance)): continue
  • legacy/helper.py:130: if not (appl := self._appliance_info_finder(appliance, appl)): continue

When _appl_thermostat_info returns an empty Munch() (signaling device removal), the empty object evaluates as falsy, triggering continue and skipping appliance processing. The defensive guard is safe and correctly implemented.

Copy link

@bouwew bouwew merged commit 569686e into main Oct 17, 2025
29 of 30 checks passed
@bouwew bouwew deleted the module-deleted branch October 17, 2025 07:09
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