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

[sleepiq] Rework cloud API and other enhancements #14180

Merged
merged 7 commits into from
Feb 18, 2023
Merged

[sleepiq] Rework cloud API and other enhancements #14180

merged 7 commits into from
Feb 18, 2023

Conversation

mhilbush
Copy link
Contributor

@mhilbush mhilbush commented Jan 7, 2023 •
edited
Loading

Fixes #11764

This PR replaces the JAX-RS implementation with Jetty due to the unreliability of the binding when calling the SleepIQ API. See the issue description for more details on the issues encountered with the JAX-RS implementation.

This PR also adds new functionality to the binding (also described in the issue description).

There are significant changes to the binding code. The 3rd party library source code and tests have been integrated into the official source code for the binding. The 3rd party library source has ben modified to conform to openHAB standards/style.

The binding has been running very stable for me and several other users for over a year. Unfortunately, my schedule in 2022 prevented me from submitting this PR sooner.

Signed-off-by: Mark Hilbush [email protected]

@mhilbush mhilbush requested a review from syphr42 as a code owner January 7, 2023 20:30
@mhilbush mhilbush added bug An unexpected problem or unintended behavior of an add-on enhancement An enhancement or new feature for an existing add-on labels Jan 7, 2023
@mhilbush mhilbush changed the title Rework cloud API and other enhancements [sleepiq] Rework cloud API and other enhancements Jan 7, 2023
@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/sleepiq-binding-issue-polling-sleep-number-api/85094/72

@jlaur jlaur removed the bug An unexpected problem or unintended behavior of an add-on label Jan 7, 2023
jlaur
jlaur requested changes Feb 15, 2023
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.

Thanks for the improvements made here. I have provided some initial comments, but haven't looked through any of the 3rdparty code yet.

...inding.sleepiq/src/3rdparty/java/org/openhab/binding/sleepiq/api/CommunicationException.java Outdated Show resolved Hide resolved
...inding.sleepiq/src/3rdparty/java/org/openhab/binding/sleepiq/api/CommunicationException.java Outdated Show resolved Hide resolved
...ding.sleepiq/src/main/java/org/openhab/binding/sleepiq/internal/SleepIQBindingConstants.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.sleepiq/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.sleepiq/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
....sleepiq/src/main/java/org/openhab/binding/sleepiq/internal/handler/SleepIQCloudHandler.java Outdated Show resolved Hide resolved
....sleepiq/src/main/java/org/openhab/binding/sleepiq/internal/handler/SleepIQCloudHandler.java Outdated Show resolved Hide resolved
....sleepiq/src/main/java/org/openhab/binding/sleepiq/internal/handler/SleepIQCloudHandler.java Outdated Show resolved Hide resolved
@mhilbush mhilbush mentioned this pull request Feb 15, 2023
Merged
@mhilbush
Copy link
Contributor Author

mhilbush commented Feb 16, 2023 •
edited
Loading

@jlaur I've migrated all the 3rd party source code (including tests) into the official binding code. As part of that, I've (hopefully) taken care of all the license headers, null annotations, Javadocs, author tags, SAT issues, etc.

I'm now moving on to address your other review feedback.

@mhilbush mhilbush force-pushed the sleepiq-rework-to-use-jetty-client branch from b5c9556 to 4599e28 Compare February 16, 2023 12:44
@mhilbush mhilbush requested a review from a team as a code owner February 16, 2023 12:44
@mhilbush
Address review feedback
1333feb 
Signed-off-by: Mark Hilbush <[email protected]>
@mhilbush
Copy link
Contributor Author

mhilbush commented Feb 16, 2023 •
edited
Loading

I've pushed the changes from your review feedback. I believe there's only 1 question outstanding (the removal of the single-chamber bed constants that are currently commented out, and the single-chamber bed thing type in thing-types.xml. Just let me know what you want to do.

Edit: I hope I didn't miss any feedback. With all the changes from moving the 3rd party code, it's possible I might've missed some feedback.

jlaur
jlaur requested changes Feb 16, 2023
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.

Thanks for this massive refactoring! The code already looks really good, so I have provided only a few minor findings.

...g.sleepiq/src/main/java/org/openhab/binding/sleepiq/internal/api/dto/SleepNumberRequest.java Outdated Show resolved Hide resolved
....sleepiq/src/main/java/org/openhab/binding/sleepiq/internal/api/enums/SleepDataInterval.java Outdated Show resolved Hide resolved
....sleepiq/src/main/java/org/openhab/binding/sleepiq/internal/api/enums/SleepDataInterval.java Outdated Show resolved Hide resolved
@mhilbush
Fixes
776c5e7 
Signed-off-by: Mark Hilbush <[email protected]>
@mhilbush
Copy link
Contributor Author

The only thing remaining at this point is the commented constants and thing type for the single-chamber bed? Remove them?

@jlaur
Copy link
Contributor

jlaur commented Feb 16, 2023

@mhilbush - perhaps you could also update the PR description to reflect recent developments. @syphr42 - an approval from you would be the icing on the cake. 🙂

@mhilbush
Copy link
Contributor Author

PR description has been updated.

syphr42
syphr42 approved these changes Feb 18, 2023
View reviewed changes
Copy link

@syphr42 syphr42 left a comment

Choose a reason for hiding this comment

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

I have been away from openHAB dev for awhile, so I can't speak to project standards being followed. However, the code looks good and the improvements over my original implementation make sense.

Great job @mhilbush! Glad to see the binding getting a significant refresh.

jlaur
jlaur approved these changes Feb 18, 2023
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

@jlaur jlaur merged commit 253b256 into openhab:main Feb 18, 2023
@jlaur jlaur added this to the 4.0 milestone Feb 18, 2023
@mhilbush mhilbush deleted the sleepiq-rework-to-use-jetty-client branch February 18, 2023 15:50
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Feb 28, 2023
@mhilbush @nemerdaud
[sleepiq] Rework cloud API and other enhancements (openhab#14180)
9f989af 
Signed-off-by: Mark Hilbush <[email protected]>
renescherer pushed a commit to renescherer/openhab-addons that referenced this pull request Mar 23, 2023
@mhilbush @renescherer
[sleepiq] Rework cloud API and other enhancements (openhab#14180)
e3f4df4 
Signed-off-by: Mark Hilbush <[email protected]>
FordPrfkt pushed a commit to FordPrfkt/openhab-addons that referenced this pull request Apr 20, 2023
@mhilbush @FordPrfkt
[sleepiq] Rework cloud API and other enhancements (openhab#14180)
27bad39 
Signed-off-by: Mark Hilbush <[email protected]>
@mhilbush mhilbush mentioned this pull request Apr 28, 2023
Merged
@lolodomo lolodomo mentioned this pull request May 5, 2023
Closed
29 tasks
@jlaur jlaur mentioned this pull request Jan 1, 2024
Merged
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

@syphr42 syphr42 syphr42 approved these changes

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

Successfully merging this pull request may close these issues.

[sleepiq] Communication with SleepIQ cloud very unreliable
4 participants
@mhilbush @openhab-bot @jlaur @syphr42