[UoM] Change Dimensionless default unit to percent #4078
Conversation
Signed-off-by: Mark Herwege <[email protected]>
|
You may be able to convince me that my position is wrong, until then I put a veto on this one. Changing the default unit of a dimension is a breaking change which may result in big issues in a lot of systems, especially when it comes to such a fundamental one like I fail to the the big benefit to change the default unit which has been |
I could go on but you get the point. This is the #1 most common problem I deal with on the forum since OH 4 was released. And while it's a breaking change, And the problem is particularly confusing to end users because of the way bindings can pass state description with a unit for display. These users end up having a Number:Dimensionless that appears to be a % everywhere they look until they try to use it in a Chart or use the battery level icon or something. Then all of a sudden the value is two orders of magnitude off and they have no idea why. (Number:Dimensionless isn't the only unit that has this problem BTW). It may have been the default since UoM was first introduced, but with the changes to UoM in OH 4 we introduced a new interaction between the state description and the Item state which is causing no end of problems and confusion to users. Previously, because the State Description was used as the In #3854 binding developers are advocating for introducing a whole new mechanism to bindings to provide hints for what unit to use by default. But the main thing driving them to this is %. For the sake of end users I think we have three choices:
If you veto 1, then I suggest 3 is the only reasonable option, even though it will result in a lot of disruption to binding developers and end users. 2 has already been vetoed and I'm not actually sure it's a viable option anyway, but it's the only way to address this problem in a non-breaking way. The big thing that makes this distinct from #3552 is I would be surprised if there are more than a dozen users among all OH users who are using ONE on purpose. |
|
I like 2, as long as it is a suggestion. Users can decide to follow that (for new items) or not. It doesn't break anything, already existing installations are not affected. I think "automatically set the unit metadata when a link is created" as a core-feature is wrong, it breaks the separation of things and items. |
|
It would be very unexpected if |
What's important is this has to be what gets applied to the Item if the user takes no action. It becomes the default, which was expressly rejected previously. If it's just a suggestion that, if ignored, causes However, it could be implemented at the MainUI level. MainUI already shows the default unit when creating the Item and offers a chance to change that unit prior to creating the Item. If that default becomes whatever the binding is pushing and that default results in This didn't used to be a problem because the binding essentially got to set the unit. It's a problem now because we expressly forbid the binding from setting the unit, but still let them change how the Item's state appears.
I agree. I've disliked this for a long time. But that's how State Description works now. Seems like that would be an even bigger breaking change than changing the default unit of Number:Dimensionless. It definitely would be more disruptive.
And it's equally unexpected for these users who have always had Number:Dimensionless Items which were "defaulting" to |
|
I very much understand the concern with this PR. But I don’t see a perfect solution. Implementing 2 provides a solution for new items defined from thing channels through mainUI. It does not help for users migrating from a previous version, or users using textual configuration. |
This is exactly what I proposed in #3854. |
But that's a migration issue, if we set the default to Imho the root cause is that the users don't have a unit set for a UoM item so that's what we should address. The other issue is a usability issue. These are very small changes which should sufficiently address the issue. Currently the UI already suggests a unit so I don't think we gain much if we introduce a way for the binding to suggest the unit. |
|
FWIW I was wondering what could be the scale of this issue, so I did some grepping on the add-ons source code..
There are probably a few more add-ons (like tado) which don't (yet) return any kind of QuantityType .. but which should probably do so.. EDIT: obviously the Venn diagram intersection of the number of entries in above bullet points is clearly far less than their simple sum.. |
+1
+1
+1 |
That is arbitrary (just like in OH km/h is the default for speed, while the base unit in physics would be m/s). I agree with the principal of always having to set a unit when creating a QuantityType item. I also agree with logging if it is not defined in textual configurations (+ documenting it should always be defined). I don’t think creating artificial dimensions (percent) is the way to go though. I understand why you would want it. However, it makes conversion to Units.ONE impossible (express percent as ratio), and it means all bindings using Dimensionless to express percent should be changed (over 100). You could in theory leave it, but it creates another way to express the same thing. I see a major issue with backward compatibility also. If not changing the default unit (and I understand why one would not want to do it), I do think the unit-hint concept could play a role in the UI to present the most meaningful unit, and bindings could provide that. I don’t see a way to do that without extending the channel or channel type configurations though. |
Just in case you misunderstand my intention I'd like to clarify: |
So, it would only live in mainUI? Textual configuration would always have to be |
What is the difference between missing unit metadata on a managed item and an item defined in an .items file, i.e. why would you only log this for file-based configuration? I primarily use file-based configuration and have some UoM items without explicitly provided unit metadata. Since unit metadata is optional, I would find it quite annoying to have anything logged because of this. For example, I have items of type I also have Also, wouldn't the somehow degrade the regional settings/measurement system? Currently you can have unpersisted items without unit provided, and I believe the configured measurement system will then be taken into consideration when determining default unit? So you can change the unit at once for all these items by simply changing the measurement system. I know this is dangerous for persisted items without unit metadata, in which case it's of course preferable to explicitly define the unit. But at least the user has the freedom to choose. I guess what I'm saying is: If we add such logging, isn't that the first step towards making unit metadata mandatory? I see this as an early warning/reminder before that happens. If we want to keep it optional, it shouldn't cause any warnings to be logged, as this would raise some concerns for perfectly valid configurations, and worse, it would be logged on each system startup, when the user really don't want to provide these units? |
|
Would we then say we only need to force a unit when persisting? It is a totally different change of course, but we might look at forcing to set a unit in the persistence configuration for QuantityTypes. That would require extending textual and UI persistence configuration. The persistence unit would then be by item and persistence service. |
|
Just some thoughts:
|
|
The idea of the log message is not bad at all but of course we should log something only for |
Actually, the real, (and perhaps only), problem is that persistence is wrongly designed. Currently each slot in a persistence database is storing only the float part of the data. Whereas it should be storing the full QuantityType i.e. a float and a unit. If that were the case, a time series of persisted data would always be consistent, even across a change in units. @mherwege suggestion to bolt on a unit to the data series, via the persistence configuration, outside the actual data series, is also a hack. The proper solution is to rework persistence to store and retrieve QuantityTypes in addition to floats. A further nice to have would be a migration tool to convert legacy persistence data streams to QuantityType data streams by manually providing a unit to the conversion tool. |
This is an implementation issue in the concrete persistence services rather than a general design issue. At least I don't think anything prohibits the persistence services from storing unit information as well? Personally I prefer a fixed unit in my MySQL tables so I can utilize the data outside openHAB without having to deal with conversions. But having e.g. a unit column shouldn't cause any problems except storage space (because it would usually be very redundant). |
Just a short note here for completeness: The impact of unit issues is probably most severe for persistence. But it is not the only place. For rules, a user could either:
|
My suggestions was to log it for all items on startup and when loading an *.items file it's only logged for the items defined in the file. Both items defined through gui and file should behave the same.
This is a non-feature because how often do you switch your system from e.g. metric to imperial. And even if you want to do it's still possible by editing all the items, it's just not that convenient.
Do you already think of other aliases? If it's only one or two it'll be fine if there are many this should be rejected.
That would obviously not allowed, that's why I wrote "force to %". Changing the
Some persistence services don't support persisting a unit or strings so that would not work there. |
Totally agree, and I am not saying to store the unit, but to set a unit parameter in the persistence strategy for the item, so it gets converted to the unit before persisting. If the parameter is not set, use the default. The challenge is obviously that persistence is often defined or group (or all) level, so not the item level.
Why is that? An item state for a QuantityType item contains the unit, also in the REST response. Does HABApp drop it by default and assume the default? That negates the whole value of units in my view. All the info is available in the REST responses. |
Almost - HABApp just drops the unit and treats the value as if it would be a value without unit. |
Perhaps use a console command instead of logging? |
But if I have to run a console command I already know that there might be an issue. The intersection of users who know how to run the console command and users who face this issue is probably very small, too. |
Ok. You are right. |
One concern I have is adding another source of confusion similar to what we have for Location. It's an Item and it's a semantic model category. I've had to help more than one user who created Location Items instead of Groups, with the semantic model Location tag and they can't figure out why they can't add Equipment to it. I fully expect users to become confused between a Number:Percent and PercentType.
To truly protect yourself from this scenario, don't use a Number:EnergyPrice Item in the first place and just use a Number Item.
The default unit is chosen based on what the default for that type of unit is for the selected measurement system selected in regional settings. A user who has a Number:Length without unit Whether or not the Item is persisted is irrelevant.
What would need to change for the UI?
I think it is important for more users than you might think.
I believe the suggestion only results in
I can see developers of the car bindings wanting Number:LongDistance (or something like that) to measure length as km or miles instead of meters and feet by default. I'm not sure what the default units for volume are but could see something similar wanted there. Essentially, any time a binding developer hits a situation where they have a lot of Channels not using the default unit, they're going to want an alias to reduce the need of their users setting the
By default I assume you mean whatever unit the Item is configured to carry (i.e.
@J-N-K posted a rule some time ago that will tell you all your Items lacking |
I may be wrong, but looking at openhab/openhab-webui#2302, this only seems to be true for state description. |
That's also my interpretation. Unit metadata is always set when creating an item through the UI. |
|
Closing this for openhab/openhab-webui#2312 While not entirely equivalent (does not solve the issue of users upgrading and having a mismatch between unit and state description), it will give a more meaningful default when creating a Dimensionless Number item through the UI. |
Closes #4077
There has been a lot of debate and confusion about the default unit for QuantityType. There is some consensus that the most used case for Dimensionless is percent, and therefore it would make more sense to have this as the default unit.
This PR is breaking as anyone that is using the current default Unit.ONE in persistence will see there persisted data scale. It will require manually changing the unit for this item to Unit.ONE. The expectation is there are very few cases where Unit.ONE is the right unit.