Skip to content

Cleanup: minor issues across secondary files #237

Description

@uspass

Cleanup: minor issues across secondary files

While working through the fixes for the previously reported bugs, I reviewed the
remaining files in the component. The issues below are minor but I reckon they're worth tidying up.
Please bear with my perfectionism.
Hope this also helps.

Affected version: 2025.12.0 (master)


Issue 1 — binary_sensor.py / number.py: NUMBER_ENTITIES not updated for new zones after reload

File: binary_sensor.py line 107, number.py lines 40, 48

Description

NUMBER_ENTITIES (the dict mapping unique IDs to IURunDuration entities) is
populated in number.async_setup_platform and number.async_setup_entry, but
async_reload_platform in binary_sensor.py never updates it.

The impact is narrow: existing zones and sequences are unaffected because the number
platform is not reloaded, so its entities remain alive and NUMBER_ENTITIES continues
to point to the same objects with any user-customized duration values intact. The
problem only affects zones and sequences newly added in YAML after a reload: a new
IUZoneRunButton or IUSequenceRunButton looks up its duration entity by unique_id
in NUMBER_ENTITIES, finds nothing, and silently falls back to the 10-minute default
regardless of what is configured.

Before

# binary_sensor.py  async_reload_platform — NUMBER_ENTITIES not updated for new entities
async def async_reload_platform(
    component: EntityComponent, coordinator: IUCoordinator
) -> bool:
    ...
    for entity in old_entities:
        await platform.async_remove_entity(entity)
    if len(new_entities) > 0:
        await platform.async_add_entities(new_entities)
        coordinator.initialise()

    return True
    # Newly added zones/sequences have no entry in NUMBER_ENTITIES →
    # their run buttons always use the 10-minute default duration

After

# binary_sensor.py  async_reload_platform
# Add to existing imports:
#   from .const import NUMBER, NUMBER_ENTITIES
#   from .number import IUZoneRunDuration, IUSequenceRunDuration

async def async_reload_platform(
    component: EntityComponent, coordinator: IUCoordinator
) -> bool:
    ...
    for entity in old_entities:
        await platform.async_remove_entity(entity)
    if len(new_entities) > 0:
        await platform.async_add_entities(new_entities)
        coordinator.initialise()

    # Add number entities for newly added zones/sequences only.
    # Existing entries are kept as-is, preserving any user-set duration values.
    number_platform: EntityPlatform = find_platform(component.hass, NUMBER)
    if number_platform is not None:
        number_entities = component.hass.data[DOMAIN].get(NUMBER_ENTITIES, {})
        new_number_entities = []
        for controller in coordinator.controllers:
            for zone in controller.zones:
                uid = f"{zone.unique_id}_run_duration"
                if uid not in number_entities:
                    entity = IUZoneRunDuration(coordinator, controller, zone)
                    number_entities[uid] = entity
                    new_number_entities.append(entity)
            for sequence in controller.sequences:
                uid = f"{sequence.unique_id}_run_duration"
                if uid not in number_entities:
                    entity = IUSequenceRunDuration(coordinator, controller, sequence)
                    number_entities[uid] = entity
                    new_number_entities.append(entity)
        if new_number_entities:
            await number_platform.async_add_entities(new_number_entities)

    return True

Issue 2 — service.py: @callback applied to all async def handlers

Lines: 59, 108, 131, 143, 189, 206

Description

All six service handler functions in service.py are decorated with @callback but
defined as async def coroutines. In Home Assistant, @callback marks a function as
a synchronous callback that must not schedule coroutines or yield to the event
loop. Applying it to a coroutine is contradictory.

In practice, HA's service infrastructure calls asyncio.iscoroutinefunction() — which
is unaffected by @callback — to decide whether to await the handler. All six
functions are therefore awaited correctly and there is no runtime failure. However, the
decorator is misleading and signals incorrect intent.

Before

# service.py  line 59
@callback                                           # incorrect: marks function as synchronous
async def async_entity_service_handler(
    entity: IUEntity, call: ServiceCall
) -> ServiceResponse:
    return entity.dispatch(call.service, call)

# service.py  lines 108, 131, 143, 189, 206 — same pattern on all inner handlers
@callback
async def reload_service_handler(call: ServiceCall) -> None:
    ...

@callback
async def coordinator_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

@callback
async def get_info_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

@callback
async def get_status_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

@callback
async def export_config_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

After

# Remove @callback from all six async handlers
async def async_entity_service_handler(
    entity: IUEntity, call: ServiceCall
) -> ServiceResponse:
    return entity.dispatch(call.service, call)

async def reload_service_handler(call: ServiceCall) -> None:
    ...

async def coordinator_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

async def get_info_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

async def get_status_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

async def export_config_service_handler(call: ServiceCall) -> ServiceResponse:
    ...

Issue 3 — schema.py: ENTITY_SCHEMA uses singular cv.entity_id for SERVICE_SKIP

Line: 402 (schema.py), line 91 (service.py)

Description

SERVICE_SKIP is registered with ENTITY_SCHEMA, which validates entity_id using
cv.entity_id (accepts a single string). Every other platform service uses
cv.entity_ids (accepts a list), allowing multiple entities to be targeted in a single
service call. skip is the only service that cannot do so.

Before

# schema.py  line 402
ENTITY_SCHEMA = {vol.Required(CONF_ENTITY_ID): cv.entity_id}    # singular

# service.py — all platform service registrations
platform.async_register_entity_service(SERVICE_ENABLE,      ENABLE_DISABLE_SCHEMA, ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_DISABLE,     ENABLE_DISABLE_SCHEMA, ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_TOGGLE,      ENABLE_DISABLE_SCHEMA, ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_CANCEL,      CANCEL_SCHEMA,         ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_TIME_ADJUST, TIME_ADJUST_SCHEMA,    ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_MANUAL_RUN,  MANUAL_RUN_SCHEMA,     ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_SUSPEND,     SUSPEND_SCHEMA,        ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_SKIP,        ENTITY_SCHEMA,         ...)  # cv.entity_id ← outlier
platform.async_register_entity_service(SERVICE_PAUSE,       PAUSE_RESUME_SCHEMA,   ...)  # cv.entity_ids
platform.async_register_entity_service(SERVICE_RESUME,      PAUSE_RESUME_SCHEMA,   ...)  # cv.entity_ids

After

# schema.py  line 402 — replace ENTITY_SCHEMA usage with a dedicated schema
SKIP_SCHEMA = {
    vol.Required(CONF_ENTITY_ID): cv.entity_ids,    # plural — consistent with all others
}

# service.py  lines 17–27 — replace ENTITY_SCHEMA with SKIP_SCHEMA in the import block
from .schema import (
    CANCEL_SCHEMA,
    ENABLE_DISABLE_SCHEMA,
    SKIP_SCHEMA,             # ← added
    # ENTITY_SCHEMA,         # ← removed: no longer used
    GET_STATUS_SCHEMA,
    ...
)

# service.py  line 91
platform.async_register_entity_service(SERVICE_SKIP, SKIP_SCHEMA, async_entity_service_handler)

Cheers,
us

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions