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

[comfoair] Extend UoM support, add semantic tags & update state descriptions #15167

Merged
merged 20 commits into from
Jul 3, 2023

Conversation

boehan
Copy link
Contributor

@boehan boehan commented Jul 1, 2023

This PR provides following improvements to the comfoair binding:

  • add UoM support for fan speed in RPM (ventilation#fanInRPM and ventilation#fanOutRPM)
  • add UoM support for non-hour time-related channels (mainly delay settings, and filter duration)
  • allow DecimalType for QuantityType channels --> this should also avoid breaking changes for channels now supporting UoM
  • add semantic tags for some channels
  • update state descriptions and options for some channels
  • (theoretically) enable support for rfc2217 (not tested and not sure if this is really possible for the RS232 connection)
  • some code improvements

boehan added 19 commits June 30, 2023 16:27
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Hans Böhm <[email protected]>
@boehan
Copy link
Contributor Author

boehan commented Jul 1, 2023

Some things are a bit unclear for me and maybe have to be adapted:

  1. I was not sure if update instructions are required / make sense if only tags or state options are updated. So far I only added them for channels were item-type changed (UoM) or read-only setting was corrected.
  2. AFAICT #3132 should enable bounds in state descriptions for UoM channels, but I was unsure if I should add corresponding units to the min and max values.
    EDIT: if I understand correctly, with above core-PR bounds use the same unit as the pattern. However, that means that I would always have to use a dedicated unit in the pattern instead of %unit%?
  3. I was thinking about also adding UoM support for channels with percentage values, but I see no real benefit being worth the effort as unit conversion doesn't make any sense here. However, if anyone has an argument why UoM should also be made available for percentage channels wherever possible, I will try to add it.

Any recommendations on these are welcome.

Signed-off-by: Hans Böhm <[email protected]>
@jlaur jlaur added the enhancement An enhancement or new feature for an existing add-on label Jul 2, 2023
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.

@jlaur jlaur merged commit 4ddb3ce into openhab:main Jul 3, 2023
@jlaur jlaur added this to the 4.0 milestone Jul 3, 2023
@jlaur jlaur changed the title [comfoair] extend UoM support, add semantic tags & update state descriptions [comfoair] Extend UoM support, add semantic tags & update state descriptions Jul 3, 2023
@boehan
Copy link
Contributor Author

boehan commented Jul 3, 2023

Thanks @jlaur for the fast review and merge.
One thing I forgot to mention: enthalpy#enthalpyTime now represents the set time (in minutes) instead of the internal number value that is sent to the device (minutes / 12). For items newly created as Number:Time this is clear, but for old plain Number items the state is now different.
I'm not sure if this requires / is worth a breaking change note (though I expect this channel to be hardly used)?

Additionally, I forgot to update the changes on read-only channels in the README. I will fix that in a follow-up PR.

@jlaur
Copy link
Contributor

jlaur commented Jul 5, 2023

For items newly created as Number:Time this is clear, but for old plain Number items the state is now different.
I'm not sure if this requires / is worth a breaking change note (though I expect this channel to be hardly used)?

If you can create a small note about this in this section, it would probably be safest - just in case some users would be affected:
https://github.com/openhab/openhab-distro/blob/ca7e75ab4db186ee73dc982af30d3aea2e24fca3/distributions/openhab/src/main/resources/bin/update.lst#L95-L116

markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Jul 8, 2023
…iptions (openhab#15167)

* enable UoM for rpm values
* support uom for non-hour time channels
* add state options for frost setting
* add data type for percentage
* support DecimalType for QuantityType channels
* support rfc2217
* improve state description for time channels
* improve code style in DataTypes
* remove unnecessary command type check
* enable UoM for enthalpy timer

Signed-off-by: Hans Böhm <[email protected]>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
…iptions (openhab#15167)

* enable UoM for rpm values
* support uom for non-hour time channels
* add state options for frost setting
* add data type for percentage
* support DecimalType for QuantityType channels
* support rfc2217
* improve state description for time channels
* improve code style in DataTypes
* remove unnecessary command type check
* enable UoM for enthalpy timer

Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Matt Myers <[email protected]>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
…iptions (openhab#15167)

* enable UoM for rpm values
* support uom for non-hour time channels
* add state options for frost setting
* add data type for percentage
* support DecimalType for QuantityType channels
* support rfc2217
* improve state description for time channels
* improve code style in DataTypes
* remove unnecessary command type check
* enable UoM for enthalpy timer

Signed-off-by: Hans Böhm <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
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