-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[evcc] Adjust to evcc version 0.123.1 #16114
Conversation
… 0.123.1 Signed-off-by: Luca Arnecke <luca@arnecke.name>
Signed-off-by: Luca Arnecke <luca@arnecke.name>
Signed-off-by: Luca Arnecke <luca@arnecke.name>
Signed-off-by: Luca Arnecke <luca@arnecke.name>
… new implementation of minSoC and plans (served by new api) Signed-off-by: Luca Arnecke <luca@arnecke.name>
Signed-off-by: Luca Arnecke <luca@arnecke.name>
5125cce
to
bf39488
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR, and thanks for updating the binding.
I’ve stopped keeping up with evcc‘s API change because I unfortunately I never started using evcc in production - blame the manufacturer of my wallbox because the announced Web API was never released.
Code looks good in general, please have a look at my comments.
....openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/api/dto/Loadpoint.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.evcc/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/api/EvccAPI.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/api/EvccAPI.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Show resolved
Hide resolved
...enhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccBindingConstants.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Luca Arnecke <luca@arnecke.name>
...enhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccBindingConstants.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my review!
I have a few (final) comments:
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...enhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccBindingConstants.java
Outdated
Show resolved
Hide resolved
...enhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccBindingConstants.java
Show resolved
Hide resolved
Signed-off-by: Luca Arnecke <luca@arnecke.name>
Signed-off-by: Luca Arnecke <luca@arnecke.name>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are getting close to the finish line!
At least from my review perspective, the add-on maintainers also have to take a look at this.
bundles/org.openhab.binding.evcc/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.evcc/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Luca Arnecke <luca@arnecke.name>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bundles/org.openhab.binding.evcc/src/main/resources/OH-INF/thing/thing-types.xml
Show resolved
Hide resolved
Signed-off-by: Michael Weger <weger.michael@gmx.net>
Signed-off-by: Michael Weger <weger.michael@gmx.net>
@lucaarn, I proposed a check-in in order to fix the loadpoint-vehicleName channel: The API parameter is called
With this change we loose again the shared channel type between |
Signed-off-by: Michael Weger <weger.michael@gmx.net>
In order to automatize the upgrade efforts, upgrade instructions shall be implemented: They will also make the
|
For Things with statically defined channels in the thing-type.xml upgrade instructions should be provided, but the evcc binding dynamically creates the channels from the code, so no upgrade instructions are needed and they also wouldn’t work I think. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, just a few minor comments:
bundles/org.openhab.binding.evcc/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
...enhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccBindingConstants.java
Show resolved
Hide resolved
...enhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccBindingConstants.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
@MikeTheTux Can you please regenerate the default translations? |
Signed-off-by: Michael Weger <weger.michael@gmx.net>
done |
agree. the current implementation removing outdated channels is the way to go |
I think I'm done from my end. |
I can do a review, but since I am only the evcc binding maintainer and no add-ons maintainer I cannot merge. |
Signed-off-by: Michael Weger <weger.michael@gmx.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, just some minor findings:
...enhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccBindingConstants.java
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
...es/org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/EvccHandler.java
Outdated
Show resolved
Hide resolved
....openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/api/dto/Loadpoint.java
Show resolved
Hide resolved
...org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/api/dto/Result.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.evcc/src/main/java/org/openhab/binding/evcc/internal/api/dto/Result.java
Show resolved
Hide resolved
Signed-off-by: Michael Weger <weger.michael@gmx.net>
Signed-off-by: Michael Weger <weger.michael@gmx.net>
In the meanwhile I implemented #15744 on-top of this branch. Shall I ...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@openhab/add-ons-maintainers Can you please review and merge this PR? I already did a full review as the evcc add-on maintainer.
Please go for 3. What really speaks against 1 is IMO that this PR is not really your PR, and it is already pretty big and want to avoid it growing bigger. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides this i18n comment, also note that some channels are removed and it depends on an external software upgrade. That makes this PR a breaking change. Please create a warning message at the openHAB distro repo to warn users when upgrading.
bundles/org.openhab.binding.evcc/src/main/resources/OH-INF/i18n/evcc_de.properties
Outdated
Show resolved
Hide resolved
* updated url of setTargetEnergy and setTargetSoC to match evcc version 0.123.1 * removed minSoc from Loadpoint (since evcc 0.123.0 part of vehicle) * renamed from targetEnergy to limitEnergy to match new evcc version * renamed from targetSoC to limitSoC to match new evcc version * plementation of vehicle object to match new evcc version 0.123.1 -> new implementation of minSoC and plans (served by new api) Signed-off-by: Luca Arnecke <luca@arnecke.name> Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Signed-off-by: Michael Weger <weger.michael@gmx.net> Co-authored-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Michael Weger <weger.michael@gmx.net> Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
* updated url of setTargetEnergy and setTargetSoC to match evcc version 0.123.1 * removed minSoc from Loadpoint (since evcc 0.123.0 part of vehicle) * renamed from targetEnergy to limitEnergy to match new evcc version * renamed from targetSoC to limitSoC to match new evcc version * plementation of vehicle object to match new evcc version 0.123.1 -> new implementation of minSoC and plans (served by new api) Signed-off-by: Luca Arnecke <luca@arnecke.name> Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Signed-off-by: Michael Weger <weger.michael@gmx.net> Co-authored-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Michael Weger <weger.michael@gmx.net> (cherry picked from commit b316096)
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
* updated url of setTargetEnergy and setTargetSoC to match evcc version 0.123.1 * removed minSoc from Loadpoint (since evcc 0.123.0 part of vehicle) * renamed from targetEnergy to limitEnergy to match new evcc version * renamed from targetSoC to limitSoC to match new evcc version * plementation of vehicle object to match new evcc version 0.123.1 -> new implementation of minSoC and plans (served by new api) Signed-off-by: Luca Arnecke <luca@arnecke.name> Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Signed-off-by: Michael Weger <weger.michael@gmx.net> Co-authored-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Michael Weger <weger.michael@gmx.net>
After the update to evcc version 0.123.0 the structure of the api changed:
-> Instead of that there is a new endpoint for the vehicles, where you can setup a plan with a plan limit soc and a plan time as well as a vehicle specific minimum soc
In order to fit the new api I fixed some issues and implemented a vehicle object. It is possible that the api focusses with coming version even more on vehicles than the loadpoints.
Fixes #16098
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
660 York Street, Suite 102,
San Francisco, CA 94110 USA
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
org.openhab.binding.evcc-4.2.0-SNAPSHOT.zip