Add Home Assistant add-on UI configuration generator#414
Conversation
Surface the Hampel outlier filter and PID controller through the Home Assistant add-on as optional Configuration fields (no defaults, so they stay hidden/disabled unless set), wiring them through config.yaml schema, translations, and run.sh. Add a third "Home Assistant add-on" target to the web config generator that emits a ready-to-paste add-on options YAML (including the newly exposed filters), restricted to a single Home Assistant power source with a pointer to custom config.ini for advanced setups. Update README and CHANGELOG.
|
Warning Review limit reached
More reviews will be available in 2 minutes and 10 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis pull request adds support for a new "homeassistant" configuration target in the web-based config generator. It includes Home Assistant add-on schema updates for Hampel filtering and PID controller parameters, runtime integration to pass these settings into the core config.ini, a new YAML-based options generator, UI field filtering by target, and updated documentation. ChangesHome Assistant Add-on Target Configuration and Generation
Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI as Web Config Generator
participant AppState as app.ts State
participant Generator as generate.ts
participant Output as Output File
User->>WebUI: Select "Home Assistant add-on" target
WebUI->>AppState: Update state.target = "homeassistant"
AppState->>AppState: coerceHaMeter() enforces single meter
AppState->>WebUI: Re-render UI with HA field filters
User->>WebUI: Configure filters & options
WebUI->>AppState: Update state with user settings
User->>WebUI: Click preview/download
WebUI->>Generator: Call generate(state)
Generator->>Generator: Detect homeassistant target
Generator->>Generator: Call generateHomeAssistant(state)
Generator->>Output: Emit YAML options block
Output-->>User: Display/download add-on options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
The add-on sets the Home Assistant host, port, token and API path automatically, so the HA target now only surfaces the grid-power entity fields instead of the full homeassistant powermeter form.
For the Home Assistant target, hide settings the add-on configures automatically or doesn't expose: general device IDs / skip-test / web editor, CT active-control / balancer / saturation fields, Marstek base URL / timezone, and the MQTT discovery/topic fields. The MQTT section is reframed as an optional custom broker, and throttle / wait-for-push are shown only once under General.
CT003 isn't just an older CT002 variant: CT002 clamps around the phases while CT003 reads via the electricity meter's interface. They share the same protocol.
Drop the hardware-matching framing: AstraMeter emulates both and the protocol is identical, so there's no difference from the battery's perspective and the choice doesn't depend on physical hardware.
CT002 is the recommended emulation for the common multi-battery case, so make it the default device type instead of Shelly Pro 3EM.
These are CT002/CT003 features, so enable them by default (the default device type is now CT002) and keep them in sync when CT emulation is toggled in the device card — edge-triggered so a manual override sticks. Guard the config.ini generators to omit an enabled-but-unconfigured [MARSTEK] / [MQTT_INSIGHTS] section so the default-on toggles never produce broken output before credentials/broker are entered.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/ts/state.ts (1)
121-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize
meterswhen restoring the Home Assistant target.
coerceHaMeter()only runs on the target button click. A loaded share link/project can already havetarget: "homeassistant"with multiple meters or a non-homeassistantfirst meter, andgenerateHomeAssistant()inweb/ts/generate.tsalways readsstate.meters[0]. That leaves restore paths generating blank or wrong YAML until the user toggles targets manually. Migration should enforce the same single-meter HA contract as the UI.Proposed fix
export function migrate(s: any): State { const d = defaultState(); s = s && typeof s === "object" ? s : {}; - const meters = Array.isArray(s.meters) && s.meters.length ? s.meters : d.meters; + const target = s.target === "esphome" || s.target === "homeassistant" ? s.target : "python"; + const restoredMeters = Array.isArray(s.meters) && s.meters.length ? s.meters.map(cleanMeter) : d.meters; + const meters = + target === "homeassistant" + ? [restoredMeters.find((m) => m.type === "homeassistant") ?? newMeter("homeassistant")] + : restoredMeters; return { ...d, ...s, - target: s.target === "esphome" || s.target === "homeassistant" ? s.target : "python", + target, general: ((): State["general"] => { const sg = s.general && typeof s.general === "object" ? s.general : {}; const dg = d.general; return { deviceTypes: Array.isArray(sg.deviceTypes) ? sg.deviceTypes.map((t: unknown) => String(t)) : dg.deviceTypes, @@ esphome: { ...d.esphome, ...(s.esphome || {}) }, ct: { fields: asObject(s.ct && s.ct.fields) }, marstek: { enabled: !!(s.marstek && s.marstek.enabled), fields: asObject(s.marstek && s.marstek.fields) }, mqttInsights: { enabled: !!(s.mqttInsights && s.mqttInsights.enabled), fields: asObject(s.mqttInsights && s.mqttInsights.fields) }, - meters: meters.map(cleanMeter), + meters, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/ts/state.ts` around lines 121 - 145, When restoring state in the returned object (where meters is computed and mapped with cleanMeter), ensure the same single-meter Home Assistant contract used by the UI is enforced: if the resolved target is "homeassistant" then coerce and keep only a single HA meter (apply coerceHaMeter to meters[0] or to the first valid meter) and drop any additional meters so generateHomeAssistant() will read a correct first meter; update the meters assignment/normalization (the meters constant and the meters: meters.map(cleanMeter) output) to perform this normalization before mapping with cleanMeter and reference coerceHaMeter and generateHomeAssistant in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/ts/generate.ts`:
- Around line 568-581: yamlScalar currently returns unquoted digit-only scalars
which can lose leading zeros; update generation so that when calling yamlScalar
from generateHomeAssistant (via the add function) you pass the option key (or
otherwise detect field type) and force quoting for known string-typed fields
(e.g., "ct_mac", "mac", "mac_address", "password", "entity_id", "id", "serial")
and for any all-digit values for those keys; keep yamlScalar’s numeric and
boolean detection for other keys but ensure add calls yamlScalar with the key so
it can decide to quote, and add a regression for ct_mac values like
"001122334455". Ensure references: yamlScalar, generateHomeAssistant, add, opts,
isBlank.
- Around line 632-640: The mqtt URI builder incorrectly treats mi.TLS as truthy
for non-boolean strings and doesn't percent-encode credentials; update the block
that reads state.mqttInsights / const mi to normalize TLS (e.g., treat mi.TLS as
true only when the value explicitly equals boolean true or the string
"true"/"1") and percent-encode USER and PASSWORD before composing the credential
portion used by add("mqtt_uri", ...); ensure you build cred using the encoded
username and encoded password (and still omit the ":" or "@" when empty) so
parse_mqtt_uri can safely unquote them.
---
Outside diff comments:
In `@web/ts/state.ts`:
- Around line 121-145: When restoring state in the returned object (where meters
is computed and mapped with cleanMeter), ensure the same single-meter Home
Assistant contract used by the UI is enforced: if the resolved target is
"homeassistant" then coerce and keep only a single HA meter (apply coerceHaMeter
to meters[0] or to the first valid meter) and drop any additional meters so
generateHomeAssistant() will read a correct first meter; update the meters
assignment/normalization (the meters constant and the meters:
meters.map(cleanMeter) output) to perform this normalization before mapping with
cleanMeter and reference coerceHaMeter and generateHomeAssistant in your change.
🪄 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: e1283e9f-4a59-45ad-9d8c-4c419fdf489c
📒 Files selected for processing (11)
CHANGELOG.mdREADME.mdha_addon/config.yamlha_addon/run.shha_addon/translations/en.yamlweb/generator.htmlweb/ts/app.tsweb/ts/generate.test.tsweb/ts/generate.tsweb/ts/schema.tsweb/ts/state.ts
- Quote string-typed add-on options (ct_mac, entity aliases, transforms, credentials, mqtt_uri) so all-digit values like a MAC keep leading zeros and aren't coerced to numbers. - Normalize mqtt_uri building: treat TLS as on only for explicit true/'true'/'1' (restored state may carry a string), and percent-encode the username/password. - Enforce the single-Home-Assistant-meter contract on state restore so generateHomeAssistant() always reads a correct first meter. Adds regressions for the MAC, TLS/credential, and restore cases.
Summary
Adds a new "Home Assistant add-on (UI options)" target to the web config generator that produces YAML configuration options for the Home Assistant add-on's Configuration tab. This complements the existing
config.iniand ESPHome targets, making it easier for users to configure the add-on through the UI without manually editing YAML.Key Changes
New generator function (
generateHomeAssistant) that emits YAML key-value pairs for the add-on's Configuration schema, including:Updated state model to support
"homeassistant"as a valid target alongside"python"and"esphome"UI improvements:
config.inifor advanced setupsastrameter-options.yamlfor this target)Add-on configuration schema (
config.yaml) and translations (en.yaml) extended with new filter options:hampel_window,hampel_n_sigma,hampel_min_threshold(Hampel outlier filter)pid_kp,pid_ki,pid_kd,pid_output_max,pid_mode(PID controller)Add-on runtime (
run.sh) updated to pass these new filter options from the Configuration tab to the generated configTests added to verify correct YAML generation for full filter sets, minimal configs, and calculated power scenarios
Documentation updated (README, CHANGELOG) to reflect the new target and explain the add-on's single-meter constraint
Implementation Details
The generator intelligently handles:
The UI enforces the add-on's architectural constraint (single Home Assistant sensor) by automatically collapsing the meter list when this target is selected, while still allowing users to switch back to
config.inifor multi-meter or advanced scenarios.https://claude.ai/code/session_01QFtgjskrRiwoJ96kstNyX2
Summary by CodeRabbit
New Features
Documentation