Skip to content

Conversation

@bouwew
Copy link
Contributor

@bouwew bouwew commented Aug 31, 2025

This should fix the failure of deleting a dead device.

Summary by CodeRabbit

  • Bug Fixes

    • Improved device removal flow and error handling so unregistering proceeds more gracefully and issues are logged with reduced severity.
  • Chores

    • Bumped plugwise-usb dependency to v0.44.14 and updated package version to v0.55.12.
  • Documentation

    • Added an "Ongoing" subsection to the changelog linking the recent fix and the upstream v0.44.14 release.

@coderabbitai
Copy link

coderabbitai bot commented Aug 31, 2025

Walkthrough

Adds an "Ongoing" subsection to CHANGELOG, bumps the plugwise-usb dependency and integration/project versions (manifest.json and pyproject.toml), and changes async_remove_config_entry_device to derive the node MAC from device identifiers, call unregister_node(mac), log NodeError as a warning while allowing registry removal, or return False if no removable identifier is found.

Changes

Cohort / File(s) Summary
Changelog update
CHANGELOG.md
Added an "Ongoing" subsection before v0.55.10 with entries referencing issue #312, PR #324, and a link to plugwise_usb v0.44.14.
Dependency / metadata
custom_components/plugwise_usb/manifest.json, pyproject.toml
Bumped requirements pin: plugwise-usb==0.44.13plugwise-usb==0.44.14; bumped integration version and project version from 0.55.110.55.12.
Device removal logic
custom_components/plugwise_usb/__init__.py
async_remove_config_entry_device now iterates device_entry.identifiers, selects the first DOMAIN identifier whose value is not a known coordinator/stick MAC as mac, calls await api_stick.unregister_node(mac), treats NodeError as a warning and returns True to allow device registry removal; returns False if no removable identifier is found.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant HA as Home Assistant
  participant Int as plugwise_usb integration
  participant Lib as plugwise-usb library

  HA->>Int: async_remove_config_entry_device(device_entry)
  Int->>Int: inspect device_entry.identifiers\nfind first DOMAIN id not in {mac_stick, mac_coordinator}
  alt found removable identifier
    Int->>Lib: unregister_node(mac)
    alt success
      Lib-->>Int: success
      Int-->>HA: return True (allow removal)
    else NodeError
      Lib--x Int: raises NodeError
      Note right of Int #FFF3CD: log warning\nreturn True (allow removal)
    end
  else no removable identifier
    Int-->>HA: return False (prevent removal)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • dirixmjm
  • CoMPaTech
  • ArnoutD

Poem

I nibble changelogs, twitch my nose,
I hop through identifiers where the MAC line grows.
A gentle warning if a node won't part,
Versions bumped — a tiny restart.
🐰✨


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3784155 and f172dbc.

📒 Files selected for processing (2)
  • custom_components/plugwise_usb/manifest.json (1 hunks)
  • pyproject.toml (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
  • custom_components/plugwise_usb/manifest.json
⏰ 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: Setup for HA-core (release/master)
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improve_delete

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@bouwew bouwew marked this pull request as ready for review August 31, 2025 09:39
@bouwew bouwew requested a review from a team as a code owner August 31, 2025 09:39
@bouwew bouwew changed the title Fix for #312 Fix for #312, link to plugwise_usb v0.44.14 Aug 31, 2025
Copy link

@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)
custom_components/plugwise_usb/__init__.py (1)

182-199: Fix undefined identifier and correctly extract MAC

In custom_components/plugwise_usb/__init__.py at lines 182–199, the comprehension variable identifier does not leak into the outer scope, leading to a NameError and preventing any device removal. Replace the block so it:

  • Uses a local name (e.g. id_) in the any()/next() comprehensions
  • Determines removability by checking against api_stick.mac_stick and api_stick.mac_coordinator
  • Extracts mac via next((id_[1] for id_ in device_entry.identifiers if id_[0] == DOMAIN), None), logs and returns False if missing
  • Calls await api_stick.unregister_node(mac), catching both NodeError and StickError
🧹 Nitpick comments (1)
CHANGELOG.md (1)

5-9: Changelog entry OK; add release line on tag

“Ongoing” section is fine during development. On release, add a dated “v0.55.11 - 2025-08-31” header above and move these bullets under it for consistency.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d9c813a and 4d91a9f.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • custom_components/plugwise_usb/__init__.py (1 hunks)
  • custom_components/plugwise_usb/manifest.json (1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
custom_components/plugwise_usb/__init__.py

189-189: Undefined name identifier

(F821)

🔇 Additional comments (1)
custom_components/plugwise_usb/manifest.json (1)

12-12: Pin bumped to plugwise-usb 0.44.14 — verify availability and changelog alignment

Looks good. Please double-check that 0.44.14 is published and includes the device-removal fix referenced in this PR, and run hassfest after bumping to ensure manifest compliance.

Copy link

@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

🧹 Nitpick comments (1)
custom_components/plugwise_usb/__init__.py (1)

191-195: Optionally also handle StickError on unregister to avoid blocking removal on USB issues

unregister_node may raise stick-level errors; aligning handling with other calls (e.g., set_energy_intervals) avoids blocking registry removal when the stick errors out.

-        except NodeError:
-            _LOGGER.warning("Plugwise node %s unregistering failed with NodeError", mac)
+        except (NodeError, StickError) as exc:
+            _LOGGER.warning("Plugwise node %s unregistering failed: %s", mac, exc)
             return True  # Must return True for device_registry removal to happen!
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4d91a9f and cb0cd1b.

📒 Files selected for processing (1)
  • custom_components/plugwise_usb/__init__.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
custom_components/plugwise_usb/__init__.py

189-189: Undefined name identifier

(F821)

Copy link
Contributor

@dirixmjm dirixmjm left a comment

Choose a reason for hiding this comment

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

Can't beat a good nights sleep!

Copy link

@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

🧹 Nitpick comments (1)
custom_components/plugwise_usb/__init__.py (1)

191-201: Simplify: compute mac upfront and remove removable flag

Cleaner control flow, avoids state coupling between removable and mac.

Alternative refactor:

-    if removable:
-        try:
-            await api_stick.unregister_node(mac)
-        except NodeError as exc:
-            _LOGGER.warning("Plugwise node %s unregistering failed: %s", mac, exc)
-            return True # Must return True for device_registry removal to happen!
-
-        _LOGGER.debug("Plugwise device %s successfully removed", mac)
-        return True
-
-    return False
+    mac = next(
+        (
+            id_[1]
+            for id_ in device_entry.identifiers
+            if id_[0] == DOMAIN
+            and id_[1] not in {str(api_stick.mac_stick), str(api_stick.mac_coordinator)}
+        ),
+        None,
+    )
+    if not mac:
+        return False
+    try:
+        await api_stick.unregister_node(mac)
+    except NodeError as exc:
+        _LOGGER.warning("Plugwise node %s unregistering failed: %s", mac, exc)
+        return True  # Must return True for device registry removal to happen!
+    _LOGGER.debug("Plugwise device %s successfully removed", mac)
+    return True
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between dda954e and 1b4f1b4.

📒 Files selected for processing (2)
  • custom_components/plugwise_usb/__init__.py (1 hunks)
  • custom_components/plugwise_usb/manifest.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • custom_components/plugwise_usb/manifest.json
⏰ 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: Setup for HA-core (release/master)
🔇 Additional comments (1)
custom_components/plugwise_usb/__init__.py (1)

194-196: Confirm exception coverage for unregister_node in 0.44.14

If unregister_node can also raise StickError (transport/USB issues), we should catch it similarly to ensure device-registry removal proceeds for dead devices.

Would you confirm from plugwise-usb v0.44.14 docs/impl whether unregister_node may raise StickError? If yes, extend the except clause to (NodeError, StickError).

Copy link

@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

♻️ Duplicate comments (1)
custom_components/plugwise_usb/__init__.py (1)

182-191: Identifier-based MAC selection looks correct; add mac initialization to avoid “possibly undefined” and improve readability

Initialize mac before the loop to satisfy linters/type-checkers and make intent explicit. Optionally unpack the identifier tuple for clarity.

-    removable = False
-    for identifier in device_entry.identifiers:
+    removable = False
+    mac = None
+    for identifier in device_entry.identifiers:
         if (
             identifier[0] == DOMAIN
             and identifier[1] not in (str(api_stick.mac_stick), str(api_stick.mac_coordinator))
         ):
             mac = identifier[1]
             removable = True
             break
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1b4f1b4 and 9866171.

📒 Files selected for processing (1)
  • custom_components/plugwise_usb/__init__.py (1 hunks)
⏰ 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: Setup for HA-core (release/master)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 2, 2025

@bouwew bouwew merged commit d824384 into main Sep 2, 2025
12 checks passed
@bouwew bouwew deleted the improve_delete branch September 2, 2025 06:18
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