Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[shelly] Add Plus/Pro support, some bugfixes #13439

Merged
merged 3 commits into from
Oct 4, 2022
Merged

[shelly] Add Plus/Pro support, some bugfixes #13439

merged 3 commits into from
Oct 4, 2022

Conversation

markus7017
Copy link
Contributor

This PR replaces #12842

The PR adds support for the Shelly Generation 2 Plus/Pro Devices .

I tried to separate it from other changes (previous PRs), but it still has a relevant size.

@markus7017 markus7017 requested a review from fwolter September 25, 2022 12:24
@markus7017 markus7017 self-assigned this Sep 25, 2022
@markus7017 markus7017 added bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels Sep 25, 2022
@jlaur
Copy link
Contributor

jlaur commented Sep 25, 2022

@markus7017 - can you close the PR being replaced?

@jlaur jlaur removed the bug An unexpected problem or unintended behavior of an add-on label Sep 25, 2022
@lolodomo lolodomo mentioned this pull request Sep 25, 2022
Closed
@markus7017 markus7017 force-pushed the shelly_snapshot3.4_pr4 branch from e0c26b9 to 875ce4c Compare September 25, 2022 20:08
@markus7017
Copy link
Contributor Author

@fwolter @lolodomo I fixed the merge conflict and ready to start the review. Pinged you both, because you were in previous PRs.

@markus7017
Plus/Pro support and some refactoring, bugfixes
d6c9e8c 
Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017 markus7017 force-pushed the shelly_snapshot3.4_pr4 branch from 875ce4c to d6c9e8c Compare October 1, 2022 18:31
@markus7017 markus7017 requested a review from lolodomo October 1, 2022 18:44
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/shelly-beta-with-plus-pro-support/139554/1

fwolter
fwolter requested changes Oct 2, 2022
View reviewed changes
Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you say a few words about how Gen2 devices connect in general? Do the devices connect to the OH instance? Is there some authentification? Is it save to expose the servlet to the internet?

bundles/org.openhab.binding.shelly/README.md Outdated Show resolved Hide resolved
....binding.shelly/src/main/java/org/openhab/binding/shelly/internal/api2/Shelly2RpcSocket.java Outdated Show resolved Hide resolved
...ding.shelly/src/main/java/org/openhab/binding/shelly/internal/handler/ShellyBaseHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.shelly/src/main/resources/OH-INF/thing/shellyGen2_relay.xml Outdated Show resolved Hide resolved
@markus7017
Copy link
Contributor Author

Can you say a few words about how Gen2 devices connect in general? Do the devices connect to the OH instance? Is there some authentification? Is it save to expose the servlet to the internet?

The binding supports Generation 1 (first series) and Plus / Pro series of devices, which are Generation 2. Both generations uses complete different APIs

  • Generation 1: Uses http-based communication with basic authentication. In addition the devices implement CoIoT version 1 + 2, a Allterco specific IoT protocol based on Coap. Therefore the Californium framework is integrated to handle the CoAP protocol. CoAP supports multicast mode and peer mode. Using the peer mode is an easy way to have specific firewall rules protecting the device access.
  • Generation 2: Complete different API, which is based on http POSTs for device control and an WebSocket interface for event messages. In addition the binding implements an inbound WebSocket, which is required to receive wakeup / status notifications from battery powered devices, which go into sleep mode. Gen2 uses digest authentication if restricted access has been enabled in the device setup. The user is "admin" (hard-code for whatever reason) and password is taken from the binding or thing configuration. The device also supports secure WebSocket communication (wss, incl. X.5409 certificates), but this is not yet implemented in the binding. In general you might access a Shelly device directly from the Internet or exposing the servlet entry point using protected device setup. However, I would prefer a VPN in between.
  • ShellyManager is also based on a Servlet and implements some advanced device diagnostic and control functions. The Manager runs in the openHAB context so you need to login as usual being able to access the UI (http / https).

Short note on the API. Due to the fact that APIs are different, also the data formats, but device functionality is similar I decided to implement a mapping layer. Data structures are mapped form Gen2 format to Gen1 format for further processing on the upper code layers. Otherwise I would have to re-implement the complete logic, in fact redundant. Gen1 devices are still in production so removing Gen1 support was no option. Also creating a 2nd binding would increase maintenance efforts and may confuses the users.
I separate Gen1 and Gen2 APIs in packages

  • api1 = Gen1-specific
  • api2 = Gen2-spcific
  • apu = Common code
  • ShellyApiInterface defines the API interface, which then has 2 implementations

Please feel free to ask further questions if this gets a better understanding for the review.

@markus7017
Review changes applied
152412e 
Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017
Copy link
Contributor Author

@fwolter requested changes are resolved

@markus7017
review changes
4ec2c70 
Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017
Copy link
Contributor Author

@fwolter I pushed the changes, also removing some other left overs from development phase :-)

fwolter
fwolter approved these changes Oct 4, 2022
View reviewed changes
Copy link
Member

@fwolter fwolter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

@fwolter fwolter merged commit 6835c27 into openhab:main Oct 4, 2022
@fwolter fwolter added this to the 3.4 milestone Oct 4, 2022
This was linked to issues Oct 4, 2022
[shelly] DW2: An event "0" is posted to the alarm trigger channel #12339
Closed
[shelly] Motion timestamp is 1h off #12402
Closed
[shelly] Support for Gen2 devices (Plus/Pro) #12336
Closed
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/shelly-binding/56862/2813

cputoaster
cputoaster reviewed Oct 11, 2022
View reviewed changes
bundles/org.openhab.binding.shelly/src/main/resources/OH-INF/thing/shellyGen1_relay.xml
@@ -104,22 +104,8 @@
<config-description-ref uri="thing-type:shelly:relay"/>
</thing-type>

<thing-type id="shelly2-roller">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for this removal?

@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/shelly-binding/56862/2814

andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@markus7017 @andan67
[shelly] Add Plus/Pro support, some bugfixes (openhab#13439)
13bd682 
* Plus/Pro support and some refactoring, bugfixes

Signed-off-by: Markus Michels <markus7017@gmail.com>

* Review changes applied

Signed-off-by: Markus Michels <markus7017@gmail.com>

* review changes

Signed-off-by: Markus Michels <markus7017@gmail.com>

Signed-off-by: Markus Michels <markus7017@gmail.com>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
@markus7017 @andrasU
[shelly] Add Plus/Pro support, some bugfixes (openhab#13439)
6de8953 
* Plus/Pro support and some refactoring, bugfixes

Signed-off-by: Markus Michels <markus7017@gmail.com>

* Review changes applied

Signed-off-by: Markus Michels <markus7017@gmail.com>

* review changes

Signed-off-by: Markus Michels <markus7017@gmail.com>

Signed-off-by: Markus Michels <markus7017@gmail.com>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
borazslo pushed a commit to borazslo/openhab-mideaac-addon that referenced this pull request Jan 8, 2023
@markus7017 @borazslo
[shelly] Add Plus/Pro support, some bugfixes (openhab#13439)
4ce9e49 
* Plus/Pro support and some refactoring, bugfixes

Signed-off-by: Markus Michels <markus7017@gmail.com>

* Review changes applied

Signed-off-by: Markus Michels <markus7017@gmail.com>

* review changes

Signed-off-by: Markus Michels <markus7017@gmail.com>

Signed-off-by: Markus Michels <markus7017@gmail.com>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
@markus7017 @psmedley
[shelly] Add Plus/Pro support, some bugfixes (openhab#13439)
e756b1a 
* Plus/Pro support and some refactoring, bugfixes

Signed-off-by: Markus Michels <markus7017@gmail.com>

* Review changes applied

Signed-off-by: Markus Michels <markus7017@gmail.com>

* review changes

Signed-off-by: Markus Michels <markus7017@gmail.com>

Signed-off-by: Markus Michels <markus7017@gmail.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@markus7017 @nemerdaud
[shelly] Add Plus/Pro support, some bugfixes (openhab#13439)
cccbb20 
* Plus/Pro support and some refactoring, bugfixes

Signed-off-by: Markus Michels <markus7017@gmail.com>

* Review changes applied

Signed-off-by: Markus Michels <markus7017@gmail.com>

* review changes

Signed-off-by: Markus Michels <markus7017@gmail.com>

Signed-off-by: Markus Michels <markus7017@gmail.com>
@markus7017 markus7017 deleted the shelly_snapshot3.4_pr4 branch July 8, 2023 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cputoaster cputoaster cputoaster left review comments

@fwolter fwolter fwolter approved these changes

@lolodomo lolodomo Awaiting requested review from lolodomo

Assignees

@markus7017 markus7017

Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
3.4
5 participants
@markus7017 @jlaur @openhab-bot @cputoaster @fwolter