Solis v2 slot fixes by springfall2008 · Pull Request #3654 · springfall2008/batpred

springfall2008 · 2026-03-25T09:19:19Z

No description provided.

Copilot

Pull request overview

Adjusts the Solis (TOU v2) slot-writing behavior to avoid writing slot fields when the corresponding enable is off, aligning register writes with actual enabled schedules and updating the Solis v2 test expectations accordingly.

Changes:

Update write_time_windows_if_changed (TOU v2) to only write time/SOC/current registers when the relevant charge/discharge slot is enabled.
Fix CID write verification to compare readback and desired values as strings (avoids false mismatches due to type differences).
Update Solis v2 unit test expectations and bump Predbat version.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
apps/predbat/tests/test_solis.py	Updates Solis v2 test to expect only enabled-field writes (and asserts disabled discharge fields are not written).
apps/predbat/solis.py	Gates v2 register writes behind enable flags; improves write verification by string-normalizing comparisons.
apps/predbat/predbat.py	Version bump to v8.34.12.

Copilot · 2026-03-25T09:25:17Z

apps/predbat/solis.py

+                    slot_charge_enable = False
+                    slot_discharge_enable = False
+                    if "charge_enable" in slot_data:
+                        slot_charge_enable = slot_data['charge_enable']
+                    if "discharge_enable" in slot_data:
+                        slot_discharge_enable = slot_data['discharge_enable']


slot_discharge_enable is initialized from slot_data here and then reassigned again inside the if "discharge_enable" in slot_data: block below. Consider deriving the enable flags once (e.g., via slot_data.get(...)) to avoid redundant state and make the control flow easier to follow.

Suggested change

slot_charge_enable = False

slot_discharge_enable = False

if "charge_enable" in slot_data:

slot_charge_enable = slot_data['charge_enable']

if "discharge_enable" in slot_data:

slot_discharge_enable = slot_data['discharge_enable']

slot_charge_enable = bool(slot_data.get("charge_enable", False))

slot_discharge_enable = bool(slot_data.get("discharge_enable", False))

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Copilot · 2026-03-25T19:39:36Z

apps/predbat/solis.py

+            info = copy.deepcopy(data)

            if inverter_sn not in self.cached_values:
                self.cached_values[inverter_sn] = {}


read_cid() only initializes cached_infos[inverter_sn] when inverter_sn is missing from cached_values. If cached_values[inverter_sn] exists but cached_infos[inverter_sn] doesn't (e.g., after a prior write_cid() call which only populates cached_values), self.cached_infos[inverter_sn][cid] = info will raise a KeyError. Initialize cached_infos independently (e.g., setdefault) before assigning.

Suggested change

self.cached_values[inverter_sn] = {}

self.cached_values[inverter_sn] = {}

if inverter_sn not in self.cached_infos:

Copilot · 2026-03-25T19:39:37Z

apps/predbat/solis.py

            if inverter_sn not in self.cached_values:
                self.cached_values[inverter_sn] = {}
+                self.cached_infos[inverter_sn] = {}

            # Update cached values
            self.cached_values[inverter_sn].update(values)
+            self.cached_infos[inverter_sn].update(infos)


poll_inverter_data() has the same cache init issue as read_cid(): it only creates cached_infos[inverter_sn] when inverter_sn is missing from cached_values. If cached_values was created earlier without cached_infos, self.cached_infos[inverter_sn].update(infos) will KeyError. Consider initializing both caches independently (e.g., setdefault for each).

Copilot · 2026-03-25T19:39:37Z

apps/predbat/solis.py

+            if isinstance(old_value, (int, float)):
+                try:
+                    if float(old_value) == float(value):
+                        self.log(f"Solis API: CID {cid} {field_description} on {inverter_sn} is set to {value}")
+                        return True
+                except (ValueError, TypeError):
+                    pass  # Fall back to string comparison if float conversion fails


In read_and_write_cid(), the numeric verification branch if isinstance(old_value, (int, float)) will never run because read_cid() returns data['msg'] (typically a string). This can cause false verification failures when values are numerically equal but formatted differently (e.g., wrote "50.0" but read back "50"). Consider attempting float comparison whenever both sides are float-convertible (regardless of the original type), and only falling back to string comparison if conversion fails.

Suggested change

if isinstance(old_value, (int, float)):

try:

if float(old_value) == float(value):

self.log(f"Solis API: CID {cid} {field_description} on {inverter_sn} is set to {value}")

return True

except (ValueError, TypeError):

pass # Fall back to string comparison if float conversion fails

try:

if float(old_value) == float(value):

self.log(f"Solis API: CID {cid} {field_description} on {inverter_sn} is set to {value}")

return True

except (ValueError, TypeError):

pass # Fall back to string comparison if float conversion fails

Copilot · 2026-03-25T19:39:37Z

apps/predbat/tests/test_solis.py

    call = api.read_and_write_cid_calls[0]
    assert call["cid"] == SOLIS_CID_BATTERY_MAX_CHARGE_CURRENT, f"Expected CID {SOLIS_CID_BATTERY_MAX_CHARGE_CURRENT}, got {call['cid']}"
-    expected_amps = str(int(4840 / 48.4))  # = "100"
+    expected_amps = str(round(float(4840) / 48.4, 1))  # = "100.0"


This assertion hardcodes 48.4 for the volts→amps conversion, but the production default nominal_voltage was changed in this PR. To keep the test aligned with runtime behavior and avoid future drift, consider using api.nominal_voltage (or explicitly setting the mock's voltage in the test and using that value here).

Suggested change

expected_amps = str(round(float(4840) / 48.4, 1)) # = "100.0"

expected_amps = str(round(float(4840) / api.nominal_voltage, 1))

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

Copilot · 2026-03-25T19:52:58Z

apps/predbat/tests/test_solis.py

+    # nominal_voltage = 48.0V, so 1452W = 30A
    entity_id = f"number.predbat_solis_{inverter_sn}_discharge_slot4_power"
    await api.number_event(entity_id, 1452)

-    # Verify current was updated (1452W / 48.4V = 30A)
-    expected_amps = int(1452 / 48.4)  # = 30A
+    # Verify current was updated (1452W / 48.0V = 30A)
+    expected_amps = float(round(1452 / 48.0, 1))  # = 30.2A


This test’s explanatory comments are now inconsistent with the behavior being asserted: 1452W/48.0V rounds to 30.2A (not 30A), and the comment just below also states "= 30A". Please update the comments to reflect the new rounding behavior.

Copilot · 2026-03-25T19:52:58Z

apps/predbat/tests/test_solis.py

+    # nominal_voltage = 48.0V, so 4840W = 100A
    entity_id = f"number.predbat_solis_{inverter_sn}_max_charge_power"
    await api.number_event(entity_id, 4840)

    assert len(api.read_and_write_cid_calls) == 1, "Should call read_and_write_cid once"
    call = api.read_and_write_cid_calls[0]
    assert call["cid"] == SOLIS_CID_BATTERY_MAX_CHARGE_CURRENT, f"Expected CID {SOLIS_CID_BATTERY_MAX_CHARGE_CURRENT}, got {call['cid']}"
-    expected_amps = str(int(4840 / 48.4))  # = "100"
+    expected_amps = round(float(4840) / api.nominal_voltage, 1)  # = "100.0"
    assert call["value"] == expected_amps, f"Expected '{expected_amps}', got {call['value']}"


The comments in this max-power test don’t match the updated calculation: at 48.0V, 4840W converts to 100.8A (rounded to 1dp), not 100A/"100.0". Please update the comments to match the expected_amps computation.

Copilot · 2026-03-25T19:52:59Z

apps/predbat/solis.py

+                        max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_charge_current_amps)
+                        new_current = min(new_current, max_current)
+                        cached_current = float(self.cached_values.get(inverter_sn, {}).get(current_cid, max_current))


In V2 mode current clamping, max_current is pulled from cached API metadata (cached_infos[..]['sysCommand']['max']) but is not coerced to a numeric type. Solis API payloads commonly use strings for numeric fields, so min(new_current, max_current) can raise a TypeError (float vs str) and break slot writes. Convert max_current to float with a safe fallback (and similarly guard cached_current float conversion).

Suggested change

max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_charge_current_amps)

new_current = min(new_current, max_current)

cached_current = float(self.cached_values.get(inverter_sn, {}).get(current_cid, max_current))

raw_max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_charge_current_amps)

try:

max_current = float(raw_max_current)

except (TypeError, ValueError):

max_current = float(max_charge_current_amps)

new_current = min(new_current, max_current)

raw_cached_current = self.cached_values.get(inverter_sn, {}).get(current_cid, max_current)

try:

cached_current = float(raw_cached_current)

except (TypeError, ValueError):

cached_current = max_current

Copilot · 2026-03-25T19:52:59Z

apps/predbat/solis.py

+                        max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_discharge_current_amps)
+                        new_current = min(new_current, max_current)
+                        cached_current = float(self.cached_values.get(inverter_sn, {}).get(current_cid, 0))


Same issue as charge-current clamping: max_current from cached_infos[..]['sysCommand']['max'] is used directly in min() without numeric coercion, which can crash if the API returns strings. Parse max_current to float (with fallback), and consider guarding cached_current parsing too.

Suggested change

max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_discharge_current_amps)

new_current = min(new_current, max_current)

cached_current = float(self.cached_values.get(inverter_sn, {}).get(current_cid, 0))

raw_max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_discharge_current_amps)

try:

max_current = float(raw_max_current)

except (TypeError, ValueError):

max_current = max_discharge_current_amps

new_current = min(new_current, max_current)

raw_cached_current = self.cached_values.get(inverter_sn, {}).get(current_cid, 0)

try:

cached_current = float(raw_cached_current)

except (TypeError, ValueError):

cached_current = 0.0

Copilot · 2026-03-25T19:52:59Z

apps/predbat/solis.py

                # Convert watts to amps for inverter
-                amps = int(value / self.nominal_voltage)
+                amps = round(float(value) / self.nominal_voltage, 1)



number_event now rounds watts->amps to 0.1A for discharge slot power, but charge slot power conversion still truncates to an int (earlier in this method). If fractional-amp support is required for V2 slots, charge and discharge paths should use the same rounding behavior to avoid inconsistent UI behavior and unnecessary write churn.

Copilot · 2026-03-25T19:52:59Z

apps/predbat/tests/test_solis.py

+    # Power converted to amps: 2400W / 48.0V = 49.58A
+    expected_discharge_current = int(2400 / 48.0)  # ~49A


Comment/math is incorrect: 2400W / 48.0V equals 50.0A (not 49.58A). This comment now contradicts the code below and can mislead future changes.

Suggested change

# Power converted to amps: 2400W / 48.0V = 49.58A

expected_discharge_current = int(2400 / 48.0) # ~49A

# Power converted to amps: 2400W / 48.0V = 50.0A

expected_discharge_current = int(2400 / 48.0) # 50A

Copilot · 2026-03-25T19:52:59Z

apps/predbat/tests/test_solis.py

    assert f"number.{prefix}_solis_{inverter_sn_lower}_charge_slot1_power" in api.dashboard_items, "Charge slot 1 power should be published"
    charge_power = api.dashboard_items[f"number.{prefix}_solis_{inverter_sn_lower}_charge_slot1_power"]
-    expected_power = int(50 * api.nominal_voltage)  # 50A * 48.4V = 2420W
+    expected_power = int(50 * api.nominal_voltage)  # 50A * 48.0V = 2420W


Inline comment has the wrong computed value: 50A * 48.0V is 2400W (not 2420W). Please fix the comment to match the calculation.

Suggested change

expected_power = int(50 * api.nominal_voltage) # 50A * 48.0V = 2420W

expected_power = int(50 * api.nominal_voltage) # 50A * 48.0V = 2400W

Copilot · 2026-03-25T19:53:00Z

apps/predbat/tests/test_solis.py

    assert f"sensor.{prefix}_solis_{inverter_sn_lower}_battery_capacity" in api.dashboard_items, "Battery capacity should be published"
    battery_cap = api.dashboard_items[f"sensor.{prefix}_solis_{inverter_sn_lower}_battery_capacity"]
-    expected_kwh = round(100 * api.nominal_voltage / 1000.0, 2)  # 100Ah * 48.4V / 1000 = 4.84 kWh
+    expected_kwh = round(100 * api.nominal_voltage / 1000.0, 2)  # 100Ah * 48.0V / 1000 = 4.84 kWh


Inline comment has the wrong computed value: 100Ah * 48.0V / 1000 is 4.8kWh (not 4.84kWh). Please update the comment to avoid confusion.

Suggested change

expected_kwh = round(100 * api.nominal_voltage / 1000.0, 2) # 100Ah * 48.0V / 1000 = 4.84 kWh

expected_kwh = round(100 * api.nominal_voltage / 1000.0, 2) # 100Ah * 48.0V / 1000 = 4.8 kWh

springfall2008 added 2 commits March 25, 2026 09:07

Solis fix

6bdca29

Slot handling code

9c01a53

Copilot AI review requested due to automatic review settings March 25, 2026 09:19

Copilot started reviewing on behalf of springfall2008 March 25, 2026 09:19 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Solis: Rework current control for charge/discharge

533d384

springfall2008 requested a review from Copilot March 25, 2026 19:35

Copilot started reviewing on behalf of springfall2008 March 25, 2026 19:36 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

springfall2008 added 2 commits March 25, 2026 19:44

Review feedback

da15f4f

review

3ebc25c

springfall2008 requested a review from Copilot March 25, 2026 19:47

Copilot started reviewing on behalf of springfall2008 March 25, 2026 19:48 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

springfall2008 merged commit 6415df8 into main Mar 25, 2026
5 checks passed

springfall2008 deleted the solis2 branch March 25, 2026 19:56

	self.cached_values[inverter_sn] = {}
	self.cached_values[inverter_sn] = {}
	if inverter_sn not in self.cached_infos:

	expected_amps = str(round(float(4840) / 48.4, 1)) # = "100.0"
	expected_amps = str(round(float(4840) / api.nominal_voltage, 1))

-                        max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_charge_current_amps)
-                        new_current = min(new_current, max_current)
-                        cached_current = float(self.cached_values.get(inverter_sn, {}).get(current_cid, max_current))
+                        raw_max_current = self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_charge_current_amps)
+                        try:
+                            max_current = float(raw_max_current)
+                        except (TypeError, ValueError):
+                            max_current = float(max_charge_current_amps)
+                        new_current = min(new_current, max_current)
+                        raw_cached_current = self.cached_values.get(inverter_sn, {}).get(current_cid, max_current)
+                        try:
+                            cached_current = float(raw_cached_current)
+                        except (TypeError, ValueError):
+                            cached_current = max_current

		# Power converted to amps: 2400W / 48.0V = 49.58A
		expected_discharge_current = int(2400 / 48.0) # ~49A

	expected_power = int(50 * api.nominal_voltage) # 50A * 48.0V = 2420W
	expected_power = int(50 * api.nominal_voltage) # 50A * 48.0V = 2400W

	expected_kwh = round(100 * api.nominal_voltage / 1000.0, 2) # 100Ah * 48.0V / 1000 = 4.84 kWh
	expected_kwh = round(100 * api.nominal_voltage / 1000.0, 2) # 100Ah * 48.0V / 1000 = 4.8 kWh

Uh oh!

Conversation

springfall2008 commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants