diff --git a/.github/workflows/release-versioning.yml b/.github/workflows/release-versioning.yml index ff3b79f..30e399b 100644 --- a/.github/workflows/release-versioning.yml +++ b/.github/workflows/release-versioning.yml @@ -16,12 +16,12 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 - name: Setup Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: '3.x' @@ -47,10 +47,10 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Create GitHub Release from tag - uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2 with: tag_name: ${{ github.event.release.tag_name }} env: diff --git a/README.md b/README.md index d5e24f6..678091d 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Simple PID Controller -> A Home Assistant integration for real-time PID control with UI-based tuning and diagnostics. +> The Simple PID Controller is a Home Assistant integration for real-time PID control with UI-based tuning and diagnostics. --- @@ -9,13 +9,17 @@ - [Installation](#installation) - [HACS (Recommended)](#hacs-recommended) - [Manual Installation](#manual-installation) + - [Removal Instructions](#removal-instructions) - [Configuration](#configuration) - [Entities Overview](#entities-overview) - [PID Tuning Guide](#pid-tuning-guide) - [Manual Tuning](#1-manual-trial--error) - [Ziegler–Nichols Method](#2-zieglernichols-method) +- [PID Calculation Frequency and Sample Time](#pid-calculation-frequency-and-sample-time) - [Example PID Graph](#example-pid-graph) - [Support & Development](#support--development) +- [Service Actions](#service-actions) + --- @@ -40,6 +44,8 @@ | Number | `Sample Time` | PID evaluation interval | | Switch | `Auto Mode` | Toggle automatic control | | Switch | `Proportional on Measurement` | Change proportional mode | +| Switch | `Windup Protection` | Toggle windup protection | + > 💡 All entities are editable via the UI in **Settings > Devices & Services > [Your Controller] > Options**. @@ -63,22 +69,15 @@ 1. Download or clone this repository 2. Copy `simple_pid_controller` to `/config/custom_components/` -3. Ensure this folder structure: - ```text - config/ - └── custom_components/ - └── simple_pid_controller/ - ├── __init__.py - ├── manifest.json - ├── sensor.py - ├── number.py - └── switch.py - ``` -4. Restart Home Assistant +3. Restart Home Assistant + +### Removal Instructions +To remove the Simple PID Controller, navigate to **Settings > Devices & Services**, select **Simple PID Controller**, and click **Delete**. If installed manually, delete the `custom_components/simple_pid_controller` directory and restart Home Assistant. --- ## ⚙️ Configuration +The controller is configured through the UI using the Config Flow {% term config_flow %}. 1. Go to **Settings > Devices & Services** 2. Click **Add Integration** and choose **Simple PID Controller** @@ -87,6 +86,9 @@ - **Sensor Entity**: e.g., `sensor.living_room_temperature` 4. Submit and finish setup +**Default Range:** +The controller’s setpoint range defaults to **0.0 – 100.0**. To customize this range, select the integration in **Settings > Devices & Services**, click **Options**, adjust **Range Min** and **Range Max**, and save. + --- ## 📊 Entities Overview @@ -101,6 +103,7 @@ | Number | `Sample Time` | PID evaluation rate in seconds. | | Switch | `Auto Mode` | Enable/disable PID automation. | | Switch | `Proportional on Measurement` | Use measurement instead of error for P term. | +| Switch | `Windup Protection` | Toggle windup protection | --- @@ -138,6 +141,45 @@ A PID controller continuously corrects the difference between a **setpoint** and +--- + +## PID Calculation Frequency and Sample Time + +This integration recalculates the PID output at a fixed, user-configurable interval to ensure timely and consistent control updates. Internally, both the PID loop and Home Assistant’s data coordinator use the same **Sample Time**, but they each maintain their own timer, so slight differences can occur. + +- **Sample Time** + The minimum number of seconds between successive PID computations. For example, if you set `sample_time: 5`, both the PID controller and the update coordinator are scheduled to fire every 5 seconds. + +### How it works in practice + +1. **Initialization** + - On startup (or when options change), we set up a single `sample_time` value (in seconds). + - We register a periodic callback with Home Assistant’s scheduler (`async_track_time_interval` or `DataUpdateCoordinator`) using that same `sample_time`. + - We also configure the PID algorithm’s internal timer to the same `sample_time`. + +2. **Coordinator Tick** + - Every `sample_time` seconds, Home Assistant’s scheduler invokes our update method. + - We immediately read the current process variable (e.g. temperature sensor) and pass it to the PID logic. + +3. **PID Logic & Output** + - The PID algorithm checks whether at least `sample_time` seconds have elapsed since its own last computation. + - If so, it calculates the Proportional, Integral, and Derivative terms and writes the result to your target entity (e.g. a heater or set-point). + - If not (because the PID’s internal timer hasn’t quite reached the next tick), it skips the computation until its own timer allows it. + +4. **Timer Drift & Overlap** + - Both the coordinator and the PID controller schedule their next run relative to when the current one started. Under heavy load, one callback may run a few milliseconds later than expected. + - Because each timer is independent, occasional “double-ticks” or small gaps can occur: + - If the scheduler drifts early but the PID timer hasn’t yet reached `sample_time`, no computation runs. + - If the PID timer elapses first and the scheduler callback is slightly late, the update happens immediately when the scheduler finally fires. + - Over time, these small variances average out, preserving an approximately consistent interval. + +5. **Adjusting Sample Time** + - Changing `sample_time` in your integration options takes effect at the end of the current interval—no Home Assistant restart is required. + - On the next tick, both the coordinator and the PID logic will use the new interval. + +By using a single **Sample Time** for both scheduling and calculation—and understanding that each component tracks its own clock—you get predictable, evenly-spaced control updates while allowing Home Assistant’s event loop to manage timing drifts gracefully.``` + + --- ## 📈 Example PID Graph @@ -153,3 +195,9 @@ Here's an example output showing the controller responding to a setpoint: - **GitHub Repository**: [https://github.com/bvweerd/simple_pid_controller](https://github.com/bvweerd/simple_pid_controller) - **Issues & Bugs**: [Report here](https://github.com/bvweerd/simple_pid_controller/issues) +--- + +## 🔧 Service Actions +This Integration does **not** expose any custom services. All interactions are performed via UI-based entities. + + diff --git a/custom_components/simple_pid_controller/__init__.py b/custom_components/simple_pid_controller/__init__.py index ad53667..abd6091 100644 --- a/custom_components/simple_pid_controller/__init__.py +++ b/custom_components/simple_pid_controller/__init__.py @@ -8,14 +8,30 @@ from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import entity_registry as er - -from .const import DOMAIN, CONF_NAME, CONF_SENSOR_ENTITY_ID +from dataclasses import dataclass +from .coordinator import PIDDataCoordinator + +from .const import ( + DOMAIN, + CONF_NAME, + CONF_SENSOR_ENTITY_ID, + CONF_RANGE_MIN, + CONF_RANGE_MAX, + DEFAULT_RANGE_MIN, + DEFAULT_RANGE_MAX, +) _LOGGER = logging.getLogger(__name__) PLATFORMS: list[Platform] = [Platform.SENSOR, Platform.NUMBER, Platform.SWITCH] +@dataclass +class MyData: + handle: PIDDeviceHandle + coordinator: PIDDataCoordinator = None + + class PIDDeviceHandle: """Shared device handle for a PID controller config entry.""" @@ -23,6 +39,12 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry) -> None: self.hass = hass self.entry = entry self.name = entry.data.get(CONF_NAME) + self.range_min = entry.options.get( + CONF_RANGE_MIN, entry.data.get(CONF_RANGE_MIN, DEFAULT_RANGE_MIN) + ) + self.range_max = entry.options.get( + CONF_RANGE_MAX, entry.data.get(CONF_RANGE_MAX, DEFAULT_RANGE_MAX) + ) self.sensor_entity_id = entry.options.get( CONF_SENSOR_ENTITY_ID, entry.data.get(CONF_SENSOR_ENTITY_ID) ) @@ -72,7 +94,7 @@ def get_input_sensor_value(self) -> float | None: return float(state.state) except ValueError: _LOGGER.warning( - f"Sensor {self.sensor_entity_id} heeft geen geldige waarde. PID-berekening wordt overgeslagen." + f"Sensor {self.sensor_entity_id} invalid value. PID-calculation skipped." ) return None @@ -89,14 +111,25 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: raise ConfigEntryNotReady(f"Sensor {sensor_entity_id} not ready") handle = PIDDeviceHandle(hass, entry) - hass.data.setdefault(DOMAIN, {})[entry.entry_id] = handle + entry.runtime_data = MyData(handle=handle) + + # register updatelistener for optionsflow + entry.async_on_unload(entry.add_update_listener(_async_update_options_listener)) await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS) return True async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: - """Unload PID Controller entry.""" - if DOMAIN in hass.data and entry.entry_id in hass.data[DOMAIN]: - hass.data[DOMAIN].pop(entry.entry_id) - return await hass.config_entries.async_unload_platforms(entry, PLATFORMS) + """Unload a config entry.""" + if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS): + # reset runtime_data zodat tests slagen + entry.runtime_data = None + return unload_ok + + +async def _async_update_options_listener( + hass: HomeAssistant, entry: ConfigEntry +) -> None: + """Update after options are changed in optionsflow""" + await hass.config_entries.async_reload(entry.entry_id) diff --git a/custom_components/simple_pid_controller/config_flow.py b/custom_components/simple_pid_controller/config_flow.py index 349031e..899a446 100644 --- a/custom_components/simple_pid_controller/config_flow.py +++ b/custom_components/simple_pid_controller/config_flow.py @@ -16,7 +16,16 @@ from homeassistant.core import callback from homeassistant.helpers.selector import selector -from .const import DOMAIN, CONF_NAME, CONF_SENSOR_ENTITY_ID +from .const import ( + DOMAIN, + CONF_NAME, + DEFAULT_NAME, + CONF_SENSOR_ENTITY_ID, + CONF_RANGE_MIN, + CONF_RANGE_MAX, + DEFAULT_RANGE_MIN, + DEFAULT_RANGE_MAX, +) _LOGGER = logging.getLogger(__name__) @@ -32,61 +41,103 @@ def async_get_options_flow( config_entry: ConfigEntry, ) -> PIDControllerOptionsFlowHandler: """Get the options flow for this handler.""" - return PIDControllerOptionsFlowHandler(config_entry) + return PIDControllerOptionsFlowHandler() async def async_step_user( self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Handle the initial step.""" + + schema = vol.Schema( + { + vol.Required(CONF_NAME, default=DEFAULT_NAME): str, + vol.Required(CONF_SENSOR_ENTITY_ID): selector( + {"entity": {"domain": "sensor"}} + ), + vol.Optional(CONF_RANGE_MIN, default=DEFAULT_RANGE_MIN): vol.Coerce( + float + ), + vol.Optional(CONF_RANGE_MAX, default=DEFAULT_RANGE_MAX): vol.Coerce( + float + ), + } + ) + if user_input is not None: + self._async_abort_entries_match({CONF_NAME: user_input[CONF_NAME]}) + + # Validate that range_min < range_max + min_val = user_input.get(CONF_RANGE_MIN) + max_val = user_input.get(CONF_RANGE_MAX) + if min_val is not None and max_val is not None and min_val >= max_val: + return self.async_show_form( + step_id="user", data_schema=schema, errors={"base": "range_min_max"} + ) + return self.async_create_entry( title=user_input[CONF_NAME], data={ CONF_NAME: user_input[CONF_NAME], CONF_SENSOR_ENTITY_ID: user_input[CONF_SENSOR_ENTITY_ID], + CONF_RANGE_MIN: user_input[CONF_RANGE_MIN], + CONF_RANGE_MAX: user_input[CONF_RANGE_MAX], }, ) - return self.async_show_form( - step_id="user", - data_schema=vol.Schema( - { - vol.Required( - CONF_NAME, default=self.hass.config.location_name - ): str, - vol.Required(CONF_SENSOR_ENTITY_ID): selector( - {"entity": {"domain": "sensor"}} - ), - } - ), - ) + return self.async_show_form(step_id="user", data_schema=schema) class PIDControllerOptionsFlowHandler(OptionsFlow): """Handle options for PID Controller.""" - def __init__(self, config_entry: ConfigEntry) -> None: + def __init__(self) -> None: """Initialize options flow.""" - self.config_entry = config_entry + super().__init__() async def async_step_init( self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: - """Manage the options.""" - if user_input is not None: - return self.async_create_entry(title="", data=user_input) + """Manage the options form and save user input.""" + # Pre-fill form with existing options or sensible defaults current_sensor = self.config_entry.options.get( - CONF_SENSOR_ENTITY_ID, - self.config_entry.data.get(CONF_SENSOR_ENTITY_ID, ""), - ) + CONF_SENSOR_ENTITY_ID + ) or self.config_entry.data.get(CONF_SENSOR_ENTITY_ID) + current_min = self.config_entry.options.get(CONF_RANGE_MIN, DEFAULT_RANGE_MIN) + current_max = self.config_entry.options.get(CONF_RANGE_MAX, DEFAULT_RANGE_MAX) options_schema = vol.Schema( { - vol.Required(CONF_SENSOR_ENTITY_ID, default=current_sensor): selector( - {"entity": {"domain": "sensor"}} - ), + vol.Required( + CONF_SENSOR_ENTITY_ID, + default=current_sensor, + ): selector({"entity": {"domain": "sensor"}}), + vol.Required( + CONF_RANGE_MIN, + default=current_min, + ): vol.Coerce(float), + vol.Required( + CONF_RANGE_MAX, + default=current_max, + ): vol.Coerce(float), } ) + # If the user has submitted the form, create the entry + if user_input is not None: + # Validate that range_min < range_max + min_val = user_input.get(CONF_RANGE_MIN) + max_val = user_input.get(CONF_RANGE_MAX) + if min_val is not None and max_val is not None and min_val >= max_val: + return self.async_show_form( + step_id="init", + data_schema=options_schema, + errors={"base": "range_min_max"}, + ) + + return self.async_create_entry( + title=self.config_entry.title, + data=user_input, + ) + return self.async_show_form(step_id="init", data_schema=options_schema) diff --git a/custom_components/simple_pid_controller/const.py b/custom_components/simple_pid_controller/const.py index 5dec9da..de37472 100644 --- a/custom_components/simple_pid_controller/const.py +++ b/custom_components/simple_pid_controller/const.py @@ -1,5 +1,14 @@ """Constants for the PID Controller integration.""" DOMAIN = "simple_pid_controller" + CONF_NAME = "name" +DEFAULT_NAME = "sPID-x" + CONF_SENSOR_ENTITY_ID = "sensor_entity_id" + +CONF_RANGE_MIN = "range_min" +CONF_RANGE_MAX = "range_max" + +DEFAULT_RANGE_MIN = 0.0 +DEFAULT_RANGE_MAX = 100.0 diff --git a/custom_components/simple_pid_controller/diagnostics.py b/custom_components/simple_pid_controller/diagnostics.py new file mode 100644 index 0000000..204ff49 --- /dev/null +++ b/custom_components/simple_pid_controller/diagnostics.py @@ -0,0 +1,24 @@ +"""Diagnostics support for Simple PID Controller integration.""" + +from __future__ import annotations + +from typing import Any +from homeassistant.core import HomeAssistant +from homeassistant.config_entries import ConfigEntry + + +async def async_get_config_entry_diagnostics( + hass: HomeAssistant, entry: ConfigEntry +) -> dict[str, Any]: + """Return diagnostics for a config entry.""" + handle = entry.runtime_data.handle + + return { + "entry_data": entry.as_dict(), + "data": { + "name": handle.name, + "sensor_entity_id": handle.sensor_entity_id, + "range_min": handle.range_min, + "range_max": handle.range_max, + }, + } diff --git a/custom_components/simple_pid_controller/entity.py b/custom_components/simple_pid_controller/entity.py new file mode 100644 index 0000000..8b5835b --- /dev/null +++ b/custom_components/simple_pid_controller/entity.py @@ -0,0 +1,34 @@ +from __future__ import annotations + +from homeassistant.config_entries import ConfigEntry +from homeassistant.core import HomeAssistant +from homeassistant.helpers.entity import Entity, DeviceInfo + +from .const import DOMAIN +from . import PIDDeviceHandle + + +class BasePIDEntity(Entity): + """Base entity for Simple PID Controller integration.""" + + def __init__( + self, + hass: HomeAssistant, + entry: ConfigEntry, + key: str, + name: str, + ) -> None: + """Initialize the base PID entity.""" + self.hass = hass + self._entry = entry + self._handle = PIDDeviceHandle(hass, entry) + self._key = key + + # Common entity attributes + self._attr_name = f"{name}" + self._attr_has_entity_name = True + self._attr_unique_id = f"{entry.entry_id}_{key}" + self._attr_device_info = DeviceInfo( + identifiers={(DOMAIN, entry.entry_id)}, + name=self._handle.name, + ) diff --git a/custom_components/simple_pid_controller/manifest.json b/custom_components/simple_pid_controller/manifest.json index b727bac..46d93c4 100644 --- a/custom_components/simple_pid_controller/manifest.json +++ b/custom_components/simple_pid_controller/manifest.json @@ -8,7 +8,7 @@ "homekit": {}, "iot_class": "calculated", "issue_tracker": "https://github.com/bvweerd/simple_pid_controller/issues", - "quality_scale": "bronze", + "quality_scale": "silver", "requirements": ["simple-pid==2.0.1"], "ssdp": [], "version": "1.0.2", diff --git a/custom_components/simple_pid_controller/number.py b/custom_components/simple_pid_controller/number.py index a63443c..6f5288b 100644 --- a/custom_components/simple_pid_controller/number.py +++ b/custom_components/simple_pid_controller/number.py @@ -2,14 +2,27 @@ from __future__ import annotations +import logging + from homeassistant.components.number import RestoreNumber from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.entity import EntityCategory -from . import PIDDeviceHandle -from .const import DOMAIN +from .entity import BasePIDEntity +from .const import ( + CONF_RANGE_MIN, + CONF_RANGE_MAX, + DEFAULT_RANGE_MIN, + DEFAULT_RANGE_MAX, +) + +# Coordinator is used to centralize the data updates +PARALLEL_UPDATES = 0 + +_LOGGER = logging.getLogger(__name__) + PID_NUMBER_ENTITIES = [ { @@ -42,44 +55,41 @@ "default": 0.05, "entity_category": EntityCategory.CONFIG, }, + { + "name": "Sample Time", + "key": "sample_time", + "unit": "s", + "min": 0.01, + "max": 60.0, + "step": 0.01, + "default": 10.0, + "entity_category": EntityCategory.CONFIG, + }, +] + +CONTROL_NUMBER_ENTITIES = [ { "name": "Setpoint", "key": "setpoint", - "unit": "%", - "min": 0.0, - "max": 100.0, + "unit": "", "step": 1.0, - "default": 50.0, + "default": 0.5, "entity_category": None, }, { "name": "Output Min", "key": "output_min", "unit": "", - "min": -100.0, - "max": 0.0, "step": 1.0, - "default": -10.0, + "default": 0, "entity_category": EntityCategory.CONFIG, }, { "name": "Output Max", "key": "output_max", "unit": "", - "min": 0.0, - "max": 100.0, "step": 1.0, - "default": 10.0, - "entity_category": EntityCategory.CONFIG, - }, - { - "name": "Sample Time", - "key": "sample_time", - "unit": "s", - "min": 0.01, - "max": 60.0, - "step": 0.01, - "default": 10.0, + "default": 1, "entity_category": EntityCategory.CONFIG, }, ] @@ -88,17 +98,20 @@ async def async_setup_entry( hass: HomeAssistant, entry: ConfigEntry, async_add_entities: AddEntitiesCallback ) -> None: - handle: PIDDeviceHandle = hass.data[DOMAIN][entry.entry_id] - name = handle.name - entities = [PIDParameterNumber(entry, name, desc) for desc in PID_NUMBER_ENTITIES] + entities = [PIDParameterNumber(hass, entry, desc) for desc in PID_NUMBER_ENTITIES] + async_add_entities(entities) + + entities = [ + ControlParameterNumber(hass, entry, desc) for desc in CONTROL_NUMBER_ENTITIES + ] async_add_entities(entities) class PIDParameterNumber(RestoreNumber): - def __init__(self, entry: ConfigEntry, device_name: str, desc: dict) -> None: - self._attr_name = f"{desc['name']}" - self._attr_has_entity_name = True - self._attr_unique_id = f"{entry.entry_id}_{desc['key']}" + def __init__(self, hass: HomeAssistant, entry: ConfigEntry, desc: dict) -> None: + BasePIDEntity.__init__(self, hass, entry, desc["key"], desc["name"]) + RestoreNumber.__init__(self) + self._attr_icon = "mdi:ray-vertex" self._attr_mode = "box" self._attr_native_unit_of_measurement = desc["unit"] @@ -108,16 +121,85 @@ def __init__(self, entry: ConfigEntry, device_name: str, desc: dict) -> None: self._attr_native_value = desc["default"] self._attr_entity_category = desc["entity_category"] - # Device-info - self._attr_device_info = { - "identifiers": {(DOMAIN, entry.entry_id)}, - "name": device_name, - } + async def async_added_to_hass(self) -> None: + await super().async_added_to_hass() + if (last := await self.async_get_last_number_data()) is not None: + if last.native_value < self._attr_native_min_value: + self._attr_native_value = self._attr_native_min_value + elif last.native_value > self._attr_native_max_value: + self._attr_native_value = self._attr_native_max_value + else: + self._attr_native_value = last.native_value + + @property + def native_value(self) -> float: + return self._attr_native_value + + async def async_set_native_value(self, value: float) -> None: + self._attr_native_value = value + self.async_write_ha_state() + + +class ControlParameterNumber(RestoreNumber): + """Number entity for PID control parameters.""" + + def __init__(self, hass: HomeAssistant, entry: ConfigEntry, desc: dict) -> None: + BasePIDEntity.__init__(self, hass, entry, desc["key"], desc["name"]) + RestoreNumber.__init__(self) + + self._attr_icon = "mdi:ray-vertex" + self._attr_mode = "box" + self._attr_native_unit_of_measurement = desc["unit"] + self._attr_native_step = desc["step"] + self._attr_native_value = desc["default"] + self._attr_entity_category = desc["entity_category"] + self._key = desc["key"] + + # Compute range limits based on key + opts = entry.options or {} + data = entry.data or {} + range_min = opts.get( + CONF_RANGE_MIN, data.get(CONF_RANGE_MIN, DEFAULT_RANGE_MIN) + ) + range_max = opts.get( + CONF_RANGE_MAX, data.get(CONF_RANGE_MAX, DEFAULT_RANGE_MAX) + ) + + if self._key == "setpoint": + min_val, max_val = range_min, range_max + elif self._key == "output_min": + min_val, max_val = -abs(range_max), 0.0 + elif self._key == "output_max": + min_val, max_val = 0.0, range_max + else: + _LOGGER.error("Unexpected PID parameter key: %s", self._key) + min_val, max_val = DEFAULT_RANGE_MIN, DEFAULT_RANGE_MAX + + self._attr_native_min_value = min_val + self._attr_native_max_value = max_val + self._attr_native_step = desc.get("step", 1.0) + + # Initialize current value + if self._key == "setpoint": + self._attr_native_value = range_min + (range_max - range_min) * float( + desc["default"] + ) + elif self._key == "output_min": + self._attr_native_value = range_min + elif self._key == "output_max": + self._attr_native_value = range_max + else: + _LOGGER.error("Unexpected error, unknown state in number.py") async def async_added_to_hass(self) -> None: await super().async_added_to_hass() if (last := await self.async_get_last_number_data()) is not None: - self._attr_native_value = last.native_value + if last.native_value < self._attr_native_min_value: + self._attr_native_value = self._attr_native_min_value + elif last.native_value > self._attr_native_max_value: + self._attr_native_value = self._attr_native_max_value + else: + self._attr_native_value = last.native_value @property def native_value(self) -> float: diff --git a/custom_components/simple_pid_controller/quality_scale.yaml b/custom_components/simple_pid_controller/quality_scale.yaml index 201a916..129a7b6 100644 --- a/custom_components/simple_pid_controller/quality_scale.yaml +++ b/custom_components/simple_pid_controller/quality_scale.yaml @@ -1,60 +1,82 @@ rules: # Bronze - action-setup: todo - appropriate-polling: todo - brands: todo - common-modules: todo - config-flow-test-coverage: todo - config-flow: todo - dependency-transparency: todo - docs-actions: todo - docs-high-level-description: todo - docs-installation-instructions: todo - docs-removal-instructions: todo - entity-event-setup: todo - entity-unique-id: todo - has-entity-name: todo - runtime-data: todo - test-before-configure: todo - test-before-setup: todo - unique-config-entry: todo + action-setup: + status: exempt + comment: This integration does not provide any actions + appropriate-polling: + status: exempt + comment: This integration does not use polling + brands: done + common-modules: done + config-flow-test-coverage: done + config-flow: done + dependency-transparency: done + docs-actions: done + docs-high-level-description: done + docs-installation-instructions: done + docs-removal-instructions: done + entity-event-setup: done + entity-unique-id: done + has-entity-name: done + runtime-data: done + test-before-configure: + status: exempt + comment: This integration has no external dependencies + test-before-setup: + status: exempt + comment: This integration has no external dependencies + unique-config-entry: done # Silver - action-exceptions: todo - config-entry-unloading: todo - docs-configuration-parameters: todo - docs-installation-parameters: todo - entity-unavailable: todo - integration-owner: todo - log-when-unavailable: todo - parallel-updates: todo - reauthentication-flow: todo - test-coverage: todo + action-exceptions: + status: exempt + comment: This integration does not use actions + config-entry-unloading: done + docs-configuration-parameters: done + docs-installation-parameters: done + entity-unavailable: done + integration-owner: done + log-when-unavailable: done + parallel-updates: done + reauthentication-flow: + status: exempt + comment: This integration does not use authentication + test-coverage: done # Gold - devices: todo - diagnostics: todo - discovery-update-info: todo - discovery: todo - docs-data-update: todo + devices: done + diagnostics: done + discovery-update-info: + status: exempt + comment: This integration does not use discoverable devices + discovery: + status: exempt + comment: This integration does not use discoverable devices + docs-data-update: done docs-examples: todo docs-known-limitations: todo - docs-supported-devices: todo + docs-supported-devices: + status: exempt + comment: This integration does not use devices docs-supported-functions: todo docs-troubleshooting: todo docs-use-cases: todo dynamic-devices: todo - entity-category: todo + entity-category: done entity-device-class: todo entity-disabled-by-default: todo entity-translations: todo exception-translations: todo icon-translations: todo - reconfiguration-flow: todo + reconfiguration-flow: + status: exempt + comment: This integration does not use external devices repair-issues: todo stale-devices: todo # Platinum async-dependency: todo - inject-websession: todo + inject-websession: + status: exempt + comment: This integration does not use external devices strict-typing: todo diff --git a/custom_components/simple_pid_controller/sensor.py b/custom_components/simple_pid_controller/sensor.py index e6b4d99..6537018 100644 --- a/custom_components/simple_pid_controller/sensor.py +++ b/custom_components/simple_pid_controller/sensor.py @@ -16,9 +16,12 @@ from typing import Any from . import PIDDeviceHandle -from .const import DOMAIN +from .entity import BasePIDEntity from .coordinator import PIDDataCoordinator +# Coordinator is used to centralize the data updates +PARALLEL_UPDATES = 0 + _LOGGER = logging.getLogger(__name__) @@ -28,8 +31,7 @@ async def async_setup_entry( async_add_entities: AddEntitiesCallback, ) -> None: """Set up PID output and diagnostic sensors.""" - handle: PIDDeviceHandle = hass.data[DOMAIN][entry.entry_id] - name = handle.name + handle: PIDDeviceHandle = entry.runtime_data.handle # Init PID with default values pid = PID(1.0, 0.1, 0.05, setpoint=50) @@ -42,7 +44,7 @@ async def update_pid(): if input_value is None: raise ValueError("Input sensor not available") - # Lees parameters uit de UI + # Read parameters from UI kp = handle.get_number("kp") ki = handle.get_number("ki") kd = handle.get_number("kd") @@ -52,12 +54,16 @@ async def update_pid(): out_max = handle.get_number("output_max") auto_mode = handle.get_switch("auto_mode") p_on_m = handle.get_switch("proportional_on_measurement") + windup_protection = handle.get_switch("windup_protection") - # Pas live de PID-instellingen aan + # adapt PID settings pid.tunings = (kp, ki, kd) pid.setpoint = setpoint pid.sample_time = sample_time - pid.output_limits = (out_min, out_max) + if windup_protection: + pid.output_limits = (out_min, out_max) + else: + pid.output_limits = (None, None) pid.auto_mode = auto_mode pid.proportional_on_measurement = p_on_m @@ -92,21 +98,33 @@ async def update_pid(): return output # Setup Coordinator - coordinator = PIDDataCoordinator(hass, name, update_pid, interval=10) + if entry.runtime_data.coordinator is None: + entry.runtime_data.coordinator = PIDDataCoordinator( + hass, handle.name, update_pid, interval=10 + ) + coordinator = entry.runtime_data.coordinator # Wait for HA to finish starting async def start_refresh(_: Any) -> None: _LOGGER.debug("Home Assistant started, first PID-refresh started") await coordinator.async_request_refresh() - hass.bus.async_listen_once("homeassistant_started", start_refresh) + entry.async_on_unload( + hass.bus.async_listen_once("homeassistant_started", start_refresh) + ) async_add_entities( [ - PIDOutputSensor(entry, name, coordinator), - PIDContributionSensor(entry, name, "p", handle, coordinator), - PIDContributionSensor(entry, name, "i", handle, coordinator), - PIDContributionSensor(entry, name, "d", handle, coordinator), + PIDOutputSensor(hass, entry, coordinator), + PIDContributionSensor( + hass, entry, "pid_p_contrib", "P contribution", coordinator + ), + PIDContributionSensor( + hass, entry, "pid_i_contrib", "I contribution", coordinator + ), + PIDContributionSensor( + hass, entry, "pid_d_contrib", "D contribution", coordinator + ), ] ) @@ -132,7 +150,7 @@ def _listener(event): "state_changed", make_listener(f"number.{entry.entry_id}_{key}") ) - for key in ["auto_mode", "proportional_on_measurement"]: + for key in ["auto_mode", "proportional_on_measurement", "windup_protection"]: hass.bus.async_listen( "state_changed", make_listener(f"switch.{entry.entry_id}_{key}") ) @@ -142,19 +160,16 @@ class PIDOutputSensor(CoordinatorEntity[PIDDataCoordinator], SensorEntity): """Sensor representing the PID output.""" def __init__( - self, entry: ConfigEntry, device_name: str, coordinator: PIDDataCoordinator + self, hass: HomeAssistant, entry: ConfigEntry, coordinator: PIDDataCoordinator ): super().__init__(coordinator) - self._attr_unique_id = f"{entry.entry_id}_pid_output" - self._attr_name = "PID Output" - self._attr_has_entity_name = True - self._attr_native_unit_of_measurement = "%" - # Device-info - self._attr_device_info = { - "identifiers": {(DOMAIN, entry.entry_id)}, - "name": device_name, - } + name = "PID Output" + key = "pid_output" + + BasePIDEntity.__init__(self, hass, entry, key, name) + + self._attr_native_unit_of_measurement = "%" @property def native_value(self) -> float | None: @@ -168,27 +183,19 @@ class PIDContributionSensor(CoordinatorEntity[PIDDataCoordinator], SensorEntity) def __init__( self, + hass: HomeAssistant, entry: ConfigEntry, - device_name: str, - component: str, - handle: PIDDeviceHandle, + key: str, + name: str, coordinator: PIDDataCoordinator, ): super().__init__(coordinator) - self._attr_unique_id = f"{entry.entry_id}_pid_{component}_contrib" - self._attr_name = f"PID {component.upper()} Contribution" + + BasePIDEntity.__init__(self, hass, entry, key, name) + self._attr_entity_category = EntityCategory.DIAGNOSTIC self._attr_entity_registry_enabled_default = False - self._attr_has_entity_name = True - self._handle = handle - self._component = component - self._entry_id = entry.entry_id - - # Device-info - self._attr_device_info = { - "identifiers": {(DOMAIN, entry.entry_id)}, - "name": device_name, - } + self._key = key @property def native_value(self): @@ -197,5 +204,5 @@ def native_value(self): "p": contributions[0], "i": contributions[1], "d": contributions[2], - }.get(self._component) + }.get(self._key) return round(value, 2) if value is not None else None diff --git a/custom_components/simple_pid_controller/switch.py b/custom_components/simple_pid_controller/switch.py index b644493..b5d97d3 100644 --- a/custom_components/simple_pid_controller/switch.py +++ b/custom_components/simple_pid_controller/switch.py @@ -7,8 +7,10 @@ from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.entity import EntityCategory -from . import PIDDeviceHandle -from .const import DOMAIN +from .entity import BasePIDEntity + +# Coordinator is used to centralize the data updates +PARALLEL_UPDATES = 0 SWITCH_ENTITIES = [ {"key": "auto_mode", "name": "Auto Mode", "default_state": True}, @@ -17,32 +19,27 @@ "name": "Proportional on Measurement", "default_state": False, }, + { + "key": "windup_protection", + "name": "Windup Protection", + "default_state": True, + }, ] async def async_setup_entry( hass: HomeAssistant, entry: ConfigEntry, async_add_entities: AddEntitiesCallback ) -> None: - handle: PIDDeviceHandle = hass.data[DOMAIN][entry.entry_id] - name = handle.name - async_add_entities([PIDOptionSwitch(entry, name, desc) for desc in SWITCH_ENTITIES]) + async_add_entities([PIDOptionSwitch(hass, entry, desc) for desc in SWITCH_ENTITIES]) class PIDOptionSwitch(SwitchEntity, RestoreEntity): - def __init__(self, entry: ConfigEntry, device_name: str, desc: dict) -> None: - self._entry = entry - self._attr_name = f"{desc['name']}" - self._attr_has_entity_name = True - self._attr_unique_id = f"{entry.entry_id}_{desc['key']}" + def __init__(self, hass: HomeAssistant, entry: ConfigEntry, desc: dict) -> None: + BasePIDEntity.__init__(self, hass, entry, desc["key"], desc["name"]) + self._attr_entity_category = EntityCategory.CONFIG self._state = desc["default_state"] - # Device-info - self._attr_device_info = { - "identifiers": {(DOMAIN, entry.entry_id)}, - "name": device_name, - } - async def async_added_to_hass(self) -> None: """Restore previous state if available.""" await super().async_added_to_hass() diff --git a/custom_components/simple_pid_controller/translations/en.json b/custom_components/simple_pid_controller/translations/en.json new file mode 100644 index 0000000..5b25d3c --- /dev/null +++ b/custom_components/simple_pid_controller/translations/en.json @@ -0,0 +1,73 @@ +{ + "title": "Simple PID Controller", + "config": { + "step": { + "user": { + "title": "Configure Simple PID Controller", + "description": "Enter a name and choose the sensor to drive the PID loop.", + "data": { + "name": "Name", + "sensor_entity_id": "Sensor Entity" + } + } + }, + "abort": { + "single_instance_allowed": "Only a single instance with this name is allowed." + }, + "error": { + "already_configured": "A configuration with this name already exists.", + "range_min_max": "Minimum must be lower than maximum." + } + }, + "options": { + "step": { + "init": { + "title": "Edit PID Controller Options", + "description": "Modify the sensor entity and range used by the controller.", + "data": { + "sensor_entity_id": "Sensor Entity", + "range_min": "Minimum Range", + "range_max": "Maximum Range" + } + } + }, + "error": { + "range_min_max": "Minimum must be lower than maximum." + } + }, + "entity": { + "number": { + "kp": { + "name": "Kp" + }, + "ki": { + "name": "Ki" + }, + "kd": { + "name": "Kd" + }, + "setpoint": { + "name": "Setpoint" + }, + "output": { + "name": "Output" + } + }, + "switch": { + "auto_mode": { + "name": "Auto Mode" + }, + "proportional_on_measurement": { + "name": "Proportional on Measurement" + }, + "windup_protection": { + "name": "Windup Protection" + } + }, + "sensor": { + "current_value": { + "name": "Current Value" + } + } + } +} diff --git a/custom_components/simple_pid_controller/translations/nl.json b/custom_components/simple_pid_controller/translations/nl.json new file mode 100644 index 0000000..1ff0624 --- /dev/null +++ b/custom_components/simple_pid_controller/translations/nl.json @@ -0,0 +1,73 @@ +{ + "title": "PID-regelaar", + "config": { + "step": { + "user": { + "title": "Configureer PID-regelaar", + "description": "Voer een naam in en kies de sensor als input voor de PID.", + "data": { + "name": "Naam", + "sensor_entity_id": "Sensor naam" + } + } + }, + "abort": { + "single_instance_allowed": "Slechts één instantie van de PID-regelaar met deze naam is toegestaan." + }, + "error": { + "already_configured": "Er bestaat al een configuratie met deze naam.", + "range_min_max": "Minimum moet lager zijn dan maximum." + } + }, + "options": { + "step": { + "init": { + "title": "Bewerk PID-regelaar", + "description": "Wijzig de sensor en het bereik dat door de PID-regelaar wordt gebruikt.", + "data": { + "sensor_entity_id": "Sensor naam", + "range_min": "Minimum bereik", + "range_max": "Maximum bereik" + } + } + }, + "error": { + "range_min_max": "Minimum moet lager zijn dan maximum." + } + }, + "entity": { + "number": { + "kp": { + "name": "Kp" + }, + "ki": { + "name": "Ki" + }, + "kd": { + "name": "Kd" + }, + "setpoint": { + "name": "Doel" + }, + "output": { + "name": "Uitgang" + } + }, + "switch": { + "auto_mode": { + "name": "Automatische mode" + }, + "proportional_on_measurement": { + "name": "Proportioneel op meting" + }, + "windup_protection": { + "name": "Windup bescherming" + } + }, + "sensor": { + "current_value": { + "name": "Huidige waarde" + } + } + } +} diff --git a/tests/conftest.py b/tests/conftest.py index a06e1d0..4e1e8ff 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,6 +2,8 @@ from pytest_homeassistant_custom_component.common import MockConfigEntry from homeassistant.helpers.device_registry import DeviceRegistry from custom_components.simple_pid_controller.const import DOMAIN, CONF_SENSOR_ENTITY_ID +import custom_components.simple_pid_controller.sensor as sensor_mod + from homeassistant.const import CONF_NAME @@ -31,13 +33,27 @@ async def config_entry(hass, device_registry: DeviceRegistry): ) entry.add_to_hass(hass) + await hass.async_block_till_done() - device_registry.async_get_or_create( - config_entry_id=entry.entry_id, - identifiers={(DOMAIN, entry.entry_id)}, - name={CONF_NAME}, - manufacturer="Test", - model="PID Controller", - ) + # device_registry.async_get_or_create( + # config_entry_id=entry.entry_id, + # identifiers={(DOMAIN, entry.entry_id)}, + # name=entry.entry_id, + # ) return entry + + +@pytest.fixture +async def sensor_entities(hass, config_entry): + """ + catch all SensorEntity-instances + """ + created = [] + # async_add_entities callback fills 'created' + await sensor_mod.async_setup_entry( + hass, config_entry, lambda entities: created.extend(entities) + ) + # wait till all items are created + await hass.async_block_till_done() + return created diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 0b65fe8..08f593f 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -1,37 +1,174 @@ import pytest from homeassistant import config_entries +from homeassistant.data_entry_flow import FlowResultType + from custom_components.simple_pid_controller.const import ( DOMAIN, - CONF_SENSOR_ENTITY_ID, CONF_NAME, + CONF_SENSOR_ENTITY_ID, + CONF_RANGE_MIN, + CONF_RANGE_MAX, + DEFAULT_RANGE_MIN, + DEFAULT_RANGE_MAX, +) +from custom_components.simple_pid_controller.config_flow import ( + PIDControllerFlowHandler, + PIDControllerOptionsFlowHandler, ) +SENSOR_ENTITY = "sensor.test_input" + @pytest.mark.parametrize( - "source,step_id", + "user_input, expected_type, expected_data, expected_errors", [ - (config_entries.SOURCE_USER, "user"), + # Happy path without specifying ranges (defaults applied) + ( + { + CONF_NAME: "My PID", + CONF_SENSOR_ENTITY_ID: SENSOR_ENTITY, + CONF_RANGE_MIN: DEFAULT_RANGE_MIN, + CONF_RANGE_MAX: DEFAULT_RANGE_MAX, + }, + FlowResultType.CREATE_ENTRY, + { + CONF_NAME: "My PID", + CONF_SENSOR_ENTITY_ID: SENSOR_ENTITY, + CONF_RANGE_MIN: DEFAULT_RANGE_MIN, + CONF_RANGE_MAX: DEFAULT_RANGE_MAX, + }, + None, + ), + # Happy path specifying explicit valid ranges + ( + { + CONF_NAME: "My PID 2", + CONF_SENSOR_ENTITY_ID: SENSOR_ENTITY, + CONF_RANGE_MIN: 1.0, + CONF_RANGE_MAX: 10.0, + }, + FlowResultType.CREATE_ENTRY, + { + CONF_NAME: "My PID 2", + CONF_SENSOR_ENTITY_ID: SENSOR_ENTITY, + CONF_RANGE_MIN: 1.0, + CONF_RANGE_MAX: 10.0, + }, + None, + ), + # Invalid ranges (min >= max) + ( + { + CONF_NAME: "Bad PID", + CONF_SENSOR_ENTITY_ID: SENSOR_ENTITY, + CONF_RANGE_MIN: 10.0, + CONF_RANGE_MAX: 5.0, + }, + FlowResultType.FORM, + None, + {"base": "range_min_max"}, + ), ], ) -async def test_show_form(hass, source, step_id): - """When starting the flow, you get a form back.""" +async def test_async_step_user( + hass, user_input, expected_type, expected_data, expected_errors +): + """Test the user step: happy paths and validation errors.""" + # Start the user flow result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": source} + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + assert result["type"] == FlowResultType.FORM + assert result["step_id"] == "user" + + # Submit the form + flow_id = result["flow_id"] + result2 = await hass.config_entries.flow.async_configure( + flow_id, user_input=user_input + ) + + # Verify outcome + assert result2["type"] == expected_type + if expected_type == FlowResultType.CREATE_ENTRY: + assert result2["title"] == user_input[CONF_NAME] + assert result2["data"] == expected_data + else: + assert result2.get("errors") == expected_errors + + +def test_async_get_options_flow(): + """Test that async_get_options_flow returns the correct handler.""" + handler = PIDControllerFlowHandler() + options_flow = handler.async_get_options_flow(config_entry=None) + assert isinstance(options_flow, PIDControllerOptionsFlowHandler) + + +@pytest.mark.parametrize( + "new_options, expected_errors", + [ + ( + { + CONF_SENSOR_ENTITY_ID: "sensor.new", + CONF_RANGE_MIN: 1.0, + CONF_RANGE_MAX: 10.0, + }, + None, + ), + ( + { + CONF_SENSOR_ENTITY_ID: "sensor.new", + CONF_RANGE_MIN: 10.0, + CONF_RANGE_MAX: 5.0, + }, + {"base": "range_min_max"}, + ), + ], +) +async def test_options_flow(hass, config_entry, new_options, expected_errors): + """Test the options flow: happy and error scenarios.""" + # Initialize options flow + init_result = await hass.config_entries.options.async_init(config_entry.entry_id) + assert init_result["type"] == FlowResultType.FORM + assert init_result["step_id"] == "init" + + # Submit options + result2 = await hass.config_entries.options.async_configure( + init_result["flow_id"], user_input=new_options ) - assert result["type"] == "form" - assert result["step_id"] == step_id + + if expected_errors: + assert result2["type"] == FlowResultType.FORM + assert result2.get("errors") == expected_errors + else: + assert result2["type"] == FlowResultType.CREATE_ENTRY + assert result2.get("data") == new_options -async def test_create_entry(hass): - """After filling out the form, a ConfigEntry is created.""" - init = await hass.config_entries.flow.async_init( +async def test_user_flow_duplicate_abort(hass): + """Test that a duplicate config entry aborts the flow.""" + user_input = { + CONF_NAME: "Duplicate PID", + CONF_SENSOR_ENTITY_ID: SENSOR_ENTITY, + CONF_RANGE_MIN: DEFAULT_RANGE_MIN, + CONF_RANGE_MAX: DEFAULT_RANGE_MAX, + } + + # Create initial entry + init_result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": config_entries.SOURCE_USER} + ) + await hass.config_entries.flow.async_configure( + init_result["flow_id"], user_input=user_input + ) + + # Attempt duplicate + result = await hass.config_entries.flow.async_init( DOMAIN, context={"source": config_entries.SOURCE_USER} ) result2 = await hass.config_entries.flow.async_configure( - init["flow_id"], - user_input={CONF_NAME: "My PID", CONF_SENSOR_ENTITY_ID: "sensor.test"}, + result["flow_id"], user_input=user_input ) - assert result2["type"] == "create_entry" - assert result2["title"] == "My PID" - assert result2["data"][CONF_SENSOR_ENTITY_ID] == "sensor.test" + + assert result2["type"] == FlowResultType.ABORT + assert result2["reason"] == "already_configured" diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py new file mode 100644 index 0000000..9e622ee --- /dev/null +++ b/tests/test_coordinator.py @@ -0,0 +1,26 @@ +import pytest +from custom_components.simple_pid_controller.coordinator import PIDDataCoordinator +from homeassistant.helpers.update_coordinator import UpdateFailed + + +async def test_async_update_data_success(hass): + """Test that _async_update_data returns the value from update_method on success.""" + + async def fake_update(): + return 42.0 + + coordinator = PIDDataCoordinator(hass, "test", fake_update, interval=1) + result = await coordinator._async_update_data() + assert result == 42.0 + + +async def test_async_update_data_failure(hass): + """Test that _async_update_data raises UpdateFailed with proper message on exception.""" + + async def fake_update(): + raise ValueError("test error") + + coordinator = PIDDataCoordinator(hass, "test", fake_update, interval=1) + with pytest.raises(UpdateFailed) as excinfo: + await coordinator._async_update_data() + assert "PID update failed: test error" in str(excinfo.value) diff --git a/tests/test_device_handle.py b/tests/test_device_handle.py index 0ed2a6c..2f367bf 100644 --- a/tests/test_device_handle.py +++ b/tests/test_device_handle.py @@ -78,3 +78,25 @@ def test_get_input_sensor_value_invalid(hass, config_entry): # Should handle gracefully and return None assert handle.get_input_sensor_value() is None + + +@pytest.mark.parametrize("state", ["unknown", "unavailable"]) +async def test_get_switch_returns_true_when_state_unavailable( + hass, config_entry, state +): + """Regel 79: get_switch returns True if state 'unknown' or 'unavailable'.""" + fake_entity = f"switch.{config_entry.entry_id}_test_key" + handle = PIDDeviceHandle(hass, config_entry) + # Force existence of entity_id + handle._get_entity_id = lambda platform, key: fake_entity + # State to 'unknown' or 'unavailable' + hass.states.async_set(fake_entity, state) + assert handle.get_switch("test_key") is True + + +async def test_get_switch_returns_true_when_no_entity_configured(hass, config_entry): + """Regel 74: get_switch must return True if _get_entity_id None.""" + handle = PIDDeviceHandle(hass, config_entry) + # Force no entity_id + handle._get_entity_id = lambda platform, key: None + assert handle.get_switch("any_key") is True diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py new file mode 100644 index 0000000..8bf9255 --- /dev/null +++ b/tests/test_diagnostics.py @@ -0,0 +1,32 @@ +from custom_components.simple_pid_controller.diagnostics import ( + async_get_config_entry_diagnostics, +) +from custom_components.simple_pid_controller.const import ( + DOMAIN, + CONF_SENSOR_ENTITY_ID, + CONF_NAME, + DEFAULT_RANGE_MIN, + DEFAULT_RANGE_MAX, +) + + +async def test_config_entry_diagnostics(hass, config_entry): + """Test that diagnostics returns correct data for config entry.""" + result = await async_get_config_entry_diagnostics(hass, config_entry) + + # Controleer de entry_data + entry_data = result.get("entry_data") + assert entry_data["domain"] == DOMAIN + assert entry_data["entry_id"] == config_entry.entry_id + assert ( + entry_data["data"][CONF_SENSOR_ENTITY_ID] + == config_entry.data[CONF_SENSOR_ENTITY_ID] + ) + assert entry_data["data"][CONF_NAME] == config_entry.data[CONF_NAME] + + # Controleer de diagnostische payload + data = result.get("data") + assert data["name"] == config_entry.data[CONF_NAME] + assert data["sensor_entity_id"] == config_entry.data[CONF_SENSOR_ENTITY_ID] + assert data["range_min"] == DEFAULT_RANGE_MIN + assert data["range_max"] == DEFAULT_RANGE_MAX diff --git a/tests/test_init_unload.py b/tests/test_init_unload.py index 894bda1..01280bb 100644 --- a/tests/test_init_unload.py +++ b/tests/test_init_unload.py @@ -6,8 +6,27 @@ async def test_setup_and_unload_entry(hass, config_entry): """Test setting up and tearing down the entry.""" - assert config_entry.entry_id in hass.data[DOMAIN] + # runtime_data should exist… + assert hasattr(config_entry, "runtime_data") + # …and it should carry a PIDDeviceHandle… + handle = config_entry.runtime_data.handle + from custom_components.simple_pid_controller import PIDDeviceHandle + + assert isinstance(handle, PIDDeviceHandle) + + # …whose .entry has the same entry_id + assert handle.entry.entry_id == config_entry.entry_id + + # Unload-entry returned True assert await async_unload_entry(hass, config_entry) is True await hass.async_block_till_done() - assert config_entry.entry_id not in hass.data[DOMAIN] + + # Runtime data is empty + assert ( + not hasattr(config_entry, "runtime_data") or config_entry.runtime_data is None + ) + + # hass Data should be gone + + assert DOMAIN not in hass.data diff --git a/tests/test_number.py b/tests/test_number.py index bfa7588..7b30581 100644 --- a/tests/test_number.py +++ b/tests/test_number.py @@ -1,22 +1,28 @@ import pytest -from custom_components.simple_pid_controller.number import PID_NUMBER_ENTITIES +import logging +from custom_components.simple_pid_controller.number import ( + PID_NUMBER_ENTITIES, + CONTROL_NUMBER_ENTITIES, + PIDParameterNumber, + ControlParameterNumber, +) +from custom_components.simple_pid_controller.const import ( + DEFAULT_RANGE_MIN, + DEFAULT_RANGE_MAX, +) async def test_number_platform(hass, config_entry): """Check that all Number entities from PID_NUMBER_ENTITIES are created.""" numbers = hass.states.async_entity_ids("number") - assert len(numbers) == len(PID_NUMBER_ENTITIES) + assert len(numbers) == len(PID_NUMBER_ENTITIES) + len(CONTROL_NUMBER_ENTITIES) @pytest.mark.parametrize("desc", PID_NUMBER_ENTITIES) async def test_number_entity_attributes(hass, config_entry, desc): - # Build the entity_id from the entry and the key in the descriptor entity_id = f"number.{config_entry.entry_id}_{desc['key']}" - - # Check that the entity exists state = hass.states.get(entity_id) - assert state is not None, f"{entity_id} does not exist" attrs = state.attributes @@ -32,3 +38,114 @@ async def test_number_entity_attributes(hass, config_entry, desc): ## Unique ID and name # assert state.object_id == f"{config_entry.entry_id}_{desc['key']}" # assert state.name == desc.get("name", state.name) + + +@pytest.mark.parametrize( + "last_value, expected", + [ + (PID_NUMBER_ENTITIES[0]["min"] - 1, PID_NUMBER_ENTITIES[0]["min"]), + (PID_NUMBER_ENTITIES[0]["max"] + 1, PID_NUMBER_ENTITIES[0]["max"]), + ( + (PID_NUMBER_ENTITIES[0]["min"] + PID_NUMBER_ENTITIES[0]["max"]) / 2, + (PID_NUMBER_ENTITIES[0]["min"] + PID_NUMBER_ENTITIES[0]["max"]) / 2, + ), + ], +) +async def test_async_added_to_hass_clamps_pid_value( + hass, config_entry, monkeypatch, last_value, expected +): + """Test that restored PIDParameterNumber clamps values outside range to min/max.""" + desc = PID_NUMBER_ENTITIES[0] + num = PIDParameterNumber(hass, config_entry, desc) + + async def fake_get_last_number_data(): + return type("Last", (), {"native_value": last_value}) + + monkeypatch.setattr(num, "async_get_last_number_data", fake_get_last_number_data) + + await num.async_added_to_hass() + assert num.native_value == expected + + +async def test_async_added_to_hass_clamps_control_value( + hass, config_entry, monkeypatch +): + """Test that restored ControlParameterNumber clamps values outside dynamic range to min/max.""" + for desc in CONTROL_NUMBER_ENTITIES: + num = ControlParameterNumber(hass, config_entry, desc) + min_val = num._attr_native_min_value + max_val = num._attr_native_max_value + + # Below minimum should clamp to min_val + async def fake_last_below(): + return type("Last", (), {"native_value": min_val - 1}) + + monkeypatch.setattr(num, "async_get_last_number_data", fake_last_below) + await num.async_added_to_hass() + assert num.native_value == min_val + + # Above maximum should clamp to max_val + async def fake_last_above(): + return type("Last", (), {"native_value": max_val + 1}) + + monkeypatch.setattr(num, "async_get_last_number_data", fake_last_above) + await num.async_added_to_hass() + assert num.native_value == max_val + + # Value within range should be set as is + mid = (min_val + max_val) / 2 + + async def fake_last_within(): + return type("Last", (), {"native_value": mid}) + + monkeypatch.setattr(num, "async_get_last_number_data", fake_last_within) + await num.async_added_to_hass() + assert num.native_value == mid + + +@pytest.mark.parametrize( + "clazz, desc, sample_value", + [ + (PIDParameterNumber, PID_NUMBER_ENTITIES[0], PID_NUMBER_ENTITIES[0]["default"]), + ( + ControlParameterNumber, + CONTROL_NUMBER_ENTITIES[0], + CONTROL_NUMBER_ENTITIES[0]["default"], + ), + ], +) +async def test_async_set_native_value_triggers_write( + hass, config_entry, monkeypatch, clazz, desc, sample_value +): + """Test that async_set_native_value sets value and calls async_write_ha_state.""" + num = clazz(hass, config_entry, desc) + write_calls = [] + + def fake_write_state(): + write_calls.append(True) + + monkeypatch.setattr(num, "async_write_ha_state", fake_write_state) + + await num.async_set_native_value(sample_value) + assert num.native_value == sample_value + assert write_calls, "async_write_ha_state was not called" + + +@pytest.mark.parametrize("invalid_key", ["invalid1", "invalid2"]) +async def test_controlparameter_number_unexpected_key( + hass, config_entry, caplog, invalid_key +): + """Test that ControlParameterNumber logs error and uses default range for unexpected key.""" + desc = { + "name": "Invalid", + "key": invalid_key, + "unit": "", + "step": 1.0, + "default": 0.0, + "entity_category": None, + } + caplog.set_level(logging.ERROR) + num = ControlParameterNumber(hass, config_entry, desc) + assert f"Unexpected PID parameter key: {invalid_key}" in caplog.text + assert num._attr_native_min_value == DEFAULT_RANGE_MIN + assert num._attr_native_max_value == DEFAULT_RANGE_MAX diff --git a/tests/test_sensor.py b/tests/test_sensor.py index e79e9e3..b4f1579 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -1,5 +1,174 @@ -async def test_sensor_platform(hass, config_entry): - """At least one sensor must be created.""" +import pytest +from datetime import timedelta +from homeassistant.util.dt import utcnow +from pytest_homeassistant_custom_component.common import async_fire_time_changed +from custom_components.simple_pid_controller.sensor import PIDContributionSensor +from custom_components.simple_pid_controller.coordinator import PIDDataCoordinator +from custom_components.simple_pid_controller.sensor import async_setup_entry - sensors = hass.states.async_entity_ids("sensor") - assert len(sensors) >= 1 + +@pytest.mark.asyncio +async def test_pid_output_and_contributions_update(hass, config_entry): + """Test that PID output and contribution sensors update on Home Assistant start.""" + sample_time = 5 + + handle = config_entry.runtime_data.handle + + handle.get_input_sensor_value = lambda: 10.0 + handle.get_number = lambda key: { + "kp": 1.0, + "ki": 0.1, + "kd": 0.01, + "setpoint": 20.0, + "sample_time": sample_time, + "output_min": 0.0, + "output_max": 100.0, + }[key] + handle.get_switch = lambda key: True + + # 1) trigger initial update + hass.bus.async_fire("homeassistant_started") + await hass.async_block_till_done() + + # 2) fake time advancement to trigger scheduled update + future = utcnow() + timedelta(seconds=sample_time) + async_fire_time_changed(hass, future) + await hass.async_block_till_done() + + # Check PID output sensor + out_entity = f"sensor.{config_entry.entry_id}_pid_output" + state = hass.states.get(out_entity) + assert state is not None + assert float(state.state) != 0 + + +@pytest.mark.asyncio +async def test_pid_contribution_native_value_rounding_and_none(hass, config_entry): + """Test that PIDContributionSensor.native_value rounds correctly and returns None for unknown key.""" + handle = config_entry.runtime_data.handle + # Provide known contributions + handle.last_contributions = (0.1234, 1.9876, 2.5555) + coordinator = PIDDataCoordinator(hass, "test", lambda: 0, interval=1) + + # Map contribution key to expected index + mapping = [ + ("p", round(0.1234, 2)), + ("i", round(1.9876, 2)), + ("d", round(2.5555, 2)), + ] + for key, expected in mapping: + sensor = PIDContributionSensor( + hass, config_entry, key, f"pid_{key}_contrib", coordinator + ) + # Override internal handle to use test handle + sensor._handle = handle + assert sensor.native_value == expected + + # Unknown key should return None + sensor_none = PIDContributionSensor( + hass, config_entry, "x", "pid_x_contrib", coordinator + ) + sensor_none._handle = handle + assert sensor_none.native_value is None + + +@pytest.mark.asyncio +async def test_listeners_trigger_refresh_sensor(hass, config_entry, monkeypatch): + """Lines 131-132: coordinator.async_request_refresh called on sensor state change.""" + # Prepare handle + handle = config_entry.runtime_data.handle + handle.get_input_sensor_value = lambda: 0.0 + handle.get_number = lambda key: 0.0 + handle.get_switch = lambda key: True + + # Capture listeners + listeners = [] + monkeypatch.setattr( + type(hass.bus), + "async_listen", + lambda self, event, cb: listeners.append((event, cb)), + ) + + # Run setup to register listeners + entities = [] + await async_setup_entry(hass, config_entry, lambda ents: entities.extend(ents)) + coordinator = entities[0].coordinator + + # Patch refresh method + called = [] + monkeypatch.setattr( + coordinator, "async_request_refresh", lambda: called.append(True) + ) + + # Simulate state change event for kp + entry_id = config_entry.entry_id + test_entity = f"number.{entry_id}_kp" + callback = next(cb for evt, cb in listeners if evt == "state_changed") + + from types import SimpleNamespace + + event = SimpleNamespace(data={"entity_id": test_entity}) + callback(event) + assert ( + called + ), "Coordinator.async_request_refresh was not called on sensor state change" + + +@pytest.mark.asyncio +async def test_update_pid_raises_on_missing_input(hass, config_entry): + """Line 47: update_pid should raise ValueError when input sensor unavailable.""" + handle = config_entry.runtime_data.handle + # Force no input value + handle.get_input_sensor_value = lambda: None + # Provide defaults for numbers and switches + handle.get_number = lambda key: 0.0 + handle.get_switch = lambda key: True + # Setup entry to get coordinator with update_method + entities: list = [] + await async_setup_entry(hass, config_entry, lambda e: entities.extend(e)) + coordinator = entities[0].coordinator + with pytest.raises(ValueError) as excinfo: + await coordinator.update_method() + assert "Input sensor not available" in str(excinfo.value) + + +@pytest.mark.asyncio +async def test_update_pid_output_limits_none_when_windup_protection_disabled( + monkeypatch, hass, config_entry +): + """Line 68: update_pid should set output_limits to (None, None) when windup_protection is False.""" + from custom_components.simple_pid_controller import sensor as sensor_module + + # Dummy PID to inspect output_limits + class DummyPID: + def __init__(self, kp, ki, kd, setpoint): + self._proportional = 1.0 + self._integral = 1.0 + self._last_output = None + self.tunings = (kp, ki, kd) + self.setpoint = setpoint + self.sample_time = 1.0 + self.output_limits = (0.0, 0.0) + self.auto_mode = True + self.proportional_on_measurement = False + + def __call__(self, input_value): + return 0.0 + + monkeypatch.setattr(sensor_module, "PID", DummyPID) + handle = config_entry.runtime_data.handle + # Provide valid input and parameters + handle.get_input_sensor_value = lambda: 1.0 + handle.get_number = lambda key: 0.0 + # windup_protection False, other switches True + handle.get_switch = lambda key: False if key == "windup_protection" else True + entities: list = [] + await async_setup_entry(hass, config_entry, lambda e: entities.extend(e)) + coordinator = entities[0].coordinator + # Execute update_method + await coordinator.update_method() + # Extract pid from closure + freevars = coordinator.update_method.__code__.co_freevars + pid_idx = freevars.index("pid") + pid = coordinator.update_method.__closure__[pid_idx].cell_contents + assert pid.output_limits == (None, None) diff --git a/tests/test_switch.py b/tests/test_switch.py index 17986d4..e659abd 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -1,5 +1,8 @@ import pytest -from custom_components.simple_pid_controller.switch import SWITCH_ENTITIES +from custom_components.simple_pid_controller.switch import ( + SWITCH_ENTITIES, + PIDOptionSwitch, +) @pytest.mark.asyncio @@ -8,13 +11,14 @@ async def test_switch_operations(hass, config_entry): for desc in SWITCH_ENTITIES: entity_id = f"switch.{config_entry.entry_id}_{desc['key']}" - # Default state should be 'on' + # Default state should match description state = hass.states.get(entity_id) assert state is not None, f"Switch {entity_id} does not exist" if desc["default_state"]: assert state.state == "on" else: assert state.state == "off" + # Turn off and verify await hass.services.async_call( "switch", "turn_off", {"entity_id": entity_id}, blocking=True @@ -26,3 +30,29 @@ async def test_switch_operations(hass, config_entry): "switch", "turn_on", {"entity_id": entity_id}, blocking=True ) assert hass.states.get(entity_id).state == "on" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("last_state, expected", [("on", True), ("off", False)]) +async def test_async_added_to_hass_restores_previous_state( + hass, config_entry, monkeypatch, last_state, expected +): + """Test that PIDOptionSwitch.async_added_to_hass restores previous state (lines 47-48).""" + # Use first entity descriptor + desc = SWITCH_ENTITIES[0] + switch = PIDOptionSwitch(hass, config_entry, desc) + + # Fake a previous state + class LastState: + def __init__(self, state): + self.state = state + + async def fake_get_last_state(): + return LastState(last_state) + + monkeypatch.setattr(switch, "async_get_last_state", fake_get_last_state) + # Set initial state opposite to expected to ensure restoration occurs + switch._state = not expected + + await switch.async_added_to_hass() + assert switch.is_on is expected