Skip to content

Use correct suffix for elevation#4454

Merged
balloob merged 2 commits into
home-assistant:devfrom
ludeeus:use-correct-system-during-onboarding
Jan 12, 2020
Merged

Use correct suffix for elevation#4454
balloob merged 2 commits into
home-assistant:devfrom
ludeeus:use-correct-system-during-onboarding

Conversation

@ludeeus
Copy link
Copy Markdown
Member

@ludeeus ludeeus commented Jan 12, 2020

Fixes #4452

feet

@ludeeus ludeeus changed the title Use correct suffix for elevation WIP: Use correct suffix for elevation Jan 12, 2020
@ludeeus ludeeus changed the title WIP: Use correct suffix for elevation Use correct suffix for elevation Jan 12, 2020
@balloob balloob merged commit b15270d into home-assistant:dev Jan 12, 2020
@ludeeus ludeeus deleted the use-correct-system-during-onboarding branch January 12, 2020 15:40
@waded
Copy link
Copy Markdown

waded commented Jan 12, 2020

As I understand it the core config is stored in meters. (https://www.home-assistant.io/docs/configuration/basic/) Could someone help me see how this change would not lead to stored config value, as used by other components, being misinterpreted by a factor >3 if entered in feet? Where does conversion happen before it lands in config and is used by components? I looked, but I didn't see anything code like that.

As I mentioned in #4452, I would think you'd need to convert at the UI layer if you want to allow/display input in feet here, given the rest of the stack. It doesn't seem as simple to me as just changing the label, and that isn't what I was asking for.

@bramkragten
Copy link
Copy Markdown
Member

I think he is right, elevation in Home Assistant is always in meters. So it should be converted on save, fetch and change of the unit system to be correct with the label.

@ludeeus ludeeus mentioned this pull request Jan 12, 2020
3 tasks
@lock lock Bot locked and limited conversation to collaborators Jan 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Onboarding should allow elevation input in feet OR meters

5 participants