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

Default imperial unit for Length changed from inches to feet #3552

Closed
robnielsen opened this issue Apr 14, 2023 · 21 comments · Fixed by #3587
Closed

Default imperial unit for Length changed from inches to feet #3552

robnielsen opened this issue Apr 14, 2023 · 21 comments · Fixed by #3587
Labels
bug An unexpected problem or unintended behavior of the Core

Comments

@robnielsen
Copy link

The default imperial unit for Length is Inches (see https://www.openhab.org/docs/concepts/units-of-measurement.html), but now it's reporting units in feet. It has been inches since openHAB 1.

2023-04-03 17:57:48.828 [INFO ] [openhab.event.ItemStateUpdatedEvent ] - Item 'nRainHour' updated to 0 ft
2023-04-03 17:57:48.835 [INFO ] [openhab.event.ItemStateUpdatedEvent ] - Item 'nRainToday' updated to 0 ft
2023-04-03 17:57:48.837 [INFO ] [openhab.event.ItemStateUpdatedEvent ] - Item 'nRainWeek' updated to 0 ft
2023-04-03 17:57:48.838 [INFO ] [openhab.event.ItemStateUpdatedEvent ] - Item 'nRainMonth' updated to 0.025590551181102362 ft
@robnielsen robnielsen added the bug An unexpected problem or unintended behavior of the Core label Apr 14, 2023
@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-0-milestone-discussion/145133/175

@robnielsen
Copy link
Author

It's raining here now.

2023-04-15 08:31:04.950 [INFO ] [openhab.event.ItemStateUpdatedEvent ] - Item 'nRainHour' updated to 6.561679790026247E-4 ft

@J-N-K
Copy link
Member

J-N-K commented Apr 15, 2023

It's debatable if the bug is in core or in the documentation. When the default units were added in #3143, it was suggested to use feet: #3143 (comment).

@J-N-K J-N-K added enhancement An enhancement or new feature of the Core bug An unexpected problem or unintended behavior of the Core and removed bug An unexpected problem or unintended behavior of the Core enhancement An enhancement or new feature of the Core labels Apr 15, 2023
@robnielsen
Copy link
Author

robnielsen commented Apr 15, 2023

It's definitely a regression from prior behavior (pre 4.0), especially when dealing with devices that report values in mm or less. For example the Netatmo rain gauge has an accuracy of 0.1 mm which is 0.000328084 ft. The common way to measure rain in the US is in inches, and now the log file contains 0.0833333 ft for an inch

Prior to #3143, the value in the log file was 0.00393701 in for 0.1 mm.

@robnielsen
Copy link
Author

Thinking more about this, I can't think of any use cases that would prefer feet over inches for home automation.

@spacemanspiff2007
Copy link
Contributor

@J-N-K
This issue should be gone with the new UoM concept in 4.0, however this once again shows that implicitly picking a default unit and scale (in this case length) will always lead to confusion.
So I am once again begging you:

Please let's not do

Number:Length MyItem

which will currently default to

Number:Length MyItem {unit="ft"}

but instead make the user pick the normalization like this:

Number MyItem {unit="ft"}

@robnielsen would have picked the correct unit intuitively in this case:

Number MyItem {unit="in"}

@J-N-K
Copy link
Member

J-N-K commented Apr 16, 2023

The dimension will not be stripped from the item, if we do that, then UoM improvements will not make it into OH4. It requires too many refactoring also in openhab-addons.

@robnielsen
Copy link
Author

Is there a reason that OH can't use the unit associated with the QuantityType? Netatmo creates a QuantityType with the unit MILLI(SIUnits.METRE) for rain measurements. I would suggest that anything less than SIUnits.METRE should be represented in inches instead of feet. Or the simpler solution, just revert back to the way it used to work with inches.

@robnielsen
Copy link
Author

robnielsen commented Apr 17, 2023

It snowed yesterday, and its starting to melt:

2023-04-17 09:48:43.936 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'nRainHour' changed from 0 ft to 3.2808398950131233E-4 ft
2023-04-17 09:48:43.946 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'nRainToday' changed from 0 ft to 3.2808398950131233E-4 ft
2023-04-17 09:48:43.948 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'nRainWeek' changed from 0 ft to 3.2808398950131233E-4 ft

This is 0.004 in in the Netatmo app, but is 0.001 in basicUI. It should be 0.004

@J-N-K
Copy link
Member

J-N-K commented Apr 17, 2023

@robnielsen The issue with using the attached unit is that we don't know the unit e.g. when restoring data from persistence (because persistence stores only the numeric value, not the unit).

Did you change the state description as suggested in the other issue (%.2f in)?

@robnielsen
Copy link
Author

@J-N-K, yes I could, but the issue is that changing to feet as the default is a breaking change, with no real benefit.

@kaikreuzer
Copy link
Member

@J-N-K & @rkoshak: If this is indeed a regression, does anything speak against changing the default unit from ft to in?
I tend to agree with @robnielsen that for home automation, most relevant lengths are be measured in inches.

@J-N-K
Copy link
Member

J-N-K commented Apr 30, 2023

I don't care, this should be decided by those who use imperial units :-)

@kaikreuzer
Copy link
Member

Fully agree, that's why I also tagged @rkoshak. 😄

@mhilbush
Copy link
Contributor

I also would vote for inches.

@robnielsen
Copy link
Author

I use imperial units and vote for inches.

@rkoshak
Copy link

rkoshak commented Apr 30, 2023

I don't have a strong opinion any way. Back when @J-N-K asked for my opinion on setting the defaults I thought it would make sense to have the defaults somewhat match the metric defaults. Since meters is the default for length and it makes no sense to choose yards as the imperial default I compromised on feet. As has always been the case, if the default isn't right, it can be overridden through the state description (or hopefully the new UoM approach in OH 4).

Ultimately you'll have someone who isn't happy. For weather you want inches. For the milage on you car you want miles. No default is going to work for everyone.

But I wonder if the default for metric should be considered. Maybe cm makes more sense there?

Ultimately I'm more about consistent than anything.

@J-N-K
Copy link
Member

J-N-K commented Apr 30, 2023

If we go back to m/in we are "consistent" in the sense "it has always been that way".

@kaikreuzer
Copy link
Member

That's for me the strongest argument here - backward compatibility.

@J-N-K
Copy link
Member

J-N-K commented May 1, 2023

What about area? It is currently sq. ft., but was not set originally. Shall we change that to sq. in., too?

@mhilbush
Copy link
Contributor

mhilbush commented May 1, 2023

What about area? It is currently sq. ft

Sq ft is the most commonly used unit for area in my experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants