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

[velux] Vane position channel is visible when required #13271

Merged
merged 14 commits into from
Sep 11, 2022
Merged

[velux] Vane position channel is visible when required #13271

merged 14 commits into from
Sep 11, 2022

Conversation

andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Aug 16, 2022 •
edited
Loading

Background

In #12618 we added support for a vane position channel on venetian style blinds. And in that PR the respective vanePosition channel was created statically for ALL types of shutters and blinds regardless of whether the actual shutter/blind does physically support vanes. This could be confusing for users who have shutters / blinds that do not support vanes.

Solution

In this PR the binding queries each shutter/blind in the hub, and if that device actually supports vanes, then the binding will create a vanePosition channel dynamically.

Resolves #13260

Acknowledgments

PS many thanks to @pacive, @splatch, @lolodomo and @jlaur for support via the community forum..

https://community.openhab.org/t/dynamically-created-channel-not-showing-up-in-main-ui/138150

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

@andrewfg
[velux] make vanePosition channel dynamic
cd989f7 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
[velux] remove inversion
d81fe2e 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
[velux] remove spurious logging
e879944 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
[velux] simplify lambda
6425073 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
[velux] adopt kind recommendations from community forum
7c16997 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg added the enhancement An enhancement or new feature for an existing add-on label Aug 16, 2022
@andrewfg andrewfg requested a review from gs4711 as a code owner August 16, 2022 10:41
@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/dynamically-created-channel-not-showing-up-in-main-ui/138150/21

@andrewfg
[velux] clean up comment
819d003 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@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/dynamically-created-channel-not-showing-up-in-main-ui/138150/24

@andrewfg andrewfg requested review from lolodomo and pacive August 16, 2022 13:08
@andrewfg
[velux] delete infamous 'extensible' attribute
785f743 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from splatch August 16, 2022 17:49
@andrewfg andrewfg self-assigned this Aug 16, 2022
pacive
pacive approved these changes Aug 17, 2022
View reviewed changes
Copy link
Member

@pacive pacive left a comment

Choose a reason for hiding this comment

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

Looks good

@andrewfg
[velux] adopt reviewer suggestions
b2945c9 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested review from lolodomo and removed request for splatch August 18, 2022 14:05
@andrewfg andrewfg mentioned this pull request Aug 18, 2022
Closed
@andrewfg
Copy link
Contributor Author

andrewfg commented Aug 19, 2022 •
edited
Loading

@lolodomo please hold; in the handler calling updateThing() causes its thingUpdated() to be called, which in turn calls the dispose() and initialize() methods; so there may be a risk of recursion; in fact I don't see this, but let me just check it again..

EDIT: ok I checked again. It is all good :)

@andrewfg
[velux] confirm no recursion, plus cosmetics
9d0e333 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
This was referenced Aug 21, 2022
Merged
Merged
@andrewfg andrewfg marked this pull request as draft August 21, 2022 19:39
@andrewfg
Copy link
Contributor Author

Converted this to draft until the way forward for other similar PRs has been resolved.

@andrewfg
[velux] revert from channel creation to channel removal
81bf483 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg marked this pull request as ready for review August 22, 2022 14:47
@andrewfg andrewfg requested a review from jlaur August 22, 2022 14:47
@andrewfg andrewfg changed the title [velux] Channel 'vanePosition' is created dynamically only when required [velux] Channel 'vanePosition' only visible when required Aug 22, 2022
@andrewfg andrewfg changed the title [velux] Channel 'vanePosition' only visible when required [velux] Vane position channel is visible when required Aug 22, 2022
@andrewfg
[velux] change category of window position from 'blinds' to 'window'
a988f61 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
[velux] remove vane category
c98b5e6 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

@lolodomo just an update to avoid any (further) confusion..

  1. Last week I put this PR on hold because I wanted to use a fully harmonised approach across [velux] Vane position channel is visible when required #13271 (this), [hdpowerview] Channels are visible depending on shade capabilities #13275 and [tado] Channels are visible depending on device type and capabilities #13301.
  2. In the meantime we reached agreement on [hdpowerview] Channels are visible depending on shade capabilities #13275 and that has now been finalised and merged
  3. And [velux] Vane position channel is visible when required #13271 (this) and [tado] Channels are visible depending on device type and capabilities #13301 are both now also fully aligned with [hdpowerview] Channels are visible depending on shade capabilities #13275

=> So we are ready to go on this one again! :)

@andrewfg
[velux] remove configuration; apply reviewer suggestions
50b1a5e 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
[velux] apply reviewer suggestions
b0e3bab 
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

@lolodomo I added an alert #1414 to the release notes concerning the category correction of window position channels.

lolodomo
lolodomo approved these changes Sep 11, 2022
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM

@lolodomo lolodomo merged commit 9123e13 into openhab:main Sep 11, 2022
@lolodomo lolodomo added this to the 3.4 milestone Sep 11, 2022
@andrewfg andrewfg mentioned this pull request Sep 11, 2022
Merged
@andrewfg andrewfg mentioned this pull request Sep 27, 2022
Closed
leifbladt pushed a commit to leifbladt/openhab-addons that referenced this pull request Oct 15, 2022
@andrewfg @leifbladt
[velux] Vane position channel is visible when required (openhab#13271)
cf8fccd 
* [velux] make vanePosition channel dynamic
* [velux] change category of window position from 'blinds' to 'window'

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@andrewfg @andan67
[velux] Vane position channel is visible when required (openhab#13271)
de4e8ac 
* [velux] make vanePosition channel dynamic
* [velux] change category of window position from 'blinds' to 'window'

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
@andrewfg @andrasU
[velux] Vane position channel is visible when required (openhab#13271)
20dc7c3 
* [velux] make vanePosition channel dynamic
* [velux] change category of window position from 'blinds' to 'window'

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
marcelGoerentz pushed a commit to marcelGoerentz/openhab-addons that referenced this pull request Nov 14, 2022
@andrewfg @MarcelGoerentz-eaton
[velux] Vane position channel is visible when required (openhab#13271)
b45cbe1 
* [velux] make vanePosition channel dynamic
* [velux] change category of window position from 'blinds' to 'window'

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Feb 23, 2023
@andrewfg @psmedley
[velux] Vane position channel is visible when required (openhab#13271)
28227d7 
* [velux] make vanePosition channel dynamic
* [velux] change category of window position from 'blinds' to 'window'

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@andrewfg @nemerdaud
[velux] Vane position channel is visible when required (openhab#13271)
5a928dc 
* [velux] make vanePosition channel dynamic
* [velux] change category of window position from 'blinds' to 'window'

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@splatch splatch splatch left review comments

@pacive pacive pacive approved these changes

@gs4711 gs4711 gs4711 approved these changes

@lolodomo lolodomo lolodomo approved these changes

@jlaur jlaur Awaiting requested review from jlaur

Assignees

@andrewfg andrewfg

Labels
enhancement An enhancement or new feature for an existing add-on (potentially) not backward compatible
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.

[velux] Create vane position channel dynamically
7 participants
@andrewfg @openhab-bot @splatch @lolodomo @gs4711 @jlaur @pacive