refactor(export): name the export_limit freeze/idle sentinels - #4606
Open
chalfontchubby wants to merge 1 commit into
Open
refactor(export): name the export_limit freeze/idle sentinels#4606chalfontchubby wants to merge 1 commit into
chalfontchubby wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
export_limits_best(and its per-inverter mirrorexport_limits) uses99.0/100.0asundocumented sentinel values on top of a genuine 0-100 percent scale: any real value is a
literal export-to-this-SoC% target,
99.0means "freeze export" (hold SoC, export only PVsurplus) and
100.0means "idle/off". Nothing in the code named this - every reader had toalready know the convention or re-derive it from a comment at each call site.
EXPORT_LIMIT_FREEZE = 99.0/EXPORT_LIMIT_IDLE = 100.0toconst.py.plan.py,execute.py,prediction.py,output.py,gateway.py,inverter.py, andenphase.py'sschedule reconciler (
_desired_schedule_families, which mirrors the same encoding for itsown DTG/RBD split).
rate_gas.get(start, 99)iboost fallback (coincidentalsame literal, unrelated to export at all), genuine charge-side
100/0values (real"charge to full"/"off", not ambiguous the way export's overload is), and percent-to-fraction
unit conversions (
* soc_max / 100.0).solax.pyneeded no change - its freeze detectioncompares target vs current SoC dynamically rather than against a fixed sentinel.
deye_const.py/sunsynk_const.py's ownFREEZE_EXPORT_SOC = 99was intentionally notconsolidated into the shared constant: both are explicitly self-contained, reverse-engineered
device-wire-protocol values (
# VERIFY@SPIKE), a separate concern from Predbat's internalplanning sentinel that just happens to coincide numerically.
prediction_kernel.cpphardcodes theidentical two literals independently (it can't import
const.py) and left it functionallyuntouched - 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
is_export_freeze(limit)/is_export_idle(limit)/is_export_active(limit)) instead of raw< EXPORT_LIMIT_FREEZEcomparisons would read moreobviously 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.
prediction_kernel.cpprather than just mirroringthem 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#definewith a cross-referencing comment in each file, not real sharing. Named the C++ sidetoo 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 passedrun_pre_commithooks (ruff, black, cspell) - cleansame-valued literal, to confirm each was the same sentinel concept and not a coincidental
numeric collision
🤖 Generated with Claude Code