Skip to content

Huawei SUN2000: add returnEnergy for grid and battery#29790

Merged
premultiply merged 10 commits into
evcc-io:masterfrom
CiNcH83:huawei_importexport
Jun 2, 2026
Merged

Huawei SUN2000: add returnEnergy for grid and battery#29790
premultiply merged 10 commits into
evcc-io:masterfrom
CiNcH83:huawei_importexport

Conversation

@CiNcH83
Copy link
Copy Markdown
Contributor

@CiNcH83 CiNcH83 commented May 9, 2026

Depends on #29805.

Notes:
Huawei does not provide self-consumption information for the inverter (no returnenergy for pv).

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

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:

  • Renaming the existing energy fields to import/export changes the metric contract; consider either keeping a backward-compatible energy alias or explicitly handling the migration for existing consumers of this template.
  • The import register for the battery meter uses two separate if blocks for storageunit 1 and 2; if another value is passed, no address is set, so it may be safer to add an else/default branch or validation to avoid generating an invalid configuration.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Renaming the existing `energy` fields to `import`/`export` changes the metric contract; consider either keeping a backward-compatible `energy` alias or explicitly handling the migration for existing consumers of this template.
- The `import` register for the battery meter uses two separate `if` blocks for `storageunit` 1 and 2; if another value is passed, no address is set, so it may be safer to add an `else`/default branch or validation to avoid generating an invalid configuration.

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.

@CiNcH83 CiNcH83 force-pushed the huawei_importexport branch from f92fb19 to e7a1fc0 Compare May 10, 2026 09:36
@andig andig added the devices Specific device support label May 10, 2026
@andig andig marked this pull request as draft May 10, 2026 10:02
@CiNcH83
Copy link
Copy Markdown
Contributor Author

CiNcH83 commented May 19, 2026

@andig Think we are missing #29805.

@github-actions github-actions Bot added the stale Outdated and ready to close label May 26, 2026
@github-actions github-actions Bot closed this Jun 1, 2026
@andig andig reopened this Jun 1, 2026
@github-actions github-actions Bot removed the stale Outdated and ready to close label Jun 1, 2026
@CiNcH83 CiNcH83 marked this pull request as ready for review June 2, 2026 12:33
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • For the battery returnenergy register selection, consider handling or validating cases where .storageunit is not "1" or "2" to avoid rendering a config without an address.
  • The source, included modbus block, and timeout are repeated for each new meter; consider extracting these into a small reusable template/helper to reduce duplication and potential for configuration drift across meters.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- For the battery `returnenergy` register selection, consider handling or validating cases where `.storageunit` is not `"1"` or `"2"` to avoid rendering a config without an address.
- The `source`, included `modbus` block, and `timeout` are repeated for each new meter; consider extracting these into a small reusable template/helper to reduce duplication and potential for configuration drift across meters.

## Individual Comments

### Comment 1
<location path="templates/definition/meter/huawei-sun2000-hybrid.yaml" line_range="180-183" />
<code_context>
+    {{- include "modbus" . | indent 2 }}
+    timeout: {{ .timeout }}
+    register:
+      {{- if eq .storageunit "1" }}
+      address: 37066 # [Energy storage unit 1] Total charge
+      {{- end }}
+      {{- if eq .storageunit "2" }}
+      address: 37753 # [Energy storage unit 2] Total charge
+      {{- end }}
</code_context>
<issue_to_address>
**issue:** Handle the case where `.storageunit` is neither "1" nor "2" to avoid a register without an address.

As written, a `storageunit` value other than "1" or "2" produces a `returnenergy` register with no `address`. This may surface only as a runtime config error downstream. Either validate/constrain `.storageunit` earlier or add a default/`else` branch so invalid values fail explicitly instead of yielding a partially defined register.
</issue_to_address>

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.

Comment thread templates/definition/meter/huawei-sun2000-hybrid.yaml Outdated
@CiNcH83 CiNcH83 changed the title Huawei SUN2000: support import/export for grid/battery Huawei SUN2000: support returnenergy for grid and battery Jun 2, 2026
@CiNcH83 CiNcH83 changed the title Huawei SUN2000: support returnenergy for grid and battery Huawei SUN2000: add returnenergy for grid and battery Jun 2, 2026
@CiNcH83 CiNcH83 changed the title Huawei SUN2000: add returnenergy for grid and battery Huawei SUN2000: add returnEnergy for grid and battery Jun 2, 2026
@CiNcH83
Copy link
Copy Markdown
Contributor Author

CiNcH83 commented Jun 2, 2026

/cc @premultiply

@premultiply premultiply self-assigned this Jun 2, 2026
@premultiply premultiply enabled auto-merge (squash) June 2, 2026 16:43
@premultiply premultiply merged commit 9218cf0 into evcc-io:master Jun 2, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

devices Specific device support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants