-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[govee] Fix invalid status response handling #17048
Conversation
lsiepel
commented
Jul 13, 2024
•
edited
Loading
edited
- Fixes [govee] binding crashes silently if receiving "wrong" values #17041
- Improve logging a bit
- Add some basic tests
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix. I have added a few questions.
...g.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java
Outdated
Show resolved
Hide resolved
...hab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java
Outdated
Show resolved
Hide resolved
...hab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
...org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java
Outdated
Show resolved
Hide resolved
...g.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, hopefully last iteration, found additional minor things that could be improved.
...org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM. @stefan-hoehn, would you like to review this fix?
...hab.binding.govee/src/main/java/org/openhab/binding/govee/internal/CommunicationManager.java
Outdated
Show resolved
Hide resolved
...org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java
Show resolved
Hide resolved
...openhab.binding.govee/src/test/java/org/openhab/binding/govee/internal/GoveeHandlerMock.java
Outdated
Show resolved
Hide resolved
...g.govee/src/test/java/org/openhab/binding/govee/internal/GoveeSerializeGoveeHandlerTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...org.openhab.binding.govee/src/main/java/org/openhab/binding/govee/internal/GoveeHandler.java
Show resolved
Hide resolved
@lsiepel Thanks for jumping in. You probably fixed it much better than I could have ever done it myself 🙏🏻 |
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com> Signed-off-by: Patrik Gfeller <patrik.gfeller@proton.me>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>