Fix the accumulator helper being called with the wrong arity - #1039
Open
it-rec wants to merge 1 commit into
Open
Fix the accumulator helper being called with the wrong arity#1039it-rec wants to merge 1 commit into
it-rec wants to merge 1 commit into
Conversation
update_accum() takes (self, accum_value), but both meter accumulator sensors call it as update_accum(self, value, value). Every call raises TypeError, and because the call is wrapped in a bare "except Exception: return None" the error is swallowed, so the affected entities silently report None instead of a value. Pass the value once, and narrow the handler to ValueError, which is what update_accum legitimately raises for a non-increasing accumulator. A programming error is no longer hidden.
it-rec
force-pushed
the
fix/meter-accum-call-arity
branch
from
August 17, 2026 05:04
fd3fef6 to
df58a68
Compare
Owner
|
This went unnoticed because the other places update_accum() was used were actually removed years ago, and these were forgotten about because nobody uses them to report a problem (measurements are not supported by the most common meters provided by SolarEdge). It can be fixed but this is more of a reminder to stop using them and delete the helper completely. |
Author
|
Agree. Should I do it? |
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.
Problem
update_accum()is defined asupdate_accum(self, accum_value)inhelpers.py, but both meter accumulator sensors call it with three arguments:Every call raises
TypeError: update_accum() takes 2 positional arguments but 3 were given. The call sits inside a bareexcept Exception: return None, so the error is swallowed and the affected entities simply reportNoneforever. That is also why this went unnoticed: nothing appears in the log.Changes
ValueError, which is whatupdate_accumlegitimately raises for a non-increasing accumulator. A programming error is no longer hidden.Testing
Verified by reading the signature against the call sites - the mismatch is unconditional and does not depend on hardware.
On my own system the affected entities stay unavailable even after this fix, because my meter does not implement the corresponding scale factors:
M_VAh_SFandM_varh_SFboth return the not-implemented sentinel, and the existing scale-factor range check correctly keeps the entities unavailable. So I can confirm the crash is gone, but I cannot confirm a value on real hardware - that needs a meter that implements those registers.Checklist