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.

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

[shelly] Improved Motion Support, Support CoIoT Unicast, fixes #10220

Merged
merged 7 commits into from
Mar 3, 2021

Conversation

markus7017
Copy link
Contributor

@markus7017 markus7017 commented Feb 23, 2021

The PR adds some improvements

  • New channels device#sensorSleepTime, sensor#motionAction for Shelly Motion, sensor#motionTimestamp fixed on CoAP updazes
  • Support for CoIoT Unicast (the Motion doesn't support Multicast anymore), works also for other devices
  • H&T device#charger fixed
  • Version strong was not updated on RESTARTED in call cases (timing)
  • some other optimizations

Signed-off-by: Markus Michels <[email protected]>
@markus7017 markus7017 self-assigned this Feb 23, 2021
@markus7017 markus7017 added the enhancement An enhancement or new feature for an existing add-on label Feb 23, 2021
@markus7017 markus7017 requested a review from fwolter February 23, 2021 19:08
@markus7017 markus7017 added the work in progress A PR that is not yet ready to be merged label Feb 24, 2021
@markus7017
Copy link
Contributor Author

markus7017 commented Feb 24, 2021

@fwolter Allterco updated their firmware, which brings an important feature - helps the users and reduces my support. I want to include this in this PR.

Nevertheless, could you please have a first look to the Code and give feedback on code structure. The is a bigger one and adds a complete new component org.openhab.binding.shelly.internal.manager. I tried to encapsulate as good as I can by having the is separate Component and a clear interface to the Thing handler (ShellyManagerInterface).

I would work on your feedback before starting the review. So far I set the WIP label.

@fwolter
Copy link
Member

fwolter commented Feb 24, 2021

If these are features which are independent from each other, it would be better to split these and file another PR for the updated firmware.

@markus7017
Copy link
Contributor Author

@fwolter It's one new component Shelly Manager, which has SOME dependencies/changes in the base code, but the new Manager component depends on the changes in the base code. It's not "10 independent features in one PR"

@fwolter
Copy link
Member

fwolter commented Feb 24, 2021

Ok. As the backlog is quite extensive, I'm afraid I can't review this in the short-term.

@markus7017
Copy link
Contributor Author

@fwolter would it help if I split it

  • Part I: Base changes and supporting stuff (html+images, updated readme) - yes, it goes into the bundle, but doesn't get visible to the user
  • Part II: Actual Shelly Manager component

I estimate Part I is about 1/3rd of the total

@fwolter
Copy link
Member

fwolter commented Feb 24, 2021

It makes sense if the two parts are useable from each other independently. If the changes in the first part are useless without part II, it doesn't make sense to split it.

@markus7017 markus7017 added bug An unexpected problem or unintended behavior of an add-on and removed work in progress A PR that is not yet ready to be merged labels Feb 26, 2021
@markus7017
Copy link
Contributor Author

@fwolter I reduced the scope of this PR to include regular changes, but not the Manager
I kindly request to do the review on short-term so I get out of the sitioation that the PR doesn't matches the current development and I need to keep this branch up-to-date

sensorSleepTime is now adadvanced; Roller set position 0/100 is mapped
to UP/DOWN; Reference to Shelly Manager removed from README

Signed-off-by: Markus Michels <[email protected]>
@markus7017 markus7017 changed the title [shelly] Shelly Manager [shelly] Improved Motion support, CoIoT multicast, fixed Feb 27, 2021
@markus7017
Copy link
Contributor Author

@fwolter does that work for you?

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.

There are some checkstyle warnings, which could be fixed easily.

@markus7017 markus7017 changed the title [shelly] Improved Motion support, CoIoT multicast, fixed [shelly] Improved Motion support, CoIoT multicast, fixes Mar 2, 2021
Signed-off-by: Markus Michels <[email protected]>
restarted, README updated, moved channel sensorSleepTime from group
device to sensors

Signed-off-by: Markus Michels <[email protected]>
@markus7017 markus7017 changed the title [shelly] Improved Motion support, CoIoT multicast, fixes [shelly] Improved Motion Support, Support CoIoT Unicast, fixes Mar 3, 2021
@markus7017
Copy link
Contributor Author

@fwolter Review changes are applied, was able to fix 2 more bugs, nothing open know at the moment

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.

There are two zip files in this PR.

Signed-off-by: Markus Michels <[email protected]>
@markus7017
Copy link
Contributor Author

@fwolter Changes applied a) regex b) not the smartes idea to work on a copy of the documents :-), zip files removed

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.

LGTM

Great that you figured out the regex!

@fwolter fwolter merged commit 3af0392 into openhab:main Mar 3, 2021
@fwolter fwolter added this to the 3.1 milestone Mar 3, 2021
@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/1942

@markus7017
Copy link
Contributor Author

@fwolter We spoke abozt the regex expression, could you help to define the pattern, which extracts the version, but also -rc2" as sub identifier, samples for version strings:

20210226-072307/v1.1.0@f31e1d2b
20201124-092159/v1.9.0@57ac4ad8
20210115-103919/v1.9.4@e2732e05
20210226-115010/v1.9.6-dimmer2@1873d25e
20210122-154345/v1.10.0-rc1@00eeaa9b
20210303-090126/v1.10.0-rc2-153-gd87589bc6-master
20210209-101435/v1.10.0-rc2-g0cf9ee3-heads/v1.10.0-rc2

Currently I have the following pattern: "v\d+\.\d+\.\d+"
things gets the version, but strips the "-rc1" or "-rc2" or "-dimmer2"

Feedback welcome

@markus7017 markus7017 deleted the shelly_snapshot3.1-3 branch March 5, 2021 19:12
@fwolter
Copy link
Member

fwolter commented Mar 5, 2021

v\d+\.\d+\.\d+(-[a-z0-9]*)? should work.

themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
…ab#10220)

* New feature: Shelly Manager

Signed-off-by: Markus Michels <[email protected]>

* Removed Shelly Manager to reduce PR size (will be another PR)

Signed-off-by: Markus Michels <[email protected]>

* CoIoT initialization handles new COIOT options for the device,
sensorSleepTime is now adadvanced; Roller set position 0/100 is mapped
to UP/DOWN; Reference to Shelly Manager removed from README

Signed-off-by: Markus Michels <[email protected]>

* Nullpointer check added on settings.coiot (4Pro has this null)

Signed-off-by: Markus Michels <[email protected]>

* README updated

Signed-off-by: Markus Michels <[email protected]>

* Use regex to extract fw version from string, check fw version to detect
restarted, README updated, moved channel sensorSleepTime from group
device to sensors

Signed-off-by: Markus Michels <[email protected]>

* Review changes

Signed-off-by: Markus Michels <[email protected]>
Signed-off-by: John Marshall <[email protected]>
computergeek1507 pushed a commit to computergeek1507/openhab-addons that referenced this pull request Jul 13, 2021
…ab#10220)

* New feature: Shelly Manager

Signed-off-by: Markus Michels <[email protected]>

* Removed Shelly Manager to reduce PR size (will be another PR)

Signed-off-by: Markus Michels <[email protected]>

* CoIoT initialization handles new COIOT options for the device,
sensorSleepTime is now adadvanced; Roller set position 0/100 is mapped
to UP/DOWN; Reference to Shelly Manager removed from README

Signed-off-by: Markus Michels <[email protected]>

* Nullpointer check added on settings.coiot (4Pro has this null)

Signed-off-by: Markus Michels <[email protected]>

* README updated

Signed-off-by: Markus Michels <[email protected]>

* Use regex to extract fw version from string, check fw version to detect
restarted, README updated, moved channel sensorSleepTime from group
device to sensors

Signed-off-by: Markus Michels <[email protected]>

* Review changes

Signed-off-by: Markus Michels <[email protected]>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
…ab#10220)

* New feature: Shelly Manager

Signed-off-by: Markus Michels <[email protected]>

* Removed Shelly Manager to reduce PR size (will be another PR)

Signed-off-by: Markus Michels <[email protected]>

* CoIoT initialization handles new COIOT options for the device,
sensorSleepTime is now adadvanced; Roller set position 0/100 is mapped
to UP/DOWN; Reference to Shelly Manager removed from README

Signed-off-by: Markus Michels <[email protected]>

* Nullpointer check added on settings.coiot (4Pro has this null)

Signed-off-by: Markus Michels <[email protected]>

* README updated

Signed-off-by: Markus Michels <[email protected]>

* Use regex to extract fw version from string, check fw version to detect
restarted, README updated, moved channel sensorSleepTime from group
device to sensors

Signed-off-by: Markus Michels <[email protected]>

* Review changes

Signed-off-by: Markus Michels <[email protected]>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
…ab#10220)

* New feature: Shelly Manager

Signed-off-by: Markus Michels <[email protected]>

* Removed Shelly Manager to reduce PR size (will be another PR)

Signed-off-by: Markus Michels <[email protected]>

* CoIoT initialization handles new COIOT options for the device,
sensorSleepTime is now adadvanced; Roller set position 0/100 is mapped
to UP/DOWN; Reference to Shelly Manager removed from README

Signed-off-by: Markus Michels <[email protected]>

* Nullpointer check added on settings.coiot (4Pro has this null)

Signed-off-by: Markus Michels <[email protected]>

* README updated

Signed-off-by: Markus Michels <[email protected]>

* Use regex to extract fw version from string, check fw version to detect
restarted, README updated, moved channel sensorSleepTime from group
device to sensors

Signed-off-by: Markus Michels <[email protected]>

* Review changes

Signed-off-by: Markus Michels <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on
Projects
None yet
3 participants