Repository navigation
Solis, reset reserve on startup - #3716
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a SolisCloud startup routine intended to reset/touch battery reserve-related registers so Solis writes work reliably after component startup.
Changes:
- Introduces
startup_reset_registers()to toggle backup/reserve mode and write reserve SOC during initialrun(first=True). - Calls the new reset routine for each discovered inverter on startup.
- Updates the
MockBase.get_argsignature and trims the ad-hoctest_solis_apiloop behavior.
| """ | ||
| current_mode = self.get_current_solis_mode_value(device_sn) | ||
| new_mode = current_mode | (1 << SOLIS_BIT_BACKUP_MODE) | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, "5", field_description="Test write reserve SOC to 5%") | ||
| new_mode = current_mode & ~(1 << SOLIS_BIT_BACKUP_MODE) | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") |
There was a problem hiding this comment.
startup_reset_registers() does not restore the original storage mode: if SOLIS_BIT_BACKUP_MODE was already set in current_mode, the final write (current_mode & ~(1<<SOLIS_BIT_BACKUP_MODE)) will permanently turn battery reserve/backup mode off after startup. Save the original mode and restore it verbatim (or toggle the bit) so startup doesn't change user configuration unintentionally.
| """ | |
| current_mode = self.get_current_solis_mode_value(device_sn) | |
| new_mode = current_mode | (1 << SOLIS_BIT_BACKUP_MODE) | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, "5", field_description="Test write reserve SOC to 5%") | |
| new_mode = current_mode & ~(1 << SOLIS_BIT_BACKUP_MODE) | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | |
| Temporarily enable backup mode to test reserve SOC, then restore the original mode. | |
| """ | |
| current_mode = self.get_current_solis_mode_value(device_sn) | |
| new_mode = current_mode | (1 << SOLIS_BIT_BACKUP_MODE) | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, "5", field_description="Test write reserve SOC to 5%") | |
| # Restore original storage mode so we do not change user configuration | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(current_mode), field_description=f"battery reserve to (mode: {new_mode} -> {current_mode})") |
| @@ -2780,6 +2791,7 @@ async def run(self, seconds, first): | |||
| 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") | |||
| await self.startup_reset_registers(sn) # Reset registers on startup to ensure we have write access and correct initial state | |||
There was a problem hiding this comment.
At the point startup_reset_registers() is called, SOLIS_CID_STORAGE_MODE hasn’t been polled yet (startup only reads SOLIS_CID_TOU_V2_MODE), so get_current_solis_mode_value() can fall back to its default and you may write an incorrect mode value to the inverter. Consider explicitly reading/polling SOLIS_CID_STORAGE_MODE (and any other required CIDs) before computing new_mode.
| await self.startup_reset_registers(sn) # Reset registers on startup to ensure we have write access and correct initial state | ||
|
|
There was a problem hiding this comment.
startup_reset_registers() is invoked during startup even when control_enable is false or the switch.{prefix}_set_read_only flag is on. That bypasses the write-guarding logic used later in run() and can still change inverter settings in “read-only” mode. Gate this call behind the same read-only/control checks (or a dedicated config flag).
| await self.startup_reset_registers(sn) # Reset registers on startup to ensure we have write access and correct initial state | |
| # Only reset registers on startup when control is enabled and not in read-only mode | |
| control_enabled = getattr(self, "control_enable", True) | |
| read_only = getattr(self, "set_read_only", False) | |
| if control_enabled and not read_only: | |
| await self.startup_reset_registers(sn) # Reset registers on startup to ensure we have write access and correct initial state |
| current_mode = self.get_current_solis_mode_value(device_sn) | ||
| new_mode = current_mode | (1 << SOLIS_BIT_BACKUP_MODE) | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, "5", field_description="Test write reserve SOC to 5%") |
There was a problem hiding this comment.
startup_reset_registers() hard-codes the reserve SOC write to "5" and labels it as a "Test" write. This will override whatever reserve SOC the user/inverter currently has on every restart, which is a surprising side-effect for a startup path. Prefer restoring the existing value, using a configured target, and update the log description to reflect non-test behavior.
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, "5", field_description="Test write reserve SOC to 5%") | |
| reserve_soc = self.cached_values.get(device_sn, {}).get(SOLIS_CID_BATTERY_RESERVE_SOC) | |
| if reserve_soc is not None: | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, str(reserve_soc), field_description="Preserve battery reserve SOC on startup") |
| """ | ||
| current_mode = self.get_current_solis_mode_value(device_sn) | ||
| new_mode = current_mode | (1 << SOLIS_BIT_BACKUP_MODE) | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, "5", field_description="Test write reserve SOC to 5%") | ||
| new_mode = current_mode & ~(1 << SOLIS_BIT_BACKUP_MODE) | ||
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | ||
|
|
There was a problem hiding this comment.
The return values from read_and_write_cid() inside startup_reset_registers() are ignored, so failures won’t affect poll_success and may leave the inverter in a partially-updated state (e.g., backup bit set but reserve SOC write failed). Capture the results and either revert/restore the original mode on failure or propagate the failure up so startup can mark the poll unsuccessful.
| """ | |
| current_mode = self.get_current_solis_mode_value(device_sn) | |
| new_mode = current_mode | (1 << SOLIS_BIT_BACKUP_MODE) | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_BATTERY_RESERVE_SOC, "5", field_description="Test write reserve SOC to 5%") | |
| new_mode = current_mode & ~(1 << SOLIS_BIT_BACKUP_MODE) | |
| await self.read_and_write_cid(device_sn, SOLIS_CID_STORAGE_MODE, str(new_mode), field_description=f"battery reserve to (mode: {current_mode} -> {new_mode})") | |
| This briefly enables backup mode, writes a low reserve SOC, then restores | |
| the original storage mode. Any failure during these writes will attempt | |
| to restore the original mode and will raise an exception so callers can | |
| treat startup as unsuccessful. | |
| """ | |
| current_mode = self.get_current_solis_mode_value(device_sn) | |
| # Enable backup bit temporarily | |
| backup_mode = current_mode | (1 << SOLIS_BIT_BACKUP_MODE) | |
| ok = await self.read_and_write_cid( | |
| device_sn, | |
| SOLIS_CID_STORAGE_MODE, | |
| str(backup_mode), | |
| field_description=f"battery reserve to (mode: {current_mode} -> {backup_mode})", | |
| ) | |
| if not ok: | |
| self.log(f"Warn: Failed to set backup mode for inverter {device_sn} (mode {current_mode} -> {backup_mode}) during startup_reset_registers") | |
| raise RuntimeError(f"Solis startup_reset_registers failed to set backup mode for {device_sn}") | |
| # Test write reserve SOC to 5% | |
| ok = await self.read_and_write_cid( | |
| device_sn, | |
| SOLIS_CID_BATTERY_RESERVE_SOC, | |
| "5", | |
| field_description="Test write reserve SOC to 5%", | |
| ) | |
| if not ok: | |
| self.log(f"Warn: Failed to write reserve SOC for inverter {device_sn} during startup_reset_registers, attempting to restore original storage mode") | |
| # Best-effort restore of original mode before propagating failure | |
| try: | |
| await self.read_and_write_cid( | |
| device_sn, | |
| SOLIS_CID_STORAGE_MODE, | |
| str(current_mode), | |
| field_description=f"restore storage mode after reserve SOC write failure (mode: {backup_mode} -> {current_mode})", | |
| ) | |
| except Exception: | |
| # Swallow secondary failure here; original exception is more important | |
| self.log(f"Warn: Additional failure while restoring storage mode for inverter {device_sn} after reserve SOC write failure") | |
| raise RuntimeError(f"Solis startup_reset_registers failed to write reserve SOC for {device_sn}") | |
| # Clear backup bit and restore original mode | |
| restored_mode = current_mode & ~(1 << SOLIS_BIT_BACKUP_MODE) | |
| ok = await self.read_and_write_cid( | |
| device_sn, | |
| SOLIS_CID_STORAGE_MODE, | |
| str(restored_mode), | |
| field_description=f"battery reserve to (mode: {backup_mode} -> {restored_mode})", | |
| ) | |
| if not ok: | |
| self.log(f"Warn: Failed to restore storage mode for inverter {device_sn} during startup_reset_registers, attempting to reset to original mode") | |
| try: | |
| await self.read_and_write_cid( | |
| device_sn, | |
| SOLIS_CID_STORAGE_MODE, | |
| str(current_mode), | |
| field_description=f"restore original storage mode after failure (mode: {restored_mode} -> {current_mode})", | |
| ) | |
| except Exception: | |
| self.log(f"Warn: Additional failure while restoring original storage mode for inverter {device_sn}") | |
| raise RuntimeError(f"Solis startup_reset_registers failed to restore storage mode for {device_sn}") |
…ception - Remove accidental debug log lines from solis.py read_cid, read_batch, and write_cid methods that were left in from the solis_fix PR (#3716) - Fix misleading description in startup_reset_registers third API call - Add cleanup_pool() helper method to PredBat to properly terminate multiprocessing pool workers - Call cleanup_pool() in reset() to clean up any orphaned pool from a previous failed run on restart - Call cleanup_pool() in update_time_loop and run_time_loop finally blocks to ensure pool workers are terminated when an exception propagates out of the prediction loop (prevents orphaned worker processes that could contribute to the bootloop) Agent-Logs-Url: https://github.com/springfall2008/batpred/sessions/67d7d029-d664-4901-979c-e2b278b7a45b Co-authored-by: springfall2008 <48591903+springfall2008@users.noreply.github.com>
No description provided.