[WlanThermo] Add support for new Nano V3, Mini V3, Link V1, Mini V1/V2(ESP32) devices [V3.x] #9579

CSchlipp · 2020-12-29T13:29:51Z

This PR adds support for the new Nano V3, Mini V2 (with ESP32-Upgrade), and Link V1 devices.

The API of these hardware versions has changed compared to the previous devices, but is the same for the whole new platform (thus named "esp32"). This commit follows the same approach as the initial contribution and contains auto-generated classes for parsing the new JSON output.

The thing description xml has been spit up to different files to ease the future development.
No further changes to existing things, thus backwards compatible.

Configuration of different thing types has been merged to avoid duplicate code.

~~Backport of changes to 2.5.x branch via PR #9580 (same changes)~~

fwolter

Your code doesn't build, because there are some checkstyle errors. You could take a look at target/code-analysis/report.html.

...in/java/org/openhab/binding/wlanthermo/internal/api/esp32/WlanThermoEsp32CommandHandler.java

.../src/main/java/org/openhab/binding/wlanthermo/internal/api/esp32/WlanThermoEsp32Handler.java

bundles/org.openhab.binding.wlanthermo/src/main/resources/OH-INF/thing/channel-types.xml

bundles/org.openhab.binding.wlanthermo/src/main/resources/OH-INF/thing/thing-types-mini.xml

CSchlipp · 2021-01-01T14:08:32Z

Thanks for the review!
All required changes, except for the scheduler, are included with the last commits. Some of the improvements have also been ported to the other supported hardware versions.

Local builds do not show any blocking checkstyle errors, let's wait for the Jenkins build...

CSchlipp · 2021-01-01T17:16:00Z

License Headers were missing.
Why isn't this reported during local mvn install?

Signed-off-by: Christian Schlipp <[email protected]>

further improved logging Signed-off-by: Christian Schlipp <[email protected]>

Signed-off-by: Christian Schlipp <[email protected]>

Specify Quantity Type for Temperature and Power Channels Signed-off-by: Christian Schlipp <[email protected]>

Signed-off-by: Christian Schlipp <[email protected]>

Revert to default scheduler Signed-off-by: Christian Schlipp <[email protected]>

CSchlipp · 2021-01-03T21:05:56Z

The build failed during the Feed-Binding Integration Tests - seemingly unrelated to this binding or the changes within this PR.
Could it be related to #9610?

Signed-off-by: Christian Schlipp <[email protected]>

fwolter

There are some checkstyle warnings left. You could take a look at target/code-analysis/report.html.

...ng.wlanthermo/src/main/java/org/openhab/binding/wlanthermo/internal/api/esp32/UtilEsp32.java

...in/java/org/openhab/binding/wlanthermo/internal/api/esp32/WlanThermoEsp32CommandHandler.java

bundles/org.openhab.binding.wlanthermo/src/main/resources/OH-INF/thing/channel-types.xml

...org.openhab.binding.wlanthermo/src/main/resources/OH-INF/thing/channel-group-types-esp32.xml

.../src/main/java/org/openhab/binding/wlanthermo/internal/api/esp32/WlanThermoEsp32Handler.java

Signed-off-by: Christian Schlipp <[email protected]>

CSchlipp · 2021-01-15T07:43:51Z

Argh, missed the last spotless:apply commit. Would you label it for rebuild again, please?
Sorry for the inconvenience caused...

cpmeister · 2021-01-15T07:58:29Z

Argh, missed the last spotless:apply commit. Would you label it for rebuild again, please?
Sorry for the inconvenience caused...

No problem.

cpmeister · 2021-01-15T08:08:56Z

Just need to get a completed jenkins build that succeeds. Did you ever address @fwolter's comment about the target/code-analysis/report.html?

I couldn't find any checkstyle warnings in my local build anymore.
There are some warnings about potential null pointer access due to Gson having @nullable return objects. All of them should now be covered by a check for null before accessing the object, so this should be false positives.

Actually, the potential null pointer access warnings I see there would be caused by using nullable fields directly instead of caching the field into a local variable first. The null checker assumes that any nullable field can become null at any time due to concurrency. And yes, I understand that it is often a wrong assumption but it is also the safest assumption.
So please make an effort to address those null pointer warnings.

Ensure Gson objects are NonNull Generify Handlers Generify Utils Signed-off-by: Christian Schlipp <[email protected]>

Signed-off-by: Christian Schlipp <[email protected]>

CSchlipp · 2021-01-17T17:14:27Z

Actually, the potential null pointer access warnings I see there would be caused by using nullable fields directly instead of caching the field into a local variable first. The null checker assumes that any nullable field can become null at any time due to concurrency. And yes, I understand that it is often a wrong assumption but it is also the safest assumption.
So please make an effort to address those null pointer warnings.

Guess I've found a quite pretty solution for the null pointer warnings.
Additionally, I've added unit tests for all models and moved a lot of duplicated code & logic to parent classes.

Would you please trigger a rebuild? Thanks!

...ding.wlanthermo/src/main/java/org/openhab/binding/wlanthermo/internal/WlanThermoHandler.java

...ava/org/openhab/binding/wlanthermo/internal/api/esp32/WlanThermoEsp32CommandHandlerTest.java

...in/java/org/openhab/binding/wlanthermo/internal/api/nano/WlanThermoNanoV1CommandHandler.java

move test json to resources Signed-off-by: Christian Schlipp <[email protected]>

Signed-off-by: Christian Schlipp <[email protected]>

cpmeister

LGTM

cpmeister · 2021-01-18T21:05:19Z

The build failed, looks like you need to run spotless.

Signed-off-by: Christian Schlipp <[email protected]>

CSchlipp · 2021-01-18T21:27:54Z

This is driving me nuts some day...
Sorry, fixed.

CSchlipp · 2021-01-19T06:24:26Z

Thanks again!
I've updated the PR name to be more precise in the changelogs.

…b#9579) * Add support for ESP32 devices * Add Unit Tests Ensure Gson objects are NonNull Generify Handlers Generify Utils Signed-off-by: Christian Schlipp <[email protected]> Signed-off-by: John Marshall <[email protected]>

…b#9579) * Add support for ESP32 devices * Add Unit Tests Ensure Gson objects are NonNull Generify Handlers Generify Utils Signed-off-by: Christian Schlipp <[email protected]>

CSchlipp mentioned this pull request Dec 29, 2020

[WlanThermo] Add support for new ESP32-powered devices [V2.5.x] #9580

Closed

CSchlipp changed the title ~~[WlanThermo] Add support for new ESP32-powered devices~~ [WlanThermo] Add support for new ESP32-powered devices [V3.x] Dec 29, 2020

Hilbrand added the enhancement An enhancement or new feature for an existing add-on label Dec 29, 2020

fwolter self-assigned this Dec 31, 2020

fwolter requested changes Dec 31, 2020

View reviewed changes

CSchlipp force-pushed the wlanThermoBinding-v3 branch from 1d81981 to b164d7b Compare December 31, 2020 13:56

CSchlipp added 14 commits January 3, 2021 15:54

Add support for ESP32 devices

8a18317

Signed-off-by: Christian Schlipp <[email protected]>

Fix Compiler warnings in V3 branch

31a5d04

Signed-off-by: Christian Schlipp <[email protected]>

moved auto-generated classed to dto package

24680ba

Signed-off-by: Christian Schlipp <[email protected]>

fix typo

7f85970

Signed-off-by: Christian Schlipp <[email protected]>

fix imports

5662306

Signed-off-by: Christian Schlipp <[email protected]>

reduce logs

dd494b9

Signed-off-by: Christian Schlipp <[email protected]>

fix formatting

3021e06

Signed-off-by: Christian Schlipp <[email protected]>

removed nullable config

6356ac2

further improved logging Signed-off-by: Christian Schlipp <[email protected]>

Only log payload if response code != 200

5855a56

Signed-off-by: Christian Schlipp <[email protected]>

fix compiler warnings

c76ecfa

Signed-off-by: Christian Schlipp <[email protected]>

fix channel naming

db1da51

Specify Quantity Type for Temperature and Power Channels Signed-off-by: Christian Schlipp <[email protected]>

fixed checkstyle error

ebe4011

Signed-off-by: Christian Schlipp <[email protected]>

Add license headers

c675da7

Signed-off-by: Christian Schlipp <[email protected]>

Rebase & Update License Headers

65628c9

Signed-off-by: Christian Schlipp <[email protected]>

CSchlipp force-pushed the wlanThermoBinding-v3 branch from ae80ec0 to 65628c9 Compare January 3, 2021 15:00

Fix scheduled task not cancelled on dispose

d365c23

Revert to default scheduler Signed-off-by: Christian Schlipp <[email protected]>

Retrigger Jenkins build

aff5cb5

Signed-off-by: Christian Schlipp <[email protected]>

CSchlipp requested a review from fwolter January 5, 2021 16:23

fwolter requested changes Jan 6, 2021

View reviewed changes

cpmeister requested changes Jan 6, 2021

View reviewed changes

fix constants

609d213

Signed-off-by: Christian Schlipp <[email protected]>

spotless:apply

26aef3f

Signed-off-by: Christian Schlipp <[email protected]>

cpmeister added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 15, 2021

CSchlipp added 3 commits January 17, 2021 17:43

Add Unit Tests

a273604

Ensure Gson objects are NonNull Generify Handlers Generify Utils Signed-off-by: Christian Schlipp <[email protected]>

Check if pitmaster is enabled for ESP32 devices

d7c3d69

Signed-off-by: Christian Schlipp <[email protected]>

Revert RSSI conversion for NanoV1 devices

483da47

Signed-off-by: Christian Schlipp <[email protected]>

cpmeister added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 17, 2021

cpmeister requested changes Jan 17, 2021

View reviewed changes

CSchlipp added 2 commits January 18, 2021 07:48

fix Fallthrough bug introduced by change of return handling

1270e69

move test json to resources Signed-off-by: Christian Schlipp <[email protected]>

remove unused throws

00c450f

Signed-off-by: Christian Schlipp <[email protected]>

cpmeister approved these changes Jan 18, 2021

View reviewed changes

cpmeister added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 18, 2021

remove unused import

7b15ff9

Signed-off-by: Christian Schlipp <[email protected]>

cpmeister added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jan 18, 2021

cpmeister merged commit d8e5a57 into openhab:main Jan 18, 2021

cpmeister added this to the 3.1 milestone Jan 18, 2021

CSchlipp changed the title ~~[WlanThermo] Add support for new ESP32-powered devices [V3.x]~~ [WlanThermo] Add support for new Nano V3, Mini V3, Link V1, Mini V1/V2(ESP32) devices [V3.x] Jan 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WlanThermo] Add support for new Nano V3, Mini V3, Link V1, Mini V1/V2(ESP32) devices [V3.x] #9579

[WlanThermo] Add support for new Nano V3, Mini V3, Link V1, Mini V1/V2(ESP32) devices [V3.x] #9579

CSchlipp commented Dec 29, 2020 •

edited

Loading

fwolter left a comment •

edited

Loading

CSchlipp commented Jan 1, 2021

CSchlipp commented Jan 1, 2021

CSchlipp commented Jan 3, 2021

fwolter left a comment

CSchlipp commented Jan 15, 2021

cpmeister commented Jan 15, 2021

cpmeister commented Jan 15, 2021

CSchlipp commented Jan 17, 2021

cpmeister left a comment

cpmeister commented Jan 18, 2021

CSchlipp commented Jan 18, 2021

CSchlipp commented Jan 19, 2021

[WlanThermo] Add support for new Nano V3, Mini V3, Link V1, Mini V1/V2(ESP32) devices [V3.x] #9579

[WlanThermo] Add support for new Nano V3, Mini V3, Link V1, Mini V1/V2(ESP32) devices [V3.x] #9579

Conversation

CSchlipp commented Dec 29, 2020 • edited Loading

fwolter left a comment • edited Loading

Choose a reason for hiding this comment

CSchlipp commented Jan 1, 2021

CSchlipp commented Jan 1, 2021

CSchlipp commented Jan 3, 2021

fwolter left a comment

Choose a reason for hiding this comment

CSchlipp commented Jan 15, 2021

cpmeister commented Jan 15, 2021

cpmeister commented Jan 15, 2021

CSchlipp commented Jan 17, 2021

cpmeister left a comment

Choose a reason for hiding this comment

cpmeister commented Jan 18, 2021

CSchlipp commented Jan 18, 2021

CSchlipp commented Jan 19, 2021

CSchlipp commented Dec 29, 2020 •

edited

Loading

fwolter left a comment •

edited

Loading