Increase vehicle data refresh timeout from 10s to 30s#445
Conversation
My integration showed the car as unavailable almost permanently for the past few weeks. After checking the logs, the main culprit was timeouts: increasing the timeout shows the full data fetch actually takes ~21s. It's a long chain of sequential calls to Smart's cloud (vehicle status -> OTA info -> journal authorisation -> trip journal -> ...). The old 10s timeout tripped asyncio.timeout -> UpdateFailed -> entities went unavailable even when the API was healthy. Raise the timeout to 30s via a named API_TIMEOUT constant.
📝 WalkthroughWalkthroughIntroduces a new ChangesCoordinator Timeout Update
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (1)
custom_components/smarthashtag/coordinator.py (1)
24-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving
API_TIMEOUTtoconst.py.The constant is well-named and documented, but per the repo's Python guidelines, constants should live in
const.pyrather than being defined inline in coordinator.py.♻️ Suggested move
-# Timeout (seconds) for a full vehicle data refresh. A healthy refresh is a -# chain of sequential Smart/Geely cloud calls that can legitimately take ~20s. -API_TIMEOUT = 30 +from .const import API_TIMEOUTAnd in
const.py:# Timeout (seconds) for a full vehicle data refresh. A healthy refresh is a # chain of sequential Smart/Geely cloud calls that can legitimately take ~20s. API_TIMEOUT = 30As per coding guidelines, "Use constants from
const.pyinstead of magic strings/numbers."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@custom_components/smarthashtag/coordinator.py` around lines 24 - 27, Move the API_TIMEOUT constant out of coordinator.py and into const.py to match the repo’s constant placement guidelines. Update the SmartHashtag coordinator to import API_TIMEOUT from const.py, and remove the inline definition so the Coordinator logic continues using the same symbol without changing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@custom_components/smarthashtag/coordinator.py`:
- Around line 24-27: Move the API_TIMEOUT constant out of coordinator.py and
into const.py to match the repo’s constant placement guidelines. Update the
SmartHashtag coordinator to import API_TIMEOUT from const.py, and remove the
inline definition so the Coordinator logic continues using the same symbol
without changing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38b88dfa-d6dc-4b28-b1b9-09704943c405
📒 Files selected for processing (1)
custom_components/smarthashtag/coordinator.py
|
after a couple of days, I can say it never worked that well for so long. Never unavailable. Could it be because I have 3 cars it was always slower than everyone else so I had more timeouts? |
I shouldn't have posted this... tonight I got back to the usual hours of "unavailable". But this time just reloading the integration was enough like it used to be. I think it proves this is a fix in itself already I am working on a different branch to test improvement to the login journey resilence. I think this is the main issue but this timeout in this PR is already helping. |
1 participant
Problem
My integration showed the car as
unavailablealmost permanently for the past few weeks. After checking the logs, the main culprit was timeouts.The data update coordinator wraps
account.get_vehicles()inasyncio.timeout(10). But a full successful refresh actually takes ~21s, because it's a long chain of sequential calls to Smart's cloud:Debug log from a healthy refresh:
With the 10s timeout, this fetch reliably trips
asyncio.timeout→ raisesUpdateFailed→ all entities gounavailable, even when the API is perfectly healthy.Change
Raise the timeout to 30s via a named
API_TIMEOUTconstant (matching the existingMAX_TRANSIENT_FAILURESstyle), so a normal refresh can complete and entities stay available.Possible follow-up
Expose this as a configurable option in the options flow alongside
scan_interval, since fetch time varies with how many sub-calls the backend performs.Summary by CodeRabbit