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

[hdpowerview] Added support for rear blackout panel #12098

Merged
merged 7 commits into from
Jan 25, 2022
Merged

[hdpowerview] Added support for rear blackout panel #12098

merged 7 commits into from
Jan 25, 2022

Conversation

andrewfg
Copy link
Contributor

Fixes #12097

  • Added support for a second rear blackout panel on shades with capabilities:8 and capabilities:9
  • Refactored the enum constant names for PRIMARY_POSITION, SECONDARY_POSITION etc.
  • Added JUNIT tests for shades with capabilities:8 and capabilities:9

Signed-off-by: Andrew Fiddian-Green [email protected]

@andrewfg andrewfg added bug An unexpected problem or unintended behavior of an add-on additional testing preferred The change works for the pull request author. A test from someone else is preferred though. labels Jan 23, 2022
@andrewfg andrewfg requested a review from jlaur January 23, 2022 17:08
@andrewfg andrewfg requested a review from andylintner as a code owner January 23, 2022 17:08
@andrewfg andrewfg marked this pull request as draft January 23, 2022 17:08
@andrewfg
Copy link
Contributor Author

andrewfg commented Jan 23, 2022 •
edited
Loading

@jlaur and @arroyoj => This PR adds further support for shades with capabilities:8, 9, and I want to make sure that it has not introduced any regressions in shades with dual motors i.e. shades with capabilities:1, 2, 4, 5, 7 so I have initially marked it as DRAFT in the hope that you can both review it against regressions.

EDIT: the JAR file is here https://github.com/andrewfg/openhab-addons/tree/jar-uploads/bundles/org.openhab.binding.hdpowerview

@andrewfg andrewfg added enhancement An enhancement or new feature for an existing add-on and removed bug An unexpected problem or unintended behavior of an add-on labels Jan 23, 2022
@jlaur
Copy link
Contributor

jlaur commented Jan 23, 2022

@andrewfg - cool! I have only one two-motor shade, a top down/bottom up. It's in the bedroom and I can't test it right now, but I'll get it done one of the upcoming days.

@jlaur
Copy link
Contributor

jlaur commented Jan 24, 2022

I have only one two-motor shade, a top down/bottom up. It's in the bedroom and I can't test it right now, but I'll get it done one of the upcoming days.

@andrewfg - the bedroom is currently Covid-19 isolation zone, and I'm not in it, but I just managed to make full use of a 10 minute timeslot. :-) I tested reading and writing position/secondary for the TDBU shade, and it works exactly as before. So I can confirm that this type of shade was not affected by the changes.

@andrewfg
Copy link
Contributor Author

bedroom is currently Covid-19 isolation zone, and I'm not in it

I wish whoever is in the room a quick recovery. Stay safe!

@arroyoj
Copy link

arroyoj commented Jan 25, 2022

@andrewfg, that's great that you've been able to identify additional shade types and capabilities and expand the support in this binding. I tested my top-down/bottom-up shades (capabilities: 7) with the JAR that you posted and they worked fine. I didn't see any regressions.

@andrewfg andrewfg marked this pull request as ready for review January 25, 2022 08:41
@andrewfg andrewfg removed the additional testing preferred The change works for the pull request author. A test from someone else is preferred though. label Jan 25, 2022
@andrewfg andrewfg requested a review from a team January 25, 2022 08:42
jlaur
jlaur requested changes Jan 25, 2022
View reviewed changes
Copy link
Contributor

@jlaur jlaur 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 in general, just a few minor comments.

...c/main/java/org/openhab/binding/hdpowerview/internal/database/ShadeCapabilitiesDatabase.java Show resolved Hide resolved
...binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/HDPowerViewJUnitTests.java Outdated Show resolved Hide resolved
...binding.hdpowerview/src/test/java/org/openhab/binding/hdpowerview/HDPowerViewJUnitTests.java Outdated Show resolved Hide resolved
jlaur
jlaur approved these changes Jan 25, 2022
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

LGTM - thanks!

@jlaur jlaur merged commit 8a2d507 into openhab:main Jan 25, 2022
@jlaur jlaur added this to the 3.3 milestone Jan 25, 2022
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
@andrewfg
[hdpowerview] Added support for rear blackout panel (openhab#12098)
1a626a9 
* [hdpowerview] refactor enum constant names
* [hdpowerview] add support for blackout shades
* [hdpowerview] unit tests for capabilities 8 & 9
* [hdpowerview] delete no longer valid comment
* [hdpowerview] blackout shade position is never UNDEF
* [hdpowerview] updated read me
* [hdpowerview] refactor unit tests into two classes

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg deleted the 12097-hdpowerview-capabilities8 branch February 7, 2022 18:49
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Apr 27, 2022
@andrewfg @NickWaterton
[hdpowerview] Added support for rear blackout panel (openhab#12098)
389c055 
* [hdpowerview] refactor enum constant names
* [hdpowerview] add support for blackout shades
* [hdpowerview] unit tests for capabilities 8 & 9
* [hdpowerview] delete no longer valid comment
* [hdpowerview] blackout shade position is never UNDEF
* [hdpowerview] updated read me
* [hdpowerview] refactor unit tests into two classes

Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Nick Waterton <[email protected]>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
@andrewfg @andan67
[hdpowerview] Added support for rear blackout panel (openhab#12098)
c3a3e23 
* [hdpowerview] refactor enum constant names
* [hdpowerview] add support for blackout shades
* [hdpowerview] unit tests for capabilities 8 & 9
* [hdpowerview] delete no longer valid comment
* [hdpowerview] blackout shade position is never UNDEF
* [hdpowerview] updated read me
* [hdpowerview] refactor unit tests into two classes

Signed-off-by: Andrew Fiddian-Green <[email protected]>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
@andrewfg @andrasU
[hdpowerview] Added support for rear blackout panel (openhab#12098)
19139ed 
* [hdpowerview] refactor enum constant names
* [hdpowerview] add support for blackout shades
* [hdpowerview] unit tests for capabilities 8 & 9
* [hdpowerview] delete no longer valid comment
* [hdpowerview] blackout shade position is never UNDEF
* [hdpowerview] updated read me
* [hdpowerview] refactor unit tests into two classes

Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andras Uhrin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur approved these changes

@andylintner andylintner Awaiting requested review from andylintner andylintner is a code owner

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
3.3
Development

Successfully merging this pull request may close these issues.

[hdpowerview] Rear blackout panel position not supported
3 participants
@andrewfg @jlaur @arroyoj