From a8c8827a5b1902b980464a67270cc7e923d7f0c9 Mon Sep 17 00:00:00 2001 From: Bonne Eggleston Date: Thu, 27 Aug 2026 08:33:16 -0700 Subject: [PATCH] [modbus_controller] Switch write_lambda return value is the wire value only (#18788) Co-authored-by: J. Nick Koston --- .../switch/modbus_switch.cpp | 18 ++-- .../uart_mock_modbus_lambda_invert.yaml | 95 +++++++++++++++++++ tests/integration/test_uart_mock_modbus.py | 55 +++++++++++ 3 files changed, 161 insertions(+), 7 deletions(-) create mode 100644 tests/integration/fixtures/uart_mock_modbus_lambda_invert.yaml diff --git a/esphome/components/modbus_controller/switch/modbus_switch.cpp b/esphome/components/modbus_controller/switch/modbus_switch.cpp index c942ff1e6f..7bf45366c0 100644 --- a/esphome/components/modbus_controller/switch/modbus_switch.cpp +++ b/esphome/components/modbus_controller/switch/modbus_switch.cpp @@ -65,6 +65,7 @@ void ModbusSwitch::write_state(bool state) { // so a rapidly-changing value writes the latest, not every intermediate. this->clear_tx_queue_for_device(); modbus::helpers::PduBuffer data; + bool write_value = state; if (this->write_transform_func_.has_value()) { // The lambda may drive the write itself via item->write_*/queue_pdu(), override the written value (return a // value), or (deprecated) fill `data` with a custom PDU. @@ -92,26 +93,29 @@ void ModbusSwitch::write_state(bool state) { ESP_LOGV(TAG, "Communication handled by lambda - exiting control"); return; } + // The returned bool is the wire value only; the entity still reports the requested state. A polled + // entity needs the read lambda inverted to match, or the next poll flips the display back. ESP_LOGV(TAG, "Value overwritten by lambda"); - state = val.value(); + write_value = val.value(); } - ESP_LOGV(TAG, "write_state '%s': new value = %s type = %d address = %X offset = %x", this->get_name().c_str(), - ONOFF(state), (int) this->register_type, this->start_address, this->offset); + ESP_LOGV(TAG, "write_state '%s': new value = %s (wire = %s) type = %d address = %X offset = %x", + this->get_name().c_str(), ONOFF(state), ONOFF(write_value), (int) this->register_type, this->start_address, + this->offset); bool queued; if (this->register_type == EntityType::COIL) { // offset for coil and discrete inputs is the coil/register number not bytes if (this->use_write_multiple_) { - std::array states{state}; + std::array states{write_value}; queued = this->write_multiple_coils(this->write_address(), states); } else { - queued = this->write_single_coil(this->write_address(), state); + queued = this->write_single_coil(this->write_address(), write_value); } } else { if (this->use_write_multiple_) { - std::array states{static_cast(state ? (0xFFFF & this->bitmask) : 0)}; + std::array states{static_cast(write_value ? (0xFFFF & this->bitmask) : 0)}; queued = this->write_multiple_registers(this->write_address(), states); } else { - queued = this->write_single_register(this->write_address(), state ? 0xFFFF & this->bitmask : 0u); + queued = this->write_single_register(this->write_address(), write_value ? 0xFFFF & this->bitmask : 0u); } } if (!queued) { diff --git a/tests/integration/fixtures/uart_mock_modbus_lambda_invert.yaml b/tests/integration/fixtures/uart_mock_modbus_lambda_invert.yaml new file mode 100644 index 0000000000..41afce70d6 --- /dev/null +++ b/tests/integration/fixtures/uart_mock_modbus_lambda_invert.yaml @@ -0,0 +1,95 @@ +esphome: + name: uart-mock-modbus-lambda-invert + +host: +api: +logger: + level: VERBOSE + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +# Dummy uart entry to satisfy modbus's DEPENDENCIES = ["uart"] +# The actual UART bus used is the uart_mock component below +uart: + baud_rate: 115200 + port: /dev/null + +uart_mock: + - id: virtual_uart_server + baud_rate: 9600 + auto_start: true + debug: + on_tx: + - then: + - uart_mock.inject_rx: + id: virtual_uart_controller + data: !lambda return data; + - id: virtual_uart_controller + baud_rate: 9600 + auto_start: true + debug: + on_tx: + - then: + - uart_mock.inject_rx: + id: virtual_uart_server + data: !lambda return data; + +globals: + - id: reg40 + type: uint16_t + initial_value: "5" + +modbus: + - uart_id: virtual_uart_server + id: virtual_modbus_server + role: server + - uart_id: virtual_uart_controller + id: virtual_modbus_controller + role: client + turnaround_time: 10ms + +modbus_controller: + - address: 1 + modbus_id: virtual_modbus_controller + id: modbus_controller_1 + update_interval: 1s + +modbus_server: + - address: 1 + modbus_id: virtual_modbus_server + id: modbus_server_1 + registers: + - address: 0x40 + value_type: U_WORD + read_lambda: return id(reg40); + write_lambda: id(reg40) = x; return true; + +# An active-low holding switch: the write_lambda inverts the wire value, but the entity must still +# report the REQUESTED state. assumed_state keeps the register unpolled, so the published state comes +# only from write_state() - turning ON writes 0x0000 yet the switch shows ON. +switch: + - platform: modbus_controller + modbus_controller_id: modbus_controller_1 + name: "invert_switch" + register_type: holding + address: 0x40 + assumed_state: true + write_lambda: |- + return !x; + +sensor: + - platform: modbus_controller + modbus_controller_id: modbus_controller_1 + name: "reg_40" + address: 0x40 + register_type: holding + value_type: U_WORD + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + # This test does not have anything to start (mock is autostart) diff --git a/tests/integration/test_uart_mock_modbus.py b/tests/integration/test_uart_mock_modbus.py index ae9a481630..f88febabf5 100644 --- a/tests/integration/test_uart_mock_modbus.py +++ b/tests/integration/test_uart_mock_modbus.py @@ -1058,6 +1058,61 @@ async def test_uart_mock_modbus_lambda_write( await tracker.await_change(wrote_30, "reg_30", timeout=4.0) +@pytest.mark.asyncio +async def test_uart_mock_modbus_lambda_invert( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that a write_lambda's return value is the wire value only. + + `invert_switch` is an active-low holding switch whose write_lambda returns !x. Turning it ON must + write 0x0000 to the register (observed through the independent reg_40 sensor) while the entity + reports ON - the requested state, not the inverted wire value. Turning it OFF writes 0xFFFF and + reports OFF. The switch is assumed_state, so the published state comes only from write_state(). + """ + + tracker = SensorTracker(["reg_40"]) + initial = tracker.expect("reg_40", 5) + wrote_on = tracker.expect("reg_40", 0) + wrote_off = tracker.expect("reg_40", 65535) + + async with ( + run_compiled(yaml_config), + api_client_connected() as client, + ): + entities = await tracker.setup_and_start_scenario(client) + await tracker.await_change(initial, "reg_40", timeout=4.0) + + switch = find_entity(entities, "invert_switch", SwitchInfo) + assert switch is not None, "invert_switch not found" + + client.switch_command(switch.key, True) + # The wire byte carries the inverted value... + await tracker.await_change(wrote_on, "reg_40", timeout=4.0) + # ...while the entity reports the requested state. Switch states are deduped, so this relies on + # wait_for_state's fresh subscribe_states re-dumping every entity's current state. + await wait_for_state( + client, + lambda s: ( + getattr(s, "key", None) == switch.key + and getattr(s, "state", None) is True + ), + timeout=6.0, + ) + + client.switch_command(switch.key, False) + await tracker.await_change(wrote_off, "reg_40", timeout=4.0) + await wait_for_state( + client, + lambda s: ( + getattr(s, "key", None) == switch.key + and getattr(s, "state", None) is False + ), + timeout=6.0, + ) + + @pytest.mark.asyncio async def test_uart_mock_modbus_deprecated_write_buffer( yaml_config: str,