openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.55k forks source link

[onecta] Initial contribution #16730

Open adr001db opened 1 month ago

adr001db commented 1 month ago

New Binding for Daikin using Onecta.

With the newer Daikin units it is no longer possible to control them directly (as is done in the other Daikin binding). The units can only be connected to the Daikin cloud called Onecta. The units can then ‘only’ be controlled with the Onecta app on a phone or tablet. This binding makes it possible to still control the units with OpenHAB. It’s now done by connecting the binding to Daikin’s Onecta. The unit information is then received from the Daikin cloud just like the app. Commands to the units also run via the Daikin Cloud. Older units can also be controlled with this binding as long as they are registered in Onecta.

Description

Please give a few sentences describing the overall goals of the pull request. Give enough details to make the improvement and changes of the PR understandable to both developers and tech-savy users.

Please keep the following in mind:

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/

The Jar can be found at: https://community.openhab.org/t/daikin-onecta-cloud-binding-4-0-0-0-5-0-0-0

Don't forget to submit a thread about your Add-on in the openHAB community: https://community.openhab.org/c/add-ons -->

lsiepel commented 1 month ago

Thanks for your contribution. As there are many PR's currently being reviewed, it might take some time, evantually it will be picked up by one of the maintainers.

Can you remove the ecobee files? Add yourself to the codewoner file and perform some kind of self-review with this checklist in mind it will speed up the review process if easy spotted issues can be fixed upfront.

Edit: Also the build fails and you might want to check SAT warnings.

adr001db commented 1 month ago

Hi @lsiepel, Thanks for the first comments on my pullrequest.

But can you please help me. I'am struggeling with the First commit. This commit is not mine. I tried to delete it, but I don't think it went well.

What should I do with this ?

Greetings Alexander

afbeelding

lsiepel commented 1 month ago

If you refer to DCO, some guidance is here: https://github.com/openhab/openhab-addons/pull/16730/checks?check_run_id=24814490170

The builds fail to spotless: https://github.com/openhab/openhab-addons/actions/runs/9030368073/job/24814490996?pr=16730

That commit that you refer to does not seem to be a problem.

adr001db commented 1 month ago

Thanks than i will ignore it.

lsiepel commented 1 month ago

I think you merged some commits from remote directly to your feature branch. You should update your main from remote, then update your feature branch from main.

adr001db commented 1 month ago

Hi @lsiepel and others,

Sorry to bother you again. I am very new to this development environment and need some help. I think I really messed up this pull request. I tried to correct one wrong "signed off" but it has gotten worse now. What is the best thing I can do now? just continue with this pull request or create a new branch with my code and process the comments you made naturally?

Thanks a lot for the help

Greetings Alexander

adr001db commented 1 month ago

@lsiepel Just to double check.

First a Git pull on the main branch then merge main into the Onecta branch then push the Onecta brange

Greetings Alexander

lsiepel commented 1 month ago

hen merge main into the Onecta branch

Depends a bit on your setup, but usually you have to pull from each parent. openhab-addons => your fork at github => your local clone => your local feature branch.

adr001db commented 1 month ago

Hi @lsiepel

After a few frustrations :o) I think I managed to have a clean commit series again. But there are a lot of reviewers now on this pull request. Can they be removed? Or should I ignore this ?

I hope you don't mind me directing my questions to you.

Greetings Alexander

lsiepel commented 1 month ago

Commits, reviewers and dco are fixed, now it needs to build. Also some comments to look at.

adr001db commented 3 weeks ago

Hi fellow developers,

Yesterday I ran into the problem that I could no longer build my project locally. Part of the error message was: ` [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 27.994 s [INFO] Finished at: 2024-05-22T16:11:49+02:00 [INFO] ------------------------------------------------------------------------

[ERROR] Failed to execute goal org.apache.karaf.tooling:karaf-maven-plugin:4.4.5:verify (karaf-feature-verification) on project org.openhab.binding.onecta: Feature resolution failed for [openhab-binding-onecta/4.2.0.SNAPSHOT] [ERROR] Message: Unable to resolve root: missing requirement [root] osgi.identity; osgi.identity=openhab-binding-onecta; type=karaf.feature; version=4.2.0.SNAPSHOT; filter:="(&(osgi.identity=openhab-binding-onecta)(type=karaf.feature)(version>=4.2.0.SNAPSHOT))" [caused by: Unable to resolve openhab-binding-onecta/4.2.0.SNAPSHOT: missing requirement [openhab-binding-onecta/4.2.0.SNAPSHOT] osgi.identity; osgi.identity=openhab-runtime-base; type=karaf.feature [caused by: Unable to resolve openhab-runtime-base/4.2.0.SNAPSHOT: missing requirement [openhab-runtime-base/4.2.0.SNAPSHOT] osgi.identity; osgi.identity=openhab-core-model-item; type=karaf.feature [caused by: Unable to resolve openhab-core-model-item/4.2.0.SNAPSHOT: missing requirement [openhab-core-model-item/4.2.0.SNAPSHOT] osgi.identity; osgi.identity=org.openhab.core.model.item; type=osgi.bundle; version="[4.2.0.202405201643,4.2.0.202405201643]"; resolution:=mandatory [caused by: Unable to resolve org.openhab.core.model.item/4.2.0.202405201643: missing requirement [org.openhab.core.model.item/4.2.0.202405201643] osgi.wiring.bundle; osgi.wiring.bundle=org.eclipse.xtext.common.types; filter:="(osgi.wiring.bundle=org.eclipse.xtext.common.types)" [caused by: Unable to resolve org.eclipse.xtext.common.types/2.35.0.v20240430-1012: missing requirement [org.eclipse.xtext.common.types/2.35.0.v20240430-1012] osgi.wiring.bundle; osgi.wiring.bundle=org.objectweb.asm; bundle-version="[9.7.0,9.8.0)"; filter:="(&(osgi.wiring.bundle=org.objectweb.asm)(bundle-version>=9.7.0)(!(bundle-version>=9.8.0)))"]]]]][ERROR] Repositories: { [ERROR] file:C:\Data\openhab\openhab-addons\bundles\org.openhab.binding.onecta\target/feature/feature.xml [ERROR] mvn:org.apache.karaf.features/framework/4.4.5/xml/features [ERROR] mvn:org.apache.karaf.features/specs/4.4.5/xml/features [ERROR] mvn:org.apache.karaf.features/standard/4.4.5/xml/features [ERROR] mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-core/4.2.0-SNAPSHOT/xml/features [ERROR] mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-tp/4.2.0-SNAPSHOT/xml/features [ERROR] mvn:org.ops4j.pax.web/pax-web-features/8.0.24/xml/features [ERROR] } [ERROR] Resources: { [ERROR] mvn:com.fasterxml.jackson.core/jackson-annotations/2.16.0 [ERROR] mvn:com.fasterxml.jackson.core/jackson-core/2.16.0 [ERROR] mvn:com.fasterxml.jackson.core/jackson-databind/2.16.0 [ERROR] mvn:com.fasterxml.jackson.dataformat/jackson-dataformat-cbor/2.16.0 [ERROR] mvn:com.fasterxml.jackson.dataformat/jackson-dataformat-xml/2.16.0 ` After searching I saw that a merge had been done in which the karaf.version had gone from 4.4.5 to 4.4.6.

To solve this I performed a "Sync Fork". And now, fortunately, it works again locally.

My question is, is this the right way? Or is there a better way?

Greetings Alexander

lsiepel commented 3 weeks ago

To solve this I performed a "Sync Fork". And now, fortunately, it works again locally.

My question is, is this the right way? Or is there a better way?

Some changes (karaf update) have been applied to openhab-addons/main and that sometimes breaks other builds. In that case you have to pull the changes from main into your fork and into your feature branch. That is what you did with this commit: Merge branch 'openhab:main' into onecta' So yes this seems perfectly fine! A common mistake is that openhab-addons/main is directly pulled into your feature branch (skipping your fork/main) and that generates a lot of crap.

adr001db commented 3 weeks ago

Hi,

I'm in doubt, which is better ?

  if (getManagementPoint(managementPointType) != null
                && getManagementPoint(managementPointType).getTemperatureControl() != null
                && getManagementPoint(managementPointType).getTemperatureControl().getValue() != null
                && getManagementPoint(managementPointType).getTemperatureControl().getValue().getOperationModes() != null
                && getManagementPoint(managementPointType).getTemperatureControl().getValue().getOperationModes().getOperationMode(getCurrentOperationMode()) != null
                && getManagementPoint(managementPointType).getTemperatureControl().getValue().getOperationModes().getOperationMode(getCurrentOperationMode()).getSetpoints() != null
                && getManagementPoint(managementPointType).getTemperatureControl().getValue().getOperationModes().getOperationMode(getCurrentOperationMode()).getSetpoints().getRoomTemperature() != null
        ) {
            return getManagementPoint(this.managementPointType).getTemperatureControl().getValue().getOperationModes()
                    .getOperationMode(getCurrentOperationMode()).getSetpoints().getRoomTemperature().getMinValue();
        } else {
            return null;
        }

or

try {
    return getManagementPoint(this.managementPointType).getTemperatureControl().getValue().getOperationModes()
            .getOperationMode(getCurrentOperationMode()).getSetpoints().getRoomTemperature().getMinValue();
     } catch (NullPointerException e) {
           return null;
    }

I prefer the one with the catch NullPointerException

greetings alexander

david-pace commented 3 weeks ago

I would propose to use Optional.ofNullable(), which can be used for these kind of null check chains:

return Optional.ofNullable(getManagementPoint(this.managementPointType))
    .map(mp -> mp.getTemperatureControl())
    .map(tc -> tc.getValue()
    ...
    .orElse(null);

You can also replace the map() lambdas with method references (your IDE should propose this automatically) :+1:

Here are some references:

lsiepel commented 2 days ago

@adr001db jftr about three weeks are left for the next stable release

adr001db commented 2 days ago

Thanks, but unfortunately i do not have much time to work on the binding at the moment.