From 2d767c4c7e940ba639653e83964929d70c3d9e25 Mon Sep 17 00:00:00 2001 From: bvweerd Date: Tue, 12 May 2026 13:01:42 +0200 Subject: [PATCH 1/3] feat: make step size configurable for all number entities via options flow Closes #135. All number entities (Kp, Ki, Kd, sample time, setpoint, output min/max, startup value) now read their step size from entry.options using a generic step_{key} pattern. Defaults match existing hardcoded values, so existing installations are unaffected. Also fixes a pre-existing bug in ControlParameterNumber.__init__ where _attr_native_step was assigned twice, with the second overwriting the first. --- .../simple_pid_controller/config_flow.py | 13 ++++ .../simple_pid_controller/const.py | 13 ++++ .../simple_pid_controller/number.py | 12 +++- .../simple_pid_controller/strings.json | 20 +++++- .../translations/en.json | 62 ++++++++++++------- .../translations/nl.json | 20 +++++- tests/test_config_flow.py | 27 +++++++- tests/test_number.py | 59 ++++++++++++++++++ 8 files changed, 198 insertions(+), 28 deletions(-) diff --git a/custom_components/simple_pid_controller/config_flow.py b/custom_components/simple_pid_controller/config_flow.py index b64a162..16a2317 100644 --- a/custom_components/simple_pid_controller/config_flow.py +++ b/custom_components/simple_pid_controller/config_flow.py @@ -25,10 +25,12 @@ CONF_INPUT_RANGE_MAX, CONF_OUTPUT_RANGE_MIN, CONF_OUTPUT_RANGE_MAX, + CONF_STEP_PREFIX, DEFAULT_INPUT_RANGE_MIN, DEFAULT_INPUT_RANGE_MAX, DEFAULT_OUTPUT_RANGE_MIN, DEFAULT_OUTPUT_RANGE_MAX, + DEFAULT_STEPS, ) _LOGGER = logging.getLogger(__name__) @@ -146,6 +148,16 @@ async def async_step_init( CONF_OUTPUT_RANGE_MAX, DEFAULT_OUTPUT_RANGE_MAX ) + step_fields = { + vol.Optional( + f"{CONF_STEP_PREFIX}{key}", + default=self.config_entry.options.get( + f"{CONF_STEP_PREFIX}{key}", DEFAULT_STEPS[key] + ), + ): selector({"number": {"min": 0.0001, "max": 100.0, "step": 0.001, "mode": "box"}}) + for key in DEFAULT_STEPS + } + options_schema = vol.Schema( { vol.Required( @@ -168,6 +180,7 @@ async def async_step_init( CONF_OUTPUT_RANGE_MAX, default=current_output_max, ): vol.Coerce(float), + **step_fields, } ) diff --git a/custom_components/simple_pid_controller/const.py b/custom_components/simple_pid_controller/const.py index 592b1b5..d4a9b8e 100644 --- a/custom_components/simple_pid_controller/const.py +++ b/custom_components/simple_pid_controller/const.py @@ -16,3 +16,16 @@ DEFAULT_INPUT_RANGE_MAX = 100.0 DEFAULT_OUTPUT_RANGE_MIN = 0.0 DEFAULT_OUTPUT_RANGE_MAX = 100.0 + +CONF_STEP_PREFIX = "step_" + +DEFAULT_STEPS: dict[str, float] = { + "kp": 0.0001, + "ki": 0.0001, + "kd": 0.0001, + "sample_time": 0.01, + "setpoint": 0.01, + "output_min": 1.0, + "output_max": 1.0, + "starting_output": 1.0, +} diff --git a/custom_components/simple_pid_controller/number.py b/custom_components/simple_pid_controller/number.py index b429ceb..b8fc4f3 100644 --- a/custom_components/simple_pid_controller/number.py +++ b/custom_components/simple_pid_controller/number.py @@ -16,10 +16,12 @@ CONF_INPUT_RANGE_MAX, CONF_OUTPUT_RANGE_MIN, CONF_OUTPUT_RANGE_MAX, + CONF_STEP_PREFIX, DEFAULT_INPUT_RANGE_MIN, DEFAULT_INPUT_RANGE_MAX, DEFAULT_OUTPUT_RANGE_MIN, DEFAULT_OUTPUT_RANGE_MAX, + DEFAULT_STEPS, ) # Coordinator is used to centralize the data updates @@ -129,7 +131,9 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry, desc: dict) -> None: self._attr_native_unit_of_measurement = desc["unit"] self._attr_native_min_value = desc["min"] self._attr_native_max_value = desc["max"] - self._attr_native_step = desc["step"] + self._attr_native_step = (entry.options or {}).get( + f"{CONF_STEP_PREFIX}{desc['key']}", DEFAULT_STEPS[desc["key"]] + ) self._attr_native_value = desc["default"] self._attr_entity_category = desc["entity_category"] @@ -162,7 +166,6 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry, desc: dict) -> None: 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"] @@ -208,7 +211,10 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry, desc: dict) -> None: self._attr_native_min_value = min_val self._attr_native_max_value = max_val - self._attr_native_step = desc.get("step", 1.0) + self._attr_native_step = opts.get( + f"{CONF_STEP_PREFIX}{self._key}", + DEFAULT_STEPS.get(self._key, desc.get("step", 1.0)), + ) # Initialize current value if self._key == "setpoint": diff --git a/custom_components/simple_pid_controller/strings.json b/custom_components/simple_pid_controller/strings.json index 6bdd0a9..93d3b4e 100644 --- a/custom_components/simple_pid_controller/strings.json +++ b/custom_components/simple_pid_controller/strings.json @@ -28,7 +28,25 @@ "input_range_min": "Minimum Input Range", "input_range_max": "Maximum Input Range", "output_range_min": "Minimum Output Range", - "output_range_max": "Maximum Output Range" + "output_range_max": "Maximum Output Range", + "step_kp": "Kp Step Size", + "step_ki": "Ki Step Size", + "step_kd": "Kd Step Size", + "step_sample_time": "Sample Time Step Size", + "step_setpoint": "Setpoint Step Size", + "step_output_min": "Output Min Step Size", + "step_output_max": "Output Max Step Size", + "step_starting_output": "Startup Value Step Size" + }, + "data_description": { + "step_kp": "Increment size for the Kp parameter.", + "step_ki": "Increment size for the Ki parameter.", + "step_kd": "Increment size for the Kd parameter.", + "step_sample_time": "Increment size for the sample time parameter.", + "step_setpoint": "Increment size for the setpoint picker.", + "step_output_min": "Increment size for the output minimum.", + "step_output_max": "Increment size for the output maximum.", + "step_starting_output": "Increment size for the startup value." } } } diff --git a/custom_components/simple_pid_controller/translations/en.json b/custom_components/simple_pid_controller/translations/en.json index 6e12af2..0a5d4b2 100644 --- a/custom_components/simple_pid_controller/translations/en.json +++ b/custom_components/simple_pid_controller/translations/en.json @@ -34,7 +34,25 @@ "input_range_min": "Minimum Input Range", "input_range_max": "Maximum Input Range", "output_range_min": "Minimum Output Range", - "output_range_max": "Maximum Output Range" + "output_range_max": "Maximum Output Range", + "step_kp": "Kp Step Size", + "step_ki": "Ki Step Size", + "step_kd": "Kd Step Size", + "step_sample_time": "Sample Time Step Size", + "step_setpoint": "Setpoint Step Size", + "step_output_min": "Output Min Step Size", + "step_output_max": "Output Max Step Size", + "step_starting_output": "Startup Value Step Size" + }, + "data_description": { + "step_kp": "Increment size for the Kp parameter.", + "step_ki": "Increment size for the Ki parameter.", + "step_kd": "Increment size for the Kd parameter.", + "step_sample_time": "Increment size for the sample time parameter.", + "step_setpoint": "Increment size for the setpoint picker.", + "step_output_min": "Increment size for the output minimum.", + "step_output_max": "Increment size for the output maximum.", + "step_starting_output": "Increment size for the startup value." } } }, @@ -42,7 +60,7 @@ "range_min_max": "Minimum must be lower than maximum." } }, - "entity": { + "entity": { "number": { "kp": { "name": "Kp" @@ -75,23 +93,23 @@ "current_value": { "name": "Current Value" } - } - } - , - "services": { - "set_output": { - "name": "Set PID output", - "description": "Set or reset the PID controller output.", - "fields": { - "preset": { - "name": "Preset", - "description": "Use a preset output: zero_start, last_known_value or startup_value." - }, - "value": { - "name": "Value", - "description": "Manual output value between output_min and output_max." - } - } - } - } -} + } + } + , + "services": { + "set_output": { + "name": "Set PID output", + "description": "Set or reset the PID controller output.", + "fields": { + "preset": { + "name": "Preset", + "description": "Use a preset output: zero_start, last_known_value or startup_value." + }, + "value": { + "name": "Value", + "description": "Manual output value between output_min and output_max." + } + } + } + } +} diff --git a/custom_components/simple_pid_controller/translations/nl.json b/custom_components/simple_pid_controller/translations/nl.json index 0d73623..f812e1c 100644 --- a/custom_components/simple_pid_controller/translations/nl.json +++ b/custom_components/simple_pid_controller/translations/nl.json @@ -33,7 +33,25 @@ "input_range_min": "Minimum Input Bereik", "input_range_max": "Maximum Input Bereik", "output_range_min": "Minimum Output Bereik", - "output_range_max": "Maximum Output Bereik" + "output_range_max": "Maximum Output Bereik", + "step_kp": "Kp Stapgrootte", + "step_ki": "Ki Stapgrootte", + "step_kd": "Kd Stapgrootte", + "step_sample_time": "Steektijd Stapgrootte", + "step_setpoint": "Stapgrootte doelwaarde", + "step_output_min": "Stapgrootte min. uitvoer", + "step_output_max": "Stapgrootte max. uitvoer", + "step_starting_output": "Stapgrootte startwaarde" + }, + "data_description": { + "step_kp": "Stapgrootte voor de Kp-parameter.", + "step_ki": "Stapgrootte voor de Ki-parameter.", + "step_kd": "Stapgrootte voor de Kd-parameter.", + "step_sample_time": "Stapgrootte voor de steektijd.", + "step_setpoint": "Stapgrootte voor de setpoint-kiezer.", + "step_output_min": "Stapgrootte voor de minimale uitvoer.", + "step_output_max": "Stapgrootte voor de maximale uitvoer.", + "step_starting_output": "Stapgrootte voor de startwaarde." } } }, diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 1d28955..24f3fa0 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -4,6 +4,7 @@ from homeassistant.data_entry_flow import FlowResultType from custom_components.simple_pid_controller.const import ( + CONF_STEP_PREFIX, DOMAIN, CONF_NAME, CONF_SENSOR_ENTITY_ID, @@ -15,6 +16,7 @@ DEFAULT_INPUT_RANGE_MAX, DEFAULT_OUTPUT_RANGE_MIN, DEFAULT_OUTPUT_RANGE_MAX, + DEFAULT_STEPS, ) from custom_components.simple_pid_controller.config_flow import ( PIDControllerFlowHandler, @@ -184,7 +186,30 @@ async def test_options_flow(hass, config_entry, new_options, expected_errors): assert result2.get("errors") == expected_errors else: assert result2["type"] == FlowResultType.CREATE_ENTRY - assert result2.get("data") == new_options + # Step defaults are added by voluptuous; check submitted fields are present + assert new_options.items() <= result2.get("data", {}).items() + + +async def test_options_flow_with_custom_steps(hass, config_entry): + """Test that step options round-trip correctly through the options flow.""" + custom_steps = {f"{CONF_STEP_PREFIX}{key}": 0.5 for key in DEFAULT_STEPS} + new_options = { + CONF_SENSOR_ENTITY_ID: "sensor.new", + CONF_INPUT_RANGE_MIN: 1.0, + CONF_INPUT_RANGE_MAX: 10.0, + CONF_OUTPUT_RANGE_MIN: 1.0, + CONF_OUTPUT_RANGE_MAX: 10.0, + **custom_steps, + } + + init_result = await hass.config_entries.options.async_init(config_entry.entry_id) + result2 = await hass.config_entries.options.async_configure( + init_result["flow_id"], user_input=new_options + ) + + assert result2["type"] == FlowResultType.CREATE_ENTRY + for key, val in custom_steps.items(): + assert result2["data"][key] == val async def test_user_flow_duplicate_abort(hass): diff --git a/tests/test_number.py b/tests/test_number.py index b0d942a..91c1328 100644 --- a/tests/test_number.py +++ b/tests/test_number.py @@ -7,10 +7,13 @@ ControlParameterNumber, ) from custom_components.simple_pid_controller.const import ( + CONF_STEP_PREFIX, DEFAULT_INPUT_RANGE_MIN, DEFAULT_INPUT_RANGE_MAX, DEFAULT_OUTPUT_RANGE_MIN, DEFAULT_OUTPUT_RANGE_MAX, + DEFAULT_STEPS, + DOMAIN, ) @@ -163,3 +166,59 @@ async def test_controlparameter_number_unexpected_key( assert f"Unknown PID key '{invalid_key}'. Using default values:" in caplog.text assert num._attr_native_min_value == expected_min assert num._attr_native_max_value == expected_max + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize("desc", PID_NUMBER_ENTITIES) +async def test_pid_number_step_from_options(hass, config_entry, desc): + """Test that PIDParameterNumber reads step from CONF_STEP_PREFIX+key in options.""" + from types import SimpleNamespace + from pytest_homeassistant_custom_component.common import MockConfigEntry + + custom_step = 0.5 + entry = MockConfigEntry( + domain=DOMAIN, + entry_id=f"step_test_{desc['key']}", + data=config_entry.data, + options={f"{CONF_STEP_PREFIX}{desc['key']}": custom_step}, + ) + entry.runtime_data = SimpleNamespace(handle=config_entry.runtime_data.handle) + + num = PIDParameterNumber(hass, entry, desc) + assert num._attr_native_step == custom_step + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize("desc", PID_NUMBER_ENTITIES) +async def test_pid_number_step_defaults(hass, config_entry, desc): + """Test that PIDParameterNumber falls back to DEFAULT_STEPS when no option is set.""" + num = PIDParameterNumber(hass, config_entry, desc) + assert num._attr_native_step == DEFAULT_STEPS[desc["key"]] + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize("desc", CONTROL_NUMBER_ENTITIES) +async def test_control_number_step_from_options(hass, config_entry, desc): + """Test that ControlParameterNumber reads step from CONF_STEP_PREFIX+key in options.""" + from types import SimpleNamespace + from pytest_homeassistant_custom_component.common import MockConfigEntry + + custom_step = 2.0 + entry = MockConfigEntry( + domain=DOMAIN, + entry_id=f"step_test_{desc['key']}", + data=config_entry.data, + options={f"{CONF_STEP_PREFIX}{desc['key']}": custom_step}, + ) + entry.runtime_data = SimpleNamespace(handle=config_entry.runtime_data.handle) + + num = ControlParameterNumber(hass, entry, desc) + assert num._attr_native_step == custom_step + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize("desc", CONTROL_NUMBER_ENTITIES) +async def test_control_number_step_defaults(hass, config_entry, desc): + """Test that ControlParameterNumber falls back to DEFAULT_STEPS when no option is set.""" + num = ControlParameterNumber(hass, config_entry, desc) + assert num._attr_native_step == DEFAULT_STEPS[desc["key"]] From 860b6361c6ce4ec24609130afd2f04cdbc263ba3 Mon Sep 17 00:00:00 2001 From: bvweerd Date: Tue, 12 May 2026 13:09:01 +0200 Subject: [PATCH 2/3] test: add non-happy path tests for step selector validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests that step values below the minimum (0.0001), above the maximum (100.0), and exactly at both boundaries are correctly handled by the options flow — invalid values raise InvalidData, boundary values are accepted. --- tests/test_config_flow.py | 54 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 24f3fa0..3b01d16 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -1,7 +1,9 @@ import pytest +import pytest + from homeassistant import config_entries -from homeassistant.data_entry_flow import FlowResultType +from homeassistant.data_entry_flow import FlowResultType, InvalidData from custom_components.simple_pid_controller.const import ( CONF_STEP_PREFIX, @@ -212,6 +214,54 @@ async def test_options_flow_with_custom_steps(hass, config_entry): assert result2["data"][key] == val +_BASE_OPTIONS = { + CONF_SENSOR_ENTITY_ID: "sensor.new", + CONF_INPUT_RANGE_MIN: 1.0, + CONF_INPUT_RANGE_MAX: 10.0, + CONF_OUTPUT_RANGE_MIN: 1.0, + CONF_OUTPUT_RANGE_MAX: 10.0, +} + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize("invalid_step", [0.0, -1.0, -0.0001]) +async def test_options_flow_step_below_min_rejected(hass, config_entry, invalid_step): + """Test that a step value below the selector minimum raises InvalidData.""" + init_result = await hass.config_entries.options.async_init(config_entry.entry_id) + with pytest.raises(InvalidData): + await hass.config_entries.options.async_configure( + init_result["flow_id"], + user_input={**_BASE_OPTIONS, f"{CONF_STEP_PREFIX}kp": invalid_step}, + ) + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize("invalid_step", [100.001, 200.0, 1000.0]) +async def test_options_flow_step_above_max_rejected(hass, config_entry, invalid_step): + """Test that a step value above the selector maximum raises InvalidData.""" + init_result = await hass.config_entries.options.async_init(config_entry.entry_id) + with pytest.raises(InvalidData): + await hass.config_entries.options.async_configure( + init_result["flow_id"], + user_input={**_BASE_OPTIONS, f"{CONF_STEP_PREFIX}setpoint": invalid_step}, + ) + + +@pytest.mark.usefixtures("setup_integration") +@pytest.mark.parametrize( + "boundary_step", [0.0001, 100.0] +) +async def test_options_flow_step_boundary_values_accepted(hass, config_entry, boundary_step): + """Test that step values at the exact selector boundaries are accepted.""" + init_result = await hass.config_entries.options.async_init(config_entry.entry_id) + result = await hass.config_entries.options.async_configure( + init_result["flow_id"], + user_input={**_BASE_OPTIONS, f"{CONF_STEP_PREFIX}kp": boundary_step}, + ) + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["data"][f"{CONF_STEP_PREFIX}kp"] == boundary_step + + async def test_user_flow_duplicate_abort(hass): """Test that a duplicate config entry aborts the flow.""" user_input = { @@ -241,3 +291,5 @@ async def test_user_flow_duplicate_abort(hass): assert result2["type"] == FlowResultType.ABORT assert result2["reason"] == "already_configured" + + From 20ef5d95c9739eea7da22c26aa64ee2b69c7883d Mon Sep 17 00:00:00 2001 From: bvweerd Date: Tue, 12 May 2026 13:15:13 +0200 Subject: [PATCH 3/3] style: apply black and ruff formatting --- .../simple_pid_controller/config_flow.py | 4 +++- tests/test_config_flow.py | 11 ++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/custom_components/simple_pid_controller/config_flow.py b/custom_components/simple_pid_controller/config_flow.py index 16a2317..7e479d5 100644 --- a/custom_components/simple_pid_controller/config_flow.py +++ b/custom_components/simple_pid_controller/config_flow.py @@ -154,7 +154,9 @@ async def async_step_init( default=self.config_entry.options.get( f"{CONF_STEP_PREFIX}{key}", DEFAULT_STEPS[key] ), - ): selector({"number": {"min": 0.0001, "max": 100.0, "step": 0.001, "mode": "box"}}) + ): selector( + {"number": {"min": 0.0001, "max": 100.0, "step": 0.001, "mode": "box"}} + ) for key in DEFAULT_STEPS } diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 3b01d16..5218184 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -1,6 +1,5 @@ import pytest -import pytest from homeassistant import config_entries from homeassistant.data_entry_flow import FlowResultType, InvalidData @@ -248,10 +247,10 @@ async def test_options_flow_step_above_max_rejected(hass, config_entry, invalid_ @pytest.mark.usefixtures("setup_integration") -@pytest.mark.parametrize( - "boundary_step", [0.0001, 100.0] -) -async def test_options_flow_step_boundary_values_accepted(hass, config_entry, boundary_step): +@pytest.mark.parametrize("boundary_step", [0.0001, 100.0]) +async def test_options_flow_step_boundary_values_accepted( + hass, config_entry, boundary_step +): """Test that step values at the exact selector boundaries are accepted.""" init_result = await hass.config_entries.options.async_init(config_entry.entry_id) result = await hass.config_entries.options.async_configure( @@ -291,5 +290,3 @@ async def test_user_flow_duplicate_abort(hass): assert result2["type"] == FlowResultType.ABORT assert result2["reason"] == "already_configured" - -