Enhance CT002 consumer discovery with ARP lookup and device branding#303
Conversation
Rebrand device info: identifiers use astrameter_consumer_ prefix, manufacturer is Marstek, name is AstraMeter Consumer, model field replaces model_id. Add Bluetooth MAC connections (when consumer_id is a valid 12-char hex MAC) and network MAC via best-effort ARP lookup from /proc/net/arp. https://claude.ai/code/session_01RTxVw1HWeMwgaEV1C7Bb5w
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 44 seconds. ⌛ 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe changes modify the CT002 consumer Home Assistant MQTT discovery process to use updated branding (AstraMeter/Marstek), employ stable device identifiers, and augment device metadata with network connectivity information retrieved via ARP table lookup. Changes
Sequence DiagramsequenceDiagram
participant Event as CT002 Event
participant Service as Service
participant ARP as ARP Lookup
participant Discovery as Discovery Builder
participant HA as Home Assistant MQTT
Event->>Service: CT002 consumer event with battery_ip
Service->>Service: Extract battery_ip from event
Service->>ARP: _arp_lookup(battery_ip)
ARP->>ARP: Read /proc/net/arp
ARP-->>Service: Return MAC address
Service->>Discovery: build_ct002_consumer_discovery(network_mac, battery_ip, ...)
Discovery->>Discovery: Build device metadata with connections
Discovery-->>Service: Discovery payload
Service->>HA: Publish MQTT discovery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Keep model_id instead of model for device type. Add battery_ip as an "ip" connection type in the device discovery payload. https://claude.ai/code/session_01RTxVw1HWeMwgaEV1C7Bb5w
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/b2500_meter/mqtt_insights/service.py (1)
349-360: Retry the MAC backfill after first-discovery misses.
battery_ipis emitted on every CT002 event insrc/b2500_meter/ct002/ct002.py:585-625, but_arp_lookup()only runs inside the first-discovery branch. If that first lookup returns"", this consumer never gets itsnetwork_macconnection until reconnect/remove. Consider retrying until a valid MAC is found and republishing discovery once it is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/b2500_meter/mqtt_insights/service.py` around lines 349 - 360, The code only calls _arp_lookup when handling the first-discovery path, so if that lookup returns an empty network_mac the consumer never gets updated later; modify the consumer processing (where build_ct002_consumer_discovery is used) to retry _arp_lookup whenever battery_ip is present and network_mac is empty (e.g., on subsequent CT002 events or on a short retry/backoff) and, once a valid MAC is found, republish discovery via build_ct002_consumer_discovery with the new network_mac so the consumer gets updated; reference the _arp_lookup function for the lookup and build_ct002_consumer_discovery for republishing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/b2500_meter/mqtt_insights/discovery.py`:
- Around line 189-203: The code currently normalizes consumer_id into mac_slug
(via _sanitize_id(...).lower().replace("-", "").replace("_", "")) which
collapses distinct non-MAC IDs; change the logic so you only strip
hyphens/underscores when the value is a verified 12-hex MAC: compute a sanitized
value (e.g., raw_slug = _sanitize_id(consumer_id).lower()), then if
re.fullmatch(r"[0-9a-f]{12}", raw_slug.replace("-", "").replace("_", "")) treat
it as a MAC (build mac_slug by removing -/_ and use it for
identifiers/connections and BT MAC conversion), otherwise use the original
node_id (or raw_slug without stripping) as the unique identifier in
device_info["identifiers"] and name to avoid merging different non-MAC consumer
IDs; update references to mac_slug, device_info, and BT connection logic
accordingly.
---
Nitpick comments:
In `@src/b2500_meter/mqtt_insights/service.py`:
- Around line 349-360: The code only calls _arp_lookup when handling the
first-discovery path, so if that lookup returns an empty network_mac the
consumer never gets updated later; modify the consumer processing (where
build_ct002_consumer_discovery is used) to retry _arp_lookup whenever battery_ip
is present and network_mac is empty (e.g., on subsequent CT002 events or on a
short retry/backoff) and, once a valid MAC is found, republish discovery via
build_ct002_consumer_discovery with the new network_mac so the consumer gets
updated; reference the _arp_lookup function for the lookup and
build_ct002_consumer_discovery for republishing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e45e33e8-be1d-4dab-baaf-4932dff48f5c
📒 Files selected for processing (3)
src/b2500_meter/mqtt_insights/discovery.pysrc/b2500_meter/mqtt_insights/mqtt_insights_test.pysrc/b2500_meter/mqtt_insights/service.py
| mac_slug = _sanitize_id(consumer_id).lower().replace("-", "").replace("_", "") | ||
| identifiers = [node_id, f"hame_energy_{mac_slug}"] | ||
|
|
||
| device_info: dict = { | ||
| "identifiers": identifiers, | ||
| "name": f"HAME Energy {device_type} {mac_slug}" | ||
| "identifiers": [f"astrameter_consumer_{mac_slug}"], | ||
| "name": f"AstraMeter Consumer {device_type} {mac_slug}" | ||
| if device_type | ||
| else f"HAME Energy {mac_slug}", | ||
| "manufacturer": "HAME Energy", | ||
| else f"AstraMeter Consumer {mac_slug}", | ||
| "manufacturer": "Marstek", | ||
| } | ||
| connections: list[list[str]] = [] | ||
| if re.fullmatch(r"[0-9a-f]{12}", mac_slug): | ||
| bt_mac = ":".join( | ||
| mac_slug[i : i + 2] for i in range(0, len(mac_slug), 2) | ||
| ).upper() | ||
| connections.append(["bluetooth", bt_mac]) |
There was a problem hiding this comment.
Preserve uniqueness for non-MAC consumer IDs.
mac_slug becomes the only device identifier here, but the normalization strips - and _ even when consumer_id is not actually a MAC. Distinct IDs like abc-def and abc_def now collapse to the same astrameter_consumer_abcdef, so Home Assistant will merge different consumers into one device. Keep the stripped form only for verified MACs, and fall back to node_id otherwise.
Suggested fix
- mac_slug = _sanitize_id(consumer_id).lower().replace("-", "").replace("_", "")
+ safe_consumer_id = _sanitize_id(consumer_id).lower()
+ mac_slug = safe_consumer_id.replace("-", "").replace("_", "")
+ is_mac = re.fullmatch(r"[0-9a-f]{12}", mac_slug) is not None
+ consumer_suffix = mac_slug if is_mac else safe_consumer_id
device_info: dict = {
- "identifiers": [f"astrameter_consumer_{mac_slug}"],
- "name": f"AstraMeter Consumer {device_type} {mac_slug}"
+ "identifiers": (
+ [f"astrameter_consumer_{mac_slug}", node_id] if is_mac else [node_id]
+ ),
+ "name": f"AstraMeter Consumer {device_type} {consumer_suffix}"
if device_type
- else f"AstraMeter Consumer {mac_slug}",
+ else f"AstraMeter Consumer {consumer_suffix}",
"manufacturer": "Marstek",
}
connections: list[list[str]] = []
- if re.fullmatch(r"[0-9a-f]{12}", mac_slug):
+ if is_mac:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/b2500_meter/mqtt_insights/discovery.py` around lines 189 - 203, The code
currently normalizes consumer_id into mac_slug (via
_sanitize_id(...).lower().replace("-", "").replace("_", "")) which collapses
distinct non-MAC IDs; change the logic so you only strip hyphens/underscores
when the value is a verified 12-hex MAC: compute a sanitized value (e.g.,
raw_slug = _sanitize_id(consumer_id).lower()), then if
re.fullmatch(r"[0-9a-f]{12}", raw_slug.replace("-", "").replace("_", "")) treat
it as a MAC (build mac_slug by removing -/_ and use it for
identifiers/connections and BT MAC conversion), otherwise use the original
node_id (or raw_slug without stripping) as the unique identifier in
device_info["identifiers"] and name to avoid merging different non-MAC consumer
IDs; update references to mac_slug, device_info, and BT connection logic
accordingly.
Track consumers with missing network MAC in _pending_arp set. On subsequent CT002 events, retry the ARP lookup and republish discovery once a valid MAC is resolved. https://claude.ai/code/session_01RTxVw1HWeMwgaEV1C7Bb5w
2 participants
Summary
This PR improves the MQTT Home Assistant discovery for CT002 consumers by adding network MAC address resolution via ARP lookup and updating device branding from "HAME Energy" to "AstraMeter" with Marstek as the manufacturer.
Key Changes
ARP Lookup Implementation: Added
_arp_lookup()async function that queries/proc/net/arpto resolve IP addresses to MAC addresses. Gracefully handles missing files and invalid entries.Device Branding Updates:
hame_energy_*toastrameter_consumer_*model_idfield tomodelin device infoConnection Information:
AA:BB:CC:DD:EE:FF)network_macparameter is provided (resolved via ARP lookup frombattery_ip)Enhanced Discovery Parameters:
build_ct002_consumer_discovery()now accepts optionalnetwork_macparameter_handle_ct002_event()performs ARP lookup onbattery_ipfield and passes result to discovery builderTesting
Added comprehensive test coverage for:
https://claude.ai/code/session_01RTxVw1HWeMwgaEV1C7Bb5w
Summary by CodeRabbit
New Features
Updates