Skip to content

Assign _attr_* fields rather than overriding properties to return custom field values#62

Draft
MarijnS95 wants to merge 5 commits intoniltrip:mainfrom
MarijnS95:sensor-attr
Draft

Assign _attr_* fields rather than overriding properties to return custom field values#62
MarijnS95 wants to merge 5 commits intoniltrip:mainfrom
MarijnS95:sensor-attr

Conversation

@MarijnS95
Copy link

@MarijnS95 MarijnS95 commented Feb 1, 2026

Depends on #60, #61 (but I can rebase this off, depending on the order you want to accept/merge things)

The default property implementations for Entity and SensorEntity return the _attr_* fields. Hence it is unnecessary to craft our own fields only to return them out of overridden property getters; we should instead directly assign them to self._attr_* in __init__().

https://github.com/home-assistant/core/blob/dev/homeassistant/helpers/entity.py
https://github.com/home-assistant/core/blob/dev/homeassistant/components/sensor/__init__.py

@MarijnS95 MarijnS95 changed the title Sensor attr Assign _attr_* fields rather than overriding properties to return custom field values Feb 1, 2026
self._unit = getattr(endpoint, "unit", None)
(
self._attr_device_class,
self._attr_native_unit_of_measurement,
Copy link
Author

Choose a reason for hiding this comment

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

Drive-by fix: afaik SensorEntity shouldn't set unit_of_measurement but native_unit_of_measurement. That way HASS can automatically convert to the unit of preference by the user (i.e. Celsius to Fahrenheit).

Of course, we should check in the JSON (or otherwise) if the native unit that the EcoFlow device returns is always Celsius, but that's strictly for another issue.

@jdammers
Copy link
Collaborator

jdammers commented Feb 4, 2026

@MarijnS95 some minor changes are needed to make Lint / Ruff happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants