-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[sbus] Fix bridge, lux & motion sensors #19404
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
Conversation
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Please divide a proper description |
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
hey @lsiepel , not sure what you are looking for but let me give it a try:
|
The start post is empty. I was looking for a description about this PR. What enhancement or bug does it fix. Was it tested by you or others, links to background information. Anything that can be helpful for either users that come here from the release notes or maintainers who review the PR |
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.
Pull Request Overview
This PR fixes and consolidates the sensor handling in the SBUS binding by replacing separate motion and lux sensor device types with a unified multi-sensor approach for 9-in-1 devices.
- Replaces individual
motion-sensor
andlux-sensor
thing types with a unifiedmulti-sensor
type - Improves error handling across handlers by adding null-safe error messages
- Adds proper status updates when async messages are processed
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
thing-types.xml | Consolidates motion and lux sensor types into unified multi-sensor type |
SbusHandlerFactory.java | Updates factory to create new Sbus9in1SensorsHandler for multi-sensor devices |
BindingConstants.java | Removes separate sensor constants and adds unified multi-sensor constant |
Sbus9in1SensorsHandler.java | New coordinator handler for 9-in-1 devices that routes messages to specialized handlers |
Sbus9in1ContactHandler.java | New specialized contact handler for 9-in-1 devices |
Multiple handler files | Adds null-safe error messages and status updates for async message processing |
pom.xml | Updates j2sbus dependency to version 1.6.4 |
README.md | Updates documentation to reflect new multi-sensor approach |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...ing.sbus/src/main/java/org/openhab/binding/sbus/internal/handler/Sbus9in1SensorsHandler.java
Outdated
Show resolved
Hide resolved
…ding/sbus/internal/handler/Sbus9in1SensorsHandler.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Ciprian Pascu <ciprianpascu@yahoo.com>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
This PR is ready to be merged. Please review it. |
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 this contribution. This is a partial review mainly for the readme. Will continue later on. The readme should be focused on the openHAB model. For instance the channel sections talk about traditional and other sensors. It would be better to stick to the thing type contact-sensor
Similar remarks in the same file.
Another question, this does seem to be a breaking change. As existing things are removed, an upgrade notice should be created.
bundles/org.openhab.binding.sbus/src/main/resources/OH-INF/i18n/sbus.properties
Show resolved
Hide resolved
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
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.
just two minor comments, otherwise LGTM
...ing.sbus/src/main/java/org/openhab/binding/sbus/internal/handler/Sbus9in1ContactHandler.java
Outdated
Show resolved
Hide resolved
...nding.sbus/src/main/java/org/openhab/binding/sbus/internal/handler/SbusLuxSensorHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
thank you. |
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, LGTM
No description provided.