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

[boschshc] Add support for Smoke Detector II #16357

Merged
merged 2 commits into from
Feb 4, 2024
Merged

[boschshc] Add support for Smoke Detector II #16357

merged 2 commits into from
Feb 4, 2024

Conversation

pat-git023
Copy link
Contributor

@pat-git023 pat-git023 commented Feb 3, 2024 •
edited
Loading

Add Smoke Detector 2

Description

This PR adds support of the smoke detector 2 to the Bosch Smart Home Binding (#16243 )

Testing

Your pull request will automatically be built and available under the following folder:
org.openhab.binding.boschshc/4.2.0-SNAPSHOT

@pat-git023
feat: add support for smoke detector 2
3b1b25d 
Signed-off-by: Patrick Gell <patgit023@gmail.com>
@pat-git023
Copy link
Contributor Author

@david-pace
During testing I noticed a little difference for the communication quality level. Even though my signal strength is 3 the item in openHAB shows it as 'good'. From the state enum org.openhab.binding.boschshc.internal.services.communicationquality.dto.CommunicationQualityState I would have excpeted the state to be 'NORMAL'.
Do you know where the mapping for the UI happens?
Screenshot 2024-02-03 at 08 53 48
Screenshot 2024-02-03 at 08 54 08

@jlaur
Copy link
Contributor

jlaur commented Feb 3, 2024

Even though my signal strength is 3 the item in openHAB shows it as 'good'.

Perhaps this can help with some context: #16093 (comment)

@david-pace david-pace mentioned this pull request Feb 3, 2024
Closed
@david-pace
Copy link
Member

david-pace commented Feb 3, 2024 •
edited
Loading

Interestingly the exact same question was asked in #14562 (comment). We use an openHAB system channel and translate the Bosch-specific communication quality to an openHAB-specific signal strength as discussed here.

The mapping is:

UNKNOWN -> 0
BAD -> 1
MEDIUM -> 2
NORMAL -> 3
GOOD -> 4

As you can see, GOOD is the best Bosch communication quality, so it is translated to the best openHAB signal strength 4 which is labeled as "excellent" in the UI. In my opinion this is the best mapping but if you have a better idea feel free to propose a different mapping.

david-pace
david-pace approved these changes Feb 3, 2024
View reviewed changes
Copy link
Member

@david-pace david-pace left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great 😎
Just some things to double-check:

  • did you re-generate the i18n file in the end after having edited everything in thing-types.xml?
  • Did you check that there are no SAT warnings in target/code-analysis/report.html?

.../java/org/openhab/binding/boschshc/internal/devices/smokedetector/SmokeDetector2Handler.java Outdated
}

private void updateChannels(CommunicationQualityServiceState communicationQualityServiceState) {
logger.debug("***** update communication quality with {}", communicationQualityServiceState.quality);
Copy link
Member

Choose a reason for hiding this comment

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

Is this log really needed?
If so, maybe we should remove the asterisks and Capitalize all log messages: Updating communication quality for Smoke Detector II with value {}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

upps, that was just a debug while I did some troubleshooting and used the asterisks to find it faster in the log 😬

will remove the whole log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

log has been removed

@david-pace david-pace added the enhancement An enhancement or new feature for an existing add-on label Feb 3, 2024
@pat-git023
chore: remove debug line
465cba3 
Signed-off-by: Patrick Gell <patgit023@gmail.com>
@pat-git023
Copy link
Contributor Author

@david-pace david-pace changed the title [boschshc] Add support for Smoke Detector 2 [boschshc] Add support for Smoke Detector II Feb 4, 2024
@david-pace david-pace linked an issue Feb 4, 2024 that may be closed by this pull request
[boschshc] Support for Smoke Alarm II #16243
Closed
jlaur
jlaur approved these changes Feb 4, 2024
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 adding support for this new device!

@jlaur jlaur merged commit f45cff8 into openhab:main Feb 4, 2024
3 checks passed
@jlaur jlaur added this to the 4.2 milestone Feb 4, 2024
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@pat-git023 @austvik
[boschshc] Add support for Smoke Detector II (openhab#16357)
0f0d20a 
Signed-off-by: Patrick Gell <patgit023@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@david-pace david-pace david-pace approved these changes

@jlaur jlaur jlaur approved these changes

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

@lsiepel lsiepel Awaiting requested review from lsiepel

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

Successfully merging this pull request may close these issues.

[boschshc] Support for Smoke Alarm II
3 participants
@pat-git023 @jlaur @david-pace