QuantityType.as(PercentType.class) fixes with dimensionless units

[ssalonen]

Resolves #3296

Note: this is not changing behaviour of silly things like QuantityType("0.05 m") converted to PercentType. (changed in 169bd4c)
Note2: this is changing behaviour in backwards-compatible way with dimensionless QuantityTypes

Signed-off-by: Sami Salonen ssalonen@gmail.com

[J-N-K]

I think you should check if the dimension is "dimensionless" and return null if that is not the case.

[ssalonen]

Sure, agree it would sense that non-compatible units (example meter) are disregarded. It would be even more breaking change but then again how many are relying such odd logic

How to check dimension specifically? I have been using the unit conversion to support transformed units (%/10) as well.

[J-N-K]

The more general question is if this should be allowed at all. A value of 120% is perfectly fine for a Number:Dimensionless but is not an allowed value or PercentType, as that is only 0-100%.

[ssalonen]

Yeah it still will respect the range 0%-100%, just taking the unit into account.

Apart from backwards compatibility, I do not see a reason why we would not convert unit accordingly? In a way I find it similar to sending Fahrenheit to Item with Celsius state description

[J-N-K]

You can use UnitUtils.getDimensionName(Unit<?> unit).

My comment was more general regarding QuantityType.as(PercentType.class), not your change. Since PercentType and DecimalType/QuantityType have different use-cases, why would you need a conversion between them?

[ssalonen]

Since PercentType and DecimalType/QuantityType have different use-cases, why would you need a conversion between them?

for what it's worth...

The use case that surfaced this issue came up with modbus binding and scaling of numeric values for Dimmer item.

Modbus binding offers gain/offset profile to scale unitless numbers into QuantityType.

It happens to be that the unit syntax allows very convenient way to scale the data with raw data of modbus:

Consider we have unsigned byte (uint8 in binding terminology) representing light brightness, from 0% to 100%. This could be scaled using "1 %/255" (as opposed to mystic gain of "0.00392156... %").

(sure there are other ways as well, eg coming up with separate script profiles for read/write ops)

---

The fact that PercentType does not inherit its properties from QuantityType feels like pre-UoM design to me actually. After all, both dimensionless and PercentType represent dimensionless value, the other one having only its values limited to a range.

[J-N-K]

Don't get me wrong. If you add the dimension check, I'll merge this one.

But In general I think that this should be re-visted in the light of #3282 and a better UoM Handling in general. For me PercentType and QuantityType are completely different and should not be convertible. Since the data from modbus is e.g. 226, you have to construct a QuantityType first, then scale it and finally convert it to PercentType, what would also have been possible by using 226 / 255 * 100 which is IMO much clearer.

[ssalonen]

Don't get me wrong. If you add the dimension check, I'll merge this one.

Absolutely, I value your input a great deal. I think it is very important the whatever the end decision, things are used in a correct way, and the intent is clear with all these types.

But In general I think that this should be re-visted in the light of #3282 and a better UoM Handling in general.

Agreed.

For example, I just found another nasty UX issue (IMO):

It is not just possible to send state update of "50 %" to DimmerItem (in a way following the analogy of not constructing java types https://community.openhab.org/t/js-item-postupdate-with-quantitytype/130894/2?u=ssalonen)... DimmerItem only accepts PercentType (and ON/OFF), and PercentType cannot be constructed from "50 %".

For me PercentType and QuantityType are completely different and should not be convertible.

Yes I guess this is the crux of of difference in our understanding.

Or to be more precise, I do not have too much of an opinion on the XXXType but instead Items that accept these types.

When working with Items accepting PercentType (e.g. Dimmer or RollerShutter item), I am always hesitating how the number should be represented. Should that number should represent a fraction (value between 0..1) or sort of a percentage value 0...100? If I check it now, I forget it again after one year.

It turns out that when using DecimalType or QuantityType, one should use a fraction when dealing with e.g. Dimmer. So actually your example demonstrates how easy it is to get it wrong,

• 226/255 * 100 (of DecimalType) would try to set Item state to 8862% which is not allowed.
• 226/255 * 100 (of PercentType) would try to set Item state to 88.62% which is allowed.

When I realized I could utilize...well, percents (number with unit %, i.e. QuantityType)... the ambiguity is lost, and intent is clear even when looking at the rule / code / script after 1 year.

Since the data from modbus is e.g. 226, you have to construct a QuantityType first, then scale it and finally convert it to PercentType, what would also have been possible by using 226 / 255 * 100 which is IMO much clearer.

To my knowledge gainOffset profile of modbus binding is the only openHAB profile that provides two-way scaling of numbers with unit, a typical requirement with modbus.

Many modbus devices avoid floating points due to historical reason, and instead scale the numbers such that they are integers (e.g. 226 integer in modbus could represent 22.6 Celsius). In some cases offset is needed to avoid overflowing the small data types that are typical with modbus.

There are other ways to scale things, sure, but then one needs to build the "handler->item" and "item->handler" gains/offsets by hand separately, an error prone task. This is the reason I created the profile in first place, to allow specifying the gain/offsets once, and the right thing will happen with commands (item->handler) as well. It would be really nice for the user if all items dealing with numbers (Dimmer, Rollershutter, Number, Number:dimension) would work with QuantityType since then you do not have to know java classes such as PercentType, QuantityType and DecimalType...

If we would need to distinguish between the different types, would it not mean that we need to add parameters to these "number profiles" to select the "state class" to use?

---

[ssalonen]

UnitUtils.getDimensionName("%/10") seems to return Angle

Actually, the way the helper method has been implemented, makes it unstable. The return value is dependant on field ordering of org.openhab.core.library.unit.Units. By putting public static final Unit<Dimensionless> PERCENT = addUnit(tech.units.indriya.unit.Units.PERCENT); before public static final Unit<Angle> DEGREE_ANGLE = addUnit(NonSI.DEGREE_ANGLE); the same piece of code would return Dimensionless.

---

Let me know what you think, whether this change still makes sense?

[J-N-K]

No, that doesn't make sense. And if I'm not mistaken, toUnit will already return null if the value is incompatible. Maybe you can add a unit test for that: assertNull(QuantityType<>("1 m").as(PercentType.class));

[ssalonen]

And if I'm not mistaken, toUnit will already return null if the value is incompatible.

That is correct. But do note that I kept the original silly behaviour which disregards non-compatible units and just proceeds with the "value" part (toBigDecimal). I can add a test for that

What I tried to communicate above that it seems that there is no reliable way to get Dimension from unit, and in a way that is understandable... in a way information is lost, and hard to make distinction with radian angles and dimensionless bare numbers?

[J-N-K]

Dimensionless is the only one where this is possible. For "regular" units it works fine.

I would prefer to return null if the QuantityType is not Dimensionless (or at least convertible).

[ssalonen]

All right, incompatible units result now in null. See tests.

---

As a separate note for OH4: I got curious why there is asymmetry between constructors and the as conversion method.

For example, one would think that if OnOffType.ON.as(PercentType.class) is allowed, so would PercentType("ON").

String constructor allows one to use string state update in scripts: my_dimmer_item.postUpdate("ON"),

as conversion enables my_dimmer_item.postUpdate(OnOffType.ON) (? I think, looking at ScriptBusEventImpl implementation)

[It is of course entirely another thing that whether this particular OnOff->PercentType kind of conversion should be supported in the first place)

[J-N-K]

LGTM, thanks.