-
-
Notifications
You must be signed in to change notification settings - Fork 428
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
[RfC] Make it possible for bindings to provide unit hints #3854
Comments
This issue has been mentioned on openHAB Community. There might be relevant details there: |
This sounds like a reasonable enhancement. |
I don't think so because this XML is part of the add-on config and, as I understand it, not very dynamic in nature. Maybe the servlet behind the REST API call above could be enhanced to query the binding for the configured unit, assuming the binding knows it? Personally, I'd not have this somewhat edge case hold up the overall approach. I'd split it into two issues. First let's handle the case where the binding absolutely knows from the start which units are appropriate. Then later come back with a new issue/pr and handle the cases like HTTP, MQTT, modbus, etc. where the unit isn't known until the Thing is created and configured. The latter is much more complex and will likely require a brand new API I would think. |
This would indeed be very practical for binding writers like me, in particular for humidity which will never actually change its unit.
But when my binding posts this state: it does get displayed with a % sign, but the value stored in the persistence layer and shown in the log is 0.45 and not 45% Having a unit hint set to % would surely prefill the item with % when created, but this does not help migrating from OH3 where the same code was working properly. |
This issue has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/oh-3-to-oh-4-most-percentage-values-are-now-divided-by-100/153460/12 |
That middle one is not correct. I don't know if that was correct at the time I wrote it or if it was always incorrect. It is true that if you have managed Items, the upgrade tool will add But as currently implemented, if a unit different from the
That's because of the State Description Pattern which can display the value with a different unit, but it no longer controls the unit carried by the Item's state.
That's because the 45% is being converted to the system default
If you had managed Items most of the cases should have been caught and handled by the upgradeTool. If you are using .items files, well you wanted all that control so you'll have to make the changes manually. IIRC the upgradeTool, which runs as part of the upgrade process creates
If 1 or 2 doesn't apply, no And, this was clearly explained with a lengthy description in the list of breaking changes between OH 3 and OH 4. It's not and never was expected to work without any end user changes. It's a breaking change after all. Everyone did their best to make the transition as seamless as possible but there will be cases that require manual intervention. |
Things is, I never defined any unit nor state description pattern on any of my items, which I believe is standard behavior for most users. And as a result, the "percent" ones still have no unit and description pattern. Sorry for the digression on that issue, but I had to get a clarification on the state of things, and I must say, as a binding writer, I would very much welcome the "unit hint" along with a default pattern. Also, you mention this:
It's actually already possible to completely tweak an Thing creation via code by using |
That used to work sometimes and it never was consistent. The State Description Pattern was used to figure out the unit. But it lead to all sorts of side effects, edge cases, etc. I've spent way more time dealing with those than I have dealing with people who don't have a But since OH 4.1, it is standard behavior for most users to define the If you are a .items file users, well, watching the announcements for breaking changes is part of the job you signed up for.
To play devil's advocate (if you look at the original issue you'll see I advocated for a way for binding authors to push a unit to the Item) what do you as a binding author know about what units I as an end user prefer? As a binding author, wouldn't it even be easier if you just pick whatever unit you want to publish, safe in the knowledge that the end user will get that value converted to the unit they want. For example, now there really is no reason for any binding author to mess with discovering the system default units and deciding whether to publish °C or °F (for example). Those of us in the US that use °F will have that as our default unit and the published °C will be converted automatically. The binding author just needs to publish °C or even Kelvin if that's what they want to. All of the confusion on the forum and problems encountered revolve around two different issues:
|
I agree with @rkoshak . Why should a binding author dictate e.g. how my items are persisted. Implicitly applying units from the binding obfuscates how the UoM logic works.
I agree. Even though percent is not unit given how prevalent Percent values are there should have been a |
The point of this issue is not to force a unit on the user but rather suggest one as the default value shown on the MainUI page when creating an item. The current interface would still allow to change it and the way I see it, the suggestion would only be provided by bindings for percentage channels anyway.
Yes, that would have been nice, but I don't see how one would migrate the dimensionless to percent automatically when upgrading to OH4+ |
For managed Items, the upgradetool could handle that fairly easily. Just change the type for any Number:Dimensionless that doesn't have a For text based .items files, well you'll always have to adjust to breaking changes manually anyway. But I don't think a new UoM type is the way to go. I think simply changing the default for The confusion comes from:
2 is particularly pernicious because of the way bindings can push state description patterns. Without the |
And point 2 is exactly the reason why I got "beefed up" at this situation. |
The challenge there was it was a deliberate and definitive decision to not allow bindings to influence the That's a big reason why I keep pushing changing the default unit for |
@obones I second this. Had to support a user with this exact problem just last weekend. seime/openhab-esphome#12 (comment) . |
The one thing being overlooked in all these discussions is that an item can be linked to two or more channels (and things) at the same time. I know this is not very common, but it is possible. So, suppose hints are implemented, which hint from which binding takes precedence? This already leads to issues today with state descriptions as these can be defined on the channel (thing). But that is just a representation thing. It is not predictable which one takes precedence. |
Just to be clear, you mean the UI would suggest |
Yes |
This issue has been mentioned on openHAB Community. There might be relevant details there: |
@jlaur in your OP you suggested two variants for setting the And so I was thinking to suggest a third option -- namely adding a new attribute on its respective
HOWEVER as I was starting to write this post, it occurred to me in a sudden flash, that the existing 'state:pattern' attribute is in fact already the 'unit-hint' that you allude to in the OP. So perhaps it is not necessary to add anything new at all. Or?? |
Except that this state is only for display and the whole issue here is that percentage values, while displayed as 54.3% thanks to the state pattern are stored as 0.543 in the persistence layer, in effect making any new history useless for many migrating users. |
Ok. I get your point -- so my original suggestion to add an attribute to the 'state' element -- therefore -- applies. IMHO extending the 'state' element is more consistent than adding a new element, or adding a new attribute on another element.
|
Please read the whole discussion. An item can be linked to multiple things and channels. If each binding gives different suggestions, this again creates conflict and confusion. I would even argue that the binding defining a state description is a mistake for the same reason, but it is not as bad as defining a unit. Decoupling unit from state description has been the right thing to do, so let’s not introduce it again. I just think that Unit.ONE is not the best default unit for QuantityType:Dimensionless. |
Or the type for the Item might be changed by a profile. This has actually caused some very challenging problems for users.
Since the metadata is passed to the Item at Link time, if I understand correctly, wouldn't it be the Link that was created last that takes precedence?
I would take it one step further and if no Note that it isn't "nothing". Right now the default unit for Number:Dimensionless is ONE which is how the upstream library denotes a basic ratio. This is the only unit that is not represented with the Item, but in all other ways it is treated as a unit.
I have been advocating for making it a best practice in rules to always explicitly declare the units being used and if you have units, don't convert them to raw numbers. This results in more robust and self documenting rules. For example in JS Scripting
as opposed to
While the latter one is the shortest, we've completely thrown away the unit information and are just assuming that the units match. If the Item happens to be carrying °C or K the rule will completely but silently fail. Thanks to the very changes made to UoM that generated this thread, that's less likely to happen than it used to be, but it's still a possibility. The former example will still work, and it's clear in the rule what units you are using. This is one of the main purposes of UoM in the first place. That won't help those who are using the latter approach but frankly, I don't think it's a super bad thing for those users to reconsider their approach. If they don't want to use UoM, then don't use them and change everything to be a plain A lot of bad habits were formed with the slightly broken earlier implementations of UoM and those habits should be broken.
One thing I wish was that it was more clear in MainUI when this actually happens. Right now Items just seem to magically have State Description metadata even though none is defined. And even if you create some manually, it doesn't seem to override all of the State Description. But unless it's in the add-on docs, the end users really have no idea what it is.
The way it's currently implemented I agree. But I also know binding authors really want to control this stuff for the end users.
I'll open a new issue. I think opening a new issue would get more attention and the discussion can be more focused. Or if anyone here has the skills to submit a PR, that would be even better. I suspect the change would be relatively simple. |
Fair enough, if we agree it is not used anywhere else. But I can see the expectation from users coming that linking any existing item also updates this. |
I'm not sure if there are other relevant places, but the idea is only to assist users by providing a better UI default. I do not suggest to break the decoupling in any way or influence the actual default unit for an item when there is no unit configured. |
This issue has been mentioned on openHAB Community. There might be relevant details there: |
@mherwege - many thanks for implementing this proposal. 👍 |
This issue has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/solar-forecast-pv/137681/208 |
#3481 was quite a leap forward in terms of making UoM predictable and reliable.
Now that the smoke has cleared, I see a small opportunity to improve/fine-tune the user experience without changing anything in the UoM logic itself.
A few examples where user experience could improve:
Number:Volume
channel. Default metric unit m³ does not make much sense in this context. In most cases it should be either litres or gallons.Number:Length
channel. Default metric unit m is not ideal. In most cases km would be preferred (similar for imperial).Number:Dimensionless
channel. Default unit one is not ideal, % would be expected.Number:Dimensionless
channel. Default unit one is not ideal, dB would be expected.Since the decision of which unit to use is (and should be) entirely up to the user, my only suggestion would be to add a possibility for bindings to give a hint about recommended units. The only usage of this hint would be for the UI to suggest reasonable default units when creating new items for channels.
In other words, this will not control or override anything, or make the inner UoM logic more complicated in any way. It would exist in parallel as something exposed by the relevant REST methods to be consumed by the UI in order to better assist the user. Additionally, it should be easy/light-weight for bindings to provide such hints, and the bindings should not have to deal with
LocaleProvider
etc. or duplicate any logic that could be implemented in core and webui.The way I see this could be hinted by bindings would be to extend channel and channel-type definitions with an optional new tag. For example:
Or perhaps:
And likewise for channel:
I'm not sure if it makes sense for channels. Perhaps it would be better to require a custom channel type in order to provide such hints, as being able to override at channel level will increase the complexity. I can't think of any use cases since channel types needing specialized hints are probably too generic anyway.
Now, a more complex example - the washing machine:
In this case two units are provided. This is because the hints should be interpreted in the context of the regional settings. So for a US user, gal should be suggested, and for a European user, l should be suggested. I think this distinction is important if we want to get it right. The hint doesn't dictate anything, but in order to be truly helpful, we should still respect those settings, like we also do when item unit is completely missing and must be defaulted.
Calling a method like this:
http://openhab:8080/rest/channel-types/miele%3AwaterConsumption
should then provide this hint (if this can be added while maintaining backwards compatibility):
I'm not sure where the processing of this list of units should happen. Ultimately, for regional settings in Europe, the unit to default would be "l". I'm not into the architecture here, just outlining my proposal as detailed as possible.
See also:
The text was updated successfully, but these errors were encountered: