Replace useless warning message, various small corrections/improvements #319
Conversation
WalkthroughAdjusts logging text and severity across connection, requests, network, and node modules; changes stick-failure handling to clear seq_id and surface a NodeError via assign_error; adds unsubscribe cleanup; restructures queue retry loop; bumps version and updates changelog. Changes
Sequence Diagram(s)sequenceDiagram
participant Queue as StickQueue
participant Req as PlugwiseRequest
participant USB as USB Stick
participant Proc as Response Processor
Queue->>Req: submit/send loop (driven by request.resend)
Req->>USB: transmit frame
USB-->>Proc: StickResponse
alt StickResponseType == SUCCESS
Proc->>Req: set result / resume request
else StickResponseType == FAILED
Proc->>Req: prev_seq_id = self._seq_id
Proc->>Req: self._seq_id = None
Proc->>Req: assign_error(NodeError("Stick failed request {prev_seq_id!r}"))
Req-->>Queue: error handled / stop or retry per resend
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #319 +/- ##
=======================================
Coverage 80.58% 80.59%
=======================================
Files 36 36
Lines 7547 7555 +8
=======================================
+ Hits 6082 6089 +7
- Misses 1465 1466 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
plugwise_usb/messages/requests.py (5)
218-218: Prefer 'timeout for' over 'timeout to' (grammar/clarity).Minor wording fix for better English and consistency across logs and exceptions.
- _LOGGER.info("USB-stick response timeout to %s", self) + _LOGGER.info("USB-stick response timeout for %s", self)- StickTimeout(f"USB-stick response timeout to {self}") + StickTimeout(f"USB-stick response timeout for {self}")- f"No device response to {self} within {NODE_TIME_OUT} seconds" + f"No device response for {self} within {NODE_TIME_OUT} seconds"Also applies to: 228-228, 233-233
218-223: Optional: reduce timeout log noise after first attempt.To align with the PR objective of reducing log noise, consider demoting repeated timeout logs to debug while keeping the first occurrence as info.
- if stick_timeout: - _LOGGER.info("USB-stick response timeout to %s", self) - else: - _LOGGER.info( - "No response received for %s within %s seconds", self, NODE_TIME_OUT - ) + if stick_timeout: + if self._send_counter > 0: + _LOGGER.debug("USB-stick response timeout for %s", self) + else: + _LOGGER.info("USB-stick response timeout for %s", self) + else: + if self._send_counter > 0: + _LOGGER.debug( + "No response received for %s within %s seconds", self, NODE_TIME_OUT + ) + else: + _LOGGER.info( + "No response received for %s within %s seconds", self, NODE_TIME_OUT + )
250-263: Fix logging: log the actual response and include expected/actual seq_id.The warnings currently log self._response (likely None here) instead of the incoming response, reducing usefulness. Also include both expected and actual seq_id values.
if self._seq_id is None: _LOGGER.warning( "Received %s as reply to %s without a seq_id assigned", - self._response, + response, self, ) return False if self._seq_id != response.seq_id: _LOGGER.warning( - "Received %s as reply to %s which is not correct (expected seq_id=%s)", - self._response, - self, - str(self.seq_id), + "Received %s as reply to %s with mismatched seq_id (expected=%r, got=%r)", + response, + self, + self.seq_id, + response.seq_id, ) return False
299-305: Preserve seq_id in FAILED ack error message.seq_id is set to None before being logged in the exception, resulting in a vague message. Capture it first.
if stick_response.ack_id == StickResponseType.FAILED: - self._unsubscribe_from_node() - self._seq_id = None - self._response_future.set_exception( - NodeError(f"Stick failed request {self._seq_id}") - ) + prev_seq_id = self._seq_id + self._unsubscribe_from_node() + self._seq_id = None + self._response_future.set_exception( + NodeError(f"Stick failed request {prev_seq_id!r}") + ) return
1-1: Nit: minor docstring grammar.Adjust phrasing for correctness.
-"""All known request messages to be send to plugwise devices.""" +"""All known request messages to be sent to plugwise devices."""- """Base class for request messages to be send from by USB-Stick.""" + """Base class for request messages to be sent by USB-Stick."""Also applies to: 61-61
📜 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.
📒 Files selected for processing (2)
plugwise_usb/connection/sender.py(0 hunks)plugwise_usb/messages/requests.py(2 hunks)
💤 Files with no reviewable changes (1)
- plugwise_usb/connection/sender.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bouwew
PR: plugwise/python-plugwise-usb#311
File: plugwise_usb/nodes/circle.py:550-551
Timestamp: 2025-08-13T13:20:25.205Z
Learning: In the plugwise_usb library, the PlugwiseMessage class always provides a timestamp in responses, so defensive null checks for response.timestamp are not necessary when calling _available_update_state().
🧬 Code Graph Analysis (1)
plugwise_usb/messages/requests.py (1)
plugwise_usb/exceptions.py (1)
StickTimeout(40-41)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
plugwise_usb/network/__init__.py (4)
420-423: Warning-level log may increase noise; dedupe per MAC (and consider keeping at debug).Raising this to WARNING will generate repeated warnings for offline nodes during each discovery attempt, which seems counter to the PR’s goal of reducing log noise. Also, the same MAC can be appended to
_registry_stragglersmultiple times, causing duplicate retries and duplicate warnings.Recommend:
- Only warn once per MAC when it first becomes a straggler, and
- Avoid appending duplicates.
Apply this diff to gate the warning and avoid duplicates:
- if node_info is None: - _LOGGER.warning("Node %s with unknown NodeType not responding, is it offline?", mac) - self._registry_stragglers.append(mac) - return False + if node_info is None: + if mac not in self._registry_stragglers: + _LOGGER.warning( + "Node %s with unknown NodeType not responding, is it offline?", + mac, + ) + self._registry_stragglers.append(mac) + else: + _LOGGER.debug( + "Node %s still not responding; waiting for next retry", + mac, + ) + return FalseIf the intent is strictly to reduce warning-level noise, consider keeping this log at DEBUG and reserving WARNING for exceptional conditions.
324-329: Fix StickError message formatting (currently builds a tuple, not a formatted message).
StickErroris constructed with an extra positional argument, resulting in exception args like(message, mac)instead of a single formatted string. This will render poorly in logs.Apply this diff:
- except StickTimeout as err: - raise StickError( - "The zigbee network coordinator (Circle+/Stealth+) with mac " - + "'%s' did not respond to ping request. Make " - + "sure the Circle+/Stealth+ is within reach of the USB-stick !", - self._controller.mac_coordinator, - ) from err + except StickTimeout as err: + raise StickError( + f"The zigbee network coordinator (Circle+/Stealth+) with mac " + f"'{self._controller.mac_coordinator}' did not respond to ping request. " + "Make sure the Circle+/Stealth+ is within reach of the USB-stick!" + ) from err
295-304: Unsubscribe all node message subscriptions on stop to prevent leaks.
_unsubscribe_to_protocol_eventsclears onlynode_awakeandstick_eventsubscriptions, leavingnode_joinandnode_rejoinsubscriptions active. This can cause callbacks to linger after stop.Apply this diff:
def _unsubscribe_to_protocol_events(self) -> None: """Unsubscribe to events from protocol.""" if self._unsubscribe_node_awake is not None: self._unsubscribe_node_awake() self._unsubscribe_node_awake = None + if self._unsubscribe_node_join is not None: + self._unsubscribe_node_join() + self._unsubscribe_node_join = None + if self._unsubscribe_node_rejoin is not None: + self._unsubscribe_node_rejoin() + self._unsubscribe_node_rejoin = None if self._unsubscribe_stick_event is not None: self._unsubscribe_stick_event() self._unsubscribe_stick_event = None
475-483: Avoid duplicate NodeEvent.LOADED notifications.
_load_nodealready notifiesNodeEvent.LOADED. The loop here notifies again for each successful load, causing duplicate events to subscribers.Apply this diff to only log failures here:
- result_index = 0 - for mac in nodes_not_loaded: - if load_result[result_index]: - await self._notify_node_event_subscribers(NodeEvent.LOADED, mac) - else: - _LOGGER.debug( - "_load_discovered_nodes | Load request for %s failed", mac - ) - result_index += 1 + result_index = 0 + for mac in nodes_not_loaded: + if not load_result[result_index]: + _LOGGER.debug( + "_load_discovered_nodes | Load request for %s failed", mac + ) + result_index += 1
📜 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.
📒 Files selected for processing (1)
plugwise_usb/network/__init__.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/network/__init__.py (2)
plugwise_usb/api.py (1)
mac(316-317)plugwise_usb/nodes/node.py (1)
mac(235-237)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugwise_usb/messages/requests.py (3)
1-1: Nit: tighten grammar and capitalize proper noun in module docstring."to be sent to plugwise devices" reads awkwardly and "Plugwise" is a proper noun.
-"""All known request messages to be sent to plugwise devices.""" +"""All known request messages sent to Plugwise devices."""
61-61: Fix awkward phrasing in base class docstring."to be sent from by USB-Stick" is grammatically incorrect.
- """Base class for request messages to be sent from by USB-Stick.""" + """Base class for request messages sent by USB-Stick."""
218-218: Fix typo in stick-timeout log message ("fot" → "for").Small typo that would leak into logs.
- _LOGGER.info("USB-stick response timeout fot %s", self) + _LOGGER.info("USB-stick response timeout for %s", self)
📜 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.
📒 Files selected for processing (1)
plugwise_usb/messages/requests.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/messages/requests.py (2)
plugwise_usb/exceptions.py (3)
StickTimeout(40-41)NodeTimeout(28-29)NodeError(24-25)plugwise_usb/messages/__init__.py (2)
seq_id(38-40)seq_id(43-45)
🔇 Additional comments (4)
plugwise_usb/messages/requests.py (4)
228-229: LGTM: clearer StickTimeout message.Message is now more natural ("timeout for"). Good consistency.
233-234: LGTM: clearer NodeTimeout message."response for" reads better than "response to". Good improvement.
251-254: LGTM: more informative logging when seq_id is missing.Logging the actual response object (not the internal cached/None) makes triage easier.
258-263: LGTM: improved mismatched seq_id log with expected/got details.Including expected and actual seq_id greatly helps debugging.
bouwew
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugwise_usb/network/__init__.py (3)
331-334: Consistency nit: capitalize “Zigbee” and “MAC” and slightly tighten wordingTo match terminology used elsewhere in the codebase (e.g., “Zigbee” and “MAC”) and keep phrasing concise.
Apply:
- f"The zigbee network coordinator (Circle+/Stealth+) with mac " - f"'{self._controller.mac_coordinator}' did not respond to the ping request. " - "Make sure the Circle+/Stealth+ is within reach of the USB-stick!" + f"The Zigbee network coordinator (Circle+/Stealth+) with MAC " + f"'{self._controller.mac_coordinator}' did not respond to the ping request. " + "Ensure the Circle+/Stealth+ is within range of the USB-stick."Optional: if you want to standardize wording with other modules that use “USB-stick response timeout for …”, consider aligning this phrasing as well.
426-428: Raising to WARNING for offline/unknown nodes likely reintroduces noisy logsThis will be emitted on every retry in _discover_stragglers, producing repetitive warnings when nodes are offline. Given the PR goal to reduce warning noise, consider keeping this at debug or, if visibility is desired, log it once at info/warn per MAC.
Option A (keep it quiet):
- _LOGGER.warning( - "Node %s with unknown NodeType not responding, is it offline?", mac - ) + _LOGGER.debug( + "Node %s with unknown NodeType not responding, is it offline?", mac + )Option B (warn once per device): add a set to track warned MACs and only warn once per runtime.
Outside this block, add to init:
self._warned_unknown_offline: set[str] = set()Modify the logging here:
if mac not in self._warned_unknown_offline: _LOGGER.warning( "Node %s with unknown NodeType not responding, is it offline?", mac ) self._warned_unknown_offline.add(mac)Please verify in HA logs that this change doesn’t create new warning spam for offline devices.
483-489: Minor: simplify index tracking with enumerateRemoves manual index management and makes the loop clearer.
- result_index = 0 - for mac in nodes_not_loaded: - if not load_result[result_index]: + for idx, mac in enumerate(nodes_not_loaded): + if not load_result[idx]: _LOGGER.debug( "_load_discovered_nodes | Load request for %s failed", mac ) - result_index += 1
📜 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.
📒 Files selected for processing (2)
plugwise_usb/messages/requests.py(6 hunks)plugwise_usb/network/__init__.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugwise_usb/messages/requests.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/network/__init__.py (5)
plugwise_usb/__init__.py (1)
mac_coordinator(138-140)plugwise_usb/connection/__init__.py (1)
mac_coordinator(81-90)plugwise_usb/api.py (1)
mac(316-317)plugwise_usb/nodes/node.py (1)
mac(235-237)plugwise_usb/messages/__init__.py (1)
mac(53-57)
🔇 Additional comments (1)
plugwise_usb/network/__init__.py (1)
300-305: LGTM: unsubscribe join/rejoin listeners on shutdownGood addition to prevent lingering subscriptions and potential duplicate callbacks after restart.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugwise_usb/connection/queue.py (1)
88-114: Remove the mypy unreachable suppression by making branch reachability explicitThe
# type: ignore[unreachable]stems fromwhile request.resend:plus theelif request.resend:/elsechain. Make the loop unconditional and control flow explicit withbreak/continue. This preserves behavior, avoids the suppression, and reads clearer.Apply:
- while request.resend: - _LOGGER.debug("submit | start (%s) %s", request.retries_left, request) + while True: + if not request.resend: + break + _LOGGER.debug("submit | start (%s) %s", request.retries_left, request) if not self._running or self._stick is None: raise StickError( f"Cannot send message {request.__class__.__name__} for" + f"{request.mac_decoded} because queue manager is stopped" ) await self._add_request_to_queue(request) if request.no_response: return None try: response: PlugwiseResponse = await request.response_future() return response except (NodeTimeout, StickTimeout) as exc: if isinstance(request, NodePingRequest): # For ping requests it is expected to receive timeouts, so lower log level _LOGGER.debug( "%s, cancel because timeout is expected for NodePingRequests", exc, ) - elif request.resend: - _LOGGER.debug("%s, retrying", exc) - else: - _LOGGER.debug("%s, cancel request", exc) # type: ignore[unreachable] + continue + if request.resend: + _LOGGER.debug("%s, retrying", exc) + continue + _LOGGER.debug("%s, cancel request", exc) + break except StickError as exc: _LOGGER.error(exc) raise StickError( f"No response received for {request.__class__.__name__} " + f"to {request.mac_decoded}" ) from exc except BaseException as exc: raise StickError( f"No response received for {request.__class__.__name__} " + f"to {request.mac_decoded}" ) from exc
📜 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.
📒 Files selected for processing (2)
plugwise_usb/connection/queue.py(1 hunks)plugwise_usb/nodes/circle.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- plugwise_usb/nodes/circle.py
⏰ 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: Run pytest using Python 3.13
🔇 Additional comments (1)
plugwise_usb/connection/queue.py (1)
113-113: Downgrading “cancel request” to DEBUG is appropriate for offline-node noise reductionThis aligns with the PR’s goal to remove noisy warnings when nodes are offline and timeouts are expected. Good call.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugwise_usb/connection/queue.py (2)
92-96: Nit: missing space in error message (“for{mac}”)There’s no space between “for” and the MAC, making the message harder to read.
- f"Cannot send message {request.__class__.__name__} for" - + f"{request.mac_decoded} because queue manager is stopped" + f"Cannot send message {request.__class__.__name__} for " + f"{request.mac_decoded} because queue manager is stopped"
106-112: Log wording suggests “cancel” but the code retries for ping timeoutsThe message “cancel because timeout is expected for NodePingRequests” is misleading: you
continue(retry) rather than cancel. Consider rewording to avoid confusion, e.g., “timeout expected for NodePingRequest, will retry if allowed”.
📜 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.
📒 Files selected for processing (2)
plugwise_usb/connection/queue.py(2 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/connection/queue.py (1)
plugwise_usb/messages/requests.py (1)
resend(329-331)
🔇 Additional comments (1)
plugwise_usb/connection/queue.py (1)
88-90: Incorrect review:resendis a @Property — attribute access is correctThe repository shows
@property def resend(self) -> boolin plugwise_usb/messages/requests.py (lines ~328–331), sorequest.resendis a boolean property. The original suggestion to callrequest.resend()is incorrect and would break the code.Files/locations confirming this:
- plugwise_usb/messages/requests.py —
@property def resend(...) -> bool(lines ~328–331)- plugwise_usb/connection/queue.py — uses
request.resend(lines ~89 and ~113)- plugwise_usb/connection/manager.py — uses
request.resend(line ~164)Action: ignore the original suggestion — no change required for these call sites.
Likely an incorrect or invalid review comment.
trying to reduce amount of warning messages for an offline node
bouwew
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugwise_usb/network/__init__.py (3)
300-305: Good unsubscribe cleanup for node join/rejoin; consider DRY and safety guard.Great addition to ensure join/rejoin subscriptions are properly torn down. Two small nits:
- Factor the repeated “if unsub: call + set None” into a helper to reduce duplication.
- Optionally wrap unsubscribe calls in a try/except to avoid teardown explosions if an unsubscribe raises (e.g., racey double-call).
Example helper (outside this range) you could add:
def _safe_unsubscribe(self, attr_name: str) -> None: unsub = getattr(self, attr_name) if unsub: try: unsub() except Exception: # be strict if you prefer _LOGGER.debug("Unsubscribe failed for %s", attr_name, exc_info=True) finally: setattr(self, attr_name, None)Then here:
self._safe_unsubscribe("_unsubscribe_node_join") self._safe_unsubscribe("_unsubscribe_node_rejoin")
331-334: Timeout message is clearer; minor copyedit for consistency (Zigbee/MAC/USB-Stick casing).To align with casing used elsewhere in the module (“USB-Stick zigbee network class.”) and common conventions, consider:
- f"The zigbee network coordinator (Circle+/Stealth+) with mac " - f"'{self._controller.mac_coordinator}' did not respond to the ping request. " - "Make sure the Circle+/Stealth+ is within reach of the USB-stick!" + f"The Zigbee network coordinator (Circle+/Stealth+) with MAC " + f"'{self._controller.mac_coordinator}' did not respond to the ping request. " + "Make sure the Circle+/Stealth+ is within reach of the USB-Stick!"
426-428: Promoting to WARNING is fine; gate it to first occurrence per node to avoid log spam.This warning will recur on every retry in _discover_stragglers for offline nodes, which can be noisy. Log WARNING only once per MAC, then DEBUG thereafter, and avoid duplicate entries in _registry_stragglers.
- if node_info is None: - _LOGGER.warning( - "Node %s with unknown NodeType not responding, is it offline?", mac - ) - self._registry_stragglers.append(mac) - return False + if node_info is None: + if mac not in self._registry_stragglers: + _LOGGER.warning( + "Node %s with unknown NodeType not responding, is it offline?", mac + ) + self._registry_stragglers.append(mac) + else: + _LOGGER.debug( + "Node %s with unknown NodeType still not responding (likely offline)", mac + ) + return FalseOptional follow-up: converting
_registry_stragglersto a set would also prevent duplicates and simplify membership checks.
📜 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.
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)plugwise_usb/connection/queue.py(2 hunks)plugwise_usb/connection/sender.py(0 hunks)plugwise_usb/messages/requests.py(6 hunks)plugwise_usb/network/__init__.py(4 hunks)plugwise_usb/nodes/circle.py(1 hunks)pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- plugwise_usb/connection/sender.py
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (4)
- pyproject.toml
- plugwise_usb/nodes/circle.py
- plugwise_usb/messages/requests.py
- plugwise_usb/connection/queue.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugwise_usb/network/__init__.py (2)
plugwise_usb/__init__.py (1)
mac_coordinator(138-140)plugwise_usb/connection/__init__.py (1)
mac_coordinator(81-90)
⏰ 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: Check commit
🔇 Additional comments (1)
plugwise_usb/network/__init__.py (1)
484-488: Load failure is only logged at DEBUG — confirm this won’t hinder triage.If a node consistently fails to load, there won’t be a visible INFO/WARNING trail. That might make field diagnostics harder. Two options:
- Keep this at DEBUG but add a per-MAC counter and escalate to WARNING after N consecutive failures.
- Or enrich the debug with a reason if
_load_nodecan surface one.Do you already emit a user-facing WARNING elsewhere on persistent load failures (e.g., from within node.load())? If not, would you like a small per-MAC backoff counter here to warn after repeated failures?
This reverts commit 990025c.
|
I see many
No open request for StickResponse()messages in my HA Logs.This is caused by a Circle that is offline. Repeated
NodePingRequestsare not answered leading to many of these warning messages.Looking at the debug-log, there are several more clear debug-messages present related to this issue, therefore I propose to remove sending this particularly unclear message in
sender.py, and "upgrading" the message innetwork/__init__.pyline 426 to a warning.Also several nitpicks from CRAI are coming along in this PR :)
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Documentation
Chores