Ranges (min,max,step) should be provided with a unit

[lsiepel]

A common scenario for a thermostat's is that they have a limited operation range. To support these ranges the xml thing type structure has min, max and step xml attributes. After the introduction of UoM, these attributes where not adapted. Binding's now typically have a state pattern of %unit% and a unitHint set. The min/max/step is hardcoded to a value and it is not known to what unit it relates.

To workaround this, bindings often choose to support the widest range. With that i mean, the min is set to the value corrosponding to the Celsius and max to the Fahrenheit representation of the operating range. It is obvious that this gives issues. as the actual operating range is more limited.

I would propose to add a xml attribute next to min, max, step to specify the unit for that range: "rangeUnit" It's value would be the actual unit just like unitHint. This attribute could be used to determine the min/max/step. This can happen at the same place, just with a little UoM logic added.
If the attribute is missing it would fallback to the current behaviour. So an opt-in, full backwards comptable solution that handles all cases.

Alternatives:
Use unitHint to determine the used unit. Rules out here: openhab/openhab-addons#17638 (comment) (in essence: this is a UI hint and should not be misused for something else)
Use state pattern. This would not cover all bindings as many have it set to %unit%

Related to: #3132
Related to: openhab/openhab-addons#17638 (solving this for just mireds/mireks)

[boehan]

Just my thoughts on this:
Another alternative would be to allow units within the definition of min, max, step. Full state description definition could then, e.g., look like this:
<state min="12 °C" max="30 °C" step="0.5 K" pattern="%.1f %unit%" readOnly="false"></state>

Potential advantages:

• the state description would keep its clean structure (in consistency with pattern) and doesn't need an additional parameter
• it would allow to define different units for min, max and step (may not be needed and probably doesn't make much sense in above example, even with °F instead of °C, but could be useful for wide ranges of max - min compared to step (e.g., min/max on m vs. step in cm)

IMO, changes would be minimal:

• support UoM in parsing of min, max and step instead of decimal only
• fall back to current behavior (or state pattern as proposed in #3132)
  • if no unit is given
  • and/or log an error if given units are not compatible
• allow the definition of units in min, max and step fields in UI (and ideally restrict to compatible units)

[lsiepel]

Just my thoughts on this: Another alternative would be to allow units within the definition of min, max, step. Full state description definition could then, e.g., look like this: <state min="12 °C" max="30 °C" step="0.5 K" pattern="%.1f %unit%" readOnly="false"></state>

• it would allow to define different units for min, max and step (may not be needed and probably doesn't make much sense in above example, even with °F instead of °C, but could be useful for wide ranges of max - min compared to step (e.g., min/max on m vs. step in cm)

Specifying the unit in each element seems very redundant and is error prone. It’s good to have all options on the table tough.

On a side note: having backwards compatibility is key, regardless of the solution. Having a first fallback step to the state pattern unit is breaking.

[andrewfg]

My thoughts..

1. UnitHint cannot be used since it applies at the channel element and not on the state element.
2. Adding UoM to all attributes min/max/step would be a breaking change with unknown consequences.
3. Parsing the state pattern would not always work. Especially if min/max/step are hard coded across continents, and the state is soft coded for continent specific units. (e.g. Celsius vs Fahrenheit)
4. THEREFORE the OP's suggestion to add one single attribute on the state element is by far the best proposal.

[lsiepel]

While experimenting with the code to come up with a PR, I have some questions. I experimente4d with this pattern as part of a temperature channel:

<state min="0.0" max="35.0" step="0.5" pattern="%.1f %unit%"/>

While using file based configuration, i was able to update the state to 50 C (outside of range) from the console and from basicui. In core i could only find a MinMaxValidator that applies to ConfigDescriptionParameter and not to the StateDescription Is there any validation to the min/max value at all?

JFTR: while looking at all this, i also realised there is a min/max/step setting for the config parameters. I was not aiming to provide UoM range validation for those configuration parameters at this moment.

[andrewfg]

^
I think there should be several PRs here. To introduce the attribute, change and upload the xml schema, to pass it to the items, include in the rest api, do the validation etc.

[lsiepel]

^ I think there should be several PRs here. To introduce the attribute, change and upload the xml schema, to pass it to the items, include in the rest api, do the validation etc.

Yes multiple. For now i'm experimenting and want to get a clear picture of how everything ties together. I'm using the core API's but have not yet had the need to looked into the core code in detail, so it is somewhat challenging.

[lsiepel]

Just created a single PR to get early feedback, there is still something to discuss about the validation.

Where do we validate this range?

1. validate before a command is send to the bus. Maybe at EventBus.java
2. validate in the UI (not 100% sure if all UI's currently do, but seems to)
3. validate in the binding's handleCommand method
4. other suggestions?

I think the UI's need to be adapted anyway.