Repository navigation
fix(homebridge): stop the Grid Charging switch rewriting the export rule - #141
Conversation
b5fbdff to
f662995
Compare
|
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 configurationConfiguration used: Repository: Teslemetry/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe 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. ChangesGrid charging command
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to 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 ReviewSecurity architecture risk: 🔵 Low · up to 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 Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
f662995 to
1391a8d
Compare
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_onlyornever), so every site reporting any rule was sentneverand a site reporting none was sent an inventedbattery_ok, silently turning solar or battery export off for owners who toggled the switch or had it in a scene. The switch now sends onlydisallow_charge_from_grid_with_solar_installedand thecurrentExportSettingstate is deleted; the API accepts either body field alone, and theundefined as nevercast is there only because the SDK'sgridImportExportsignature still makes the rule mandatory, which is left for the library fix rather than widened here. The previous test that pinned thebattery_okdefault 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_ruleis an optional string enum on the export side that this switch no longer reads, anddisallow_charge_from_grid_with_solar_installedis a boolean that site info reports only when true, so the existing absent-means-allowed read is unchanged.