Skip to content

Conversation

@zoomy01
Copy link
Contributor

@zoomy01 zoomy01 commented Jun 14, 2025

Copy link
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 @zoomy01 - I've reviewed your changes and they look great!


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.

@VolkerK62
Copy link
Collaborator

Vorschlag:
den Parameter dischargepower als positiven Wert ausführen und das entsprechende Scaling innerhalb batterymode machen.
Ich denke, das ist intuitiver.

@andig andig added the devices Specific device support label Jun 14, 2025
Parameter dischargepower in positiven Wert geändert, Umwandlung erfolgt im Template
@zoomy01
Copy link
Contributor Author

zoomy01 commented Jun 14, 2025

den Parameter dischargepower bräuchte es innerhalb von evvc eigentlich nicht, da er für nichts genutzt wird.

Gibt es eine Möglichkeit den Ist-Wert aus Register 1074 zu irgendwo zu speichern, wenn hold aktiviert wird und beim Wechsel auf normal diesen Wert wieder zurück zu schreiben?

@andig
Copy link
Member

andig commented Jun 14, 2025

Nein, gibts nicht :/

@andig andig changed the title Varta: Add battery control Varta: add battery control Jun 14, 2025
@andig andig requested a review from premultiply June 15, 2025 08:35
@andig andig marked this pull request as draft June 15, 2025 08:36
Case 3 ergänzt, Parameter umbenannt
@zoomy01 zoomy01 closed this Jun 15, 2025
@zoomy01 zoomy01 deleted the patch-2 branch June 15, 2025 11:54
@zoomy01 zoomy01 restored the patch-2 branch June 15, 2025 11:56
@zoomy01 zoomy01 reopened this Jun 15, 2025
maxdischargepower ergänzt
@zoomy01
Copy link
Contributor Author

zoomy01 commented Jun 15, 2025

Wäre toll wenn nochmal jemand drüber gucken könnte. Ich weiß leider nicht warum die defaults.yaml einen Konflikt hat und freue mich über einen Tipp.

watchdog ergänzt
@zoomy01 zoomy01 marked this pull request as ready for review June 16, 2025 21:40
Copy link
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 @zoomy01 - I've reviewed your changes - here's some feedback:

  • The batterymode block’s indentation looks off compared to the surrounding YAML—please align it with the other fields to avoid rendering issues.
  • You’re hardcoding timeout: 120s in the batterymode section instead of using the {{ .watchdog }} parameter—consider switching to the template variable for flexibility.
  • The maxdischargepower param is defined both in varta.yaml and in defaults.yaml—please consolidate it to a single definition to avoid duplication.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `batterymode` block’s indentation looks off compared to the surrounding YAML—please align it with the other fields to avoid rendering issues.
- You’re hardcoding `timeout: 120s` in the `batterymode` section instead of using the `{{ .watchdog }}` parameter—consider switching to the template variable for flexibility.
- The `maxdischargepower` param is defined both in `varta.yaml` and in `defaults.yaml`—please consolidate it to a single definition to avoid duplication.

## Individual Comments

### Comment 1
<location> `templates/definition/meter/varta.yaml:27` </location>
<code_context>
   - name: host
   - name: port
     default: 502
+  - name: watchdog
+    type: duration
+    default: 120s
</code_context>

<issue_to_address>
Add description/help for 'watchdog' parameter

Adding a localized description or help text for 'watchdog' will enhance documentation and make the parameter easier to understand for users.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
  - name: watchdog
    type: duration
    default: 120s
    usages: ["battery"]
    advanced: true
=======
  - name: watchdog
    type: duration
    default: 120s
    help:
      de: Zeit in Sekunden, nach der der Watchdog auslöst, wenn keine Kommunikation erfolgt
      en: Time in seconds after which the watchdog triggers if no communication occurs
    usages: ["battery"]
    advanced: true
>>>>>>> REPLACE

</suggested_fix>

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.

sourcery-ai comments
@andig
Copy link
Member

andig commented Jun 17, 2025

Was fehlt hier noch?

zoomy01

This comment was marked as off-topic.

small correction
unit: W for maxdischargepower
@zoomy01
Copy link
Contributor Author

zoomy01 commented Jun 17, 2025

Was fehlt hier noch?

von meiner Seite nicht. ;)

@andig andig enabled auto-merge (squash) June 18, 2025 18:41
@andig andig merged commit b3267cd into evcc-io:master Jun 18, 2025
6 checks passed
andig pushed a commit that referenced this pull request Aug 11, 2025
StarF666 pushed a commit to StarF666/evcc that referenced this pull request Oct 1, 2025
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