Skip to content

Conversation

cipianpascu
Copy link
Contributor

No description provided.

Ciprian Pascu added 4 commits September 28, 2025 23:55
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>
@lsiepel
Copy link
Contributor

lsiepel commented Sep 29, 2025

Please divide a proper description

Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
@cipianpascu
Copy link
Contributor Author

Please divide a proper description

hey @lsiepel , not sure what you are looking for but let me give it a try:

  1. the timeout property that I introduced with my previous PR broke the bridge. Now it should work again.
  2. After a bit of thinking, I decided to group the motion, lux (and contact) into one thing. It is better for the long run.

@lsiepel
Copy link
Contributor

lsiepel commented Sep 29, 2025

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

@lsiepel lsiepel requested a review from Copilot September 29, 2025 20:49
Copy link

@Copilot Copilot AI left a 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 and lux-sensor thing types with a unified multi-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.

cipianpascu and others added 4 commits September 30, 2025 20:39
…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>
@cipianpascu
Copy link
Contributor Author

This PR is ready to be merged. Please review it.

Copy link
Contributor

@lsiepel lsiepel 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 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.

Ciprian Pascu added 8 commits October 4, 2025 14:16
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>
Copy link
Contributor

@lsiepel lsiepel left a 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

@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Oct 20, 2025
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
@cipianpascu
Copy link
Contributor Author

just two minor comments, otherwise LGTM

thank you.

Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@lsiepel lsiepel merged commit 2aac5e8 into openhab:main Oct 21, 2025
2 checks passed
@lsiepel lsiepel added this to the 5.1 milestone Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An enhancement or new feature for an existing add-on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants