[Netatmo] Binding rewrite without external dependencies #12357

clinique · 2022-02-23T15:55:59Z

This rework of the Netatmo binding was initiated in order to remove external dependencies (swagger, okio, okhttp...).
It ended as a complete rework of it.

Should now be stable :

Most of work done but to be tested more :

Webhook
Smoke detector (won't be full part of it due to a bug in Netatmo API)

To be started (if someone volunteers):

Adding doorbell => inception ( @Novanic )

Latest testable version here.

This PR replaces PR #10831, that already replaced PR #9486

Signed-off-by: Gaël L'hopital [email protected]

clinique · 2022-02-23T16:01:09Z

Due to huge number of conflicting file I closed previous PR and restarted clean.
Updated file headers to 2022.
Updated to SNAPSHOT 3.3.0
Removall of deprecated Netatmo API initiated.

This version runs on my production system, so you should be able to test it right now. Some code cleaning still has to be done due to majors changes introduced by API evolutions.

@mdillmann, @Novanic , @robnielsen, @jlaur , @lolodomo : FYI

jlaur

@clinique - a few warnings to fix. Can you provide a diff between this PR and the most recent abandoned PR? This would make it easier to catch up with what happened in the meantime, since we are multiple reviewers that need to get back into this.

...b.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/dto/NASnapshot.java

...nhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/ApiResponse.java

...binding.netatmo/src/test/java/org/openhab/binding/netatmo/internal/api/dto/NAObjectTest.java

jlaur

@clinique - another review round done. Only minor findings as previous PR was already in a quite good state.

...penhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/EnergyApi.java

bundles/org.openhab.binding.netatmo/README.md

...ava/org/openhab/binding/netatmo/internal/handler/propertyhelper/LastEventPropertyHelper.java

...main/java/org/openhab/binding/netatmo/internal/handler/propertyhelper/NHCPropertyHelper.java

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/thing/weather.xml

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo.properties

....binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/HomeHandler.java

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo.properties

.../src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CameraCapability.java

...penhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/ApiBridge.java

jlaur · 2022-03-03T18:55:43Z

Yes, I identified this problem on token refresh. Will check but should be solved.

Cool. Solved, but not yet pushed, I assume?

@clinique - just checking: is this issue addressed in one of the commits 96563c0 and 82220e1, or are you still working on this?

bundles/org.openhab.binding.netatmo/README.md

clinique · 2022-03-04T07:22:25Z

Yes, I identified this problem on token refresh. Will check but should be solved.

Cool. Solved, but not yet pushed, I assume?

@clinique - just checking: is this issue addressed in one of the commits 96563c0 and 82220e1, or are you still working on this?

You can test with last version. Should be adressed.

bundles/org.openhab.binding.netatmo/README.md

jlaur · 2022-03-18T16:17:10Z

Correct syntax should be :

Perfect, thanks!

Signed-off-by: Laurent Garnier <[email protected]>

lolodomo · 2022-05-06T16:31:42Z

PR submitted that fixes the issue. I will run the test again.

lolodomo · 2022-05-06T17:06:26Z

Regarding the automatic computation of the refresh interval for the weather station, the result is a little strange, sometimes the binding computes 10 minutes, sometimes 5 minutes. This is probably not something new so not something that should delay the merge but definitively something to analyse more in details. Maybe the station is not always pushing at the same frequency ?

lolodomo · 2022-05-06T18:02:29Z

I had the intention yesterday to test few other channels like known and unknown persons on the home thing but my camera is no more able to detect persons (it detects movements but not persons) so I forgive.
So, to be clear, on my side, as soon as @clinique will merge my last submitted PR, I will approve this PR.
@jlaur: can you also approve it or at least tell me if you do not approve it ?
I read that there would be a potential Milestone 5 in preparation this weekend. If this is true, this would be excellent if the PR could be included in this new milestone.

jlaur · 2022-05-06T18:55:12Z

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo.properties

+conf-error-no-username = Cannot connect to Netatmo bridge as no username is available in the configuration
+conf-error-no-password = Cannot connect to Netatmo bridge as no password is available in the configuration
+status-bridge-offline = Bridge is not connected to Netatmo API
+device-not-connected = Device is not connected


Quite minor, but could this be rephrased to:

Suggested change

device-not-connected = Device is not connected

device-not-connected = Device is not reachable

or is it used in more than this exact scenario? Is it always a module (additional indoor/outdoor)? In this case it could even be "Module is not reachable".

Devices and modules are Netatmo wording. I prefer use Thing.

I should wait for a new submit?

You just received it !

jlaur

Thanks a lot to @clinique for this massive overhaul of the binding. Also kudos to @lolodomo for joining the review in multiple iterations and helping a lot to improve the quality by review, testing and contributions.

jlaur · 2022-05-06T21:43:43Z

...src/main/java/org/openhab/binding/netatmo/internal/handler/capability/RefreshCapability.java

+
+    private Duration dataValidity;
+    private Instant dataTimeStamp = Instant.now();
+    private @Nullable Instant dataTimeStamp0;


Perhaps Instant.MIN could be used to avoid @Nullable. @lolodomo - consider for your pending PR.

Refresh strategy fixed when thing is OFFLINE

lolodomo

LGTM, thank you Gaël

Signed-off-by: clinique <[email protected]>

clinique · 2022-05-06T22:13:31Z

Thanks a lot to @clinique for this massive overhaul of the binding. Also kudos to @lolodomo for joining the review in multiple iterations and helping a lot to improve the quality by review, testing and contributions.

Yes, big thank you for helping me finalizing this nearly two years journey ! Great help from your detailed review, tests, PRs...I was a bit exhausted and you gave me energy.

@mdillmann, @Novanic , @robnielsen, @jlaur , @lolodomo : kudos !

lolodomo · 2022-05-06T22:21:40Z

[INFO] --- sat-plugin:0.12.0:report (sat-all) @ org.openhab.binding.netatmo ---
[INFO] Individual report appended to summary report.
[ERROR] Code Analysis Tool has found:
 1 error(s)!
 8 warning(s)
 0 info(s)
[ERROR] org.openhab.binding.netatmo.internal.handler.capability.RefreshCapability.java:[105]

Signed-off-by: clinique <[email protected]>

lolodomo · 2022-05-06T22:41:07Z

@clinique : please add an entry in file distributions/openhab/src/main/resources/bin/update.lst (openhab-distro) to mention the breaking change.
And ideally prepare a short chapter that Kai could include in the future release notes.

lolodomo · 2022-05-06T22:42:10Z

End of a long story ;)
Now you can look at the new authentication method ;)

lolodomo · 2022-05-06T22:44:45Z

And thank you for your patience on this PR.

clinique · 2022-05-06T22:46:03Z

And thank you for your patience on this PR.

We all have been patient.

kaikreuzer · 2022-05-07T06:22:33Z

@clinique : please add an entry in file distributions/openhab/src/main/resources/bin/update.lst (openhab-distro) to mention the breaking change.

Would be awesome, if you could do that by tonight as we are planning to do the M5 build today and we will need to warn people about that change.

lolodomo · 2022-05-07T06:53:24Z

And we will have to clean up Crowdin for different translations and fill new translations. Not necessarily today :)

lolodomo · 2022-05-07T07:04:03Z

And we will have to clean up Crowdin for different translations and fill new translations. Not necessarily today :)

Clean up was done apparently automatically in French and German translations. We just have to create new ones.

clinique · 2022-05-07T17:39:06Z

@clinique : please add an entry in file distributions/openhab/src/main/resources/bin/update.lst (openhab-distro) to mention the breaking change.

Would be awesome, if you could do that by tonight as we are planning to do the M5 build today and we will need to warn people about that change.

PR pushed. I'm not used to push against this repo, hope it is fine...

openhab-bot · 2022-05-24T17:25:50Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/netatmo-thermostat/114729/58

…nhab#12357) Signed-off-by: clinique <[email protected]> Also-by: Laurent Garnier <[email protected]>

…nhab#12357) Signed-off-by: clinique <[email protected]> Also-by: Laurent Garnier <[email protected]> Signed-off-by: Andras Uhrin <[email protected]>

…nhab#12357) Signed-off-by: clinique <[email protected]> Also-by: Laurent Garnier <[email protected]>

clinique requested review from cweitkamp and lolodomo as code owners February 23, 2022 15:56

clinique self-assigned this Feb 23, 2022

jlaur requested changes Feb 23, 2022

View reviewed changes

This comment was marked as resolved.

Sign in to view

clinique force-pushed the NetatmoOH33 branch from c72f8ec to df04e05 Compare February 25, 2022 06:17

jlaur requested changes Feb 25, 2022

View reviewed changes

jlaur reviewed Mar 3, 2022

View reviewed changes

....binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/HomeHandler.java Outdated Show resolved Hide resolved

jlaur reviewed Mar 3, 2022

View reviewed changes

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo.properties Outdated Show resolved Hide resolved

jlaur reviewed Mar 3, 2022

View reviewed changes

.../src/main/java/org/openhab/binding/netatmo/internal/handler/capability/CameraCapability.java Outdated Show resolved Hide resolved

jlaur reviewed Mar 3, 2022

View reviewed changes

...penhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/ApiBridge.java Outdated Show resolved Hide resolved

J-N-K reviewed Mar 3, 2022

View reviewed changes

bundles/org.openhab.binding.netatmo/README.md Show resolved Hide resolved

clinique requested review from robnielsen, J-N-K and jlaur March 8, 2022 15:06

jlaur reviewed Mar 8, 2022

View reviewed changes

bundles/org.openhab.binding.netatmo/README.md Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

openhab deleted a comment from jlaur Mar 18, 2022

This comment was marked as resolved.

Sign in to view

Refresh strategy fixed when thing is OFFLINE

f8e56df

Signed-off-by: Laurent Garnier <[email protected]>

jlaur reviewed May 6, 2022

View reviewed changes

jlaur approved these changes May 6, 2022

View reviewed changes

jlaur reviewed May 6, 2022

View reviewed changes

Merge pull request #20 from lolodomo/refresh_strategy_when_offline

ebccc88

Refresh strategy fixed when thing is OFFLINE

lolodomo approved these changes May 6, 2022

View reviewed changes

Integrated last comments of @jlaur

23b1b7b

Signed-off-by: clinique <[email protected]>

Correction of pmd

45526e2

Signed-off-by: clinique <[email protected]>

lolodomo merged commit adda4c8 into openhab:main May 6, 2022

lolodomo added this to the 3.3 milestone May 6, 2022

clinique deleted the NetatmoOH33 branch May 6, 2022 22:39

jlaur changed the title ~~[Netatmo] Binding rewrite without external dependencies - step 3~~ [Netatmo] Binding rewrite without external dependencies May 7, 2022

andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022

[Netatmo] Binding rewrite without external dependencies - step 3 (ope…

8ab459d

…nhab#12357) Signed-off-by: clinique <[email protected]> Also-by: Laurent Garnier <[email protected]>

psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023

[Netatmo] Binding rewrite without external dependencies - step 3 (ope…

e01ad53

…nhab#12357) Signed-off-by: clinique <[email protected]> Also-by: Laurent Garnier <[email protected]>

clinique mentioned this pull request Mar 6, 2024

[netatmo] API limit reached handling #16489

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Netatmo] Binding rewrite without external dependencies #12357

[Netatmo] Binding rewrite without external dependencies #12357

clinique commented Feb 23, 2022 •

edited

Loading

clinique commented Feb 23, 2022 •

edited

Loading

jlaur left a comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

jlaur left a comment

jlaur commented Mar 3, 2022

clinique commented Mar 4, 2022

This comment was marked as resolved.

This comment was marked as resolved.

jlaur commented Mar 18, 2022

This comment was marked as resolved.

lolodomo commented May 6, 2022

lolodomo commented May 6, 2022

lolodomo commented May 6, 2022 •

edited

Loading

jlaur May 6, 2022

clinique May 6, 2022 •

edited

Loading

lolodomo May 6, 2022

clinique May 6, 2022

jlaur left a comment

jlaur May 6, 2022

clinique May 6, 2022

lolodomo left a comment

clinique commented May 6, 2022

lolodomo commented May 6, 2022 •

edited

Loading

lolodomo commented May 6, 2022 •

edited

Loading

lolodomo commented May 6, 2022

lolodomo commented May 6, 2022

clinique commented May 6, 2022

kaikreuzer commented May 7, 2022

lolodomo commented May 7, 2022 •

edited

Loading

lolodomo commented May 7, 2022

clinique commented May 7, 2022

openhab-bot commented May 24, 2022

	device-not-connected = Device is not connected
	device-not-connected = Device is not reachable

[Netatmo] Binding rewrite without external dependencies #12357

[Netatmo] Binding rewrite without external dependencies #12357

Conversation

clinique commented Feb 23, 2022 • edited Loading

clinique commented Feb 23, 2022 • edited Loading

jlaur left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

jlaur left a comment

Choose a reason for hiding this comment

jlaur commented Mar 3, 2022

clinique commented Mar 4, 2022

This comment was marked as resolved.

This comment was marked as resolved.

jlaur commented Mar 18, 2022

This comment was marked as resolved.

lolodomo commented May 6, 2022

lolodomo commented May 6, 2022

lolodomo commented May 6, 2022 • edited Loading

jlaur May 6, 2022

Choose a reason for hiding this comment

clinique May 6, 2022 • edited Loading

Choose a reason for hiding this comment

lolodomo May 6, 2022

Choose a reason for hiding this comment

clinique May 6, 2022

Choose a reason for hiding this comment

jlaur left a comment

Choose a reason for hiding this comment

jlaur May 6, 2022

Choose a reason for hiding this comment

clinique May 6, 2022

Choose a reason for hiding this comment

lolodomo left a comment

Choose a reason for hiding this comment

clinique commented May 6, 2022

lolodomo commented May 6, 2022 • edited Loading

lolodomo commented May 6, 2022 • edited Loading

lolodomo commented May 6, 2022

lolodomo commented May 6, 2022

clinique commented May 6, 2022

kaikreuzer commented May 7, 2022

lolodomo commented May 7, 2022 • edited Loading

lolodomo commented May 7, 2022

clinique commented May 7, 2022

openhab-bot commented May 24, 2022

clinique commented Feb 23, 2022 •

edited

Loading

clinique commented Feb 23, 2022 •

edited

Loading

lolodomo commented May 6, 2022 •

edited

Loading

clinique May 6, 2022 •

edited

Loading

lolodomo commented May 6, 2022 •

edited

Loading

lolodomo commented May 6, 2022 •

edited

Loading

lolodomo commented May 7, 2022 •

edited

Loading