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

Add statistics capabilities to hp_ilo integration #65900

Closed
wants to merge 5 commits into from
Closed
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions homeassistant/components/hp_ilo/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@
import hpilo
import voluptuous as vol

from homeassistant.components.sensor import PLATFORM_SCHEMA, SensorEntity
from homeassistant.components.sensor import (
CONF_STATE_CLASS,
DEVICE_CLASSES_SCHEMA,
PLATFORM_SCHEMA,
STATE_CLASSES_SCHEMA,
SensorEntity,
)
from homeassistant.const import (
CONF_DEVICE_CLASS,
CONF_HOST,
CONF_MONITORED_VARIABLES,
CONF_NAME,
Expand Down Expand Up @@ -62,6 +69,8 @@
),
vol.Optional(CONF_UNIT_OF_MEASUREMENT): cv.string,
vol.Optional(CONF_VALUE_TEMPLATE): cv.template,
vol.Optional(CONF_DEVICE_CLASS): DEVICE_CLASSES_SCHEMA,
vol.Optional(CONF_STATE_CLASS): STATE_CLASSES_SCHEMA,
Comment on lines +72 to +73
Copy link
Member

Choose a reason for hiding this comment

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

This is an integration that connects to a device or service. As per ADR-0010.

For integrations that connect to devices or services, we no longer accept new YAML configuration or changes.

This integration needs to be refactored first to use a configuration flow and config entries first.

More information about this can be found in Architecture Decision Record:

See our developer documentation on how to get started creating a configuration flow for this integration:

https://developers.home-assistant.io/docs/config_entries_config_flow_handler

Copy link
Contributor

Choose a reason for hiding this comment

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

@sibalzer FWIW, I started such an config flow, including auto-discovery (but couldn't find the time to wrap it up, i.e. to migrate old config entries and to configure which sensor types shoudl automatically be generated). Feel free to copy from there: https://github.com/chkuendig/hass-hp_ilo-beta

}
)
],
Expand Down Expand Up @@ -103,6 +112,8 @@ def setup_platform(
sensor_type=monitored_variable[CONF_SENSOR_TYPE],
sensor_value_template=monitored_variable.get(CONF_VALUE_TEMPLATE),
unit_of_measurement=monitored_variable.get(CONF_UNIT_OF_MEASUREMENT),
device_class=monitored_variable.get(CONF_DEVICE_CLASS),
state_class=monitored_variable.get(CONF_STATE_CLASS),
)
devices.append(new_device)

Expand All @@ -120,18 +131,31 @@ def __init__(
sensor_name,
sensor_value_template,
unit_of_measurement,
device_class,
state_class,
):
"""Initialize the HP iLO sensor."""
self._hass = hass
self._name = sensor_name
self._unit_of_measurement = unit_of_measurement
self._ilo_function = SENSOR_TYPES[sensor_type][1]
self.hp_ilo_data = hp_ilo_data

if sensor_value_template is not None:
sensor_value_template.hass = hass
self._sensor_value_template = sensor_value_template

if sensor_type == "server_power_readings":
if unit_of_measurement is None:
unit_of_measurement = "W"
if device_class is None:
device_class = "power"
if state_class is None:
state_class = "measurement"
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the rest of the code, I don't think these should be hard-coded.


self._unit_of_measurement = unit_of_measurement
self._device_class = device_class
self._state_class = state_class
Copy link
Contributor

Choose a reason for hiding this comment

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

Use shorthand attributes instead

Suggested change
self._device_class = device_class
self._state_class = state_class
self._attr_device_class = device_class
self._attr_state_class = state_class


self._state = None
self._state_attributes = None

Expand All @@ -142,6 +166,16 @@ def name(self):
"""Return the name of the sensor."""
return self._name

@property
def device_class(self):
"""Return the state class of the sensor."""
return self._device_class

@property
def state_class(self):
"""Return the state class of the sensor."""
return self._state_class

Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed if shorthand attributes are used

Suggested change
@property
def device_class(self):
"""Return the state class of the sensor."""
return self._device_class
@property
def state_class(self):
"""Return the state class of the sensor."""
return self._state_class

@property
def native_unit_of_measurement(self):
"""Return the unit of measurement of the sensor."""
Expand Down