Add device defined action responses #1436
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1436 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 3312 3335 +23
=========================================
+ Hits 3312 3335 +23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #1436 will not alter performanceComparing Summary
Footnotes |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds protobuf enums/messages and client/model/core/test support to allow user-defined service actions to return server-sent ExecuteServiceResponse messages identified by call IDs and handled via optional response callbacks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant APIClient
participant Server
participant Callback
User->>APIClient: execute_service(service, data, on_response=cb, return_response=true)
APIClient->>APIClient: allocate call_id, register listener(call_id -> cb)
APIClient->>Server: ExecuteServiceRequest(call_id, return_response=true, data)
Note over Server: perform user-defined action (may emit response)
Server->>APIClient: ExecuteServiceResponse(call_id, success, response_data)
APIClient->>Callback: invoke cb(ExecuteServiceResponseModel)
APIClient->>APIClient: unregister listener for call_id
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2025-04-16T01:35:06.073ZApplied to files:
🧬 Code graph analysis (1)aioesphomeapi/client.py (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
aioesphomeapi/api.proto(3 hunks)aioesphomeapi/client.py(4 hunks)aioesphomeapi/core.py(2 hunks)aioesphomeapi/model.py(2 hunks)tests/test_client.py(4 hunks)tests/test_model.py(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Useruff check --fixto check and fix Python linting issues before committing
Useruff formatto format Python code before committing
Files:
aioesphomeapi/core.pyaioesphomeapi/client.pyaioesphomeapi/model.pytests/test_client.pytests/test_model.py
**
⚙️ CodeRabbit configuration file
- Do not generate or add any sequence diagrams
Files:
aioesphomeapi/core.pyaioesphomeapi/api.protoaioesphomeapi/client.pyaioesphomeapi/model.pytests/test_client.pytests/test_model.py
🧬 Code graph analysis (4)
aioesphomeapi/core.py (1)
aioesphomeapi/model.py (1)
ExecuteServiceResponse(1255-1261)
aioesphomeapi/client.py (3)
aioesphomeapi/model.py (2)
ExecuteServiceResponse(1255-1261)UserService(1243-1251)tests/test_client.py (1)
on_response(2255-2256)aioesphomeapi/connection.py (1)
add_message_callback(817-822)
tests/test_client.py (4)
aioesphomeapi/model.py (5)
ExecuteServiceResponse(1255-1261)APIVersion(100-102)UserService(1243-1251)UserServiceArg(1223-1239)UserServiceArgType(1204-1212)tests/conftest.py (1)
api_client(209-238)aioesphomeapi/client.py (2)
APIClient(201-1640)execute_service(1316-1371)tests/common.py (2)
mock_data_received(65-76)generate_plaintext_packet(109-110)
tests/test_model.py (1)
aioesphomeapi/model.py (10)
ExecuteServiceResponse(1255-1261)SupportsResponseType(1215-1219)convert(39-43)convert(178-181)UserService(1243-1251)from_pb(86-87)from_pb(1319-1362)from_pb(1408-1414)from_pb(1448-1459)from_pb(1491-1503)
🪛 GitHub Actions: CI
aioesphomeapi/client.py
[error] 1360-1360: flake8: F824 'nonlocal unsub' is unused: name is never assigned in scope
🪛 GitHub Check: py 3.14 on ubuntu-latest (skip_cython)
aioesphomeapi/client.py
[failure] 1360-1360:
F824 nonlocal unsub is unused: name is never assigned in scope
🔇 Additional comments (13)
aioesphomeapi/core.py (2)
8-55: ExecuteServiceResponse import wiring looks correctImporting
ExecuteServiceResponsealongsideExecuteServiceRequestkeeps the core surface in sync with the proto definitions; usage below inMESSAGE_TYPE_TO_PROTOensures it is not unused.
364-496: Protocol mapping for ExecuteServiceResponse is consistentAdding
131: ExecuteServiceResponseat the end ofMESSAGE_TYPE_TO_PROTOmaintains the sequential message-number mapping and ensures ExecuteServiceResponse frames are decoded to the right proto type. No functional issues detected.aioesphomeapi/model.py (2)
1215-1219: SupportsResponseType enum integrates cleanly with APIIntEnumThe
SupportsResponseTypeenum follows the existingAPIIntEnumpattern, uses protobuf-default values (0,1,2,100), and will gracefully map unknown integers toNoneviaconvert(). This is compatible with the converter_field mechanism and back-compat expectations.
1243-1261: UserService.supports_response and ExecuteServiceResponse model are well-structured
UserService.supports_responseusesconverter_fieldwith defaultSupportsResponseType.NONE, so older payloads deserialize correctly and unknown values degrade toNone.ExecuteServiceResponsemirrors the proto shape (call_id/success/error_message/response_data) and respects protobuf defaults via appropriate dataclass defaults.No behavioral or compatibility issues spotted here.
tests/test_model.py (3)
8-69: Import surface updates for new response types are consistentAliases
ExecuteServiceResponsePbandSupportsResponseTypePbfromapi_pb2, and their model counterparts, are introduced in line with existing naming patterns. They are used later in tests and do not introduce unused-symbol issues.Also applies to: 70-151
272-327: Including ExecuteServiceResponse in generic pb conversion test is appropriateExtending
test_basic_pb_conversionswith(ExecuteServiceResponse, ExecuteServiceResponsePb)ensures the new model participates in the samefrom_pbsanity checks as other models, which is a good safety net for future changes.
1780-1910: New tests thoroughly cover SupportsResponseType and ExecuteServiceResponse behavior
test_supports_response_type_convertandtest_supports_response_type_valuesvalidate enum mapping, including unknown values returningNone.test_user_service_with_supports_responsechecks protobuf, dict, and to_dict paths forUserService.supports_response, confirming converter behavior and defaulting toNONE.test_execute_service_responseexercisesfrom_pb(success and error cases), defaults,from_dict, andto_dictforExecuteServiceResponse.These tests accurately reflect the model semantics and should catch common regression modes.
tests/test_client.py (3)
16-56: New ExecuteServiceResponse imports are consistent and correctly aliasedUsing
ExecuteServiceResponsePbfromapi_pb2andExecuteServiceResponseModelfrommodelmatches the proto/model aliasing pattern used elsewhere in the tests and lines up with the client’sexecute_serviceimplementation.Also applies to: 107-146
946-1092: Tests around call_id and inferred return_response cover key combinations
test_execute_service_with_call_idasserts that specifying a non-zerocall_idwithouton_responseleavesreturn_response=False, and that the defaultcall_idis 0, both reflected in the outgoingExecuteServiceRequest.test_execute_service_return_response_combinationssystematically covers the matrix ofcall_id{0, non-zero} ×on_response{None, callback}, ensuring thereturn_responseinference logic inAPIClient.execute_servicebehaves as intended.These tests tightly couple to the intended API contract without over-constraining implementation details.
2245-2331: Callback-based ExecuteService response handling is well-validated
test_execute_service_with_response_callbackexercises the full callback path:
- Verifies that a matching
ExecuteServiceResponsePbproduces a singleExecuteServiceResponseModelinstance with the expected data.- Confirms auto-unsubscribe by asserting that a second response with the same
call_idis ignored.- Ensures responses with a different
call_idare ignored, while a subsequent correctcall_iddelivers exactly one callback.This matches the implementation in
aioesphomeapi.client.execute_serviceand guards against regressions in the call_id filtering/unsubscribe logic.aioesphomeapi/api.proto (2)
857-864: LGTM - Well-designed enum with future-proofing.The
SUPPORTS_RESPONSE_STATUS = 100value is appropriately set high to avoid conflicts with potential future Home Assistant values, and the comment explains this design decision clearly.
901-916: New request/response fields and message are well-structured.The
ExecuteServiceRequestextensions and newExecuteServiceResponsemessage follow existing patterns in the codebase (similar toHomeassistantActionRequest/HomeassistantActionResponse). Thefield_ifdefguards ensure proper conditional compilation.aioesphomeapi/client.py (1)
48-48: New imports properly aligned with proto and model changes.The
ExecuteServiceResponseproto message andExecuteServiceResponseModelmodel class are correctly imported to support the new response handling functionality.Also applies to: 138-138
There was a problem hiding this comment.
Pull request overview
This PR adds client-side support for user-defined service responses from ESPHome devices. It enables ESPHome devices to send response data back to the client when executing user-defined services via the api.respond action.
Key changes:
- Added
ExecuteServiceResponsemessage (id=131) with call_id matching, success/error status, and response data - Added
SupportsResponseTypeenum to indicate service response capabilities (NONE, OPTIONAL, ONLY, STATUS) - Extended
execute_service()method to accept optionalcall_idandon_responsecallback parameters with auto-unsubscribe functionality
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| aioesphomeapi/api.proto | Added SupportsResponseType enum, ExecuteServiceResponse message, and new fields to ExecuteServiceRequest and ListEntitiesServicesResponse |
| aioesphomeapi/api_pb2.py | Auto-generated protobuf Python code reflecting the proto changes |
| aioesphomeapi/model.py | Added Python models for SupportsResponseType enum and ExecuteServiceResponse dataclass |
| aioesphomeapi/core.py | Registered ExecuteServiceResponse message type (id=131) |
| aioesphomeapi/client.py | Extended execute_service() with callback support and auto-unsubscribe logic |
| tests/test_model.py | Added comprehensive tests for new enum conversions, model serialization, and UserService updates |
| tests/test_client.py | Added tests for execute_service with call_id, return_response logic, and callback behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
aioesphomeapi/model.py (1)
1255-1262: Add pylint disable comment for consistency.The
response_datafield is missing the# pylint: disable=invalid-field-callcomment that is consistently used throughout this file for all otherbytesfields withdefault_factory(see lines 149, 155, 600, 1199, 1400, 1616, 1686).Apply this diff:
@_frozen_dataclass_decorator class ExecuteServiceResponse(APIModelBase): call_id: int = 0 # Call ID that matches the original request success: bool = False # Whether the service execution succeeded error_message: str = "" # Error message if success = false response_data: bytes = field( default_factory=bytes - ) # JSON response data from ESPHome + ) # pylint: disable=invalid-field-call + # JSON response data from ESPHome
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
aioesphomeapi/api.proto(3 hunks)aioesphomeapi/model.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Useruff check --fixto check and fix Python linting issues before committing
Useruff formatto format Python code before committing
Files:
aioesphomeapi/model.py
**
⚙️ CodeRabbit configuration file
- Do not generate or add any sequence diagrams
Files:
aioesphomeapi/model.pyaioesphomeapi/api.proto
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: py 3.11 on windows-latest (skip_cython)
🔇 Additional comments (6)
aioesphomeapi/model.py (2)
1216-1221: LGTM!The enum definition is correct and matches the protobuf specification. The STATUS=100 value is intentionally set higher to avoid conflicts with future values, as documented in the proto file.
1250-1252: LGTM!The
supports_responsefield is properly configured with the correct default value and converter, matching the protobuf specification.aioesphomeapi/api.proto (4)
858-865: LGTM!The enum definition is well-structured with a clear comment explaining the STATUS value placement. The values correctly map to the Python model.
879-879: LGTM!The field is properly positioned with a sequential field number and correctly typed to match the new enum.
902-904: LGTM!The new fields are properly guarded and follow the established pattern from
HomeassistantActionRequest. Field numbering is sequential and appropriate.
906-917: LGTM!The message definition is well-structured and mirrors the established pattern from
HomeassistantActionResponse. The ifdef guards and field options are appropriate, and the message ID (131) is unique and sequential.
aioesphomeapi/client.py
Outdated
| service: UserService, | ||
| data: ExecuteServiceDataType, | ||
| *, | ||
| call_id: int = 0, |
There was a problem hiding this comment.
Might be a bit safer to make this a sentinel value so its harder to forget/mis-use the api
There was a problem hiding this comment.
I'm not 100% on the flow here, but does the caller need to know about the call_id?
if on_response is not None:
self._call_id_counter += 1
call_id = self._call_id_counter
return_response = True
else:
call_id = 0
return_response = FalseThere was a problem hiding this comment.
Hmm.. I guess you could have multiple HA instances controlling the device.
There was a problem hiding this comment.
Either way you have collisions. Probably still a cleaner API to have aioesphomeapi have the counter logic instead of HA having to know about it
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
aioesphomeapi/client.py (1)
1319-1371: execute_service callback behavior is correct; consider documenting semanticsThe updated
execute_service:
- Auto‑generates a non‑zero
call_idwheneveron_responseis provided, leaving it0otherwise.- Always passes the provided
return_responseflag through as‑is.- Registers a temporary
ExecuteServiceResponsehandler that:
- Filters by
call_id,- Converts to
ExecuteServiceResponseModel,- Unsubscribes itself after the first matching response.
This matches the new tests and avoids race conditions by registering the callback before sending the request.
As a small improvement, it may be worth adding a brief docstring or API comment clarifying:
- That
call_idis managed internally and monotonically increasing.- That
on_responsecan be used for status‑only responses even whenreturn_responseisFalse.- That if the device never responds, the callback remains registered until the connection is torn down.
That extra documentation would make the new behavior easier for external callers to reason about.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
aioesphomeapi/client.py(6 hunks)tests/test_client.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Useruff check --fixto check and fix Python linting issues before committing
Useruff formatto format Python code before committing
Files:
tests/test_client.pyaioesphomeapi/client.py
**
⚙️ CodeRabbit configuration file
- Do not generate or add any sequence diagrams
Files:
tests/test_client.pyaioesphomeapi/client.py
🧠 Learnings (1)
📚 Learning: 2025-04-16T01:35:06.073Z
Learnt from: bdraco
Repo: esphome/aioesphomeapi PR: 1156
File: tests/test_log_runner.py:187-187
Timestamp: 2025-04-16T01:35:06.073Z
Learning: In the aioesphomeapi project, the maintainers prefer using `asyncio.get_running_loop()` without fallbacks to `asyncio.get_event_loop()`, as this makes incorrect usage fail fast rather than having confusing outcomes. This was decided during a major version increment, making it an intentional breaking change.
Applied to files:
aioesphomeapi/client.py
🧬 Code graph analysis (2)
tests/test_client.py (2)
aioesphomeapi/model.py (5)
ExecuteServiceResponse(1256-1262)APIVersion(100-102)UserService(1244-1252)UserServiceArg(1224-1240)UserServiceArgType(1205-1213)aioesphomeapi/client.py (2)
APIClient(203-1641)execute_service(1318-1372)
aioesphomeapi/client.py (4)
aioesphomeapi/model.py (2)
ExecuteServiceResponse(1256-1262)UserService(1244-1252)tests/test_client.py (1)
on_response(2273-2274)aioesphomeapi/client_base.py (1)
_get_connection(363-371)aioesphomeapi/connection.py (1)
add_message_callback(817-822)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: py 3.11 on windows-latest (skip_cython)
🔇 Additional comments (3)
tests/test_client.py (2)
55-55: ExecuteService call_id and return_response tests match client semanticsThe new imports and tests around
execute_servicecorrectly exercise the updated API:
test_execute_service_with_call_idverifies:
call_id == 0andreturn_response is Falsewhenon_responseis not provided.- Non-zero, monotonically increasing
call_idvalues whenon_responseis set, without assuming a specific starting value.test_execute_service_return_response_combinationscleanly covers the four important combinations ofon_responseandreturn_response, asserting thatreturn_responseis passed through unchanged andcall_idis only generated when a callback is present.This gives good confidence that the client-side behavior for request wiring is stable and backward compatible for existing callers.
Also applies to: 125-125, 1047-1141
2252-2358: End‑to‑end ExecuteServiceResponse callback behavior is well covered
test_execute_service_with_response_callbackthoroughly validates the callback flow:
- Captures the auto-generated
call_idfrom the actualExecuteServiceRequestsent on the connection.- Confirms that a matching
ExecuteServiceResponsePbis converted toExecuteServiceResponseModeland delivered once.- Asserts that the callback auto-unsubscribes after the first matching response.
- Verifies that responses with incorrect
call_idare ignored and that a second request incrementscall_id.No issues found; this is a solid regression test for the new response-handling path.
aioesphomeapi/client.py (1)
7-7: Call‑ID counter and ExecuteServiceResponse wiring are consistentUsing a module‑level
_CALL_ID_COUNTER = itertools.count(1)and reservingcall_id == 0for calls without a response callback matches the new tests and keeps the pb/model imports (ExecuteServiceResponse/ExecuteServiceResponseModel) cleanly separated. No functional or style issues stand out here.Also applies to: 49-49, 139-139, 170-170
aioesphomeapi/client.py
Outdated
| from .util import create_eager_task | ||
|
|
||
| _LOGGER = logging.getLogger(__name__) | ||
| _CALL_ID_COUNTER = itertools.count(1) |
What does this implement/fix?
Add client-side support for user-defined service responses. This allows ESPHome devices to send response data back to the client when executing user-defined services via the
api.respondaction.Changes:
call_idandreturn_responsefields toExecuteServiceRequestExecuteServiceResponsemessage (id=131) withcall_id,success,error_message, andresponse_datafieldsSupportsResponseTypeenum (NONE, OPTIONAL, ONLY, STATUS) to indicate service response capabilitysupports_responsefield toListEntitiesServicesResponse/UserServicemodelexecute_service()to accept optionalon_responsecallback andreturn_responseparametersUsage:
Related PRs:
Types of changes
Related issue or feature (if applicable):
Pull request in esphome (if applicable):
Checklist:
tests/folder).