Code quality improvements on utility_meter (#129918)

* clean

* update snapshot

* move name, native_value and native_unit_of_measurement to _attr's

* Apply suggestions from code review

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

---------

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>
This commit is contained in:
Diogo Gomes 2024-11-08 23:17:43 +00:00 committed by GitHub
parent 2802b77f21
commit 9f7e6048f8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 103 additions and 164 deletions

View File

@ -379,14 +379,13 @@ class UtilityMeterSensor(RestoreSensor):
self.entity_id = suggested_entity_id self.entity_id = suggested_entity_id
self._parent_meter = parent_meter self._parent_meter = parent_meter
self._sensor_source_id = source_entity self._sensor_source_id = source_entity
self._state = None
self._last_period = Decimal(0) self._last_period = Decimal(0)
self._last_reset = dt_util.utcnow() self._last_reset = dt_util.utcnow()
self._last_valid_state = None self._last_valid_state = None
self._collecting = None self._collecting = None
self._name = name self._attr_name = name
self._input_device_class = None self._input_device_class = None
self._unit_of_measurement = None self._attr_native_unit_of_measurement = None
self._period = meter_type self._period = meter_type
if meter_type is not None: if meter_type is not None:
# For backwards compatibility reasons we convert the period and offset into a cron pattern # For backwards compatibility reasons we convert the period and offset into a cron pattern
@ -419,8 +418,8 @@ class UtilityMeterSensor(RestoreSensor):
def start(self, attributes: Mapping[str, Any]) -> None: def start(self, attributes: Mapping[str, Any]) -> None:
"""Initialize unit and state upon source initial update.""" """Initialize unit and state upon source initial update."""
self._input_device_class = attributes.get(ATTR_DEVICE_CLASS) self._input_device_class = attributes.get(ATTR_DEVICE_CLASS)
self._unit_of_measurement = attributes.get(ATTR_UNIT_OF_MEASUREMENT) self._attr_native_unit_of_measurement = attributes.get(ATTR_UNIT_OF_MEASUREMENT)
self._state = 0 self._attr_native_value = 0
self.async_write_ha_state() self.async_write_ha_state()
@staticmethod @staticmethod
@ -495,13 +494,13 @@ class UtilityMeterSensor(RestoreSensor):
) )
return return
if self._state is None: if self.native_value is None:
# First state update initializes the utility_meter sensors # First state update initializes the utility_meter sensors
for sensor in self.hass.data[DATA_UTILITY][self._parent_meter][ for sensor in self.hass.data[DATA_UTILITY][self._parent_meter][
DATA_TARIFF_SENSORS DATA_TARIFF_SENSORS
]: ]:
sensor.start(new_state_attributes) sensor.start(new_state_attributes)
if self._unit_of_measurement is None: if self.native_unit_of_measurement is None:
_LOGGER.warning( _LOGGER.warning(
"Source sensor %s has no unit of measurement. Please %s", "Source sensor %s has no unit of measurement. Please %s",
self._sensor_source_id, self._sensor_source_id,
@ -512,10 +511,12 @@ class UtilityMeterSensor(RestoreSensor):
adjustment := self.calculate_adjustment(old_state, new_state) adjustment := self.calculate_adjustment(old_state, new_state)
) is not None and (self._sensor_net_consumption or adjustment >= 0): ) is not None and (self._sensor_net_consumption or adjustment >= 0):
# If net_consumption is off, the adjustment must be non-negative # If net_consumption is off, the adjustment must be non-negative
self._state += adjustment # type: ignore[operator] # self._state will be set to by the start function if it is None, therefore it always has a valid Decimal value at this line self._attr_native_value += adjustment # type: ignore[operator] # self._attr_native_value will be set to by the start function if it is None, therefore it always has a valid Decimal value at this line
self._input_device_class = new_state_attributes.get(ATTR_DEVICE_CLASS) self._input_device_class = new_state_attributes.get(ATTR_DEVICE_CLASS)
self._unit_of_measurement = new_state_attributes.get(ATTR_UNIT_OF_MEASUREMENT) self._attr_native_unit_of_measurement = new_state_attributes.get(
ATTR_UNIT_OF_MEASUREMENT
)
self._last_valid_state = new_state_val self._last_valid_state = new_state_val
self.async_write_ha_state() self.async_write_ha_state()
@ -544,7 +545,7 @@ class UtilityMeterSensor(RestoreSensor):
_LOGGER.debug( _LOGGER.debug(
"%s - %s - source <%s>", "%s - %s - source <%s>",
self._name, self.name,
COLLECTING if self._collecting is not None else PAUSED, COLLECTING if self._collecting is not None else PAUSED,
self._sensor_source_id, self._sensor_source_id,
) )
@ -584,14 +585,16 @@ class UtilityMeterSensor(RestoreSensor):
return return
_LOGGER.debug("Reset utility meter <%s>", self.entity_id) _LOGGER.debug("Reset utility meter <%s>", self.entity_id)
self._last_reset = dt_util.utcnow() self._last_reset = dt_util.utcnow()
self._last_period = Decimal(self._state) if self._state else Decimal(0) self._last_period = (
self._state = 0 Decimal(self.native_value) if self.native_value else Decimal(0)
)
self._attr_native_value = 0
self.async_write_ha_state() self.async_write_ha_state()
async def async_calibrate(self, value): async def async_calibrate(self, value):
"""Calibrate the Utility Meter with a given value.""" """Calibrate the Utility Meter with a given value."""
_LOGGER.debug("Calibrate %s = %s type(%s)", self._name, value, type(value)) _LOGGER.debug("Calibrate %s = %s type(%s)", self.name, value, type(value))
self._state = Decimal(str(value)) self._attr_native_value = Decimal(str(value))
self.async_write_ha_state() self.async_write_ha_state()
async def async_added_to_hass(self): async def async_added_to_hass(self):
@ -607,10 +610,11 @@ class UtilityMeterSensor(RestoreSensor):
) )
if (last_sensor_data := await self.async_get_last_sensor_data()) is not None: if (last_sensor_data := await self.async_get_last_sensor_data()) is not None:
# new introduced in 2022.04 self._attr_native_value = last_sensor_data.native_value
self._state = last_sensor_data.native_value
self._input_device_class = last_sensor_data.input_device_class self._input_device_class = last_sensor_data.input_device_class
self._unit_of_measurement = last_sensor_data.native_unit_of_measurement self._attr_native_unit_of_measurement = (
last_sensor_data.native_unit_of_measurement
)
self._last_period = last_sensor_data.last_period self._last_period = last_sensor_data.last_period
self._last_reset = last_sensor_data.last_reset self._last_reset = last_sensor_data.last_reset
self._last_valid_state = last_sensor_data.last_valid_state self._last_valid_state = last_sensor_data.last_valid_state
@ -618,39 +622,6 @@ class UtilityMeterSensor(RestoreSensor):
# Null lambda to allow cancelling the collection on tariff change # Null lambda to allow cancelling the collection on tariff change
self._collecting = lambda: None self._collecting = lambda: None
elif state := await self.async_get_last_state():
# legacy to be removed on 2022.10 (we are keeping this to avoid utility_meter counter losses)
try:
self._state = Decimal(state.state)
except InvalidOperation:
_LOGGER.error(
"Could not restore state <%s>. Resetting utility_meter.%s",
state.state,
self.name,
)
else:
self._unit_of_measurement = state.attributes.get(
ATTR_UNIT_OF_MEASUREMENT
)
self._last_period = (
Decimal(state.attributes[ATTR_LAST_PERIOD])
if state.attributes.get(ATTR_LAST_PERIOD)
and is_number(state.attributes[ATTR_LAST_PERIOD])
else Decimal(0)
)
self._last_valid_state = (
Decimal(state.attributes[ATTR_LAST_VALID_STATE])
if state.attributes.get(ATTR_LAST_VALID_STATE)
and is_number(state.attributes[ATTR_LAST_VALID_STATE])
else None
)
self._last_reset = dt_util.as_utc(
dt_util.parse_datetime(state.attributes.get(ATTR_LAST_RESET))
)
if state.attributes.get(ATTR_STATUS) == COLLECTING:
# Null lambda to allow cancelling the collection on tariff change
self._collecting = lambda: None
@callback @callback
def async_source_tracking(event): def async_source_tracking(event):
"""Wait for source to be ready, then start meter.""" """Wait for source to be ready, then start meter."""
@ -675,7 +646,7 @@ class UtilityMeterSensor(RestoreSensor):
_LOGGER.debug( _LOGGER.debug(
"<%s> collecting %s from %s", "<%s> collecting %s from %s",
self.name, self.name,
self._unit_of_measurement, self.native_unit_of_measurement,
self._sensor_source_id, self._sensor_source_id,
) )
self._collecting = async_track_state_change_event( self._collecting = async_track_state_change_event(
@ -690,22 +661,15 @@ class UtilityMeterSensor(RestoreSensor):
self._collecting() self._collecting()
self._collecting = None self._collecting = None
@property
def name(self):
"""Return the name of the sensor."""
return self._name
@property
def native_value(self):
"""Return the state of the sensor."""
return self._state
@property @property
def device_class(self): def device_class(self):
"""Return the device class of the sensor.""" """Return the device class of the sensor."""
if self._input_device_class is not None: if self._input_device_class is not None:
return self._input_device_class return self._input_device_class
if self._unit_of_measurement in DEVICE_CLASS_UNITS[SensorDeviceClass.ENERGY]: if (
self.native_unit_of_measurement
in DEVICE_CLASS_UNITS[SensorDeviceClass.ENERGY]
):
return SensorDeviceClass.ENERGY return SensorDeviceClass.ENERGY
return None return None
@ -718,11 +682,6 @@ class UtilityMeterSensor(RestoreSensor):
else SensorStateClass.TOTAL_INCREASING else SensorStateClass.TOTAL_INCREASING
) )
@property
def native_unit_of_measurement(self):
"""Return the unit the value is expressed in."""
return self._unit_of_measurement
@property @property
def extra_state_attributes(self): def extra_state_attributes(self):
"""Return the state attributes of the sensor.""" """Return the state attributes of the sensor."""

View File

@ -41,7 +41,17 @@
'status': 'collecting', 'status': 'collecting',
'tariff': 'tariff0', 'tariff': 'tariff0',
}), }),
'last_sensor_data': None, 'last_sensor_data': dict({
'last_period': '0',
'last_reset': '2024-04-05T00:00:00+00:00',
'last_valid_state': 3,
'native_unit_of_measurement': 'kWh',
'native_value': dict({
'__type': "<class 'decimal.Decimal'>",
'decimal_str': '3',
}),
'status': 'collecting',
}),
'name': 'Energy Bill tariff0', 'name': 'Energy Bill tariff0',
'period': 'monthly', 'period': 'monthly',
'source': 'sensor.input1', 'source': 'sensor.input1',
@ -57,7 +67,17 @@
'status': 'paused', 'status': 'paused',
'tariff': 'tariff1', 'tariff': 'tariff1',
}), }),
'last_sensor_data': None, 'last_sensor_data': dict({
'last_period': '0',
'last_reset': '2024-04-05T00:00:00+00:00',
'last_valid_state': 7,
'native_unit_of_measurement': 'kWh',
'native_value': dict({
'__type': "<class 'decimal.Decimal'>",
'decimal_str': '7',
}),
'status': 'paused',
}),
'name': 'Energy Bill tariff1', 'name': 'Energy Bill tariff1',
'period': 'monthly', 'period': 'monthly',
'source': 'sensor.input1', 'source': 'sensor.input1',

View File

@ -91,7 +91,17 @@ async def test_diagnostics(
ATTR_LAST_RESET: last_reset, ATTR_LAST_RESET: last_reset,
}, },
), ),
{}, {
"native_value": {
"__type": "<class 'decimal.Decimal'>",
"decimal_str": "3",
},
"native_unit_of_measurement": "kWh",
"last_reset": last_reset,
"last_period": "0",
"last_valid_state": 3,
"status": "collecting",
},
), ),
( (
State( State(
@ -101,7 +111,17 @@ async def test_diagnostics(
ATTR_LAST_RESET: last_reset, ATTR_LAST_RESET: last_reset,
}, },
), ),
{}, {
"native_value": {
"__type": "<class 'decimal.Decimal'>",
"decimal_str": "7",
},
"native_unit_of_measurement": "kWh",
"last_reset": last_reset,
"last_period": "0",
"last_valid_state": 7,
"status": "paused",
},
), ),
], ],
) )

View File

@ -26,7 +26,6 @@ from homeassistant.components.utility_meter.const import (
) )
from homeassistant.components.utility_meter.sensor import ( from homeassistant.components.utility_meter.sensor import (
ATTR_LAST_RESET, ATTR_LAST_RESET,
ATTR_LAST_VALID_STATE,
ATTR_STATUS, ATTR_STATUS,
COLLECTING, COLLECTING,
PAUSED, PAUSED,
@ -760,64 +759,6 @@ async def test_restore_state(
"status": "paused", "status": "paused",
}, },
), ),
# sensor.energy_bill_tariff2 has missing keys and falls back to
# saved state
(
State(
"sensor.energy_bill_tariff2",
"2.1",
attributes={
ATTR_STATUS: PAUSED,
ATTR_LAST_RESET: last_reset_1,
ATTR_LAST_VALID_STATE: None,
ATTR_UNIT_OF_MEASUREMENT: UnitOfEnergy.MEGA_WATT_HOUR,
},
),
{
"native_value": {
"__type": "<class 'decimal.Decimal'>",
"decimal_str": "2.2",
},
"native_unit_of_measurement": "kWh",
"last_valid_state": "None",
},
),
# sensor.energy_bill_tariff3 has invalid data and falls back to
# saved state
(
State(
"sensor.energy_bill_tariff3",
"3.1",
attributes={
ATTR_STATUS: COLLECTING,
ATTR_LAST_RESET: last_reset_1,
ATTR_LAST_VALID_STATE: None,
ATTR_UNIT_OF_MEASUREMENT: UnitOfEnergy.MEGA_WATT_HOUR,
},
),
{
"native_value": {
"__type": "<class 'decimal.Decimal'>",
"decimal_str": "3f", # Invalid
},
"native_unit_of_measurement": "kWh",
"last_valid_state": "None",
},
),
# No extra saved data, fall back to saved state
(
State(
"sensor.energy_bill_tariff4",
"error",
attributes={
ATTR_STATUS: COLLECTING,
ATTR_LAST_RESET: last_reset_1,
ATTR_LAST_VALID_STATE: None,
ATTR_UNIT_OF_MEASUREMENT: UnitOfEnergy.MEGA_WATT_HOUR,
},
),
{},
),
], ],
) )
@ -852,25 +793,6 @@ async def test_restore_state(
assert state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) == UnitOfEnergy.KILO_WATT_HOUR assert state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) == UnitOfEnergy.KILO_WATT_HOUR
assert state.attributes.get(ATTR_DEVICE_CLASS) == SensorDeviceClass.ENERGY assert state.attributes.get(ATTR_DEVICE_CLASS) == SensorDeviceClass.ENERGY
state = hass.states.get("sensor.energy_bill_tariff2")
assert state.state == "2.1"
assert state.attributes.get("status") == PAUSED
assert state.attributes.get("last_reset") == last_reset_1
assert state.attributes.get("last_valid_state") == "None"
assert state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) == UnitOfEnergy.MEGA_WATT_HOUR
assert state.attributes.get(ATTR_DEVICE_CLASS) == SensorDeviceClass.ENERGY
state = hass.states.get("sensor.energy_bill_tariff3")
assert state.state == "3.1"
assert state.attributes.get("status") == COLLECTING
assert state.attributes.get("last_reset") == last_reset_1
assert state.attributes.get("last_valid_state") == "None"
assert state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) == UnitOfEnergy.MEGA_WATT_HOUR
assert state.attributes.get(ATTR_DEVICE_CLASS) == SensorDeviceClass.ENERGY
state = hass.states.get("sensor.energy_bill_tariff4")
assert state.state == STATE_UNKNOWN
# utility_meter is loaded, now set sensors according to utility_meter: # utility_meter is loaded, now set sensors according to utility_meter:
hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED) hass.bus.async_fire(EVENT_HOMEASSISTANT_STARTED)
@ -882,12 +804,7 @@ async def test_restore_state(
state = hass.states.get("sensor.energy_bill_tariff0") state = hass.states.get("sensor.energy_bill_tariff0")
assert state.attributes.get("status") == COLLECTING assert state.attributes.get("status") == COLLECTING
for entity_id in ( for entity_id in ("sensor.energy_bill_tariff1",):
"sensor.energy_bill_tariff1",
"sensor.energy_bill_tariff2",
"sensor.energy_bill_tariff3",
"sensor.energy_bill_tariff4",
):
state = hass.states.get(entity_id) state = hass.states.get(entity_id)
assert state.attributes.get("status") == PAUSED assert state.attributes.get("status") == PAUSED
@ -939,7 +856,18 @@ async def test_service_reset_no_tariffs(
ATTR_LAST_RESET: last_reset, ATTR_LAST_RESET: last_reset,
}, },
), ),
{}, {
"native_value": {
"__type": "<class 'decimal.Decimal'>",
"decimal_str": "3",
},
"native_unit_of_measurement": "kWh",
"last_reset": last_reset,
"last_period": "0",
"last_valid_state": None,
"status": "collecting",
"input_device_class": "energy",
},
), ),
], ],
) )
@ -1045,21 +973,33 @@ async def test_service_reset_no_tariffs_correct_with_multi(
State( State(
"sensor.energy_bill", "sensor.energy_bill",
"3", "3",
attributes={
ATTR_LAST_RESET: last_reset,
},
), ),
{}, {
"native_value": {
"__type": "<class 'decimal.Decimal'>",
"decimal_str": "3",
},
"native_unit_of_measurement": "kWh",
"last_reset": last_reset,
"last_period": "0",
"status": "collecting",
},
), ),
( (
State( State(
"sensor.water_bill", "sensor.water_bill",
"6", "6",
attributes={
ATTR_LAST_RESET: last_reset,
},
), ),
{}, {
"native_value": {
"__type": "<class 'decimal.Decimal'>",
"decimal_str": "6",
},
"native_unit_of_measurement": "kWh",
"last_reset": last_reset,
"last_period": "0",
"status": "collecting",
},
), ),
], ],
) )