-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[myenergi] Initial contribution #19248
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Rene Scherer <rene@scherer-online.com>
Signed-off-by: Rene Scherer <rene@scherer-online.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.
Thanks for contributing this binding. I left quite some comments, don't let it demotivate you, in general it looks good, it just are many small things to look at. I do have one slightly bigger comment burried somewhere about the seperation of concern regarding the bridge and its responsibilities. Let me know if any of my comments are unclear, it took me some time to review ;-)
When these are addressed, i can look at the remaining files (handlers + apiclient)
bundles/org.openhab.binding.myenergi/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.myenergi/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
...yenergi/src/main/java/org/openhab/binding/myenergi/internal/MyenergiGetHostFromDirector.java
Outdated
Show resolved
Hide resolved
...yenergi/src/main/java/org/openhab/binding/myenergi/internal/MyenergiGetHostFromDirector.java
Show resolved
Hide resolved
...yenergi/src/main/java/org/openhab/binding/myenergi/internal/MyenergiGetHostFromDirector.java
Show resolved
Hide resolved
Thanks very much @lsiepel for the detailed review. I've started work on the changes and hopefully will have a new version available sometime next week. |
Signed-off-by: Rene Scherer <rene@scherer-online.com>
Signed-off-by: Rene Scherer <rene@scherer-online.com>
…tances. Signed-off-by: Rene Scherer <rene@scherer-online.com>
Signed-off-by: Rene Scherer <rene@scherer-online.com>
@lsiepel I've addressed most of your suggestions above. There are few where I had some further questions and I haven't restructured the files yet, so that it's easier to trace through the initial comments. |
...b.binding.myenergi/src/main/java/org/openhab/binding/myenergi/internal/dto/ZappiSummary.java
Outdated
Show resolved
Hide resolved
....binding.myenergi/src/main/java/org/openhab/binding/myenergi/internal/dto/DaysOfWeekMap.java
Outdated
Show resolved
Hide resolved
....binding.myenergi/src/main/java/org/openhab/binding/myenergi/internal/MyenergiApiClient.java
Outdated
Show resolved
Hide resolved
Updated open comments and added a few. |
…te channels Signed-off-by: Rene Scherer <rene@scherer-online.com>
Signed-off-by: Rene Scherer <rene@scherer-online.com>
Signed-off-by: Rene Scherer <rene@scherer-online.com>
myenergi Binding
This binding supports a number of devices provided by the company myenergi. They include the Zappi car charger (EVSE), the Eddi solar diverter and the Harvi remote sensor for CT clamps.
Description
Although this is the first official PR for this binding, it has been in the market place for a while for 3.x and 4.x openHAB systems and has been used, tested and extended by various contributors. This is the latest version which aligns with the standard naming conventions and works with openHAB 5.x.
Details about the binding can be found in the market place
Testing
Your pull request will automatically be built and available under the following folder:
https://openhab.jfrog.io/ui/native/libs-pullrequest-local/org/openhab/addons/bundles/