Skip to content

Template Meter Add: Eltako Modbus#31127

Closed
SnejPro wants to merge 22 commits into
evcc-io:masterfrom
SnejPro:template-meter-add-Eltako-Modbus
Closed

Template Meter Add: Eltako Modbus#31127
SnejPro wants to merge 22 commits into
evcc-io:masterfrom
SnejPro:template-meter-add-Eltako-Modbus

Conversation

@SnejPro

@SnejPro SnejPro commented Jun 22, 2026

Copy link
Copy Markdown

Hi,

here's a proposal for a template for Eltako Modbus meters.

I had no change to test this template directly, but tested the connection with a custom device in evcc.

Greetings
Jens

@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 found 3 issues, and left some high level feedback:

  • The usage parameter is defined but never referenced in the render section; consider either using it (e.g., to specialize behavior per usage) or removing it to avoid confusion.
  • In requirements.description.de there is a typo (weerdenwerden), which might confuse users reading the German description.
  • The requirements.description is currently only provided in German; consider adding an English variant for consistency with the rest of the template metadata.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `usage` parameter is defined but never referenced in the `render` section; consider either using it (e.g., to specialize behavior per usage) or removing it to avoid confusion.
- In `requirements.description.de` there is a typo (`weerden``werden`), which might confuse users reading the German description.
- The `requirements.description` is currently only provided in German; consider adding an English variant for consistency with the rest of the template metadata.

## Individual Comments

### Comment 1
<location path="templates/definition/meter/eltako-modbus.yaml" line_range="9-10" />
<code_context>
+  description:
+    de: |
+      Getestet zusammen mit dem Modbus-Gateway von Eltako (ZGW16-IP)
+      Im Gateway müssen folgende Register aktiviert weerden: 30001, 30003, 30005, 30007, 30009, 30011, 30013, 30015, 30017, 30053, 30073, 30075 
+params:
+  - name: usage
</code_context>
<issue_to_address>
**suggestion (typo):** Fix the spelling of "weerden" in the German description.

"weerden" appears to be a typo and should be "werden" to keep the German description clear and professional for users.

```suggestion
      Getestet zusammen mit dem Modbus-Gateway von Eltako (ZGW16-IP)
      Im Gateway müssen folgende Register aktiviert werden: 30001, 30003, 30005, 30007, 30009, 30011, 30013, 30015, 30017, 30053, 30073, 30075 
```
</issue_to_address>

### Comment 2
<location path="templates/definition/meter/eltako-modbus.yaml" line_range="34-43" />
<code_context>
+      decode: int32
+    timeout: 2s
+
+  energy: # meter in kwh
+    source: modbus
+    id: {{ .meter_id }}
+    uri: {{ joinHostPort .host .port }}
+    register:
+      address: 72
+      type: input
+      decode: int32
+    timeout: 2s
+    scale: 0.01
+  returnenergy: # meter in kwh
+    source: modbus
+    id: {{ .meter_id }}
</code_context>
<issue_to_address>
**nitpick (typo):** Correct the unit casing from kwh to kWh in comments.

Suggested implementation:

```
  energy: # meter in kWh

```

```
  returnenergy: # meter in kWh

```
</issue_to_address>

### Comment 3
<location path="templates/definition/meter/eltako-modbus.yaml" line_range="10" />
<code_context>
+  description:
+    de: |
+      Getestet zusammen mit dem Modbus-Gateway von Eltako (ZGW16-IP)
+      Im Gateway müssen folgende Register aktiviert weerden: 30001, 30003, 30005, 30007, 30009, 30011, 30013, 30015, 30017, 30053, 30073, 30075 
+params:
+  - name: usage
</code_context>
<issue_to_address>
**issue (review_instructions):** Typo in German description ("weerden" → "werden"), which breaks wording consistency with other templates.

The German sentence has a typo: "weerden" should be "werden". Please adjust the wording so it’s consistent with the style and correctness of other templates’ localized descriptions.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `templates/**/*.yaml`

**Instructions:**
Verify that the changes comply to the rules defined in templates/README.md. Also check consistence (wording) with other files inside templates directory.

</details>
</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/eltako-modbus.yaml Outdated
Comment thread templates/definition/meter/eltako-modbus.yaml Outdated
Comment thread templates/definition/meter/eltako-modbus.yaml Outdated
@SnejPro

SnejPro commented Jun 22, 2026

Copy link
Copy Markdown
Author

What can I do to solve the failing check?

@andig andig marked this pull request as draft June 22, 2026 17:09
@andig

andig commented Jun 22, 2026

Copy link
Copy Markdown
Member

See https://github.com/evcc-io/evcc/blob/master/CONTRIBUTING.md#device-templates for testing plus PR must be green.

@SnejPro

SnejPro commented Jun 22, 2026

Copy link
Copy Markdown
Author

@andig
I finally found the reason for the failing checks:

grafik

Maybe the error reporting of the check could be improved.

@SnejPro SnejPro marked this pull request as ready for review June 22, 2026 21:06

@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:

  • The usage parameter is defined but never referenced in the rendered config; either wire it into the template (e.g., for conditional behavior) or remove it to avoid confusion.
  • The modbus connection block (source, id, uri, timeout) is repeated many times; consider using YAML anchors or a helper pattern to reduce duplication and make future changes less error-prone.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `usage` parameter is defined but never referenced in the rendered config; either wire it into the template (e.g., for conditional behavior) or remove it to avoid confusion.
- The modbus connection block (`source`, `id`, `uri`, `timeout`) is repeated many times; consider using YAML anchors or a helper pattern to reduce duplication and make future changes less error-prone.

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 marked this pull request as draft June 23, 2026 08:31
@andig andig added the devices Specific device support label Jun 23, 2026
Comment thread templates/definition/meter/eltako-modbus.yaml Outdated
Comment thread templates/definition/meter/eltako-modbus.yaml Outdated
Comment thread templates/definition/meter/eltako-modbus.yaml Outdated
decode: int32
scale: 0.01

powers: # phase powers in W

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are currents signed? Then powers should be removed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think so.
But when i remove them the per phase powers are missing.

Without per phase powers in .yaml:
Image

With per phase powers in .yaml:
Image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So? They're not used for anything.

@SnejPro SnejPro Jun 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Personally I'm interested in this and evcc already shows them for my Sunny Home Manager aswell.

I would suggest to make the power readings per phase optional.

@andig

andig commented Jun 23, 2026

Copy link
Copy Markdown
Member

Maybe the error reporting of the check could be improved.

You can run linting and

make porcelain

locally which will tell you exactly. Or use VScode with recommended plugins to produce „clean“ files.

@premultiply

Copy link
Copy Markdown
Member

We should implement it in mbmd (upstream) and just implement like other mbmd meter template.

@SnejPro

SnejPro commented Jun 23, 2026

Copy link
Copy Markdown
Author

We should implement it in mbmd (upstream) and just implement like other mbmd meter template.

Is it possible to test this like a template without mbmd?

@premultiply premultiply self-assigned this Jun 24, 2026
@premultiply

Copy link
Copy Markdown
Member

Replaced by #31197

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