Skip to content

refactor(planner): remove the inert battery-schedule feature - #873

Merged
woopstar merged 3 commits into
mainfrom
refactor/860-remove-battery-schedules
Sep 11, 2026
Merged

woopstar merged 3 commits into
mainfrom
refactor/860-remove-battery-schedules

Conversation

@woopstar

@woopstar woopstar commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Removes the fixed battery discharge/charge-schedule feature (batteries_schedule_1/2/3), which issue Triage: battery-schedule feature appears inert since MILP became the sole active planner #860 found to be functionally inert whenever MILP is active (the default planner path).
  • Also removes apply_arbitrage_grid_charge, which was gated entirely on at least one schedule being enabled.
  • current_required_battery / calculate_required_battery_until_solar are not touched — they're a separate mechanism feeding excess-export and the EV discharge-cap reserve, unrelated to fixed schedules.

Branch

refactor/860-remove-battery-schedules

Why

In MILP-only mode, candidate_generator.py only produces no_action, passive, and milp candidates — the schedule-consuming heuristic candidates are commented out (deleted entirely by #897, a separate PR). Both surviving fallback candidates (no_action, passive) explicitly clear all charge/discharge recommendations before scoring, and the MILP itself never reads battery_schedules at all. So whichever candidate wins, schedule-driven recommendations never reach the final plan.

This was already partially documented as intentional (docs/candidate-generation.md, docs/planner-spec.md), but the config, entities, and planner passes remained fully wired with no indication to users that they had no effect. See issue #860 for the full triage — the user chose full removal over re-wiring it as a MILP-unavailable fallback or leaving it documented as inert, explicitly overriding an earlier "keep fixed battery schedules" decision recorded in .github/memories.md (updated in this PR to reflect the reversal, and again to point at #897's follow-up cleanup).

This PR does not touch candidate_generator.py/candidate_selector.py/docs/candidate-generation.md/docs/planner-spec.md — those files are exclusively owned by #897, so the two PRs have zero file overlap and can merge in either order.

What changed

Config / UI

  • Removed the batteries_schedules config-flow step (flows/batteries_schedules.py, flows/schedule_helpers.py) and repointed ocppbatteries_wait_mode directly in both config_flow.py and options_flow.py.
  • Removed the six hsem_batteries_enable_batteries_schedule_* config keys from const.py.
  • Removed the 3 schedule switches and 6 start/end time entities (switch.py, time.py, utils/sensornames/controls.py, custom_switches/description.py, custom_times/description.py).
  • Removed the now-dead validate_time_window validator (its only caller was the schedule step).

Models

  • Deleted models/battery_schedule.py, models/battery_schedule_input.py, and the BatteryScheduleConfig dataclass in models/sensor_config.py.
  • Removed the battery_schedules field from PlannerInput and CoordinatorData.

Planner

  • Deleted planner/charging/pre_charge.py (apply_charge_schedules) and planner/charging/arbitrage_charge.py (apply_arbitrage_grid_charge).
  • Removed apply_discharge_schedules from discharge_scheduler.py.
  • Removed all three call sites from engine_core.py::_schedule_slots.
  • charge_scheduler.py / planner/charging/__init__.py are now thin re-export shims for the surviving apply_opportunistic_charge / apply_window_hysteresis.

Coordinator

  • Removed _batteries_schedules / _batteries_schedules_remaining_capacity_needed state, the build_battery_schedules() call, and the batteries_schedules kwarg threaded through build_planner_input().
  • Removed the resolver rule in recommendation_resolver.py that forced BatteriesDischargeMode based on remaining schedule capacity.

Docs / translations

  • Updated docs/architecture-overview.md, docs/config-flow-reference.md, docs/planner-guide.md, docs/sensors-reference.md, and docs/dashboard.yaml (removed the Battery Schedules dashboard section).
  • Removed the corresponding keys from translations/en.json and translations/da.json.

Tests

  • Deleted whole files that were 100% schedule-specific: test_schedule_validation.py, test_arbitrage_grid_charge.py, test_charge_scheduler_capacity.py, test_48h_second_day.py.
  • Deleted schedule-only classes/methods embedded in otherwise-unrelated files (e.g. TestP004Schedule3Default, TestBatteryAboveScheduleNeed, TestEvLoadDoesNotInflateChargeNeeded, TestScheduleValidator).
  • Updated shared fixtures (tests/planner/fixtures.py and others) that threaded a schedules/battery_schedules kwarg through PlannerInput construction.
  • tests/planner/test_cycle_cost_guard.py::_make_two_slot_input and tests/planner/test_invariants.py::test_grid_import_exceeds_stored_with_conversion_loss were redesigned to use natural price-arbitrage incentives instead of a forced discharge schedule — the MILP doesn't need a schedule to decide when to charge, so this preserves the invariant coverage without the removed feature.
  • Repointed tests that used a schedule key purely as generic example data (test_entity_platform_classes.py, test_platform_entity_refactor.py) to a surviving key (hsem_ev_deadline_time).

Test and lint results

All four quality gates pass:

./scripts/quality.sh lint     ✅ ruff format + ruff check + prettier — clean
./scripts/quality.sh typing   ✅ mypy — 0 errors
./scripts/quality.sh quality  ✅ pyright + vulture — 0 errors (1 pre-existing unrelated warning)
./scripts/quality.sh test     ✅ 2926 passed

Known limitations / follow-ups

None expected. flows/migrations.py's _V1_DEPRECATED_KEYS (a different, already-deprecated ..._min_price_difference sub-key list) is left untouched — it's a pre-existing migration cleanup unrelated to the current batteries_schedule_N toggle/start/end keys removed here.

Fixes #860

@github-actions github-actions Bot added the area:refactor Structure changes without intended behavior change label Sep 1, 2026
@woopstar woopstar added the pr-on-hold This PR is on hold and not scheduled for merge currently label Sep 1, 2026
…860

Issue #860 found that fixed battery discharge/charge schedules
(batteries_schedule_1/2/3) are functionally inert whenever MILP is active
(the default): the schedule-consuming heuristic candidates are commented
out in MILP-only mode, and both surviving candidates (no_action, passive)
discard schedule-derived recommendations before scoring. MILP itself never
reads battery_schedules at all.

The user explicitly chose full removal over re-wiring it as a
MILP-unavailable fallback or leaving it documented as inert, consciously
overriding an earlier "keep fixed battery schedules" decision recorded in
.github/memories.md (also updated here to reflect the reversal).

Removed:
- Config flow step (batteries_schedules), its schema/validator helpers,
  and the six hsem_batteries_enable_batteries_schedule_* config keys
- BatterySchedule / BatteryScheduleInput / BatteryScheduleConfig models
- The batteries_schedule_1/2/3 switches and their start/end time entities
- apply_discharge_schedules, apply_charge_schedules, and
  apply_arbitrage_grid_charge planner passes (and their dedicated modules)
- batteries_schedules_remaining_capacity_needed and the resolver rule that
  used it to force BatteriesDischargeMode
- The now-dead validate_time_window validator (only caller was the
  schedule step)
- Corresponding translations (en/da), docs, and dashboard.yaml section

current_required_battery / calculate_required_battery_until_solar are
unrelated (feed excess-export and the EV discharge-cap reserve) and are
untouched.
@woopstar
woopstar force-pushed the refactor/860-remove-battery-schedules branch from daa53d3 to 53b029b Compare September 2, 2026 12:05
…andidates

The MILP-only mode note added for issue #860 described the schedule-
consuming heuristic candidates as merely commented out. Issue #897
deletes that dead code entirely, so update the memory to point at the
final state instead of an intermediate one.
Brings the battery-schedule removal branch up to date with ~48 commits on
main so it can merge cleanly as part of the 7.0.0 release batch alongside
#910 and #904.

Conflict resolution:
- coordinator_data.py: kept main's current_wait_mode_reserve docstring
  (issue #914) alongside this branch's already-trimmed docstring; dropped
  the batteries_schedules doc lines this branch removes.
- custom_sensors/config_reader.py: dropped main's still-present
  build_battery_schedules() (removed by this branch).
- custom_sensors/working_mode_sensor.py: kept main's _write_phase_active
  try/finally wrapper (issue #951) and wait_mode_reserve_kwh threading
  (issue #914), while applying this branch's 3-arg
  resolve_current_recommendation() signature (drops the
  batteries_schedules_remaining_capacity_needed argument this branch
  removes from recommendation_resolver.py).
- translations/da.json: kept main's new unrelated error keys
  (invalid_wait_mode_behavior, port_conflict,
  ev_min_power_below_start_current) and dropped the
  batteries_schedules-only keys/step, matching what this branch already
  did to en.json.
- Modify/delete conflicts (models/battery_schedule.py,
  tests/planner/test_48h_second_day.py,
  tests/planner/test_arbitrage_grid_charge.py): took the deletion in all
  three cases; main's own concurrent edits there were trivial dead-field/
  dead-kwarg cleanups (#966, #974) that are moot once the file is removed.

Translation sync: removed the now-dead batteries_schedule_* step/entity
keys and the schedule-only error keys from de.json and es.json (added by
main after this branch's fork point, so they never got this branch's
original en.json/da.json cleanup). validate_translations.py now reports
0 missing/stale/placeholder-mismatch keys across en/da/de/es.

Also fixes two pre-existing, unrelated breakages inherited from main that
blocked a clean quality gate on this branch tip:
- tests/planner/test_invariants.py passed a stale is_read_only=True to
  PlannerInput; main's #974 vulture cleanup (ced164f) removed that dead
  field from the model entirely, and this branch's own rewritten
  conversion-loss test happened to still use it.
- tests/test_coordinator_tracking_forecast.py and
  tests/test_coordinator_tracking_solar_corrector.py (both new on main,
  issues #972/#973) called accumulate_forecast_actuals() with a
  signature that doesn't match its current implementation
  (solar_corrector_processed no longer exists;
  update_interval_minutes is required) -- 4 tests were failing at
  runtime on main's own tip, independent of this merge.

Also repointed one stray generic-example battery-schedule key in
tests/test_platform_entity_refactor.py to hsem_ev_deadline_time (the
original PR's own "repoint schedule-key example data" pass missed this
one occurrence), and updated docs/ev-charge-plan-setup.md's step-flow
diagram to drop the removed batteries_schedule_1/2/3 step.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@woopstar woopstar added major Bump major version number breaking-change Something in the release is breaking current functionality and removed pr-on-hold This PR is on hold and not scheduled for merge currently labels Sep 11, 2026
@woopstar
woopstar marked this pull request as ready for review September 11, 2026 10:47
@github-actions github-actions Bot removed breaking-change Something in the release is breaking current functionality labels Sep 11, 2026
@woopstar woopstar added the breaking-change Something in the release is breaking current functionality label Sep 11, 2026
@github-actions github-actions Bot removed the breaking-change Something in the release is breaking current functionality label Sep 11, 2026
@woopstar woopstar added the breaking-change Something in the release is breaking current functionality label Sep 11, 2026
@github-actions github-actions Bot removed the breaking-change Something in the release is breaking current functionality label Sep 11, 2026
@woopstar
woopstar merged commit 1e72108 into main Sep 11, 2026
29 of 30 checks passed
@woopstar
woopstar deleted the refactor/860-remove-battery-schedules branch September 11, 2026 12:03
woopstar pushed a commit that referenced this pull request Sep 11, 2026
Reconciles with #904 and #873, both merged to main ahead of this PR.
#873 (refactor/860-remove-battery-schedules) deleted the battery-schedule
switch/time entities (batteries_schedule_1/2/3 switches, schedule_N
start/end times) that this branch had also touched to add per-entity
_hsem_device dispatch. Resolution: dropped this branch's now-moot
hsem_device=BATTERY_ENERGY additions for those entities in switch.py,
time.py, and custom_times/description.py (including the now-unused
schedule-key imports) since the entities themselves no longer exist.
custom_switches/description.py and all four translation files merged
cleanly with no manual intervention needed.

Also repointed tests/test_device_split.py::test_switch_uses_description_hsem_device
from the now-removed "hsem_batteries_enable_batteries_schedule_1" example
key to the still-existing "hsem_dynamic_discharge_floor" switch, which is
also mapped to HSEMDevice.BATTERY_ENERGY — same coverage, valid key.

Full quality gate re-verified post-merge: lint clean, typing clean,
quality clean, 3271 tests passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
woopstar pushed a commit that referenced this pull request Sep 11, 2026
Reconciles with #904/#873/#910, all merged to main ahead of this PR.

Conflicts resolved:
- coordinator_cycle.py, coordinator_state.py, tests/test_ha_mock_integration.py:
  both sides added distinct new fields/state names — kept both.
- utils/sensornames/ev.py: kept this branch's new
  get_ev_soc_economics_sensor_{unique_id,entity_id}() (and second-EV
  equivalents), but dropped get_ev_soc_economics_sensor_name() /
  get_ev_second_soc_economics_sensor_name() — main's #912 already retired
  the _name()-getter convention repo-wide in favour of
  _attr_translation_key, which these sensors already partially adopted.

Integrated with #910's device split (issue #875):
- ev_soc_economics_sensor.py / ev_second_soc_economics_sensor.py: removed
  the leftover self._name assignment and custom `name` property override
  (dead code once _attr_translation_key is the only naming mechanism —
  main's other EV sensors don't override `name` either), and added
  self._hsem_device = HSEMDevice.EV_PRIMARY / EV_SECONDARY so these two
  new sensors attach to the correct per-subsystem device instead of the
  default CONTROLLER device.
- translations/en.json, da.json: dropped the redundant "2"/"EV 2" marker
  from ev_second_soc_economics's name to match #910's "device name
  already disambiguates primary/secondary" convention (identical to how
  ev_second_optimal_charging_plan reads the same as its primary sibling).
- translations/de.json, es.json: added the ev_soc_economics /
  ev_second_soc_economics entries outright — these two locale files were
  added by #910/main after this branch forked, so they never got this
  PR's original en/da-only translation pass.
- tests/sensors/test_ev_soc_economics_sensor.py: dropped the two
  now-invalid `sensor._name = ...` assignments in the object.__new__()
  test factories (mypy attr-defined errors) and set
  sensor._hsem_device explicitly instead, matching what the real
  __init__ now does.
- tests/test_device_split.py: added
  test_ev_primary_soc_economics_sensor / test_ev_secondary_soc_economics_sensor
  to TestPerEntityDeviceInfo, covering the new sensors' device dispatch
  the same way the existing EV calculated-power-sensor tests do.

Full quality gate re-verified post-merge: lint clean, typing clean,
quality clean, translations 0 errors across en/da/de/es, 3320 tests
passed (one MILP performance-budget test flaked once under load;
confirmed unrelated to this merge, passes in isolation).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:refactor Structure changes without intended behavior change major Bump major version number

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Triage: battery-schedule feature appears inert since MILP became the sole active planner

1 participant