Add AstraMeter device connections and improve device naming#311
Merged
Conversation
…er connection - CT002 / Shelly meter device names now include "AstraMeter". - Meter and consumer/battery devices share an `astrameter` connection tuple so Home Assistant can correlate consumers with their meter.
Resolve the add-on slug from the supervisor (`/store/addons/self`) in run.sh and forward it as `ADDON_SLUG` in the `[MQTT_INSIGHTS]` section. The MQTT Insights service then sets `via_device: <addon_slug>` on the CT002 and Shelly meter discovery payloads so Home Assistant nests them under the AstraMeter add-on device.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe add-on runtime now queries Supervisor for its slug and emits ADDON_SLUG in the generated MQTT discovery config when available; the app reads ADDON_SLUG into its config and discovery builders optionally include that slug as device.via_device and add "AstraMeter" branding to device names. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Supervisor
participant AddonRun as "ha_addon/run.sh"
participant AstraService as "AstraMeter Service"
participant MQTT as "MQTT Broker"
participant HomeAssistant as "Home Assistant"
Supervisor->>AddonRun: GET /addons/self/info (retrieve .slug)
AddonRun-->>AddonRun: parse .slug (store ADDON_SLUG)
AddonRun->>AstraService: start process (env/config includes ADDON_SLUG)
AstraService->>AstraService: read config (ADDON_SLUG -> cfg.addon_slug)
AstraService->>MQTT: publish discovery payloads (include via_device=ADDON_SLUG when set)
MQTT->>HomeAssistant: Home Assistant consumes discovery topics
HomeAssistant-->>HomeAssistant: create devices/entities linked via device.via_device
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/astrameter/mqtt_insights/mqtt_insights_test.py (1)
309-323: Consider assertingaddon_slug is Nonein default/empty config tests.You already assert the populated case; adding explicit
Noneassertions in the default/empty tests would close the regression loop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/astrameter/mqtt_insights/mqtt_insights_test.py` around lines 309 - 323, Add an assertion that addon_slug is None in the default/empty config tests to ensure regressions are caught: update the tests that call read_mqtt_insights_config for default/empty configs (the same test functions asserting broker/port/tls etc.) to include assert result.addon_slug is None so the code path handling addon_slug is validated alongside the populated case already covered in the existing test that asserts addon_slug == "34dea19a_astrameter".src/astrameter/config/config_loader.py (1)
573-573: TrimADDON_SLUGbefore coercing toNone.Small hardening: whitespace-only values should not propagate as a slug.
Suggested diff
- addon_slug=config.get(section, "ADDON_SLUG", fallback="") or None, + addon_slug=( + config.get(section, "ADDON_SLUG", fallback="").strip() or None + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/astrameter/config/config_loader.py` at line 573, The ADDON_SLUG value is being coerced to None without trimming, so whitespace-only values become a non-empty slug; fetch the raw value via config.get(section, "ADDON_SLUG", fallback=""), strip it (e.g. .strip()) and then coerce to None if the stripped string is empty—i.e., set addon_slug based on the stripped result; update the assignment that currently uses addon_slug=config.get(...) or None to perform the strip-before-none logic so whitespace-only values become None.ha_addon/run.sh (1)
149-153: Use more reliable endpoint to resolve add-on slug.The Supervisor API contract guarantees that
/store/addons/selfreturnsslugas a non-null field, so a literal"null"string guard is unnecessary. However, relying on the store layer is indirect—use/addons/self/infoinstead, which is the direct endpoint for the current add-on's metadata:addon_slug="$(bashio::api.supervisor GET '/addons/self/info' false | jq -r '.slug')"Alternatively, if available in this bashio version, use the
bashio::addonshelper which already calls this endpoint with proper defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ha_addon/run.sh` around lines 149 - 153, Replace the indirect Supervisor API call that reads '/store/addons/self' when resolving addon_slug in run.sh: use the direct add-on metadata endpoint '/addons/self/info' (or the bashio::addons helper if available) to reliably extract the slug into the addon_slug variable; update the bashio::api.supervisor invocation used where addon_slug is set and ensure the command parses the JSON .slug field (e.g., via jq -r '.slug' or the helper's output) so the check for non-empty addon_slug and the subsequent log messages behave correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ha_addon/run.sh`:
- Around line 149-153: Replace the indirect Supervisor API call that reads
'/store/addons/self' when resolving addon_slug in run.sh: use the direct add-on
metadata endpoint '/addons/self/info' (or the bashio::addons helper if
available) to reliably extract the slug into the addon_slug variable; update the
bashio::api.supervisor invocation used where addon_slug is set and ensure the
command parses the JSON .slug field (e.g., via jq -r '.slug' or the helper's
output) so the check for non-empty addon_slug and the subsequent log messages
behave correctly.
In `@src/astrameter/config/config_loader.py`:
- Line 573: The ADDON_SLUG value is being coerced to None without trimming, so
whitespace-only values become a non-empty slug; fetch the raw value via
config.get(section, "ADDON_SLUG", fallback=""), strip it (e.g. .strip()) and
then coerce to None if the stripped string is empty—i.e., set addon_slug based
on the stripped result; update the assignment that currently uses
addon_slug=config.get(...) or None to perform the strip-before-none logic so
whitespace-only values become None.
In `@src/astrameter/mqtt_insights/mqtt_insights_test.py`:
- Around line 309-323: Add an assertion that addon_slug is None in the
default/empty config tests to ensure regressions are caught: update the tests
that call read_mqtt_insights_config for default/empty configs (the same test
functions asserting broker/port/tls etc.) to include assert result.addon_slug is
None so the code path handling addon_slug is validated alongside the populated
case already covered in the existing test that asserts addon_slug ==
"34dea19a_astrameter".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a242aa47-eea8-4d08-bcfb-8b2575603768
📒 Files selected for processing (5)
ha_addon/run.shsrc/astrameter/config/config_loader.pysrc/astrameter/mqtt_insights/discovery.pysrc/astrameter/mqtt_insights/mqtt_insights_test.pysrc/astrameter/mqtt_insights/service.py
- run.sh: query the canonical /addons/self/info endpoint and parse the slug with jq instead of relying on the /store/addons/self route. - config_loader: strip ADDON_SLUG before falling back to None so whitespace-only values do not become a bogus slug. - tests: assert addon_slug is None for default/empty configs and add a whitespace-only regression test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR enhances MQTT device discovery by adding AstraMeter-specific device connections and improving device naming consistency across all device types.
Key Changes
astrameterconnection type to all device discovery payloads (CT002 consumer, CT002 device, Shelly battery, and Shelly device) to establish proper device correlation within the AstraMeter ecosystem["astrameter", "ct002_{device_id}"], even when consumer_id is not a MAC addressImplementation Details
["astrameter", "{device_type}_{device_id}"]to uniquely identify devices within the systemhttps://claude.ai/code/session_013FZJBrgH4U9vA8eqUzXYZP
Summary by CodeRabbit
New Features
Tests