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

Uses websocket to get sensor device class units #15014

Merged
merged 3 commits into from Jan 12, 2023
Merged

Conversation

piitaya
Copy link
Member

@piitaya piitaya commented Jan 6, 2023

Proposed change

Fetch sensor device class units from core instead of hardcoded ones.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

bramkragten
bramkragten previously approved these changes Jan 6, 2023
@@ -177,6 +126,24 @@ const OVERRIDE_WEATHER_UNITS = {
wind_speed: ["ft/s", "km/h", "kn", "m/s", "mph"],
};

const CONVERTIBLE_UNITS_SENSOR_DEVICE_CLASS = new Set([
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include this in the ws command from core?

export const SENSOR_DEVICE_CLASS_BATTERY = "battery";
export const SENSOR_DEVICE_CLASS_TIMESTAMP = "timestamp";

export type SensorDeviceClassUnits = { units: string[] };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like

Suggested change
export type SensorDeviceClassUnits = { units: string[] };
export type SensorDeviceClassUnits = { units: string[]; convertible: boolean; };

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I suggested in the core PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a plan to make use of this list for things other than unit conversion?
If it's only for unit conversion, then I think it's better to simply return an empty list when it's not convertible, as adjusted in home-assistant/core@9632158.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the endpoint et removed the hardcoded list in frontend

@piitaya piitaya added backend merged The backend PR for this frontend PR has been merged and removed wait for backend labels Jan 12, 2023
@balloob balloob merged commit 2eb5335 into dev Jan 12, 2023
@balloob balloob deleted the sensor_units_ws_api branch January 12, 2023 14:49
@emontnemery emontnemery mentioned this pull request Jan 24, 2023
9 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend merged The backend PR for this frontend PR has been merged cla-signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants