Add smart #5 support, fix login flow#414
Open
slider wants to merge 3 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR adds retry logic with transient connection error handling to the SmartHashtag integration configuration flow, bumps the integration version to 0.8.3 and the pysmarthashtag dependency to 0.8.2, and updates branding across documentation and docstrings to include "Smart ChangesConnection Retry Logic and Smart
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ConfigFlow
participant SmartAccount
participant Retry Logic
participant Backend
User->>ConfigFlow: Submit credentials
ConfigFlow->>Retry Logic: Start retry loop (max 5 attempts)
loop Retry up to 5 times
Retry Logic->>SmartAccount: login()
SmartAccount->>Backend: HTTP request
alt Connection success
Backend-->>SmartAccount: Success response
SmartAccount-->>Retry Logic: ✓ Complete
else Connection failure (500, 502, 503, 504, timeout)
Backend-->>SmartAccount: Error / Timeout
SmartAccount-->>Retry Logic: Raise httpx.HTTPStatusError
Retry Logic->>Retry Logic: Classify as retryable
Retry Logic->>Retry Logic: Sleep with backoff (if not last attempt)
end
end
Retry Logic-->>ConfigFlow: CannotConnect or SmartAPIError
ConfigFlow-->>User: Show "cannot_connect" or "auth" error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
custom_components/smarthashtag/config_flow.py (2)
267-292: Error classification helpers are clear and correct.The HTTP status codes
{429, 500, 502, 503, 504}are appropriate choices for transient/retryable server errors.Minor:
ConnectionErroris a subclass ofOSErrorin Python, so listing both in theisinstancecheck is redundant. Not a bug, just a slight redundancy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/smarthashtag/config_flow.py` around lines 267 - 292, Remove the redundant ConnectionError check in _is_connection_error: because ConnectionError is a subclass of OSError, drop ConnectionError from the isinstance tuple and keep OSError (and the other exceptions) so behavior is unchanged; update only the tuple in _is_connection_error, leaving _is_retryable_error and other logic intact.
226-265: Retry logic is well-implemented with appropriate backoff.The retry mechanism correctly:
- Retries up to 5 times with capped backoff (1s, 2s, 3s, 4s, 4s)
- Distinguishes connection errors (→
CannotConnect) from auth errors (→SmartAPIError)- Wraps unexpected HTTP errors appropriately
Line 265 appears unreachable given the current control flow (every iteration either returns on success or raises on the final failed attempt). Consider removing it or adding a comment explaining it's a defensive fallback:
Optional: Add clarifying comment
raise SmartAPIError(str(exception)) from exception + # Defensive fallback: should be unreachable as each iteration either returns or raises raise SmartAPIError("Could not validate credentials")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@custom_components/smarthashtag/config_flow.py` around lines 226 - 265, The final line raising SmartAPIError("Could not validate credentials") is unreachable because the for-loop either returns on success or raises on failure; remove that unreachable raise and, if desired, add a brief comment above the loop referencing that all failure paths raise within the loop (point to the retry loop using self._LOGIN_RETRIES, SmartAccount(...).login(), .get_vehicles(), and the exception handling branches using _is_retryable_error and _is_connection_error) so future readers know the post-loop raise was intentionally removed.tests/test_config_flow.py (1)
104-145: Good test coverage for the new transient error path.The test correctly:
- Simulates an HTTP 500 server error using
httpx.HTTPStatusError- Patches
asyncio.sleepto avoid delays during retries- Verifies the flow returns
{"base": "cannot_connect"}after retry exhaustionConsider adding a test case for partial retry success (e.g., fail twice then succeed) to verify the retry mechanism works correctly when the backend recovers mid-retry. This would complement the exhausted-retries test.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_config_flow.py` around lines 104 - 145, Add a new async test similar to test_form_with_transient_connection_error that confirms the retry loop can succeed mid-retry: patch custom_components.smarthashtag.config_flow.SmartAccount to return an instance whose login AsyncMock first raises httpx.HTTPStatusError twice and then returns successfully on the third call, patch asyncio.sleep to an AsyncMock to skip delays, run the config flow via hass.config_entries.flow.async_init and async_configure with the same credentials, and assert the final result is a successful entry (type "create_entry") and contains expected data instead of {"base":"cannot_connect"} to validate partial-retry success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@custom_components/smarthashtag/__init__.py`:
- Line 1: Fix the typo in the module docstring by replacing "intergration" with
"integration" in the top-level string in __init__.py (the module docstring that
currently reads 'Smart `#1/`#3/#5 for intergration with Home Assistant.').
In `@custom_components/smarthashtag/sensor.py`:
- Line 1: Update the module docstring in sensor.py to correct the typo: change
"intergration" to "integration" in the top-line docstring so the file now reads:
"Sensor platform for Smart `#1/`#3/#5 integration."; no other code changes
required.
---
Nitpick comments:
In `@custom_components/smarthashtag/config_flow.py`:
- Around line 267-292: Remove the redundant ConnectionError check in
_is_connection_error: because ConnectionError is a subclass of OSError, drop
ConnectionError from the isinstance tuple and keep OSError (and the other
exceptions) so behavior is unchanged; update only the tuple in
_is_connection_error, leaving _is_retryable_error and other logic intact.
- Around line 226-265: The final line raising SmartAPIError("Could not validate
credentials") is unreachable because the for-loop either returns on success or
raises on failure; remove that unreachable raise and, if desired, add a brief
comment above the loop referencing that all failure paths raise within the loop
(point to the retry loop using self._LOGIN_RETRIES, SmartAccount(...).login(),
.get_vehicles(), and the exception handling branches using _is_retryable_error
and _is_connection_error) so future readers know the post-loop raise was
intentionally removed.
In `@tests/test_config_flow.py`:
- Around line 104-145: Add a new async test similar to
test_form_with_transient_connection_error that confirms the retry loop can
succeed mid-retry: patch custom_components.smarthashtag.config_flow.SmartAccount
to return an instance whose login AsyncMock first raises httpx.HTTPStatusError
twice and then returns successfully on the third call, patch asyncio.sleep to an
AsyncMock to skip delays, run the config flow via
hass.config_entries.flow.async_init and async_configure with the same
credentials, and assert the final result is a successful entry (type
"create_entry") and contains expected data instead of {"base":"cannot_connect"}
to validate partial-retry success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1506c58-5b95-4f3f-8f52-6c1e5b1709da
📒 Files selected for processing (14)
README.mdcustom_components/smarthashtag/__init__.pycustom_components/smarthashtag/climate.pycustom_components/smarthashtag/config_flow.pycustom_components/smarthashtag/const.pycustom_components/smarthashtag/coordinator.pycustom_components/smarthashtag/manifest.jsoncustom_components/smarthashtag/sensor.pycustom_components/smarthashtag/sensor_groups/__init__.pycustom_components/smarthashtag/switch.pycustom_components/smarthashtag/translations/de.jsoncustom_components/smarthashtag/translations/en.jsonhacs.jsontests/test_config_flow.py
DasBasti
requested changes
May 3, 2026
| "requirements": ["pysmarthashtag==0.8.1"], | ||
| "version": "0.8.2" | ||
| "requirements": ["pysmarthashtag==0.8.2"], | ||
| "version": "0.8.3" |
Owner
There was a problem hiding this comment.
please don't increase the version. This change is not just a bugfix
| "issue_tracker": "https://github.com/DasBasti/SmartHashtag/issues", | ||
| "requirements": ["pysmarthashtag==0.8.1"], | ||
| "version": "0.8.2" | ||
| "requirements": ["pysmarthashtag==0.8.2"], |
Owner
There was a problem hiding this comment.
do not make this update in a PR that does not relate to the updates in pysmarthashtag
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR improves Smart #5 support and makes login/setup more resilient when the Smart auth backend is unstable.
Problem
Config flow login could fail with generic auth errors even when credentials were correct, due to intermittent Smart backend/API failures (for example temporary 5xx responses). This made initial setup unreliable.
Changes
config_flow.cannot_connectauthcannot_connecttranslation strings in:custom_components/smarthashtag/translations/en.jsoncustom_components/smarthashtag/translations/de.json#1/#3/#5:pysmarthashtag==0.8.20.8.3Validation
pytest -q tests/test_config_flow.pypassedpytest -qpassedruff check custom_components testspassedNotes
This does not remove all possible login failures (backend can still be down), but it improves UX by retrying and surfacing the correct error class (
cannot_connectvsauth).Summary by CodeRabbit
New Features
Bug Fixes
Chores