Skip to content

More ruff and warning cleanup #356

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

More ruff and warning cleanup #356

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dd1fddb
Fix (Zaptec)EntityDescription argument inconsistency
steinmn Oct 7, 2025
1613b52
Fix all ruff errors outside of api.py
steinmn Nov 6, 2025
0abd482
Handle the simplest ruff errors in api.py
steinmn Nov 6, 2025
2f7b7dd
Fix magic number 32, long lines and unnecessary assign before return
steinmn Nov 8, 2025
4700b77
Fix some review comments
steinmn Nov 15, 2025
0c44407
Disable line-too-long globally, since HA core does it
steinmn Nov 15, 2025
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
9 changes: 4 additions & 5 deletions custom_components/zaptec/binary_sensor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
BinarySensorEntityDescription,
)
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from .entity import ZaptecBaseEntity
from .manager import ZaptecConfigEntry
from .manager import ZaptecConfigEntry, ZaptecEntityDescription

_LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -44,13 +43,13 @@ def _post_init(self) -> None:


@dataclass(frozen=True, kw_only=True)
class ZapBinarySensorEntityDescription(BinarySensorEntityDescription):
class ZapBinarySensorEntityDescription(ZaptecEntityDescription, BinarySensorEntityDescription):
Copy link
Collaborator

@sveinse sveinse Nov 9, 2025

Choose a reason for hiding this comment

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

If I'm not mistaken, doesn't this result in a diamond inheritance? Does this have any practical effects in this usage? I've been taught to avoid them like the plague.

Copy link
Contributor Author

@steinmn steinmn Nov 15, 2025

Choose a reason for hiding this comment

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

I'll be honest, I'd never heard of diamond inheritance before. Having said that, is this really any different from
class ZaptecBinarySensor(ZaptecBaseEntity, BinarySensorEntity):
a couple of lines above this? Both inherit from the Entity class (if you dig down a bit). Also, these particular classes only add (non-overlapping) variables, so there's no ambiguity.

This was done to avoid the warning Argument of type "Iterable[EntityDescription]" cannot be assigned to parameter "descriptions" of type "Iterable[ZaptecEntityDescription]" in function "create_entities_from_descriptions". Unsure about the practical effects.

"""Class describing Zaptec binary sensor entities."""

cls: type[BinarySensorEntity]


INSTALLATION_ENTITIES: list[EntityDescription] = [
INSTALLATION_ENTITIES: list[ZaptecEntityDescription] = [
ZapBinarySensorEntityDescription(
key="active",
name="Installation", # Special case, no translation
Expand All @@ -72,7 +71,7 @@ class ZapBinarySensorEntityDescription(BinarySensorEntityDescription):
),
]

CHARGER_ENTITIES: list[EntityDescription] = [
CHARGER_ENTITIES: list[ZaptecEntityDescription] = [
ZapBinarySensorEntityDescription(
key="active",
name="Charger", # Special case, no translation
Expand Down
9 changes: 4 additions & 5 deletions custom_components/zaptec/button.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
from homeassistant.components.button import ButtonEntity, ButtonEntityDescription
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from .entity import ZaptecBaseEntity
from .manager import ZaptecConfigEntry
from .manager import ZaptecConfigEntry, ZaptecEntityDescription
from .zaptec import Charger

_LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -46,15 +45,15 @@ async def async_press(self) -> None:


@dataclass(frozen=True, kw_only=True)
class ZapButtonEntityDescription(ButtonEntityDescription):
class ZapButtonEntityDescription(ZaptecEntityDescription, ButtonEntityDescription):
"""Class describing Zaptec button entities."""

cls: type[ButtonEntity]


INSTALLATION_ENTITIES: list[EntityDescription] = []
INSTALLATION_ENTITIES: list[ZaptecEntityDescription] = []

CHARGER_ENTITIES: list[EntityDescription] = [
CHARGER_ENTITIES: list[ZaptecEntityDescription] = [
ZapButtonEntityDescription(
key="resume_charging",
translation_key="resume_charging",
Expand Down
18 changes: 11 additions & 7 deletions custom_components/zaptec/diagnostics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ async def async_get_config_entry_diagnostics(
return await _get_diagnostics(hass, config_entry)
except Exception:
_LOGGER.exception("Error getting diagnostics")
return {}
Copy link
Collaborator

@sveinse sveinse Nov 9, 2025

Choose a reason for hiding this comment

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

What's correct behavior here? Is it to return nothing like everything is ok, or would a more correct approach be to escalate the error to HA? What does other integrations do?

Copy link
Contributor Author

@steinmn steinmn Nov 15, 2025

Choose a reason for hiding this comment

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

Looking a bit more thoroughly at it, I don't see how we can ever reach this line of code anyway. Pretty much all of _get_diagnostics is try-excepted, so it would have to fail on add_failure I think? Part of me wants to delete the entire try-except here, and the more risk-averse part of me wants to leave it as-is.



async def async_get_device_diagnostics(
Expand All @@ -40,6 +41,7 @@ async def async_get_device_diagnostics(
return await _get_diagnostics(hass, config_entry)
except Exception:
_LOGGER.exception("Error getting diagnostics for device %s", device.id)
return {}


async def _get_diagnostics(
Expand Down Expand Up @@ -80,23 +82,24 @@ def add_failure(err: Exception) -> None:
try:
api = out.setdefault("api", {})

async def request(url: str) -> Any:
async def request(url: str) -> dict | list:
"""Make an API request and return the result."""
try:
result = await zaptec.request(url)
if not isinstance(result, (dict, list)):
return {
"type error": f"Expected dict, got type {type(result).__name__}, value {result}",
"type error": f"Expected dict or list, got type {type(result).__name__}, value {result}", # noqa: E501
}
return result
except Exception as err:
return {
"exception": type(err).__name__,
"err": str(err),
"tb": list(traceback.format_exc().splitlines()),
}
else:
return result
Comment on lines +99 to +100
Copy link
Collaborator

@sveinse sveinse Nov 9, 2025

Choose a reason for hiding this comment

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

Why this change? I find the way it was more readable than this change. Is there a formatting recommendation behind this one?

Copy link
Contributor Author

@steinmn steinmn Nov 9, 2025

Choose a reason for hiding this comment

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

I'm inclined to agree, but https://docs.astral.sh/ruff/rules/try-consider-else/ doesn't, and it does seem to be a convention that is in use in core 🤷


def add(url, obj, ctx=None) -> None:
def add(url: str, obj: dict | list, ctx: str = "") -> None:
api[redact(url, ctx=ctx)] = redact(obj, ctx=ctx)

data = await request(url := "installation")
Expand All @@ -109,8 +112,9 @@ def add(url, obj, ctx=None) -> None:

for circuit in data.get("Circuits", []):
add(f"circuits/{circuit['Id']}", circuit, ctx="circuit")
for charger in circuit.get("Chargers", []):
charger_in_circuits_ids.append(charger["Id"])
charger_in_circuits_ids.extend(
charger["Id"] for charger in circuit.get("Chargers", [])
)

add(url, data, ctx="hierarchy")

Expand All @@ -126,7 +130,7 @@ def add(url, obj, ctx=None) -> None:
add(url, data, ctx="charger")

data = await request(url := f"chargers/{charger_id}/state")
redact.redact_statelist(data, ctx="state")
data = redact.redact_statelist(data, ctx="state")
add(url, data, ctx="state")

except Exception as err:
Expand Down
4 changes: 2 additions & 2 deletions custom_components/zaptec/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ def create_entities_from_descriptions(

def create_entities_from_zaptec(
self,
installation_descriptions: Iterable[EntityDescription],
charger_descriptions: Iterable[EntityDescription],
installation_descriptions: Iterable[ZaptecEntityDescription],
charger_descriptions: Iterable[ZaptecEntityDescription],
) -> list[ZaptecBaseEntity]:
"""Create entities from the present Zaptec objects.

Expand Down
11 changes: 6 additions & 5 deletions custom_components/zaptec/number.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@
from homeassistant.const import EntityCategory
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from .entity import ZaptecBaseEntity
from .manager import ZaptecConfigEntry
from .manager import ZaptecConfigEntry, ZaptecEntityDescription
from .zaptec import Charger, Installation

_LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -108,6 +107,8 @@ def _post_init(self) -> None:
async def async_set_native_value(self, value: float) -> None:
"""Update to Zaptec."""
self._log_number(value)
if not self.entity_description.setting:
raise HomeAssistantError(f"No setting for {self.__class__.__qualname__}.{self.key}")
try:
await self.zaptec_obj.set_settings({self.entity_description.setting: value})
except Exception as exc:
Expand Down Expand Up @@ -144,14 +145,14 @@ async def async_set_native_value(self, value: float) -> None:


@dataclass(frozen=True, kw_only=True)
class ZapNumberEntityDescription(NumberEntityDescription):
class ZapNumberEntityDescription(ZaptecEntityDescription, NumberEntityDescription):
"""Class describing Zaptec number entities."""

cls: type[NumberEntity]
setting: str | None = None


INSTALLATION_ENTITIES: list[EntityDescription] = [
INSTALLATION_ENTITIES: list[ZaptecEntityDescription] = [
ZapNumberEntityDescription(
key="available_current",
translation_key="available_current",
Expand All @@ -174,7 +175,7 @@ class ZapNumberEntityDescription(NumberEntityDescription):
),
]

CHARGER_ENTITIES: list[EntityDescription] = [
CHARGER_ENTITIES: list[ZaptecEntityDescription] = [
ZapNumberEntityDescription(
key="charger_min_current",
translation_key="charger_min_current",
Expand Down
15 changes: 7 additions & 8 deletions custom_components/zaptec/sensor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from dataclasses import dataclass, replace
import logging
from typing import ClassVar
from typing import Final

from homeassistant import const
from homeassistant.components.sensor import (
Expand All @@ -14,11 +14,10 @@
SensorStateClass,
)
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from .entity import ZaptecBaseEntity
from .manager import ZaptecConfigEntry
from .manager import ZaptecConfigEntry, ZaptecEntityDescription
from .zaptec import ZCONST, get_ocmf_max_reader_value

_LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -76,7 +75,7 @@ class ZaptecChargeSensor(ZaptecSensorTranslate):
_log_attribute = "_attr_native_value"

# See ZCONST.charger_operation_modes for possible values
CHARGE_MODE_ICON_MAP: ClassVar[dict[str, str]] = {
CHARGE_MODE_ICON_MAP: Final[dict[str, str]] = {
"unknown": "mdi:help-rhombus-outline",
"disconnected": "mdi:power-plug-off",
"connected_requesting": "mdi:timer-sand",
Expand All @@ -100,7 +99,7 @@ class ZaptecEnengySensor(ZaptecSensor):

_log_attribute = "_attr_native_value"
# This entity use several attributes from Zaptec
_log_zaptec_key = ["signed_meter_value", "completed_session"]
_log_zaptec_key: Final = ["signed_meter_value", "completed_session"]

@callback
def _update_from_zaptec(self) -> None:
Expand Down Expand Up @@ -131,13 +130,13 @@ def _update_from_zaptec(self) -> None:


@dataclass(frozen=True, kw_only=True)
class ZapSensorEntityDescription(SensorEntityDescription):
class ZapSensorEntityDescription(ZaptecEntityDescription, SensorEntityDescription):
"""Provide a description of a Zaptec sensor."""

cls: type[SensorEntity]


INSTALLATION_ENTITIES: list[EntityDescription] = [
INSTALLATION_ENTITIES: list[ZaptecEntityDescription] = [
ZapSensorEntityDescription(
key="available_current_phase1",
translation_key="available_current_phase1",
Expand Down Expand Up @@ -207,7 +206,7 @@ class ZapSensorEntityDescription(SensorEntityDescription):
),
]

CHARGER_ENTITIES: list[EntityDescription] = [
CHARGER_ENTITIES: list[ZaptecEntityDescription] = [
ZapSensorEntityDescription(
key="charger_operation_mode",
translation_key="charger_operation_mode",
Expand Down
6 changes: 4 additions & 2 deletions custom_components/zaptec/services.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
)


async def async_setup_services(hass: HomeAssistant, manager: ZaptecManager) -> None:
async def async_setup_services(hass: HomeAssistant, manager: ZaptecManager) -> None: # noqa: C901 Too complex, will be fixed in https://github.com/custom-components/zaptec/issues/253
"""Set up services for zaptec."""

def get_as_set(service_call: ServiceCall, key: str) -> set[str]:
Expand All @@ -127,7 +127,7 @@ def get_as_set(service_call: ServiceCall, key: str) -> set[str]:

def iter_objects(
service_call: ServiceCall, mustbe: type[T]
) -> Generator[tuple[ZaptecUpdateCoordinator, T], None, None]:
) -> Generator[tuple[ZaptecUpdateCoordinator, T]]:
ent_reg = er.async_get(hass)
dev_reg = dr.async_get(hass)

Expand Down Expand Up @@ -306,6 +306,8 @@ async def service_handle_send_command(service_call: ServiceCall) -> None:
for coordinator, obj in iter_objects(service_call, mustbe=Charger):
_LOGGER.debug(" >> to %s", obj.id)
command = service_call.data.get("command")
if command is None:
raise HomeAssistantError("No Command received")
try:
await obj.command(command)
except Exception as exc:
Expand Down
9 changes: 4 additions & 5 deletions custom_components/zaptec/switch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@
from homeassistant.const import EntityCategory
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from .entity import ZaptecBaseEntity
from .manager import ZaptecConfigEntry
from .manager import ZaptecConfigEntry, ZaptecEntityDescription
from .zaptec import Charger

_LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -126,15 +125,15 @@ async def async_turn_on(self, **kwargs: Any) -> None:


@dataclass(frozen=True, kw_only=True)
class ZapSwitchEntityDescription(SwitchEntityDescription):
class ZapSwitchEntityDescription(ZaptecEntityDescription, SwitchEntityDescription):
"""Class describing Zaptec switch entities."""

cls: type[SwitchEntity]


INSTALLATION_ENTITIES: list[EntityDescription] = []
INSTALLATION_ENTITIES: list[ZaptecEntityDescription] = []

CHARGER_ENTITIES: list[EntityDescription] = [
CHARGER_ENTITIES: list[ZaptecEntityDescription] = [
ZapSwitchEntityDescription(
key="charger_operation_mode",
translation_key="charger_operation_mode",
Expand Down
13 changes: 6 additions & 7 deletions custom_components/zaptec/update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from dataclasses import dataclass
import logging
from typing import Any
from typing import Any, Final

from homeassistant import const
from homeassistant.components.update import (
Expand All @@ -14,11 +14,10 @@
)
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity import EntityDescription
from homeassistant.helpers.entity_platform import AddEntitiesCallback

from .entity import ZaptecBaseEntity
from .manager import ZaptecConfigEntry
from .manager import ZaptecConfigEntry, ZaptecEntityDescription
from .zaptec import Charger

_LOGGER = logging.getLogger(__name__)
Expand All @@ -30,7 +29,7 @@ class ZaptecUpdate(ZaptecBaseEntity, UpdateEntity):
# What to log on entity update
_log_attribute = "_attr_installed_version"
# This entity use several attributes from Zaptec
_log_zaptec_key = ["firmware_current_version", "firmware_available_version"]
_log_zaptec_key: Final = ["firmware_current_version", "firmware_available_version"]
zaptec_obj: Charger

@callback
Expand Down Expand Up @@ -58,15 +57,15 @@ async def async_install(self, version: str | None, backup: bool, **kwargs: Any)


@dataclass(frozen=True, kw_only=True)
class ZapUpdateEntityDescription(UpdateEntityDescription):
class ZapUpdateEntityDescription(ZaptecEntityDescription, UpdateEntityDescription):
"""Class describing Zaptec update entities."""

cls: type[UpdateEntity]


INSTALLATION_ENTITIES: list[EntityDescription] = []
INSTALLATION_ENTITIES: list[ZaptecEntityDescription] = []

CHARGER_ENTITIES: list[EntityDescription] = [
CHARGER_ENTITIES: list[ZaptecEntityDescription] = [
ZapUpdateEntityDescription(
key="firmware_update",
translation_key="firmware_update",
Expand Down
Loading
Loading