Skip to content

Conversation

@jesserockz
Copy link
Member

@jesserockz jesserockz commented Sep 30, 2025

What does this implement/fix?

Related PRs:

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

  • fixes

Pull request in esphome (if applicable):

  • esphome/esphome#

Checklist:

  • The code change is tested and works locally.
  • If api.proto was modified, a linked pull request has been made to esphome with the same changes.
  • Tests have been added to verify that the new code works (under tests/ folder).

@codecov
Copy link

codecov bot commented Sep 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (c777ded) to head (6eb3198).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1374   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         3263      3279   +16     
=========================================
+ Hits          3263      3279   +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 30, 2025

CodSpeed Performance Report

Merging #1374 will not alter performance

Comparing jesserockz-2025-458 (6eb3198) with main (c777ded)

Summary

✅ 11 untouched

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 30, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds request/response correlation for Home Assistant actions: extends HomeassistantActionRequest with call_id, wants_response, and response_template; adds HomeassistantActionResponse message and public API; renames handler to on_home_assistant_action_request; adds APIClient.send_homeassistant_action_response; updates core mapping, models, and tests.

Changes

Cohort / File(s) Summary
Protocol buffers
aioesphomeapi/api.proto
Added call_id, wants_response, and response_template fields to HomeassistantActionRequest. Introduced HomeassistantActionResponse message (id 130, SOURCE_CLIENT, no_delay, gated by USE_API_HOMEASSISTANT_ACTION_RESPONSES; optional response_data under USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON).
Client handling & API
aioesphomeapi/client.py, aioesphomeapi/client_base.py
Exported HomeassistantActionResponse. Renamed handler on_home_assistant_service_responseon_home_assistant_action_request and updated callback wiring/parameter names. Added APIClient.send_homeassistant_action_response(call_id, response_data, success, error_message) to construct/send the new response proto.
Core message map
aioesphomeapi/core.py
Imported HomeassistantActionResponse and added mapping 130HomeassistantActionResponse in MESSAGE_TYPE_TO_PROTO.
Models
aioesphomeapi/model.py
Added call_id, wants_response, and response_template to HomeassistantServiceCall. Added frozen dataclass HomeassistantActionResponse with call_id, response_data, success, and error_message.
Tests
tests/test_client.py
Added tests for sending HomeassistantActionResponse (success and error) verifying sent protobuf bytes; tests for HomeassistantServiceCall new fields and HomeassistantActionResponse model creation/validation.
Packaging
setup.py
Package version adjusted (41.12.1 → 41.12.0).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor HA as Home Assistant
  participant Device as ESPHome Device
  participant API as APIClient
  participant Core as Proto/Core

  Note over HA,Device: HomeassistantActionRequest (may include call_id/wants_response/response_template)
  HA->>Device: HomeassistantActionRequest(service, data, call_id, wants_response, response_template)
  Device->>API: on_home_assistant_action_request(msg)
  API->>Device: Callback with HomeassistantServiceCall(call_id, wants_response, response_template, ...)

  rect rgba(200,235,255,0.12)
    note right of Device: Device handles action and optionally responds
    Device->>API: send_homeassistant_action_response(call_id, response_data, success, error_message)
    API->>Core: Encode HomeassistantActionResponse (id=130)
    Core-->>HA: HomeassistantActionResponse(call_id, success, error_message, response_data)
  end

  alt success
    note over HA,Device: success=true
  else failure
    note over HA,Device: success=false, error_message set
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

new-feature

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive The description is largely a standard template with placeholders and does not include any actual details about the implementation of action response support or the specific changes made, making it too generic to inform reviewers about what this PR accomplishes. Please update the pull request description to include a concise summary of the key changes, such as the addition of the HomeassistantActionResponse message in api.proto, the new client API methods for sending and handling responses, and any relevant test enhancements.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately reflects the primary change of the pull request, which is adding support for retrieving action responses from Home Assistant in the aioesphomeapi library, and it is both clear and specific without including unnecessary detail.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 990c7b5 and 6eb3198.

📒 Files selected for processing (2)
  • setup.py (1 hunks)
  • tests/test_client.py (4 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1090afe and 439849f.

📒 Files selected for processing (6)
  • aioesphomeapi/api.proto (1 hunks)
  • aioesphomeapi/client.py (5 hunks)
  • aioesphomeapi/client_base.py (1 hunks)
  • aioesphomeapi/core.py (2 hunks)
  • aioesphomeapi/model.py (1 hunks)
  • tests/test_client.py (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

  • Do not generate or add any sequence diagrams

Files:

  • aioesphomeapi/core.py
  • aioesphomeapi/api.proto
  • aioesphomeapi/model.py
  • aioesphomeapi/client.py
  • tests/test_client.py
  • aioesphomeapi/client_base.py
🧬 Code graph analysis (4)
aioesphomeapi/core.py (1)
aioesphomeapi/model.py (1)
  • HomeassistantActionResponse (1167-1171)
aioesphomeapi/client.py (3)
aioesphomeapi/model.py (1)
  • HomeassistantActionResponse (1167-1171)
aioesphomeapi/client_base.py (2)
  • on_home_assistant_action_request (88-92)
  • _get_connection (363-371)
aioesphomeapi/connection.py (1)
  • send_message (760-762)
tests/test_client.py (2)
aioesphomeapi/model.py (2)
  • HomeassistantActionResponse (1167-1171)
  • HomeassistantServiceCall (1150-1163)
aioesphomeapi/client.py (1)
  • send_homeassistant_action_response (1598-1611)
aioesphomeapi/client_base.py (1)
aioesphomeapi/model.py (6)
  • HomeassistantServiceCall (1150-1163)
  • from_pb (86-87)
  • from_pb (1269-1312)
  • from_pb (1358-1364)
  • from_pb (1398-1409)
  • from_pb (1441-1453)
🔇 Additional comments (2)
aioesphomeapi/api.proto (2)

783-784: LGTM! Field additions are well-designed.

The new call_id and response_template fields properly extend the message for request/response correlation. Field numbering is sequential, types are appropriate, and protobuf's optional field handling ensures backward compatibility.


787-798: LGTM! Well-structured response message.

The HomeassistantActionResponse message follows best practices for request/response correlation:

  • Proper message ID assignment (130)
  • Correct SOURCE_CLIENT direction (response from Home Assistant)
  • call_id enables matching responses to requests
  • success + error_message pattern supports clear error handling
  • no_delay=true ensures timely delivery

@bdraco bdraco added the minor label Sep 30, 2025
@bdraco
Copy link
Member

bdraco commented Sep 30, 2025

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c8a1d6 and dbe15e4.

📒 Files selected for processing (1)
  • aioesphomeapi/client.py (5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

  • Do not generate or add any sequence diagrams

Files:

  • aioesphomeapi/client.py
🧬 Code graph analysis (1)
aioesphomeapi/client.py (3)
aioesphomeapi/model.py (1)
  • HomeassistantActionResponse (1168-1174)
aioesphomeapi/client_base.py (2)
  • on_home_assistant_action_request (88-92)
  • _get_connection (363-371)
aioesphomeapi/connection.py (1)
  • send_message (760-762)
🔇 Additional comments (4)
aioesphomeapi/client.py (4)

49-50: LGTM!

The import of HomeassistantActionResponse is correctly added to support the new public API method.


101-101: LGTM!

The import of on_home_assistant_action_request aligns with the handler rename from client_base.py and is used correctly in the subscription methods.


404-411: LGTM!

The handler update to use on_home_assistant_action_request is correct. The partial application properly binds the callback, leaving msg as the remaining parameter for the subscription mechanism.


943-982: LGTM!

The combined subscription method correctly uses on_home_assistant_action_request at line 963, maintaining consistency with the rename throughout the codebase.

jesserockz and others added 4 commits October 7, 2025 21:37
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@bdraco bdraco merged commit f0af120 into main Oct 8, 2025
13 of 14 checks passed
@bdraco bdraco deleted the jesserockz-2025-458 branch October 8, 2025 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants