-
-
Notifications
You must be signed in to change notification settings - Fork 92
Solis fixes (working on v1) and automatic battery size #3189
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
Solis control fixes
Reformatted the description of the 'soc_max' parameter for better readability.
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 automatic battery size detection for Solis inverters and includes various fixes for Solis V1 firmware support. The main enhancement allows Predbat to estimate battery capacity from historical charging data when soc_max is not configured, while maintaining manual configuration as the recommended approach.
Key changes:
- Automatic battery size detection: New
find_battery_size()method ininverter.pyanalyzes historical SoC and power data to estimate battery capacity - Solis V1 fixes: Enhanced charge/discharge slot detection logic to properly check enable flags and differentiate between in-slot and out-of-slot storage modes
- Bug fixes: Corrected rate application logic in Axle integration, fixed component restart behavior, and improved Solis entity state handling
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/components.md | Updated Solis documentation to explain automatic vs manual battery size configuration |
| docs/apps-yaml.md | Expanded battery size configuration guidance with automatic detection details |
| apps/predbat/unit_test.py | Registered new battery size detection tests in test suite |
| apps/predbat/tests/test_find_battery_size.py | Added comprehensive test suite for battery size detection algorithm |
| apps/predbat/tests/test_solis.py | Added tests for fetch_entity_data, fixed switch state assertions to use "on"/"off" |
| apps/predbat/tests/test_execute.py | Added stub find_battery_size method to mock inverter for test compatibility |
| apps/predbat/solis.py | Major fixes: fetch_entity_data for V1 control, enable flag checking, storage mode logic, entity publishing fixes, battery_power_invert config |
| apps/predbat/predbat.py | Version bump to v8.31.10 |
| apps/predbat/octopus.py | Fixed end_minutes calculation for saving sessions extending beyond forecast |
| apps/predbat/inverter.py | Implemented automatic battery size detection from historical charge data, fallback to 8kWh default |
| apps/predbat/components.py | Fixed component restart to call both initialization phases |
| apps/predbat/component_base.py | Moved first flag clearing outside api_started check for proper startup handling |
| apps/predbat/axle.py | Simplified rate addition logic using dict.get() and fixed end_minutes calculation |
| # Battery and inverter entities | ||
| self.set_arg("soc_percent", [f"sensor.predbat_solis_{device}_battery_soc" for device in devices]) | ||
| self.set_arg("battery_power", [f"sensor.predbat_solis_{device}_battery_power" for device in devices]) | ||
| self.set_arg("battery_power_invert", [f"True" for device in devices]) |
Copilot
AI
Jan 5, 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 battery_power_invert is set to the string "True" instead of the boolean True. This should be [True for device in devices] to ensure proper boolean handling in the codebase. String values may not work correctly with boolean comparisons elsewhere in the code.
| self.set_arg("battery_power_invert", [f"True" for device in devices]) | |
| self.set_arg("battery_power_invert", [True for device in devices]) |
| # Frequent polling (every minute) | ||
| if first or (seconds % 60 == 0): | ||
| for sn in self.inverter_sn: | ||
| success = await self.fetch_inverter_details(sn) # Get inverter details for all inverters |
Copilot
AI
Jan 5, 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.
Extra space before the assignment operator. Should be success = await not success = await (two spaces).
apps/predbat/solis.py
Outdated
|
|
||
| async def fetch_entity_data(self, sn): | ||
| """ | ||
| This use to fetch HA data for the charge/discharge windows directly frome Home Assistant using get_state_wrapper() |
Copilot
AI
Jan 5, 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.
Typo in docstring: "use to fetch" should be "used to fetch"
| This use to fetch HA data for the charge/discharge windows directly frome Home Assistant using get_state_wrapper() | |
| This is used to fetch HA data for the charge/discharge windows directly from Home Assistant using get_state_wrapper() |
| timestamp_str = timestamp.strftime("%Y-%m-%dT%H:%M:%S%z") | ||
|
|
||
| hour = timestamp.hour | ||
| minute_of_hour = timestamp.minute |
Copilot
AI
Jan 5, 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.
Variable minute_of_hour is not used.
| minute_of_hour = timestamp.minute |
| base.rate_export[minute] += pence_per_kwh | ||
| rate_replicate[minute] = "saving" | ||
| base.rate_export[minute] = base.rate_export.get(minute, 0) + pence_per_kwh | ||
| rate_replicate[minute] = "saving" |
Copilot
AI
Jan 5, 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.
This expression mutates a default value.
| rate_replicate[minute] = "saving" | ||
| base.rate_import[minute] = base.rate_import.get(minute, 0) + pence_per_kwh | ||
| base.load_scaling_dynamic[minute] = base.load_scaling_saving | ||
| rate_replicate[minute] = "saving" |
Copilot
AI
Jan 5, 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.
This expression mutates a default value.
| pass | ||
| if power_state is not None: | ||
| try: | ||
| max_current_amps = self.max_charge_current.get(sn, 100) if direction == 'charge' else self.max_discharge_current.get(sn, 100) | ||
| value = max(0, min(int(float(power_state) / self.nominal_voltage), max_current_amps)) | ||
| self.charge_discharge_time_windows[sn][slot][f"{direction}_current"] = value | ||
| except (ValueError, TypeError): | ||
| pass |
Copilot
AI
Jan 5, 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 | |
| if power_state is not None: | |
| try: | |
| max_current_amps = self.max_charge_current.get(sn, 100) if direction == 'charge' else self.max_discharge_current.get(sn, 100) | |
| value = max(0, min(int(float(power_state) / self.nominal_voltage), max_current_amps)) | |
| self.charge_discharge_time_windows[sn][slot][f"{direction}_current"] = value | |
| except (ValueError, TypeError): | |
| pass | |
| self.log("Solis: Ignoring invalid SOC state '{}' for inverter {} slot {} direction {}".format(soc_state, sn, slot, direction)) | |
| if power_state is not None: | |
| try: | |
| max_current_amps = self.max_charge_current.get(sn, 100) if direction == 'charge' else self.max_discharge_current.get(sn, 100) | |
| value = max(0, min(int(float(power_state) / self.nominal_voltage), max_current_amps)) | |
| self.charge_discharge_time_windows[sn][slot][f"{direction}_current"] = value | |
| except (ValueError, TypeError): | |
| self.log("Solis: Ignoring invalid power state '{}' for inverter {} slot {} direction {}".format(power_state, sn, slot, direction)) |
| max_current_amps = self.max_charge_current.get(sn, 100) if direction == 'charge' else self.max_discharge_current.get(sn, 100) | ||
| value = max(0, min(int(float(power_state) / self.nominal_voltage), max_current_amps)) | ||
| self.charge_discharge_time_windows[sn][slot][f"{direction}_current"] = value | ||
| except (ValueError, TypeError): |
Copilot
AI
Jan 5, 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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.