Openwbn-native: Add documentation for OpenWB SE configuration#31264
Draft
schreibubi wants to merge 4 commits into
Draft
Openwbn-native: Add documentation for OpenWB SE configuration#31264schreibubi wants to merge 4 commits into
schreibubi wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- In the English section, the sentence
Some Duo's contain two Modbus adapters, some just one. openWB SEreads as a fragment; either remove the trailingopenWB SEor turn it into a full sentence explaining what is special about the SE model. - The naming of the software alternates between
evccandEVCCin both German and English sections; consider standardizing on one form (likelyevccas the executable name) to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the English section, the sentence `Some Duo's contain two Modbus adapters, some just one. openWB SE` reads as a fragment; either remove the trailing `openWB SE` or turn it into a full sentence explaining what is special about the SE model.
- The naming of the software alternates between `evcc` and `EVCC` in both German and English sections; consider standardizing on one form (likely `evcc` as the executable name) to avoid confusion.
## Individual Comments
### Comment 1
<location path="templates/definition/charger/openwb-native.yaml" line_range="76-77" />
<code_context>
- Some Duo's contain two Modbus adapters, some just one.
- - The EVSE for the first connector has always the ID 1, the one for the second ID 2.
+ - A single or multiple Modbus adapters are found at `/dev/ttyUSB0`, `/dev/ttyUSB1` (some Duo) or `/dev/ttyACM0` (e.g. openWB SE).
+ Some Duo's contain two Modbus adapters, some just one. openWB SE
+ - The EVSE for the first connector has the ID 1, the second one ID 2. The openWB SE uses ID 11 and ID 12 instead.
- The different possible charge meters are:
</code_context>
<issue_to_address>
**issue (typo):** Fix the trailing fragment `openWB SE` at the end of this sentence.
The line ends with a stray `openWB SE`, which makes the docs ambiguous. Please either remove it or expand it into a complete, meaningful phrase.
```suggestion
- A single or multiple Modbus adapters are found at `/dev/ttyUSB0`, `/dev/ttyUSB1` (some Duo) or `/dev/ttyACM0` (e.g. openWB SE).
Some Duo's contain two Modbus adapters, some just one.
```
</issue_to_address>
### Comment 2
<location path="templates/definition/charger/openwb-native.yaml" line_range="83-84" />
<code_context>
+ - SDM630/SDM72 (template: eastron) with ID 105 or ID 106 for the Duo's second connector (e.g. openWB SE).
- ABB B23 (template: abb-ab) with ID 201
+ - Be aware of the limitations of the openWB SE HW regarding maximum allowed currents, see https://wiki.openwb.de/doku.php?id=openwb:technische-daten:openwb_se.
+ EVCC does not limit the current to the allowed maximum, so please configure the maximum current in EVCC accordingly.
Additional steps for showing evcc on the display (be careful, because this will allow anybody to enable charging!):
</code_context>
<issue_to_address>
**suggestion (typo):** Use a consistent spelling/capitalization of `evcc` throughout the instructions.
Most of this file uses `evcc` (lowercase), whereas this line uses `EVCC`. Unless there’s a specific reason for the different capitalization, please align it with the existing usage.
```suggestion
- Be aware of the limitations of the openWB SE HW regarding maximum allowed currents, see https://wiki.openwb.de/doku.php?id=openwb:technische-daten:openwb_se.
evcc does not limit the current to the allowed maximum, so please configure the maximum current in evcc accordingly.
```
</issue_to_address>
### Comment 3
<location path="templates/definition/charger/openwb-native.yaml" line_range="77" />
<code_context>
- Some Duo's contain two Modbus adapters, some just one.
- - The EVSE for the first connector has always the ID 1, the one for the second ID 2.
+ - A single or multiple Modbus adapters are found at `/dev/ttyUSB0`, `/dev/ttyUSB1` (some Duo) or `/dev/ttyACM0` (e.g. openWB SE).
+ Some Duo's contain two Modbus adapters, some just one. openWB SE
+ - The EVSE for the first connector has the ID 1, the second one ID 2. The openWB SE uses ID 11 and ID 12 instead.
- The different possible charge meters are:
</code_context>
<issue_to_address>
**issue (review_instructions):** This line ends in an incomplete fragment ("openWB SE") and breaks the wording/consistency with the rest of the templates.
The English text here currently ends with an unfinished fragment: `openWB SE `. Other bullet points are full sentences and do not contain trailing fragments. Please either complete the sentence (e.g. mention what is special for openWB SE here) or remove the fragment to keep the wording consistent with the surrounding instructions and other templates.
<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>
### Comment 4
<location path="templates/definition/charger/openwb-native.yaml" line_range="34" />
<code_context>
+ - SDM630/SDM72 (template: eastron) mit ID 105 oder ID 106 für den zweiten Ladepunkt bei der Duo (u.a. openWB SE).
- ABB B23 (template: abb-ab) mit ID 201
+ - Beachte die Beschränkungen der openWB SE-Hardware bezüglich der maximal zulässigen Ströme; siehe https://wiki.openwb.de/doku.php?id=openwb:technische-daten:openwb_se.
+ EVCC begrenzt den Strom nicht auf den zulässigen Maximalwert, daher bitte den maximalen Strom in EVCC entsprechend konfigurieren.
Zusätzlich für die Anzeige von evcc im Display (Achtung dann können auch Unbefugte laden!):
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The capitalization of "EVCC" is inconsistent with the rest of this file and other templates, which predominantly use "evcc".
In this template (and generally across the templates directory), the project name is written as `evcc` (lowercase). This new sentence uses `EVCC` twice, which is inconsistent with the existing wording. Please align the capitalization with the rest of the file (likely `evcc`).
<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>
### Comment 5
<location path="templates/definition/charger/openwb-native.yaml" line_range="84" />
<code_context>
+ - SDM630/SDM72 (template: eastron) with ID 105 or ID 106 for the Duo's second connector (e.g. openWB SE).
- ABB B23 (template: abb-ab) with ID 201
+ - Be aware of the limitations of the openWB SE HW regarding maximum allowed currents, see https://wiki.openwb.de/doku.php?id=openwb:technische-daten:openwb_se.
+ EVCC does not limit the current to the allowed maximum, so please configure the maximum current in EVCC accordingly.
Additional steps for showing evcc on the display (be careful, because this will allow anybody to enable charging!):
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The use of "EVCC" here is inconsistent with the rest of the file and likely other templates, which use "evcc".
Similar to the German section, this English sentence uses `EVCC` twice, while the rest of the template (and other templates) typically refer to the application as `evcc` in lowercase. To keep wording consistent across templates, consider changing `EVCC` to `evcc`.
<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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
andig
reviewed
Jun 28, 2026
| - SDM630/SDM72 (template: eastron) mit ID 105 oder ID 106 für den zweiten Ladepunkt bei der Duo (u.a. openWB SE). | ||
| - ABB B23 (template: abb-ab) mit ID 201 | ||
| - Beachte die Beschränkungen der openWB SE-Hardware bezüglich der maximal zulässigen Ströme; siehe https://wiki.openwb.de/doku.php?id=openwb:technische-daten:openwb_se. | ||
| EVCC begrenzt den Strom nicht auf den zulässigen Maximalwert, daher bitte den maximalen Strom in EVCC entsprechend konfigurieren. |
Member
There was a problem hiding this comment.
Can we please reword this to matter of fact style?
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.
Add the description on how to configure an openWB SE