Change execute_service to be async to handle timeouts internally

[jesserockz]

What does this implement/fix?

This alters execute_service to be an async function so that it can timeout while waiting for a response for a device.

Before this, the message subscription would forever be in memory if the device did not respons, even if the calling code (HA) had already given up.

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]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #1443   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         3335      3342    +7     
=========================================
+ Hits          3335      3342    +7     

☔ 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]

CodSpeed Performance Report

Merging #1443 will not alter performance

_{Comparing jesserockz-2025-545 (23fb444) with main (71362b5)}

Summary

✅ 11 untouched

[coderabbitai]

Walkthrough

execute_service in aioesphomeapi/client.py was converted from synchronous to asynchronous, a module-level DEFAULT_EXECUTE_SERVICE_TIMEOUT = 30.0 constant was added, and the method now optionally waits for a response using an asyncio.Event with guaranteed callback unsubscription; tests in tests/test_client.py were updated to await the calls and reflect timeout/async behavior.

Changes

Cohort / File(s)	Change Summary
Async method conversion aioesphomeapi/client.py	Added DEFAULT_EXECUTE_SERVICE_TIMEOUT = 30.0. Converted execute_service from sync to async def, signature now `return_response: bool
Test updates tests/test_client.py	Updated tests to await execute_service calls and adapt to async behavior. Added/changed tests to assert asyncio.TimeoutError for timeouts, use short timeouts for non-blocking checks, run expected-response flows as tasks and await after simulating device responses, and preserve assertions around call_id generation and mismatched-response handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Inspect callback registration/unregistration correctness across success, timeout, and exception paths in aioesphomeapi/client.py.
• Verify asyncio.Event signaling and asyncio.wait_for usage to avoid race conditions between callback and waiter.
• Confirm tests in tests/test_client.py comprehensively cover: timeout, returned-response path, fire-and-forget path, and mismatched call_id handling.
• Check public API/backwards-compatibility implications for callers expecting a synchronous execute_service.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly describes the main change: converting execute_service to async to handle timeouts internally, which aligns with the primary objective and implementation.
Description check	✅ Passed	The description explains the purpose of the change (handling timeouts, preventing memory leaks from subscriptions) and relates to the changeset modifications in both client.py and test_client.py.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jesserockz-2025-545

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/test_client.py (2)

1046-1092: call_id generation and timeout behavior are well covered; confirm error-type choice

These tests nicely validate that:

• call_id stays 0 when return_response is left as default,
• non‑zero call_ids are auto‑generated when return_response is set, and
• the counter increments across calls, using very short timeouts to keep tests fast (which matches the project’s preference for sub‑second tests, based on learnings).

One thing to double‑check: here you assert asyncio.TimeoutError directly from execute_service, whereas other APIs in this module typically surface TimeoutAPIError for timeouts. If you later decide to wrap this in TimeoutAPIError for consistency, these assertions will need to change accordingly; if the intention is to expose the raw asyncio.TimeoutError for this low‑level helper, the tests are already aligned.

---

2239-2336: execute_service_with_response test correctly exercises response correlation and ignores mismatched call_ids

This test does a good job of:

• Capturing the sent ExecuteServiceRequest to verify a non‑zero auto‑generated call_id.
• Simulating a matching ExecuteServiceResponse and asserting that execute_service returns the expected ExecuteServiceResponseModel.
• Verifying that a response with the wrong call_id is ignored and the task remains pending until the correct call_id arrives.

One optional hardening you might consider (not required) is passing an explicit, relatively small timeout when calling execute_service here so that, if the correlation logic regresses and no response ever matches, the test fails fast instead of potentially waiting on the default long timeout.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ed1ad9 and 23fb444.

📒 Files selected for processing (1)

• tests/test_client.py (12 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: Use ruff check --fix to check and fix Python linting issues before committing
Use ruff format to format Python code before committing

Files:

• tests/test_client.py

**

⚙️ CodeRabbit configuration file

• Do not generate or add any sequence diagrams

Files:

• tests/test_client.py

🧠 Learnings (1)

📓 Common learnings

Learnt from: bdraco
Repo: esphome/aioesphomeapi PR: 1065
File: .github/workflows/ci.yml:114-114
Timestamp: 2025-02-14T16:26:00.072Z
Learning: In aioesphomeapi, all tests are designed to complete within 1 second. The CI configuration uses a 4-second timeout which provides sufficient buffer for test execution across all environments.

Learnt from: bdraco
Repo: esphome/aioesphomeapi PR: 999
File: aioesphomeapi/client.py:610-612
Timestamp: 2024-11-23T18:39:41.119Z
Learning: In the `aioesphomeapi/client.py` file within the `APIClient` class, it's intentional to catch `BaseException` in methods like `bluetooth_device_connect` to ensure cleanup is performed after cancellations or unexpected exceptions, preventing connection leaks.

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.

🧬 Code graph analysis (1)

tests/test_client.py (2)

tests/conftest.py (2)

• auth_client (71-85)
• api_client (209-238)

aioesphomeapi/client.py (2)

• execute_service (1317-1381)
• APIClient (202-1650)

⏰ 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: Run benchmarks

🔇 Additional comments (3)

tests/test_client.py (3)

945-1044: execute_service async migration and arg mapping tests look solid

Awaiting auth_client.execute_service in both the KeyError path and the various arg/api_version cases is consistent with the new async signature, and the coverage of bool/int/float/string/array types plus legacy legacy_int and arg-order handling is preserved. I don’t see issues here.

---

1094-1128: return_response tri‑state combinations test the new semantics comprehensively

The test_execute_service_return_response_combinations function gives good coverage of the three cases:

• return_response=None → call_id == 0, no waiting.
• return_response=True → non‑zero call_id, request field set to True, and a timeout when no response arrives.
• return_response=False → non‑zero call_id, request field set to False, and the same wait/timeout semantics.

The use of a 0.01‑second timeout keeps these negative‑path tests cheap while still verifying the internal wait logic. No changes needed.

---

3338-3350: Updated execute_service usage after disconnect is consistent

Switching the “after connection closed” assertion to await client.execute_service(service, {}) keeps the existing behavior check (raising APIConnectionError) while matching the new async contract for execute_service. This is consistent with how other client methods in the test are awaited.