fix: mqtt device tracker back in business#368
Conversation
github-actions
Bot
added
fix
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds MQTT subscription for GPS latitude updates, a new Changes
Sequence Diagram(s)sequenceDiagram
participant HA as HomeAssistant
participant Entity as RobonectEntity
participant Coord as Coordinator
participant MQTT as MQTT_Broker
participant REST as Robonect_REST_API
HA->>Entity: add entity (async_added_to_hass)
Entity->>HA: await super
Entity->>Coord: _handle_coordinator_update() -- initial state push
Coord-->>Entity: coordinator update (state/data)
alt MQTT enabled
Entity->>MQTT: subscribe to {topic}/gps/latitude
MQTT-->>Entity: mqtt message (latitude)
Entity->>Entity: async_mqtt_event_received -> update attributes (inc. last_synced)
else REST-only
REST-->>Entity: REST poll/update
Entity->>Entity: apply REST attributes (remove category), update state
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
custom_components/robonect/entity.py (1)
155-159: Consider defensive guard againstNonedata.
len(self.coordinator.data)raisesTypeErrorifdataisNone. Sinceasync_added_to_hassnow calls_handle_coordinator_update()immediately, the coordinator may not have fetched data yet.Proposed fix
`@callback` def _handle_coordinator_update(self) -> None: """Handle updated data from the coordinator.""" - if len(self.coordinator.data): + if self.coordinator.data: self.update_ha_state() return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/robonect/entity.py` around lines 155 - 159, The _handle_coordinator_update method should defensively handle coordinator.data possibly being None; change the check to ensure data is not None before calling len or simply test truthiness of self.coordinator.data, e.g. guard with "if self.coordinator.data is not None and len(self.coordinator.data):" or "if self.coordinator.data:" then call update_ha_state(); update the logic in _handle_coordinator_update (and keep async_added_to_hass behavior) so no TypeError occurs when coordinator.data is None.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@custom_components/robonect/device_tracker.py`:
- Around line 62-69: Remove the trailing whitespace at the end of the argument
lines inside the mqtt_subscribe_entry call to fix the pipeline failure; locate
the block gated by CONF_MQTT_ENABLED and the call to mqtt_subscribe_entry (using
CONF_MQTT_TOPIC and async_mqtt_event_received) and delete any extra spaces after
the commas/arguments (the lines currently ending with whitespace) so the file
has no trailing spaces on those lines.
- Around line 149-152: The property extra_state_attributes and the
satellites_received update can raise TypeError because self._attributes is
initialized to None; change the initialization of _attributes (used in the class
that defines extra_state_attributes and satellites_received) to an empty dict
instead of None so the dict merge (|) and inplace merge (|=) work safely, and if
you prefer add a defensive guard in extra_state_attributes to return
(self._attributes or {}) | {"last_synced": self.last_synced} and similarly use
(self._attributes or {}).update(...) in satellites_received; update references
to _attributes accordingly so no NoneType operations occur.
In `@custom_components/robonect/entity.py`:
- Around line 150-154: The docstring for async_added_to_hass in class/entity
needs to use imperative mood; change the first line from "Called when entity is
added to Home Assistant." to an imperative like "Handle entity being added to
Home Assistant." or "Run when the entity is added to Home Assistant." Update the
docstring above async_added_to_hass to the chosen imperative phrasing (keep the
rest of the docstring unchanged).
---
Nitpick comments:
In `@custom_components/robonect/entity.py`:
- Around line 155-159: The _handle_coordinator_update method should defensively
handle coordinator.data possibly being None; change the check to ensure data is
not None before calling len or simply test truthiness of self.coordinator.data,
e.g. guard with "if self.coordinator.data is not None and
len(self.coordinator.data):" or "if self.coordinator.data:" then call
update_ha_state(); update the logic in _handle_coordinator_update (and keep
async_added_to_hass behavior) so no TypeError occurs when coordinator.data is
None.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab70a239-1acf-4850-b0f4-7936576a92a9
📒 Files selected for processing (2)
custom_components/robonect/device_tracker.pycustom_components/robonect/entity.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
custom_components/robonect/device_tracker.py (1)
149-152:⚠️ Potential issue | 🟠 Major
TypeErrorif_attributesisNone.
self._attributesis initialized toNone(line 124). The dict merge operator|will raiseTypeError: unsupported operand type(s) for |: 'NoneType' and 'dict'if Home Assistant queries this property before any data is received.The same issue affects
satellites_receivedat line 207 where|=is used.,
Proposed fix - initialize `_attributes` to empty dict
self._longitude = None self._latitude = None self._satellites = None self._battery = None - self._attributes = None + self._attributes = {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/robonect/device_tracker.py` around lines 149 - 152, The property extra_state_attributes and the satellites_received update will raise TypeError if self._attributes is None; fix by initializing self._attributes to an empty dict ({} ) where it's defined and/or defensively using a dict fallback before merging (e.g. use (self._attributes or {}) when returning extra_state_attributes and before using |= in the satellites_received code path) so merges like self._attributes | {"last_synced": self.last_synced} and self._attributes |= {...} never operate on None.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@custom_components/robonect/device_tracker.py`:
- Around line 149-152: The property extra_state_attributes and the
satellites_received update will raise TypeError if self._attributes is None; fix
by initializing self._attributes to an empty dict ({} ) where it's defined
and/or defensively using a dict fallback before merging (e.g. use
(self._attributes or {}) when returning extra_state_attributes and before using
|= in the satellites_received code path) so merges like self._attributes |
{"last_synced": self.last_synced} and self._attributes |= {...} never operate on
None.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28af5ec3-5d67-4763-98d5-7cd44d88754c
📒 Files selected for processing (1)
custom_components/robonect/device_tracker.py
github-actions
Bot
added
the
chore
1 participant
Summary by CodeRabbit