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.

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

[radoneye] Add decay channel for HW v2 and simplify the code #18125

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

joerg1985
Copy link
Contributor

This PR does fix the randoneye integration and simplifies the code by using the ConnectedBluetoothHandler.
For V2 devices a new channel decay is available showing the number of decays in the last timeframe.

Signed-off-by: Jörg Sautter joerg.sautter@gmx.net

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. Changes look good, it just needs some more:

  1. Thing upgrade instructiosn for the chagned and added channel
  2. Properties file need to be updated (mvn i18n:generate-default-translations)
  3. Optional: would be nice if you use this opportunity to fix some of the compiler null warnigs.

@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Jan 20, 2025
@joerg1985
Copy link
Contributor Author

joerg1985 commented Jan 26, 2025

@lsiepel thanks for the input, i have updated the PR.
Regarding the changes in the orginal channel, the original BECQUEREL_PER_CUBIC_METRE and Units.BECQUEREL_PER_CUBIC_METRE are identical, so there is no change.
The other channel has been added to the instructions.xml, could you please double check the id in the file.
I did read the instructions to the XML format at https://www.openhab.org/docs/developer/bindings/thing-xml.html#updating-thing-types but im still unsure.

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.

I've had a look mostly at the update instructions, and have added a few comments.

The other channel has been added to the instructions.xml, could you please double check the id in the file.
I did read the instructions to the XML format at https://www.openhab.org/docs/developer/bindings/thing-xml.html#updating-thing-types but im still unsure.

It's important to test the update instructions as otherwise they can do more harm than good. You could create a managed Thing with the binding version before this PR (e.g. from the distribution), then install this version and verify that the new channel is added to your Thing automatically, and that the existing channel is updated with the correct dimension.

@jlaur
Copy link
Contributor

jlaur commented Jan 28, 2025

For reference regarding the dimension fix: openhab/openhab-core#3608

Signed-off-by: Jörg Sautter <joerg.sautter@gmx.net>
@joerg1985
Copy link
Contributor Author

I have updated the PR and tested the update, before:
grafik

And after:
grafik

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, LGTM! Since all @lsiepel's remarks have been addressed (and I didn't find anything either), we can merge while he is ⛷️'ing. 🙂

@jlaur jlaur changed the title [radoneye] Fixed the binding for HW v1 and v2 [radoneye] Fix for HW v1 and v2 Jan 30, 2025
@jlaur jlaur changed the title [radoneye] Fix for HW v1 and v2 [radoneye] Add decay channel for HW v2 and simplify the code Jan 30, 2025
@jlaur jlaur merged commit c79fdc5 into openhab:main Jan 30, 2025
2 checks passed
@jlaur jlaur added this to the 5.0 milestone Jan 30, 2025
chilobo pushed a commit to chilobo/openhab-addons that referenced this pull request Feb 10, 2025
Signed-off-by: Jörg Sautter <joerg.sautter@gmx.net>
Signed-off-by: Christian Koch <78686276+chilobo@users.noreply.github.com>
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.

3 participants