GrowattServer: Add "Account is locked" message #142874
Conversation
home-assistant
bot
added
bugfix
cla-signed
has-tests
integration: growatt_server
small-pr
Add tests
…imo-seitz/homeassistant-core into growatt_server_account_locked_message
| if login_response["msg"] == LOGIN_INVALID_AUTH_CODE: | ||
| # Invalid auth code | ||
| return self._async_show_user_form({"base": "invalid_auth"}) | ||
| if login_response["msg"] == LOGIN_LOCKED_CODE: | ||
| # Account locked | ||
| return self._async_show_user_form({"base": "account_locked"}) | ||
| # Unknown error | ||
| return self._async_show_user_form({"base": "unknown_error"}) |
There was a problem hiding this comment.
You can do a dict lookup
{
LOGIN_INVALID_AUTH_CODE: "invalid_auth",
LOGIN_LOCKED_CODE: "account_locked"
}.get(login_response["msg"], "unknown_error")To make this more neat
| if login_response["msg"] == LOGIN_INVALID_AUTH_CODE: | ||
| raise ConfigEntryError("Username, Password or URL may be incorrect!") | ||
| if login_response["msg"] == LOGIN_LOCKED_CODE: | ||
| raise ConfigEntryError(login_response["error"]) |
There was a problem hiding this comment.
The api has a request limit which you can reach, if you use the same account for the HA-Integration and the App on the phone. A screenshot of that is inside the linkes issue #121703
| raise ConfigEntryError("Username, Password or URL may be incorrect!") | ||
| if login_response["msg"] == LOGIN_LOCKED_CODE: | ||
| raise ConfigEntryError(login_response["error"]) | ||
| raise ConfigEntryError( |
There was a problem hiding this comment.
when would this happen? This means someone has to restart whole of HA in order to make it work again
There was a problem hiding this comment.
This are the responses of the growatt cloud. There are a unknown number of causes, why a login isn't valid anymore. The implementors of the used api https://github.com/indykoning/PyPi_GrowattServer doesn't mention details there. When the user changes their password in the growatt cloud, they have to update the integratrion. The locked state will be unlocked after 24 hours, if the user hit the request limit. IMHO a restart is not necassary. Maybe a reconfiguration of the integration.
There was a problem hiding this comment.
Is that 24 hour limit a hard limit? Or will it be reset to 24h again when I do more requests?
Should we ask the user to reauthenticate in this scenario? Or will it recover?
There was a problem hiding this comment.
I don't really know, but it seems not to reset if you do more request within these 24 hours. The PyPi_GrowattServer is a reverse engineering work, so there's no offical doc of the api. The response in that case of a "lock" contains a lockDuration property which states 24 in the beginning.
{ 'error': 'Current account has been locked for 24 hours',
'lockDuration': '24',
'msg': '507',
'success': False}But these 24 doesn't change over time.
At the Growatt webpage you can add "visitor" accounts. It seems to be a good idea to use a seperate account for the integration, so the (unkown) limit shouldn't be hit.
| "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]" | ||
| "invalid_auth": "[%key:common::config_flow::error::invalid_auth%]", | ||
| "account_locked": "Account is locked for 24 hours. Please try again later.", | ||
| "unknown_error": "Unknown error occured." |
There was a problem hiding this comment.
You can use a common translation key here
There was a problem hiding this comment.
This would be test_sensor.py instead.
There was a problem hiding this comment.
Why is this called test_sensor_init? This should be in test_init.py (and I think the code in sensor/__init__.py should be moved to __init__.py in general, but that is outside of the scope of this PR)
| LOGIN_INVALID_AUTH_CODE, | ||
| LOGIN_LOCKED_CODE, | ||
| ) | ||
| from homeassistant.components.growatt_server.sensor import get_device_list |
There was a problem hiding this comment.
We should not touch internals directly, we should rather patch the library and have HA set up the integration
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Use the common translation key.
Breaking change
Proposed change
If the account is temporarily locked, display an aproperiate message.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: