[boschshc] Add bridge and device discovery #14197

GerdZanker · 2023-01-09T20:07:31Z

#14195 [boschshc] Bridge discovery is implemented via mDNS, local IP addresses are checked. If a GET returns the public SHC information,
then this shcIpAddress is reported as a discovered bridge.

Devices are always discovered after successful pairing, but a manual scan is also possible.

GerdZanker · 2023-01-23T19:41:22Z

Hello @david-pace, can you already take a look at the code and how I now write the test?
There are two TODOs regarding the list of supported things pending which I will update within the next days.

david-pace · 2023-01-25T20:21:07Z

I'll look at it as soon as I find some time, probably at the weekend.

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

...rc/main/java/org/openhab/binding/boschshc/internal/discovery/BridgeDiscoveryParticipant.java

...shc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

david-pace · 2023-01-25T20:30:12Z

...shc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

+ new AbstractMap.SimpleEntry<>("BBL", BoschSHCBindingConstants.THING_TYPE_SHUTTER_CONTROL.getId()), // Rollladenschalter
+ new AbstractMap.SimpleEntry<>("TWINGUARD", BoschSHCBindingConstants.THING_TYPE_TWINGUARD.getId()), // Twinguard
+ new AbstractMap.SimpleEntry<>("PSM", BoschSHCBindingConstants.THING_TYPE_INWALL_SWITCH.getId()), // Steckdosen Zwischenstecker
+ new AbstractMap.SimpleEntry<>("PLUG_COMPACT", BoschSHCBindingConstants.THING_TYPE_INWALL_SWITCH.getId()) // Steckdosen Zwischenstecker kompakt


we now have a dedicated thing type for the plugs (not in-wall switch)

...src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java

...shc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

david-pace

added some comments about things I saw at first glance

GerdZanker · 2023-01-27T17:28:27Z

added some comments about things I saw at first glance

Thanks a lot. I will improve your findings together with the updated list/map of devices.
I'm glad to see no big topics regarding my new mockito tests.

...shc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

...src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java

david-pace

I found a typo in the key for the 360 camera and I also have some general questions about the keys.

GerdZanker · 2023-02-19T15:43:08Z

I found a typo in the key for the 360 camera and I also have some general questions about the keys.

Fixed the typo, improved the tests code and tried to clarify the questions.
Of cause the available information about your questions is weak and I guess the mapping must be improved later one. But it's the best I know currently.

Soon I will squash and cleanup the commits and provide a bundle Jar for testing with last stable version 3.4.2

GerdZanker · 2023-02-21T19:05:01Z

Test failing, because of updated code. Branch will be rebased and fixed soon.

...schshc/src/main/java/org/openhab/binding/boschshc/internal/devices/bridge/BridgeHandler.java

david-pace

See comments, plus some checkstyle warnings have been introduced that should be fixed if possible. See file target/code-analysis/report.html. On the main branch I currently see 3 warnings related to catching NullPointerExceptions, on your branch I see 19 additional minor things that could be mitigated easily (like unnecessary empty lines etc.)

GerdZanker · 2023-03-03T18:41:14Z

See comments,

All comments should be improved, please check how I handled the InterruptException.

plus some checkstyle warnings have been introduced that should be fixed if possible. See file target/code-analysis/report.html. On the main branch I currently see 3 warnings related to catching NullPointerExceptions, on your branch I see 19 additional minor things that could be mitigated easily (like unnecessary empty lines etc.)

I did my best to get rid of all findings

david-pace

Only a few minor things remaining 👍 I also tested your change with real hardware and everything works nicely 😎

One idea for an enhancement would be the labels for the following items. It would look nicer if they did not have the surrounding - characters and proper readable names:

...rc/main/java/org/openhab/binding/boschshc/internal/discovery/BridgeDiscoveryParticipant.java

GerdZanker · 2023-03-17T16:49:35Z

Only a few minor things remaining 👍 I also tested your change with real hardware and everything works nicely 😎

Thanks for your testfeedback

One idea for an enhancement would be the labels for the following items. It would look nicer if they did not have the surrounding - characters and proper readable names:

It took some time to find a solution for nice names, but now this enhancement is part of this PR.

david-pace · 2023-03-18T16:12:30Z

...shc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

+ // convert "IntrusionDetectionSystem" into "Intrusion Detection System"
+ String[] allParts = StringUtils.splitByCharacterTypeCamelCase(niceName);
+ String[] parts = Arrays.stream(allParts).filter(p -> !p.trim().isEmpty()).collect(Collectors.toList())
+ .toArray(new String[0]);


Array conversion is not needed. You could use String.join() providing a list of strings. But there is an even better way: use Collectors.joining(" ") instead of Collectors.toList() and you're done directly 👍

david-pace

Awesome, this looks great! Thanks for improving the labels, I see that this was not trivial ;) I tested again with the actual hardware and it looks good 👍

I just have a minor suggestion which could save one line of code, but this is optional. Good work 😎

jlaur

Thanks! In addition to the review of @david-pace (👍) only a single comment from my side.

...shc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

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

GerdZanker · 2023-03-20T19:42:07Z

Out of topic, but @jlaur, do you know how I can provide a nice binding icon for boschshc? I wasn't able to find a documentation or used the wrong words.

jlaur · 2023-03-20T19:47:41Z

Out of topic, but @jlaur, do you know how I can provide a nice binding icon for boschshc? I wasn't able to find a documentation or used the wrong words.

Yes, see here: https://www.openhab.org/docs/developer/bindings/#add-your-binding-s-logo-to-the-openhab-website

jlaur · 2023-03-20T19:50:55Z

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

+2023-03-20 20:30:48.026 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'boschshc:security-camera-eyes:yourBrideName:hdm_Cameras_XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX' to inbox.
+2023-03-20 20:30:48.026 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'boschshc:smoke-detector:yourBrideName:hdm_HomeMaticIP_XXXXXXXXXXXXXXXXXXXXXXXX' to inbox.
+2023-03-20 20:30:48.027 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'boschshc:twinguard:yourBrideName:hdm_ZigBee_XXXXXXXXXXXXXXXX' to inbox.
+2023-03-20 20:30:48.028 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'boschshc:smart-bulb:yourBrideName:hdm_PhilipsHueBridge_HueLight_XXXXXXXXXXXXXXXX-XX_XXXXXXXXXXXX' to inbox.


yourBrideName intended? 👰

Arg - last minute changes 🤦‍♂️

Out of topic, but @jlaur, do you know how I can provide a nice binding icon for boschshc? I wasn't able to find a documentation or used the wrong words.

Yes, see here: https://www.openhab.org/docs/developer/bindings/#add-your-binding-s-logo-to-the-openhab-website

Thanks a lot for the link to this obvious location.
I wasn't able to get information when I searched for hints the last time, but its already some months ago - hopefully this answer was added just recently 🤔

Bridge discovery is implemented via mDNS, local IP addresses are checked. If a GET returns the public SHC information, then this shcIpAddress is reported as a discovered bridge. Devices are always discovered after successful pairing, but a manual scan is also possible. Added unit tests for Bridge and Device Discovery. Signed-off-by: Gerd Zanker <gerd.zanker@web.de>

Three AvoidCatchingNPE findings are pending in the class BoschHttpClient. Could be false positive, because NullPointer exceptions appear during tests run. Signed-off-by: Gerd Zanker <gerd.zanker@web.de>

jlaur

Last minute finding, sorry.

...shc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

Signed-off-by: Gerd Zanker <gerd.zanker@web.de>

jlaur

LGTM

* openhab#14195 Bridge and Device Discovery Bridge discovery is implemented via mDNS, local IP addresses are checked. If a GET returns the public SHC information, then this shcIpAddress is reported as a discovered bridge. Devices are always discovered after successful pairing, but a manual scan is also possible. Added unit tests for Bridge and Device Discovery. Signed-off-by: Gerd Zanker <gerd.zanker@web.de>

jlaur changed the title ~~#14195 [boschshc] Bridge and Device Discovery~~ [boschshc] Bridge and Device Discovery Jan 9, 2023

jlaur linked an issue Jan 9, 2023 that may be closed by this pull request

[boschshc] Discovery of Bridge and Things #14195

Closed

jlaur added the enhancement An enhancement or new feature for an existing add-on label Jan 9, 2023

GerdZanker force-pushed the feature/#14195-bosch-shc-discovery branch from 914b314 to e30b864 Compare January 23, 2023 19:36