-
-
Notifications
You must be signed in to change notification settings - Fork 92
Solis development, fixes for component restarts #3179
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
DB Manager crash on exit with different DB
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 adds comprehensive Solis Cloud API integration for Predbat and fixes component restart behavior with exponential backoff. The changes introduce a new 2642-line Solis integration module, update the ComponentBase lifecycle management with backoff retry logic, add comprehensive unit tests for ComponentBase, and include defensive code improvements.
Key Changes
- Added new Solis Cloud API integration with full inverter control via REST API and HMAC-SHA1 authentication
- Implemented exponential backoff retry mechanism in ComponentBase for component startup failures (60s initial, max 128 minutes)
- Added comprehensive unit tests for ComponentBase lifecycle and backoff behavior
- Added defensive hasattr check in db_manager before calling _commit_db
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/predbat/solis.py | New 2642-line Solis Cloud API integration with inverter discovery, control, time window management, and HA entity publishing |
| apps/predbat/component_base.py | Updated start() method with exponential backoff logic for component restart resilience |
| apps/predbat/tests/test_component_base.py | New test suite with 5 tests covering ComponentBase lifecycle, backoff, exception handling, and stop behavior |
| apps/predbat/config.py | Added SolisCloud inverter type definition and 5 new config options (api_key, api_secret, inverter_sn, automatic, base_url) |
| apps/predbat/components.py | Registered Solis API component with event filtering and configuration mapping |
| apps/predbat/predbat.py | Updated PREDBAT_FILES list to include solis.py and solax.py |
| apps/predbat/db_manager.py | Added defensive hasattr check before calling _commit_db to prevent AttributeError |
| apps/predbat/unit_test.py | Registered new component_base test suite in TEST_REGISTRY |
Comments suppressed due to low confidence (1)
apps/predbat/component_base.py:54
- This call to ComponentBase.initialize in an initialization method is overridden by AlertFeed.initialize.
This call to ComponentBase.initialize in an initialization method is overridden by HAInterface.initialize.
This call to ComponentBase.initialize in an initialization method is overridden by OctopusAPI.initialize.
This call to ComponentBase.initialize in an initialization method is overridden by OhmeAPI.initialize.
This call to ComponentBase.initialize in an initialization method is overridden by SolaxAPI.initialize.
This call to ComponentBase.initialize in an initialization method is overridden by SolisAPI.initialize.
This call to ComponentBase.initialize in an initialization method is overridden by PredbatMCPServer.initialize.
self.initialize(**kwargs)
| "inverter_sn": {"required": False, "config": "solis_inverter_sn"}, | ||
| "automatic": {"required": False, "config": "solis_automatic", "default": False}, | ||
| "base_url": {"required": False, "config": "solis_base_url", "default": "https://www.soliscloud.com:13333"}, | ||
| "control_enable": {"required": False, "config": "solis_control_enable", "default": True}, |
Copilot
AI
Jan 4, 2026
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 config entry "solis_control_enable" is referenced in components.py (line 255) but is not defined in the CONFIG_ITEMS dictionary in config.py. This will cause the configuration validation to fail when users try to set this option. The config item should be added to CONFIG_ITEMS with proper type definition like the other solis_* entries.
| "control_enable": {"required": False, "config": "solis_control_enable", "default": True}, |
apps/predbat/solis.py
Outdated
| async def list_operation(): | ||
| payload = {"pageSize": "100"} | ||
| data = await self._execute_request(SOLIS_INVERTER_LIST_ENDPOINT, payload) | ||
| print(data) |
Copilot
AI
Jan 4, 2026
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.
Debug print statement should be removed or converted to a log call. Using print() instead of self.log() bypasses the standard logging infrastructure and won't be captured properly in production environments.
| print(data) | |
| self.log(f"Solis API inverter list response: {data}") |
apps/predbat/solis.py
Outdated
|
|
||
| # detail contains: maxChargePowerW which can be used for battery_rate_max | ||
| max_charge_power_detail = detail.get("maxChargePowerW") | ||
| print("Max Charge Power from detail:", max_charge_power_detail) |
Copilot
AI
Jan 4, 2026
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.
Debug print statement should be removed or converted to a log call. Using print() instead of self.log() bypasses the standard logging infrastructure and won't be captured properly in production environments.
| print("Max Charge Power from detail:", max_charge_power_detail) | |
| self.log(f"Max Charge Power from detail: {max_charge_power_detail}") |
|
|
||
| # One-time startup configuration | ||
| if first: | ||
| # Create aiohttp session | ||
| timeout = aiohttp.ClientTimeout(total=SOLIS_REQUEST_TIMEOUT) | ||
| self.session = aiohttp.ClientSession(timeout=timeout) | ||
|
|
||
| # Discover inverters - always scan, filter by inverter_sn if specified | ||
| try: | ||
| self.log("Solis API: Discovering inverters...") | ||
| all_inverters = await self.get_inverter_list() | ||
|
|
||
| if all_inverters: | ||
| # Filter by configured inverter_sn if specified, otherwise use all | ||
| if self.inverter_sn: | ||
| # inverter_sn was configured, filter to only those | ||
| filtered = [inv for inv in all_inverters if inv.get("sn") in self.inverter_sn] | ||
| self.log(f"Solis API: Filtered to {len(filtered)} of {len(all_inverters)} inverter(s) based on config") | ||
| self.inverter_sn = [inv.get("sn") for inv in filtered if inv.get("sn")] | ||
| else: | ||
| # No filter configured, use all discovered inverters | ||
| self.inverter_sn = [inv.get("sn") for inv in all_inverters if inv.get("sn")] | ||
| self.log(f"Solis API: Using all {len(self.inverter_sn)} discovered inverter(s)") | ||
|
|
||
| if not self.inverter_sn: | ||
| self.log("Warn: Solis API: No inverters found after filtering") | ||
| else: | ||
| self.log("Warn: Solis API: No inverters discovered") | ||
|
|
||
| except Exception as e: | ||
| self.log(f"Error: Solis API: Inverter discovery failed: {e}") | ||
|
|
||
| # Get inverter details for all inverters | ||
| for sn in self.inverter_sn: | ||
| await self.fetch_inverter_details(sn) | ||
| await self.poll_inverter_data(sn, [SOLIS_CID_TOU_V2_MODE]) # Get TOU V2 mode status | ||
| if self.is_tou_v2_mode(sn): | ||
| self.log(f"Solis API: Inverter {sn} is in Time of Use V2 mode") | ||
| else: | ||
| self.log(f"Solis API: Inverter {sn} is in standard Time of Use mode") | ||
|
|
||
| if self.inverter_sn: | ||
| self.log(f"Solis API: Managing {len(self.inverter_sn)} inverter(s): {', '.join(self.inverter_sn)}") | ||
| self.api_started = True | ||
| else: | ||
| self.log("Error: Solis API: No inverters to manage after discovery") | ||
| self.api_started = False | ||
| return # Stop further processing if no inverters | ||
|
|
||
| # Frequent polling (every 5 minutes) | ||
| if first or (seconds % 300 == 0): | ||
| poll_success = True | ||
| for sn in self.inverter_sn: | ||
| success = await self.fetch_inverter_details(sn) # Get inverter details for all inverters | ||
| if not success: | ||
| poll_success = False | ||
|
|
||
| # Only update last_updated_time if all polls succeeded | ||
| if poll_success: | ||
| self.last_updated_time = self.base.now_utc | ||
|
|
||
| # Infrequent polling (every 60 minutes) | ||
| if first or (seconds % 3600 == 0): | ||
| for sn in self.inverter_sn: | ||
| self.log(f"Solis API: Performing infrequent data poll for inverter {sn}...") | ||
| await self.poll_inverter_data(sn, SOLIS_CID_INFREQUENT) | ||
| if self.is_tou_v2_mode(sn): | ||
| success = await self.poll_inverter_data(sn, SOLIS_CID_LIST_TOU_V2) | ||
| if not success: | ||
| poll_success = False | ||
| await self.decode_time_windows_v2(sn) | ||
| else: | ||
| self.log("Solis API: Inverter is in standard Time of Use mode, polling standard TOU data") | ||
| success = await self.poll_inverter_data(sn, SOLIS_CID_SINGLE, batch=False) | ||
| if not success: | ||
| poll_success = False | ||
| await self.decode_time_windows(sn) | ||
| # Recalculate max currents after polling infrequent data | ||
| self._calculate_max_currents(sn) | ||
|
|
||
| # Control mode | ||
| if first or (seconds % 60 == 0): | ||
| # Write to inverter using new function (handles both V1 and V2) | ||
| is_readonly = self.get_state_wrapper(f'switch.{self.prefix}_set_read_only', default='off') == 'on' | ||
| if self.control_enable and not is_readonly: | ||
| for sn in self.inverter_sn: | ||
| await self.reset_charge_windows_if_needed(sn) | ||
| await self.write_time_windows_if_changed(sn) | ||
| else: | ||
| self.log("Solis API: Control disabled, skipping writing time windows") | ||
|
|
||
|
|
||
| # Publish entities after polling | ||
| if first or (seconds % 300 == 0): | ||
| await self.publish_entities() | ||
|
|
||
| # Auto-configure Predbat if enabled | ||
| if first and self.automatic and self.inverter_sn: | ||
| await self.automatic_config() | ||
|
|
Copilot
AI
Jan 4, 2026
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 run() method in solis.py doesn't return a boolean value to indicate success/failure, but the ComponentBase.start() method expects this. When run() fails during first-time setup (e.g., when no inverters are found), it should return False to trigger the backoff mechanism. Currently, when inverter_sn is empty after discovery (line 2492), the method returns without a value (implicit None), which will be treated as False by the ComponentBase. However, when setup succeeds, there's no explicit return True statement, meaning successful runs will also return None/False, potentially triggering unnecessary backoff.
| next_retry = 0 # When to next attempt self.run() during startup backoff | ||
| backoff_interval = 60 # Start with 60 seconds between attempts | ||
| max_backoff = 128 * 60 # Maximum 128 minutes between attempts | ||
| while not self.api_stop and not self.fatal_error: | ||
| try: | ||
| if first or seconds % 60 == 0: | ||
| # Check if it's time to run | ||
| should_run = False | ||
| if first: | ||
| # During startup, only run when we've reached the next retry time | ||
| if seconds >= next_retry: | ||
| should_run = True | ||
| backoff_interval = min(backoff_interval * 2, max_backoff) | ||
| next_retry = seconds + backoff_interval | ||
| else: | ||
| # After startup, run every 60 seconds | ||
| if seconds % 60 == 0: | ||
| should_run = True | ||
|
|
||
| if should_run: | ||
| if await self.run(seconds, first): | ||
| if not self.api_started: | ||
| self.api_started = True | ||
| self.log(f"{self.__class__.__name__}: Started") | ||
| first = False # Clear first flag once started | ||
| else: | ||
| self.count_errors += 1 | ||
| self.non_fatal_error_occurred() | ||
| first = False | ||
| self.log("Warn: " + f"{self.__class__.__name__}: run() returned False") |
Copilot
AI
Jan 4, 2026
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 backoff logic has a subtle bug: backoff_interval is calculated BEFORE checking if run() succeeded. This means on the first run (when seconds=0 and next_retry=0), it will run, then immediately double the backoff_interval to 120 regardless of success. If the first run succeeds, the backoff_interval should not be increased. The interval should only be increased when run() returns False. The correct logic should be: run the method, check the result, and only if it fails, increase the backoff and set next_retry.
apps/predbat/solis.py
Outdated
| except (ValueError, TypeError): | ||
| pass |
Copilot
AI
Jan 4, 2026
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.
'except' clause does nothing but pass and there is no explanatory comment.
| except (ValueError, TypeError): | |
| pass | |
| except (ValueError, TypeError) as exc: | |
| # Invalid or non-numeric current value; leave as None so UI shows unknown | |
| self.log("Warn: Failed to convert Solis discharge_current %r for inverter %s: %s" % (current_value_amps, inverter_sn, exc)) | |
| current_value_watts = None |
apps/predbat/solis.py
Outdated
| try: | ||
| storage_mode_int = int(mode_value) | ||
| except (ValueError, TypeError): | ||
| pass |
Copilot
AI
Jan 4, 2026
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.
'except' clause does nothing but pass and there is no explanatory comment.
| pass | |
| # Invalid storage mode value from Solis; ignore and fall back to no mode | |
| storage_mode_int = None | |
| self.log("Warn: Invalid storage mode value {} for inverter {}".format(mode_value, inverter_sn)) |
apps/predbat/solis.py
Outdated
| except (ValueError, TypeError): | ||
| pass |
Copilot
AI
Jan 4, 2026
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.
'except' clause does nothing but pass and there is no explanatory comment.
| except (ValueError, TypeError): | |
| pass | |
| except (ValueError, TypeError) as exc: | |
| # Invalid or non-numeric current value from Solis API; leave power as None but log for diagnostics | |
| self.log("Warn: Failed to convert Solis max charge current '{}' to watts: {}".format(max_charge_current_amps, exc)) |
| if max_discharge_current_amps is not None: | ||
| try: | ||
| max_discharge_power_watts = int(float(max_discharge_current_amps) * self.nominal_voltage) | ||
| except (ValueError, TypeError): |
Copilot
AI
Jan 4, 2026
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.
'except' clause does nothing but pass and there is no explanatory comment.
apps/predbat/solis.py
Outdated
| except (ValueError, TypeError): | ||
| pass |
Copilot
AI
Jan 4, 2026
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.
'except' clause does nothing but pass and there is no explanatory comment.
| except (ValueError, TypeError): | |
| pass | |
| except (ValueError, TypeError) as e: | |
| self.log( | |
| "Warn: Solis invalid battery capacity '%s' for inverter %s (%s): %s" | |
| % (battery_capacity_ah, inverter_sn, inverter_name, e) | |
| ) |
No description provided.