-
Notifications
You must be signed in to change notification settings - Fork 0
v1.2.1 #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bugfix: API key in API ClientError message wasnt redacted feature: added custom base API URL config and reconfig option change: refactored config_flow navigation for better readability Changes to be committed: modified: custom_components/diveracontrol/__init__.py modified: custom_components/diveracontrol/config_flow.py modified: custom_components/diveracontrol/const.py modified: custom_components/diveracontrol/divera_api.py modified: custom_components/diveracontrol/divera_credentials.py modified: custom_components/diveracontrol/manifest.json modified: custom_components/diveracontrol/translations/de.json modified: custom_components/diveracontrol/translations/en.json
feature: updated tests to work with configurable base url Changes to be committed: modified: custom_components/diveracontrol/services.yaml modified: custom_components/diveracontrol/translations/de.json modified: custom_components/diveracontrol/translations/en.json modified: tests/test_config_flow.py modified: tests/test_divera_api.py modified: tests/test_divera_credentials.py modified: tests/test_init.py modified: tests/test_utils.py
Changes to be committed: modified: custom_components/diveracontrol/services.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the API configuration to support customizable base URLs by introducing a base_api_url parameter throughout the codebase. The version is bumped from 1.2.0 to 1.2.1.
- Adds
base_api_urlparameter to API initialization, credential validation, and configuration flows - Refactors API request handling to separate URL construction from endpoint paths
- Replaces menu-based config flow with form-based selection for better error display
- Removes device targets from service definitions, keeping only entity targets
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| custom_components/diveracontrol/manifest.json | Updates version from 1.2.0 to 1.2.1 |
| custom_components/diveracontrol/const.py | Adds D_BASE_API_URL constant for the new parameter |
| custom_components/diveracontrol/init.py | Extracts and passes base_url parameter to DiveraAPI initialization; updates migration logic |
| custom_components/diveracontrol/divera_api.py | Refactors API request method to accept partial URLs and construct full URLs using base URL; improves error messages |
| custom_components/diveracontrol/divera_credentials.py | Adds base_api_url parameter to validation methods |
| custom_components/diveracontrol/config_flow.py | Replaces menu with form-based method selection; adds base URL field to all configuration forms; improves error handling |
| custom_components/diveracontrol/translations/*.json | Updates translations to support form-based method selection and adds base URL field labels |
| custom_components/diveracontrol/services.yaml | Removes device targets from service definitions, keeping only entity targets |
| tests/test_*.py | Updates tests to pass BASE_API_URL parameter and adjusts assertions for config flow changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ucr_id: str = config_entry.data.get(D_UCR_ID) or "" | ||
| cluster_name: str = config_entry.data.get(D_CLUSTER_NAME) or "" | ||
| api_key: str = config_entry.data.get(D_API_KEY) or "" | ||
| base_url: str = config_entry.data.get(D_BASE_API_URL) or "" |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base URL extraction uses or \"\" which will result in an empty string if D_BASE_API_URL is not in config_entry.data (e.g., for existing installations upgrading to this version). This will cause API requests to fail. Consider providing a fallback to BASE_API_URL constant: base_url: str = config_entry.data.get(D_BASE_API_URL, BASE_API_URL)
|
|
||
| except ClientError as err: | ||
| raise HomeAssistantError(f"Failed to connect to Divera API: {err}") from err | ||
| url = self._redact_url(err.url.path_qs) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing err.url.path_qs assumes that the ClientError has a url attribute with a path_qs property. This may not always be the case depending on the type of ClientError. Consider adding a check or try-except block to handle cases where err.url is None or doesn't have path_qs attribute to avoid AttributeError.
| url = self._redact_url(err.url.path_qs) | |
| # Safely access err.url.path_qs if available | |
| if hasattr(err, "url") and err.url is not None and hasattr(err.url, "path_qs"): | |
| url = self._redact_url(err.url.path_qs) | |
| else: | |
| url = self._redact_url(str(getattr(err, "url", "Unknown URL"))) |
| if not existing_entry: | ||
| return self.async_abort(reason="hub_not_found") | ||
| entry_id: str = self.context["entry_id"] | ||
| config_entry: ConfigEntry = self.hass.config_entries.async_get_entry(entry_id) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reconfigure flow now assumes async_get_entry() will always return a valid ConfigEntry and assigns it directly. If the entry doesn't exist, this will raise an AttributeError instead of returning None. The previous code handled the None case explicitly. Consider adding a null check: if not config_entry: return self.async_abort(reason=\"hub_not_found\") or use the previous try-except pattern.
| config_entry: ConfigEntry = self.hass.config_entries.async_get_entry(entry_id) | |
| config_entry: ConfigEntry = self.hass.config_entries.async_get_entry(entry_id) | |
| if not config_entry: | |
| return self.async_abort(reason="hub_not_found") |
| self.update_interval_alarm = user_input[D_UPDATE_INTERVAL_ALARM] | ||
| # persist non-sensitive input so we can prefill forms if the user | ||
| # returns to the menu after validation errors | ||
| cur_step_id = self.cur_step.get("step_id") if self.cur_step else None |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code attempts to access self.cur_step which is not a documented attribute of ConfigFlow. This appears to be trying to determine the current step, but cur_step is not a standard ConfigFlow attribute. This may cause AttributeError. Consider passing the step_id as a parameter to _validate_and_proceed or using a different approach to track which form the user is on.
Changes to be committed: modified: .github/workflows/ci_pipeline.yml modified: .github/workflows/hacs.yml modified: .github/workflows/hass.yml
Changes to be committed: modified: .github/workflows/ci_pipeline.yml modified: .github/workflows/hacs.yml modified: .github/workflows/hass.yml modified: .github/workflows/test.yml
bugfix: API key in API ClientError message wasnt redacted
feature: added custom base API URL config and reconfig option
change: refactored config_flow navigation for better readability
feature: updated tests to work with configurable base url
redefined services.yaml to match HA standards