[solax] Add cloud connection support #16124
Conversation
lolodomo
added
the
enhancement
theater
force-pushed
the
retrieval_from_cloud
branch
3 times, most recently
from
f6a9c45
to
1416cc3
Compare
|
Depends on #16248 and has to be rebased on it once it's merged into main |
theater
force-pushed
the
retrieval_from_cloud
branch
from
1416cc3
to
4283fd0
Compare
* Initial rearrangement of classes and cloud response in test * Refactoring of packages and initial version of retrieval of data * Test for parsed raw data * Move the enums for status and type to the root model package * Another package refactoring + basic handler functionality * Handler works now, most build warnings removed * Add initial exception handling during the update * Small code style fixes and channel for inverter status * Add all defined channels from the XML in the handler * Add translations of the status and support for zones and I18N * Update readme * Fix error handling in the Cloud connection handler * Manual fixes openhab#16249 and openhab#16214 due to refactored packages and classes * Fix the headers for the new / refactored / moved classes for 2024 Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
theater
force-pushed
the
retrieval_from_cloud
branch
from
4283fd0
to
91dca44
Compare
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
|
I have rebased everything on top of the change from the other PR which resulted in a very unpleasant fix of conflicts. Hope someone finds time to review it before any other changes in main branch happen, so I don't have to do this again. It's very time consuming due to a lot of refactoring (including a package refactoring) in this PR. Best regards, |
There was a problem hiding this comment.
Did a full review. Most are minor comments. I also think the properties file lacks some values, so it is good to clean/re-run i18n plugin to be sure all are added correctly.
As this is on top of the previous PR and some conflicts have been resolved, i think it would be wise to spend some extra time on testing as we want to prevent regression.
...ng.solax/src/main/java/org/openhab/binding/solax/internal/handlers/AbstractSolaxHandler.java
Show resolved
Hide resolved
bundles/org.openhab.binding.solax/src/main/resources/OH-INF/thing/cloudConnectInverter.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.solax/src/main/resources/OH-INF/thing/cloudConnectInverter.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.solax/src/main/resources/OH-INF/thing/channel_types.xml
Show resolved
Hide resolved
...ab.binding.solax/src/test/java/org/openhab/binding/solax/internal/cloud/TestCloudParser.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
|
Hi @lsiepel, thanks very much for the fast review! I have fixed most of the comments, for some I left my own comments for clarification. If you decide to meanwhile merge it, please hold until I do some integration testing of the changes, related to the exception management, so I don't pass any missed runtime exception... maybe I should just catch any exception in the AbstractSolaxHandler.retrieveData() method so I ensure another schedule attempt afterwards... Cheers, |
Signed-off-by: Konstantin Polihronov <polychronov@gmail.com>
|
Hi. I changed the requested minimum refresh interval to 9s, even though from practical perspective it doesn't make sense to refresh lower than 30-60 seconds, because the cloud API data gets updated from the local module once on each 5 minutes. :) Thanks a lot for the review! |
lsiepel
changed the title
* Initial rearrangement of classes and cloud response in test Signed-off-by: Konstantin Polihronov <polychronov@gmail.com> Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Besides the direct connection to the local wi-fi module that we started to support with OpenHAB 4.1, now the binding will support also a new type - cloud connect. The documentation for the API can be obtained from the Solax web portal even though it's not very thorough.
========
Implemented a new thing type - cloud connect inverter.
Refactored the packages, model and classes to reflect that now we have two different ways to retrieve the data.
The common logic is extracted in the AbstractSolaxHandler.
The handler now requires two method implementations with the specific logics for the Local and Cloud connection:
README is updated with the new type and the examples are up to date. Made a small corrections to other parts in the README.
I'm not sure if the thing type is new if I need to create an update instructions. I would expect to NOT need it.