OpenEMS: add battery control #20948
OpenEMS: add battery control #20948
Conversation
Reviewer's GuideThis pull request adds battery control for OpenEMS devices by introducing a new File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@andig draft completed, thought about initial battery parameter - comments welcome |
There was a problem hiding this comment.
Hey @iseeberg79 - I've reviewed your changes - here's some feedback:
- The 'normal' (case 1) and 'hold' (case 2) battery modes appear to execute the identical action; consider clarifying the distinction or combining them if functionally equivalent.
- The charge mode (case 3) is unimplemented; consider adding basic charge functionality if the API supports it or clarifying why it's unavailable.
- The username for basic authentication appears hardcoded as 'x' in the template, while a 'user' parameter exists; consider using the parameter value instead.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Wieso bewirken zwei identische Posts unterschiedliches Verhalten? Hast du das getestet? |
| - name: battery | ||
| example: ess0 | ||
| help: | ||
| de: steuerbare Batterie Komponente |
There was a problem hiding this comment.
Hier fehlt noch der Hinweis, dass Fenecon dafür eine kostenpflichtige Lizenz erwartet.
There was a problem hiding this comment.
Kann ich nachtragen!
Ich brauche hier keinen POST, aber der Watchdog muss enden. Geht das auch ohne definierten case 1? War unsicher! Exklusiv Case 2 auszuführen wäre für die Funktion ausreichend. Dann würde ich ggf. über die Löschung von case 3 nachdenken wollen, und nur case 2 definieren. Ja, das Template ist getestet. OpenEMS endet nach Timeout und kehrt im Standard nach 60s bei NORMAL in den nicht limitierten Zustand zurück. |
|
Ahh, mhhm. Ich überleg mal ob sich case 1 klarer unterscheiden lässt. |
|
Bitze noch Formatierung korrigieren. |
gefunden.. erledigt |
|
Klasse PR, vielen Dank! |
There was a problem hiding this comment.
Hey @iseeberg79 - I've reviewed your changes - here's some feedback:
- The 'normal' and 'hold' battery modes currently trigger the same API call; consider if 'hold' requires a different implementation.
- Consider removing the unimplemented 'charge' mode case for clarity.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| default: 60s | ||
| help: | ||
| de: abgestimmt auf das API-Timeout | ||
| en: adjusted to the API timout |
There was a problem hiding this comment.
suggestion (typo): Typo detected in the API help message.
Replace 'timout' with 'timeout'.
| en: adjusted to the API timout | |
| en: adjusted to the API timeout |
Typo ignorieren? Cooles Werkzeug.. |
|
Just a few notes: Thanks for improving the OpenEMS-integration! 👍 |
|
Happy to contribute. I would have liked to set a constraint like lessOrEqual but did not figure out how to set via API. It would allow to charge the battery. The way it's actually implemented equals other inverter implementations and is compatible with not calculating and not setting power specific values to control the energy flow. It has positive effects and counterparts :-) |
|
There are Channels available to set the constraints: |
|
@sfeilmeier I've tried using SetActivePowerLessOrEquals - it works fine, but I've found that the controllers from openEMS try to compensate for that. It has the effect that the battery is charged even when no power is available (home use) when testing. I suspect that the PID filter is trying to compensate for a short period of time. It then settles to a true zero after a few seconds. We could adjust the template so that the channel is a constraint that allows the battery to charge (available PV energy). But we should be aware of the side effects. The current implementation sets a power of zero for the ESS device, so charging and discharging is not possible, preventing the home battery to charge the battery of the car. We have discussed a comparable situation in the past with other inverters (e.g. Plenticore) and when in doubt decided that it is ok not to charge the battery when the car is (fast-)charging because most of the energy is probably used for charging the car. The implementation seems to be more robust if the power is fixed at zero. Of course, it would be desirable to charge the battery whenever possible using remaining energy. I don't want to make a decision here. What do you prefer? Probably we need to replace with the following: |
Your reasoning is absolutely correct. |
see #11501
uses channel SetActivePowerEquals to control ess device via HTTP/REST API
relies on REST/API controller, correct ess device name and proper watchdog/timeout definition
Summary by Sourcery
Add battery control capabilities for OpenEMS devices using HTTP/REST API
New Features:
Enhancements:
Summary by Sourcery
Add battery control capabilities for OpenEMS devices using HTTP/REST API
New Features:
Documentation: