diff --git a/esphome/components/api/__init__.py b/esphome/components/api/__init__.py index 0f5cd936f5..1146b43596 100644 --- a/esphome/components/api/__init__.py +++ b/esphome/components/api/__init__.py @@ -540,17 +540,20 @@ HOMEASSISTANT_ACTION_ACTION_SCHEMA = cv.All( ) +# synchronous=False: when on_success/on_error is configured, play() stores the +# trigger args until the HomeassistantActionResponse arrives, so non-owning args +# (StringRef into the API receive buffer) must not be used. @automation.register_action( "homeassistant.action", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, - synchronous=True, + synchronous=False, ) @automation.register_action( "homeassistant.service", HomeAssistantServiceCallAction, HOMEASSISTANT_ACTION_ACTION_SCHEMA, - synchronous=True, + synchronous=False, ) async def homeassistant_service_to_code( config: ConfigType, @@ -644,6 +647,8 @@ HOMEASSISTANT_EVENT_ACTION_SCHEMA = cv.Schema( ) +# synchronous=True is safe here: the event schema has no on_success/on_error, +# so play() never stores the trigger args. @automation.register_action( "homeassistant.event", HomeAssistantServiceCallAction, diff --git a/tests/component_tests/api/__init__.py b/tests/component_tests/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/api/test_homeassistant_action.py b/tests/component_tests/api/test_homeassistant_action.py new file mode 100644 index 0000000000..611353e7c5 --- /dev/null +++ b/tests/component_tests/api/test_homeassistant_action.py @@ -0,0 +1,28 @@ +"""Tests for arg-type selection of api user-defined services with homeassistant.action.""" + +CONFIG = "tests/component_tests/api/test_homeassistant_action.yaml" + + +def test_synchronous_chain_keeps_zero_copy_args(generate_main): + """A chain of synchronous actions keeps the non-owning StringRef arg type.""" + main_cpp = generate_main(CONFIG) + + assert ( + "api::UserServiceTrigger" + '("zero_copy_args", {"message"})' in main_cpp + ) + + +def test_response_callback_args_are_owning(generate_main): + """homeassistant.action with on_success/on_error stores the trigger args + until the HomeassistantActionResponse arrives, so string args must fall + back to owning std::string; StringRef would point into the connection's + receive buffer, which is reused before the response arrives.""" + main_cpp = generate_main(CONFIG) + + assert ( + "api::UserServiceTrigger" + '("response_args", {"message"})' in main_cpp + ) + assert "api::HomeAssistantServiceCallAction" in main_cpp + assert "api::HomeAssistantServiceCallAction" not in main_cpp diff --git a/tests/component_tests/api/test_homeassistant_action.yaml b/tests/component_tests/api/test_homeassistant_action.yaml new file mode 100644 index 0000000000..4561494c9e --- /dev/null +++ b/tests/component_tests/api/test_homeassistant_action.yaml @@ -0,0 +1,43 @@ +esphome: + name: test + +esp32: + board: esp32dev + +wifi: + ssid: SomeNetwork + password: SomePassword + +logger: + +api: + actions: + # Chain of synchronous actions that never store the args: + # keeps the zero-copy StringRef arg type. + - action: zero_copy_args + variables: + message: string + then: + - logger.log: + format: "%s" + args: [message.c_str()] + # homeassistant.action with on_success/on_error stores the trigger args + # until the action response arrives, so the codegen must fall back to + # owning std::string args (StringRef would dangle once the receive + # buffer is reused). + - action: response_args + variables: + message: string + then: + - homeassistant.action: + action: notify.notify + data: + message: !lambda return message; + on_success: + - logger.log: + format: "sent %s" + args: [message.c_str()] + on_error: + - logger.log: + format: "failed (%s): %s" + args: [error.c_str(), message.c_str()] diff --git a/tests/components/api/common-base.yaml b/tests/components/api/common-base.yaml index 060254990d..d7470ee4b3 100644 --- a/tests/components/api/common-base.yaml +++ b/tests/components/api/common-base.yaml @@ -109,6 +109,34 @@ api: - name.c_str() - int_arr.size() - string_arr.size() + # Test string + array args used by homeassistant.action's deferred + # on_success/on_error response callback. homeassistant.action registers + # synchronous=False, so the api codegen must fall back to owning + # std::string / std::vector args here: the non-owning defaults would + # dangle once rx_buf_ is reused before the response arrives, and the + # non-copyable FixedVector would fail to compile when captured into + # the response callback. + - action: action_response_args + variables: + name: string + int_arr: int[] + then: + - homeassistant.action: + action: notify.notify + data: + message: !lambda 'return name;' + on_success: + - logger.log: + format: "Notified %s (%u ints)" + args: + - name.c_str() + - int_arr.size() + on_error: + - logger.log: + format: "Notify failed (%s): %s" + args: + - error.c_str() + - name.c_str() # Test ContinuationAction (IfAction with then/else branches) - action: test_if_action variables: