Skip to content

fix(homebridge): stop the Grid Charging switch rewriting the export rule - #141

Merged
Bre77 merged 1 commit into
mainfrom
fm/homebridge-fix-grid-charging
Oct 1, 2026
Merged

Bre77 merged 1 commit into
mainfrom
fm/homebridge-fix-grid-charging

Conversation

@Bre77

@Bre77 Bre77 commented Sep 30, 2026

Copy link
Copy Markdown
Member

Toggling the Powerwall "Grid Charging" switch in HomeKit also rewrote the site's export rule: the service derived a rule from a truthiness check on site_info.components.customer_preferred_export_rule, which is a string (battery_ok, pv_only or never), so every site reporting any rule was sent never and a site reporting none was sent an invented battery_ok, silently turning solar or battery export off for owners who toggled the switch or had it in a scene. The switch now sends only disallow_charge_from_grid_with_solar_installed and the currentExportSetting state is deleted; the API accepts either body field alone, and the undefined as never cast is there only because the SDK's gridImportExport signature still makes the rule mandatory, which is left for the library fix rather than widened here. The previous test that pinned the battery_ok default is replaced by a regression test over four site shapes (battery_ok, pv_only, never, no rule) asserting the rule argument is never sent in either direction; it fails on main for all four and passes with the fix, alongside the package's full suite (159 passing), typecheck and lint. Fields relied on, per the api schema: customer_preferred_export_rule is an optional string enum on the export side that this switch no longer reads, and disallow_charge_from_grid_with_solar_installed is a boolean that site info reports only when true, so the existing absent-means-allowed read is unchanged.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: Teslemetry/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: f7227b0e-41ac-4d1b-82a2-3706c8af5974

📥 Commits

Reviewing files that changed from the base of the PR and between d321ce1 and 1391a8d.

📒 Files selected for processing (3)
  • .changeset/grid-charging-export-rule.md
  • packages/homebridge-teslemetry/src/energy-services/grid-charging.ts
  • packages/homebridge-teslemetry/test/energy-services.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The Grid Charging service no longer tracks or sends the site’s export rule when toggling grid charging. Regression tests cover four site export-rule shapes, and a patch changeset records the update.

Changes

Grid charging command

Layer / File(s) Summary
Omit export rule from grid charging commands
packages/homebridge-teslemetry/src/energy-services/grid-charging.ts, packages/homebridge-teslemetry/test/energy-services.test.ts, .changeset/grid-charging-export-rule.md
The service no longer tracks the site export rule and passes undefined for that command argument. Tests cover four site export-rule shapes and verify the grid-charging flags. The changeset records the patch.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 1391a

The Grid Charging switch no longer forwards an export rule, avoiding unintended export-setting changes. No actionable merge-blocking risk remains; the change is mergeable subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1391a

The switch now limits its request to grid charging while retaining the existing site and credential scope. No expanded access was identified, but the external service's behavior when the export field is omitted has not been independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A characteristic write selects the charging value, not a destination site or credential. Each invocation remains directed through the captured site's API object and its stored siteId. The inspected change does not expand site selection or account authority; external tenant authorization was not verified.

Trust Boundaries and Controls

  • observed — The operation retains the existing remote API boundary. Its generated request declares bearer and query-token authentication options, and the root client receives the configured access token. No server-side authorization implementation is available in the inspected evidence.

Resilience and Maintainability Implications

  • observed — The setter retains shared failure handling: non-benign command refusals reject, early failures become HomeKit communication errors, and late failures restore the prior characteristic only when no newer write owns it. The handler neither serializes concurrent requests nor cancels an in-flight request when its response timer expires. These existing mechanisms do not prove remote ordering, atomic updates, or recovery after ambiguous completion.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main fix: preventing the Grid Charging switch from rewriting the export rule.
Description check ✅ Passed The description directly explains the export-rule bug, the implementation change, and the regression tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@Bre77
Bre77 force-pushed the fm/homebridge-fix-grid-charging branch from f662995 to 1391a8d Compare October 1, 2026 07:49
@Bre77
Bre77 merged commit 9f2227f into main Oct 1, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant