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

status entities for battery calibration & balancing #296

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
40 changes: 38 additions & 2 deletions custom_components/rct_power/lib/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,16 @@
from rctclient.registry import REGISTRY

from .device_info_helpers import get_battery_device_info, get_inverter_device_info
from .entity import EntityUpdatePriority, RctPowerSensorEntityDescription
from .entity import (
EntityUpdatePriority,
RctPowerSensorEntityDescription,
RctPowerBinarySensorEntityDescription,
)
from .state_helpers import (
get_first_api_reponse_value_as_absolute_state,
sum_api_response_values_as_state,
get_battery_calibration_status,
get_battery_balancing_status,
)


Expand Down Expand Up @@ -191,6 +197,31 @@ def get_matching_names(expression: str):
),
]

battery_binary_sensor_entity_descriptions: List[
RctPowerBinarySensorEntityDescription
] = [
RctPowerBinarySensorEntityDescription(
get_device_info=get_battery_device_info,
key="battery.bat_status.calibrating",
object_names=["battery.bat_status"],
name="Battery Calibration Active",
update_priority=EntityUpdatePriority.FREQUENT,
get_native_binary_value=get_battery_calibration_status,
),
RctPowerBinarySensorEntityDescription(
get_device_info=get_battery_device_info,
key="battery.bat_status.balancing",
# 'battery.status2' is not required here, this is just a hack
# so that this entity's generated id doesn't conflict with
# "battery.bat_status.calibrating"
# changing the id generation scheme would break existing installations
object_names=["battery.bat_status", "battery.status2"],
Comment on lines +214 to +218
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can avoid this hack by setting the unique_id. I honestly don't remember why I'm not using the key either. That's what you get when nobody reviews my code 🙈

Suggested change
# 'battery.status2' is not required here, this is just a hack
# so that this entity's generated id doesn't conflict with
# "battery.bat_status.calibrating"
# changing the id generation scheme would break existing installations
object_names=["battery.bat_status", "battery.status2"],
unique_id="battery.bat_status.balancing",
object_names=["battery.bat_status"],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I didn't think that we could just override the use of a generated id by setting it explicitly. Will change this.

name="Battery Balancing Active",
update_priority=EntityUpdatePriority.FREQUENT,
get_native_binary_value=get_battery_balancing_status,
),
]

inverter_sensor_entity_descriptions: List[RctPowerSensorEntityDescription] = [
RctPowerSensorEntityDescription(
get_device_info=get_inverter_device_info,
Expand Down Expand Up @@ -717,4 +748,9 @@ def get_matching_names(expression: str):
*fault_sensor_entity_descriptions,
]

all_entity_descriptions = [*sensor_entity_descriptions]
binary_sensor_entity_descriptions = [*battery_binary_sensor_entity_descriptions]

all_entity_descriptions = [
*sensor_entity_descriptions,
*binary_sensor_entity_descriptions,
]
42 changes: 41 additions & 1 deletion custom_components/rct_power/lib/entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
from typing import Any, Callable, Dict, List, Optional

from homeassistant.components.sensor import SensorEntity, SensorEntityDescription
from homeassistant.components.binary_sensor import (
BinarySensorEntity,
BinarySensorEntityDescription,
)
from homeassistant.config_entries import ConfigEntry
from homeassistant.helpers.entity import DeviceInfo, EntityDescription
from homeassistant.helpers.typing import StateType
Expand All @@ -17,7 +21,10 @@
from .device_class_helpers import guess_device_class_from_unit
from .entry import RctPowerConfigEntryData
from .multi_coordinator_entity import MultiCoordinatorEntity
from .state_helpers import get_first_api_response_value_as_state
from .state_helpers import (
get_first_api_response_value_as_state,
get_first_api_response_value_as_binary_state,
)
from .update_coordinator import RctPowerDataUpdateCoordinator


Expand Down Expand Up @@ -121,6 +128,30 @@ def device_info(self):
return self.entity_description.get_device_info(self)


class RctPowerBinarySensorEntity(BinarySensorEntity, RctPowerEntity):
entity_description: "RctPowerBinarySensorEntityDescription"

@property
def device_class(self):
"""Return the device class of the sensor."""
if device_class := super().device_class:
return device_class

return None

@property
def native_value(self):
values = [
self.get_valid_api_response_value_by_id(object_id, None)
for object_id in self.object_ids
]
return self.entity_description.get_native_binary_value(self, values)

@property
def is_on(self):
return self.native_value


class RctPowerSensorEntity(SensorEntity, RctPowerEntity):
entity_description: "RctPowerSensorEntityDescription"

Expand Down Expand Up @@ -198,6 +229,15 @@ def __post_init__(self):
]


@dataclass
class RctPowerBinarySensorEntityDescription(
RctPowerEntityDescription, BinarySensorEntityDescription
):
get_native_binary_value: Callable[
[RctPowerBinarySensorEntity, list[Optional[ApiResponseValue]]], StateType
] = get_first_api_response_value_as_binary_state


@dataclass
class RctPowerSensorEntityDescription(
RctPowerEntityDescription, SensorEntityDescription
Expand Down
48 changes: 47 additions & 1 deletion custom_components/rct_power/lib/state_helpers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Optional
from typing import Optional, Union

from homeassistant.components.sensor import SensorEntity
from homeassistant.components.binary_sensor import BinarySensorEntity
from homeassistant.helpers.typing import StateType

from .api import ApiResponseValue
Expand Down Expand Up @@ -60,3 +61,48 @@ def sum_api_response_values_as_state(
for value in values
if isinstance(value, (int, float))
)


def get_first_api_response_value_as_binary_state(
entity: BinarySensorEntity,
values: list[Optional[ApiResponseValue]],
) -> Union[None, bool]:
if len(values) <= 0:
return None

return get_api_response_value_as_binary_state(entity=entity, value=values[0])


def get_api_response_value_as_binary_state(
entity: BinarySensorEntity,
value: Optional[ApiResponseValue],
) -> Union[None, bool]:
if value is None:
return None
return bool(value)


def get_battery_calibration_status(
entity: BinarySensorEntity,
values: list[Optional[ApiResponseValue]],
) -> Union[None, bool]:
if len(values) <= 0:
return None
value = values[0]

if isinstance(value, int):
return value & 1032 != 0
weltenwort marked this conversation as resolved.
Show resolved Hide resolved
return None


def get_battery_balancing_status(
entity: BinarySensorEntity,
values: list[Optional[ApiResponseValue]],
) -> Union[None, bool]:
if len(values) <= 0:
return None
value = values[0]

if isinstance(value, int):
return value & 2048 != 0
return None
12 changes: 12 additions & 0 deletions custom_components/rct_power/sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
from .lib.context import RctPowerContext
from .lib.entities import (
battery_sensor_entity_descriptions,
battery_binary_sensor_entity_descriptions,
fault_sensor_entity_descriptions,
inverter_sensor_entity_descriptions,
)
from .lib.entity import (
RctPowerBinarySensorEntity,
RctPowerFaultSensorEntity,
RctPowerSensorEntity,
)
Expand All @@ -35,6 +37,15 @@ async def async_setup_entry(
for entity_description in battery_sensor_entity_descriptions
]

battery_binary_sensor_entities = [
Copy link
Owner

Choose a reason for hiding this comment

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

If I understand HA's component architecture correctly, setting up a binary sensor in sensor.py would not be correct. Instead, it should be set up as part of the binary_sensor platform (meaning in binary_sensor.py), which is being delegated to by the __init__.py's loop over the PLATFORMS list (which would also have to be extended to include binary sensors). In light of that I actually like your alternative suggestion of introducing a single sensor that displays the state using an enum. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH, I'm not fully aware of the component architecture, as I never built an integration myself. But it makes sense, even though it just seems to work in my installation without complying with the convention.
But nevertheless, I also think we should go for the enum alternative. I just thought about this after I made the current implementation.
I will refactor this and update the PR once I tested it.

RctPowerBinarySensorEntity(
coordinators=list(context.update_coordinators.values()),
config_entry=entry,
entity_description=entity_description,
)
for entity_description in battery_binary_sensor_entity_descriptions
]

inverter_sensor_entities = [
RctPowerSensorEntity(
coordinators=list(context.update_coordinators.values()),
Expand All @@ -56,6 +67,7 @@ async def async_setup_entry(
async_add_entities(
[
*battery_sensor_entities,
*battery_binary_sensor_entities,
*inverter_sensor_entities,
*fault_sensor_entities,
]
Expand Down