Skip to content

Sungrow: add caveat for coarse energy resolution - #32743

Draft
naltatis wants to merge 1 commit into
masterfrom
sungrow-inverter-caveat
Draft

Sungrow: add caveat for coarse energy resolution#32743
naltatis wants to merge 1 commit into
masterfrom
sungrow-inverter-caveat

Conversation

@naltatis

Copy link
Copy Markdown
Member

relates to #32711

The SG series inverter reports total PV yield (register 5003) in whole 1 kWh steps, so 15-minute energy history advances in coarse chunks. This is a hardware register limitation, not something evcc can smooth out. Document it as a template caveat.

  • add caveat with 1 kWh resolution note to sungrow-inverter template

🤖 Generated with Claude Code

@naltatis naltatis added the devices Specific device support label Aug 11, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Double-check the YAML indentation of the caveats block — as written it appears nested under the first product entry rather than at the template level, which may prevent it from being applied as intended.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Double-check the YAML indentation of the `caveats` block — as written it appears nested under the first product entry rather than at the template level, which may prevent it from being applied as intended.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@andig

andig commented Aug 11, 2026

Copy link
Copy Markdown
Member

Instead of caveat we should just remove it as we did for other templates. Or revert all those?

@naltatis

Copy link
Copy Markdown
Member Author

Instead of caveat we should just remove it as we did for other templates. Or revert all those?

Thats a tradeoff discussion: long-term accuracy vs. short-term resolution. In context of upcoming month/years statistics having meter-based values is definitely more solid and will avoid discrepancies with our data and the vendors app.

as we did for other templates

Not sure what our minimum accuracy threshold was in the past. But yes, having this consistent thought all meters of a class would help.

@andig

andig commented Aug 12, 2026

Copy link
Copy Markdown
Member

Thats a tradeoff discussion

It is. But whatever we do should be consistent. This is a new approach.

@andig
andig marked this pull request as draft August 12, 2026 08:12
@naltatis naltatis added the needs decision Unsure if we should really do this label Aug 12, 2026
@premultiply

Copy link
Copy Markdown
Member

We could use the decimal part of register 5002 (0.1 kWh, UINT16, Daily power yields) to raise the resolution of 5003-5004.

@andig

andig commented Aug 18, 2026

Copy link
Copy Markdown
Member

This would require testing if .0 aligns with the coarse counter wrap-around.

@github-actions github-actions Bot added the stale Outdated and ready to close label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devices Specific device support needs decision Unsure if we should really do this stale Outdated and ready to close

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants