[api] Use StringRef for ActionResponse error message to avoid copy (#12240)
This commit is contained in:
@@ -12,6 +12,7 @@
|
||||
#endif
|
||||
#include "esphome/core/automation.h"
|
||||
#include "esphome/core/helpers.h"
|
||||
#include "esphome/core/string_ref.h"
|
||||
|
||||
namespace esphome::api {
|
||||
|
||||
@@ -55,14 +56,16 @@ template<typename... Ts> class TemplatableKeyValuePair {
|
||||
|
||||
#ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES
|
||||
// Represents the response data from a Home Assistant action
|
||||
// Note: This class holds a StringRef to the error_message from the protobuf message.
|
||||
// The protobuf message must outlive the ActionResponse (which is guaranteed since
|
||||
// the callback is invoked synchronously while the message is on the stack).
|
||||
class ActionResponse {
|
||||
public:
|
||||
ActionResponse(bool success, std::string error_message = "")
|
||||
: success_(success), error_message_(std::move(error_message)) {}
|
||||
ActionResponse(bool success, const std::string &error_message) : success_(success), error_message_(error_message) {}
|
||||
|
||||
#ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON
|
||||
ActionResponse(bool success, std::string error_message, const uint8_t *data, size_t data_len)
|
||||
: success_(success), error_message_(std::move(error_message)) {
|
||||
ActionResponse(bool success, const std::string &error_message, const uint8_t *data, size_t data_len)
|
||||
: success_(success), error_message_(error_message) {
|
||||
if (data == nullptr || data_len == 0)
|
||||
return;
|
||||
this->json_document_ = json::parse_json(data, data_len);
|
||||
@@ -70,7 +73,8 @@ class ActionResponse {
|
||||
#endif
|
||||
|
||||
bool is_success() const { return this->success_; }
|
||||
const std::string &get_error_message() const { return this->error_message_; }
|
||||
// Returns reference to error message - can be implicitly converted to std::string if needed
|
||||
const StringRef &get_error_message() const { return this->error_message_; }
|
||||
|
||||
#ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON
|
||||
// Get data as parsed JSON object (const version returns read-only view)
|
||||
@@ -79,7 +83,7 @@ class ActionResponse {
|
||||
|
||||
protected:
|
||||
bool success_;
|
||||
std::string error_message_;
|
||||
StringRef error_message_;
|
||||
#ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON
|
||||
JsonDocument json_document_;
|
||||
#endif
|
||||
|
||||
@@ -17,6 +17,7 @@ api:
|
||||
- button.press: test_all_empty_service
|
||||
- button.press: test_rapid_service_calls
|
||||
- button.press: test_read_ha_states
|
||||
- button.press: test_action_response_error
|
||||
- number.set:
|
||||
id: ha_number
|
||||
value: 42.5
|
||||
@@ -309,3 +310,24 @@ button:
|
||||
} else {
|
||||
ESP_LOGI("test", "HA Empty State has no value (expected)");
|
||||
}
|
||||
|
||||
# Test 9: Action response error handling (tests StringRef error message)
|
||||
- platform: template
|
||||
name: "Test Action Response Error"
|
||||
id: test_action_response_error
|
||||
on_press:
|
||||
- logger.log: "Testing action response error handling"
|
||||
- homeassistant.action:
|
||||
action: nonexistent.action_for_error_test
|
||||
data:
|
||||
test_field: "test_value"
|
||||
on_error:
|
||||
- lambda: |-
|
||||
// This tests that StringRef error message works correctly
|
||||
// The error variable is std::string (converted from StringRef)
|
||||
ESP_LOGI("test", "Action error received: %s", error.c_str());
|
||||
- logger.log:
|
||||
format: "Action failed with error message length: %d"
|
||||
args: ['error.size()']
|
||||
on_success:
|
||||
- logger.log: "Action succeeded unexpectedly"
|
||||
|
||||
@@ -81,8 +81,15 @@ async def test_api_homeassistant(
|
||||
"input_number.set_value": loop.create_future(), # ha_number_service_call
|
||||
"switch.turn_on": loop.create_future(), # ha_switch_on_service_call
|
||||
"switch.turn_off": loop.create_future(), # ha_switch_off_service_call
|
||||
"nonexistent.action_for_error_test": loop.create_future(), # error_test_call
|
||||
}
|
||||
|
||||
# Future for error message test
|
||||
action_error_received_future = loop.create_future()
|
||||
|
||||
# Store client reference for use in callback
|
||||
client_ref: list = [] # Use list to allow modification in nested function
|
||||
|
||||
def on_service_call(service_call: HomeassistantServiceCall) -> None:
|
||||
"""Capture HomeAssistant service calls."""
|
||||
ha_service_calls.append(service_call)
|
||||
@@ -93,6 +100,17 @@ async def test_api_homeassistant(
|
||||
if not future.done():
|
||||
future.set_result(service_call)
|
||||
|
||||
# Immediately respond to the error test call so the test can proceed
|
||||
# This needs to happen synchronously so ESPHome receives the response
|
||||
# before logging "=== All tests completed ==="
|
||||
if service_call.service == "nonexistent.action_for_error_test" and client_ref:
|
||||
test_error_message = "Test error: action not found"
|
||||
client_ref[0].send_homeassistant_action_response(
|
||||
call_id=service_call.call_id,
|
||||
success=False,
|
||||
error_message=test_error_message,
|
||||
)
|
||||
|
||||
def check_output(line: str) -> None:
|
||||
"""Check log output for expected messages."""
|
||||
log_lines.append(line)
|
||||
@@ -131,7 +149,12 @@ async def test_api_homeassistant(
|
||||
if match:
|
||||
ha_number_future.set_result(match.group(1))
|
||||
|
||||
elif not tests_complete_future.done() and tests_complete_pattern.search(line):
|
||||
# Check for action error message (tests StringRef -> std::string conversion)
|
||||
# Use separate if (not elif) since this can come after tests_complete
|
||||
if not action_error_received_future.done() and "Action error received:" in line:
|
||||
action_error_received_future.set_result(line)
|
||||
|
||||
if not tests_complete_future.done() and tests_complete_pattern.search(line):
|
||||
tests_complete_future.set_result(True)
|
||||
|
||||
# Run with log monitoring
|
||||
@@ -144,6 +167,9 @@ async def test_api_homeassistant(
|
||||
assert device_info is not None
|
||||
assert device_info.name == "test-ha-api"
|
||||
|
||||
# Store client reference for use in service call callback
|
||||
client_ref.append(client)
|
||||
|
||||
# Subscribe to HomeAssistant service calls
|
||||
client.subscribe_service_calls(on_service_call)
|
||||
|
||||
@@ -292,6 +318,17 @@ async def test_api_homeassistant(
|
||||
assert switch_off_call.service == "switch.turn_off"
|
||||
assert switch_off_call.data["entity_id"] == "switch.test_switch"
|
||||
|
||||
# 9. Action response error test (tests StringRef error message)
|
||||
# The error response is sent automatically in on_service_call callback
|
||||
# Wait for the error to be logged (proves StringRef -> std::string works)
|
||||
error_log_line = await asyncio.wait_for(
|
||||
action_error_received_future, timeout=2.0
|
||||
)
|
||||
test_error_message = "Test error: action not found"
|
||||
assert test_error_message in error_log_line, (
|
||||
f"Expected error message '{test_error_message}' not found in: {error_log_line}"
|
||||
)
|
||||
|
||||
except TimeoutError as e:
|
||||
# Show recent log lines for debugging
|
||||
recent_logs = "\n".join(log_lines[-20:])
|
||||
|
||||
Reference in New Issue
Block a user