Skip to content

Update ocpp-abb-tac.yaml#31444

Open
hansenelectric wants to merge 6 commits into
evcc-io:masterfrom
hansenelectric:patch-2
Open

Update ocpp-abb-tac.yaml#31444
hansenelectric wants to merge 6 commits into
evcc-io:masterfrom
hansenelectric:patch-2

Conversation

@hansenelectric

Copy link
Copy Markdown

added issues with remote transaction and validation timeout

added issues with remote transaction and validation timeout

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

  • The new Setup Guide and Validation lines are currently aligned under generic’s string value, which will break the YAML; either move them out as separate keys under description or convert generic to a block scalar (|) and indent the bullet list accordingly.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `Setup Guide` and `Validation` lines are currently aligned under `generic`’s string value, which will break the YAML; either move them out as separate keys under `description` or convert `generic` to a block scalar (`|`) and indent the bullet list accordingly.

## Individual Comments

### Comment 1
<location path="templates/definition/charger/ocpp-abb-tac.yaml" line_range="12-21" />
<code_context>
   evcc: ["sponsorship", "skiptest"]
   description:
     generic: "[library.e.abb.com](https://library.e.abb.com/public/8f07987a3a284da6bf4e4f8f53cd6502/ABB_Terra_AC_Charger_OCPP1.6_ImplementationOverview%20_v1.8_FW1.6.6.pdf)"
+      Setup Guide:
+      * Select **ABB Terra AC (OCPP)** when adding the charger in evcc.
+      * Configure the charger to use the OCPP Server URL displayed by evcc.
+      * Enable **Start remote transaction on vehicle connection**.
+      * Save the charger configuration.
+      Validation:
+      * During the initial setup, validation may fail with `chargeStatus: timeout`.
+      * If this happens, save the configuration anyway and allow the charger to reconnect.
+      * If the charger does not reconnect automatically, restart evcc and wait for the charger to establish a new OCPP connection.
+      * Once connected, the Station ID will be detected automatically and the charger should become available in evcc.
 params:
   - preset: ocpp
</code_context>
<issue_to_address>
**issue (bug_risk):** The added bullet list is likely invalid YAML as a plain scalar continuation; consider switching `generic` to a block scalar.

As a plain scalar, the indented lines are parsed as a continuation of `generic`, but YAML does not allow `*` at the start of a plain scalar line, so entries like `* Select …` can break parsing or be treated as anchors. To preserve the markdown-style content and keep the YAML valid, change `generic` to a block scalar, e.g.:

```yaml
generic: |
  [library.e.abb.com](https://library.e.abb.com/public/8f07987a3a284da6bf4e4f8f53cd6502/ABB_Terra_AC_Charger_OCPP1.6_ImplementationOverview%20_v1.8_FW1.6.6.pdf)

  Setup Guide:
  * Select **ABB Terra AC (OCPP)** when adding the charger in evcc.
  ...
```
</issue_to_address>

### Comment 2
<location path="templates/definition/charger/ocpp-abb-tac.yaml" line_range="12" />
<code_context>
   evcc: ["sponsorship", "skiptest"]
   description:
     generic: "[library.e.abb.com](https://library.e.abb.com/public/8f07987a3a284da6bf4e4f8f53cd6502/ABB_Terra_AC_Charger_OCPP1.6_ImplementationOverview%20_v1.8_FW1.6.6.pdf)"
+      Setup Guide:
+      * Select **ABB Terra AC (OCPP)** when adding the charger in evcc.
+      * Configure the charger to use the OCPP Server URL displayed by evcc.
</code_context>
<issue_to_address>
**issue (review_instructions):** The added "Setup Guide" and bullet points break the existing `description.generic` structure and likely violate the template/README formatting conventions and consistency with other chargers’ descriptions.

In the existing templates, `description.generic` is typically a single markdown string (often a link plus optional short text) rather than a nested structure with additional YAML keys like `Setup Guide` and `Validation`.

Here, `generic` is still defined as a scalar string on one line:
```yaml
    generic: "[library.e.abb.com](https://library.e.abb.com/...)"
```
followed by additional indented keys:
```yaml
      Setup Guide:
      * Select **ABB Terra AC (OCPP)** ...
      ...
      Validation:
      * During the initial setup, ...
```
This is inconsistent with other `templates/definition/charger` files and likely not what the README’s schema expects for `description`. If the intention is to add setup/validation instructions, they should be folded into the `description.generic` string (e.g. using a `|` block with markdown list items), or into whatever field the README specifies for such guidance, while keeping the YAML structure valid and consistent across 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>

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/charger/ocpp-abb-tac.yaml Outdated
Comment thread templates/definition/charger/ocpp-abb-tac.yaml Outdated
@premultiply premultiply marked this pull request as draft July 4, 2026 04:21
@hansenelectric hansenelectric marked this pull request as ready for review July 8, 2026 06:43

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

  • Consider keeping the URL in markdown link format (as it was before) for consistency with other charger definitions and to ensure any markdown rendering components still generate clickable links.
  • Verify that the consumer of description.generic supports multi-line markdown with headings and bullet lists; if not, you may need to simplify the formatting or split these instructions into dedicated UI/help fields.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider keeping the URL in markdown link format (as it was before) for consistency with other charger definitions and to ensure any markdown rendering components still generate clickable links.
- Verify that the consumer of `description.generic` supports multi-line markdown with headings and bullet lists; if not, you may need to simplify the formatting or split these instructions into dedicated UI/help fields.

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/charger/ocpp-abb-tac.yaml
@premultiply premultiply marked this pull request as draft July 8, 2026 07:26
@hansenelectric hansenelectric marked this pull request as ready for review July 9, 2026 09:53

@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 expanded description.generic block is now a literal string; confirm that the downstream renderer supports Markdown headings and lists in this field, otherwise consider simplifying formatting or using plain text to avoid display issues.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The expanded `description.generic` block is now a literal string; confirm that the downstream renderer supports Markdown headings and lists in this field, otherwise consider simplifying formatting or using plain text to avoid display issues.

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.

description:
generic: "[library.e.abb.com](https://library.e.abb.com/public/8f07987a3a284da6bf4e4f8f53cd6502/ABB_Terra_AC_Charger_OCPP1.6_ImplementationOverview%20_v1.8_FW1.6.6.pdf)"
generic: |
https://library.e.abb.com/public/8f07987a3a284da6bf4e4f8f53cd6502/ABB_Terra_AC_Charger_OCPP1.6_ImplementationOverview%20_v1.8_FW1.6.6.pdf

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.

please see description guidelines in https://github.com/evcc-io/evcc/tree/master/templates#description

We should add both en/de translations.


## Setup Guide

* Select **ABB Terra AC (OCPP)** when adding the charger in evcc.

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.

This is self-referencing. This text is shown in docs and also in the actual ui once the user has selected the device. Please focus on preparations the user has to make on charger level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants