Skip to content

refactor(export): name the export_limit freeze/idle sentinels - #4606

Open
chalfontchubby wants to merge 1 commit into
mainfrom
refactor/soc-sentinel-constants
Open

refactor(export): name the export_limit freeze/idle sentinels#4606
chalfontchubby wants to merge 1 commit into
mainfrom
refactor/soc-sentinel-constants

Conversation

@chalfontchubby

Copy link
Copy Markdown
Collaborator

Summary

export_limits_best (and its per-inverter mirror export_limits) uses 99.0/100.0 as
undocumented sentinel values on top of a genuine 0-100 percent scale: any real value is a
literal export-to-this-SoC% target, 99.0 means "freeze export" (hold SoC, export only PV
surplus) and 100.0 means "idle/off". Nothing in the code named this - every reader had to
already know the convention or re-derive it from a comment at each call site.

  • Added EXPORT_LIMIT_FREEZE = 99.0 / EXPORT_LIMIT_IDLE = 100.0 to const.py.
  • Replaced every genuine sentinel comparison with the named constant across plan.py,
    execute.py, prediction.py, output.py, gateway.py, inverter.py, and enphase.py's
    schedule reconciler (_desired_schedule_families, which mirrors the same encoding for its
    own DTG/RBD split).
  • Deliberately left untouched: the rate_gas.get(start, 99) iboost fallback (coincidental
    same literal, unrelated to export at all), genuine charge-side 100/0 values (real
    "charge to full"/"off", not ambiguous the way export's overload is), and percent-to-fraction
    unit conversions (* soc_max / 100.0). solax.py needed no change - its freeze detection
    compares target vs current SoC dynamically rather than against a fixed sentinel.
    deye_const.py/sunsynk_const.py's own FREEZE_EXPORT_SOC = 99 was intentionally not
    consolidated into the shared constant: both are explicitly self-contained, reverse-engineered
    device-wire-protocol values (# VERIFY@SPIKE), a separate concern from Predbat's internal
    planning sentinel that just happens to coincide numerically.
  • Value-preserving only, no behaviour change. Confirmed prediction_kernel.cpp hardcodes the
    identical two literals independently (it can't import const.py) and left it functionally
    untouched - just added a comment pointing back to the new Python names. No parity revision
    bump or binary rebuild needed since nothing about the compiled logic changed.

Considered and deferred

  • Predicate helpers (is_export_freeze(limit) / is_export_idle(limit) /
    is_export_active(limit)) instead of raw < EXPORT_LIMIT_FREEZE comparisons would read more
    obviously at each of the ~35 call sites than re-deriving "below freeze means real target"
    from a bare comparison every time. Deferred to a follow-up PR rather than bundled in here -
    it's a second refactor on the same call sites so soon after this one, and would need writing
    in both Python and the C++ kernel mirror.
  • Sharing the actual numeric values with prediction_kernel.cpp rather than just mirroring
    them by literal - there's no existing codegen/shared-header precedent in this codebase for
    that; the only precedent (PREDBAT_MAX_CARS/PK_MAX_CARS) is a manually-kept-in-sync
    #define with a cross-referencing comment in each file, not real sharing. Named the C++ side
    too as a follow-up worth considering if this class of Python/kernel drift risk comes up again,
    but didn't want to couple it to this PR given it would mean rebuilding all six platform
    binaries for what is otherwise a zero-behaviour-change rename.

Test plan

  • ./run_all --quick - 743 tests, 4 slow skipped, all passed
  • run_pre_commit hooks (ruff, black, cspell) - clean
  • Manually reviewed every one of the ~50 replaced sites in context, and every left-alone
    same-valued literal, to confirm each was the same sentinel concept and not a coincidental
    numeric collision

🤖 Generated with Claude Code

export_limits_best used 99.0 (freeze export) and 100.0 (idle/off) as
undocumented magic numbers throughout plan.py, execute.py, prediction.py,
output.py, gateway.py, inverter.py and enphase.py's schedule reconciler.
Introduce EXPORT_LIMIT_FREEZE/EXPORT_LIMIT_IDLE in const.py and use them
at every genuine sentinel comparison, leaving unrelated same-valued
literals (the iboost gas-rate fallback, real charge-to-full/off values,
percent-to-fraction conversions) untouched.

Value-preserving only - prediction_kernel.cpp still hardcodes the same
two literals independently and is annotated to point back here, but is
otherwise unchanged, so no parity revision bump or binary rebuild is
needed for this change.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant