Support for reconfigure and accompanying improvements#191
Merged
sveinse merged 5 commits intocustom-components:masterfrom Jul 15, 2025
Merged
Support for reconfigure and accompanying improvements#191sveinse merged 5 commits intocustom-components:masterfrom
sveinse merged 5 commits intocustom-components:masterfrom
Conversation
… for requests and streams. * Refactor of configuration flows * Add support for reconfigure config flow * Add support for reauthenticate config flow * Refactor user setup config flow * Refactor stream methods * Rename Installation._stream() to stream_main() * Move Account.update() to Installation.stream_update() * Make sure only one stream can run * Implement Installation.stream_close() * Refactor API methods * Remove Account.check_login() * Add Account.login() * Rename Account._retry_request() to _request_worker() * Improve integration setup * Move Account initalization to early setup and fail there * Handle streams from the coordinator * Move first-time-setup to ZaptecUpdateCoordinator._first_time_setup()
steinmn
reviewed
Jul 15, 2025
custom_components/zaptec/api.py
Outdated
Comment on lines
451
to
458
| if (charger := self._account.map.get(charger_id)) is None: | ||
| _LOGGER.warning("Got update for unknown charger id %s", charger_id) | ||
| return | ||
|
|
||
| charger = self.map.get(charger_id) | ||
| if charger is None: | ||
| _LOGGER.warning("Got update for unknown charger id %s", charger_id) | ||
| return |
Contributor
There was a problem hiding this comment.
This looks like a copypasta gone wrong, I think the first charger assignment is the correct one, and the second is an artifact from before the function was moved from the account object?
Collaborator
Author
There was a problem hiding this comment.
Good catch. It is right, but I changed the text a little bit to be a little more informative of the error.
Contributor
There was a problem hiding this comment.
But the installation class doesn't have a map attribute, that is in the account class:
2025-07-15 13:14:38.060 ERROR (MainThread) [custom_components.zaptec.api] Couldn't process stream message: 'map'
Traceback (most recent call last):
File "/workspaces/zaptec/custom_components/zaptec/api.py", line 158, in __getattr__
return self._attrs[to_under(key)]
~~~~~~~~~~~^^^^^^^^^^^^^^^
KeyError: 'map'
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/workspaces/zaptec/custom_components/zaptec/api.py", line 402, in stream_main
self.stream_update(json_result)
~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^
File "/workspaces/zaptec/custom_components/zaptec/api.py", line 467, in stream_update
charger = self.map.get(charger_id)
^^^^^^^^
File "/workspaces/zaptec/custom_components/zaptec/api.py", line 160, in __getattr__
raise AttributeError(exc) from exc
AttributeError: 'map'
Collaborator
Author
There was a problem hiding this comment.
Oh, right. How did that miss my testing? I'll fix this.
Collaborator
Author
|
I isolated out the formatting changes into a separate PR, so this PR should now be easier and more on-point to review. |
steinmn
reviewed
Jul 15, 2025
custom_components/zaptec/api.py
Outdated
Comment on lines
463
to
470
| if (charger := self._account.map.get(charger_id)) is None: | ||
| _LOGGER.warning("Got update for unknown charger id %s", charger_id) | ||
| return | ||
|
|
||
| charger = self.map.get(charger_id) | ||
| if charger is None: | ||
| _LOGGER.warning("Got update for unregistered charger, id %s", charger_id) | ||
| return |
Contributor
There was a problem hiding this comment.
Suggested change
| if (charger := self._account.map.get(charger_id)) is None: | |
| _LOGGER.warning("Got update for unknown charger id %s", charger_id) | |
| return | |
| charger = self.map.get(charger_id) | |
| if charger is None: | |
| _LOGGER.warning("Got update for unregistered charger, id %s", charger_id) | |
| return | |
| if (charger := self._account.map.get(charger_id)) is None: | |
| _LOGGER.warning("Got update for unregistered charger, id %s", charger_id) | |
| return |
Collaborator
Author
There was a problem hiding this comment.
New version pushed
* Minor docstring updates
Collaborator
Author
|
Thank you @steinmn |
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.
This PR started with the intention to fix #76: Reconfigure the Zaptec integration. However, once pulling the threads in the code revealed a few things that had to be fixed for this to work. When reconfiguring an integration, it needs to shut down the old to start a new, and the current design didn't do that very well. In particular the stream methods didn't clean up properly and were failing miserably on reconfigure.
Next the setup weren't implemented in a good way, as any login/password issues should be handled earlier so it had to be improved. We needed a separate login method instead of just refreshing the token when doing the first access.
There are a few method being changed from this PR. However the
api.pyfile exists to serve the integration, so I think its ok to do these changes.This PR is the first in a series to move towards a more Zaptec API friendly design.