Merge pull request #31 from fboundy/patch

Fix Work Mode select/switch bit conflicts; rename discharge slots

Author: fboundy

custom_components/solisconnect/sensor_data/hybrid_sensors.py

@@ -3458,18 +3458,19 @@ def _apply_control_metadata() -> None:
         "Backup SOC": "Backup Mode SOC",
     }
 
-    timed_charge_registers: dict[int, tuple[str, int]] = {}
+    # TOU V2 slot fields: 6 charge slots (43708+) and 6 discharge slots (43750+),
+    # named "Timed Charge X 1-6" and "Timed Discharge X 1-6" respectively.
+    timed_slot_registers: dict[int, tuple[str, str, int]] = {}
     for slot in range(1, 7):
         charge_base = 43708 + ((slot - 1) * 7)
-        timed_charge_registers[charge_base] = ("SOC", slot)
-        timed_charge_registers[charge_base + 1] = ("Current", slot)
-        timed_charge_registers[charge_base + 2] = ("Voltage", slot)
+        timed_slot_registers[charge_base] = ("Charge", "SOC", slot)
+        timed_slot_registers[charge_base + 1] = ("Charge", "Current", slot)
+        timed_slot_registers[charge_base + 2] = ("Charge", "Voltage", slot)
 
-        discharge_slot = slot + 6
         discharge_base = 43750 + ((slot - 1) * 7)
-        timed_charge_registers[discharge_base] = ("SOC", discharge_slot)
-        timed_charge_registers[discharge_base + 1] = ("Current", discharge_slot)
-        timed_charge_registers[discharge_base + 2] = ("Voltage", discharge_slot)
+        timed_slot_registers[discharge_base] = ("Discharge", "SOC", slot)
+        timed_slot_registers[discharge_base + 1] = ("Discharge", "Current", slot)
+        timed_slot_registers[discharge_base + 2] = ("Discharge", "Voltage", slot)
 
     for group in hybrid_sensors:
         for entity in group.get("entities", []):
@@ -3487,10 +3488,10 @@ def _apply_control_metadata() -> None:
                 continue
 
             register = int(registers[0])
-            timed_control = timed_charge_registers.get(register)
+            timed_control = timed_slot_registers.get(register)
             if timed_control:
-                field, slot = timed_control
-                entity["name"] = f"Timed Charge {field} {slot}"
+                direction, field, slot = timed_control
+                entity["name"] = f"Timed {direction} {field} {slot}"
                 entity["control"] = True
 
 

custom_components/solisconnect/sensor_data/select_sensors.py

@@ -36,13 +36,18 @@ def get_select_sensors(inverter_config):
                 "name": "Work Mode",
                 "control": True,
                 "entities": [
-                    # Adheres to RS485_MODBUS ESINV-33000ID Hybrid Inverter V3.2 / Appendix 8
-                    {"bit_position": 0, "name": "Self-Use", "conflicts_with": [0, 6, 11]},
-                    {"bit_position": 0, "name": "Self-Use + TOU", "conflicts_with": [0, 6, 11], "requires": [1]},
-                    {"bit_position": 2, "name": "Off-Grid Operation", "conflicts_with": [0, 1, 2]},
-                    {"bit_position": 6, "name": "Feed-in Priority", "conflicts_with": [0, 6, 11]},
-                    {"bit_position": 6, "name": "Feed-in + TOU", "conflicts_with": [0, 6, 11], "requires": [1]},
-                    {"bit_position": 11, "name": "Peak Shaving", "conflicts_with": [0, 4, 6, 11]},
+                    # Adheres to RS485_MODBUS ESINV-33000ID Hybrid Inverter V3.2 / Appendix 8.
+                    # Each option clears every OTHER mode bit it is incompatible with; the
+                    # previous lists conflicted with their own bit and never cleared the
+                    # Timed Charge bit (1), so switching modes could leave invalid bit
+                    # combos that the inverter rejects/normalises (controls "bounce off"),
+                    # and plain "Self-Use"/"Feed-in" could never turn TOU back off.
+                    {"bit_position": 0, "name": "Self-Use", "conflicts_with": [1, 2, 6, 11]},
+                    {"bit_position": 0, "name": "Self-Use + TOU", "conflicts_with": [2, 6, 11], "requires": [1]},
+                    {"bit_position": 2, "name": "Off-Grid Operation", "conflicts_with": [0, 1, 6, 11]},
+                    {"bit_position": 6, "name": "Feed-in Priority", "conflicts_with": [0, 1, 2, 11]},
+                    {"bit_position": 6, "name": "Feed-in + TOU", "conflicts_with": [0, 2, 11], "requires": [1]},
+                    {"bit_position": 11, "name": "Peak Shaving", "conflicts_with": [0, 1, 2, 4, 6]},
                 ],
             },
         ]

custom_components/solisconnect/sensor_data/switch_sensors.py

@@ -37,7 +37,10 @@ def get_switch_sensors(inverter_config):
                 "entities": [
                     # Adheres to RS485_MODBUS ESINV-33000ID Hybrid Inverter V3.2 / Appendix 8
                     {"bit_position": 0, "name": "Mode - Self Use", "conflicts_with": [2, 6, 11], "control": True},
-                    {"bit_position": 1, "name": "Mode - Timed Charge", "requires_any": [0, 6], "control": True},
+                    # Timed Charge (TOU) only operates alongside Self-Use (0) or Feed-In (6);
+                    # clear Off-Grid/Peak-Shaving first or the fallback Self-Use bit forms a
+                    # combo the firmware rejects and the switch bounces off on the next poll.
+                    {"bit_position": 1, "name": "Mode - Timed Charge", "requires_any": [0, 6], "conflicts_with": [2, 11], "control": True},
                     {"bit_position": 2, "name": "Mode - Off-Grid", "conflicts_with": [0, 1, 6, 11], "control": True},
                     {"bit_position": 3, "name": "Mode - Battery Wake Up", "control": True},
                     {"bit_position": 4, "name": "Mode - Backup", "conflicts_with": [11], "control": True},
@@ -152,12 +155,12 @@ def get_switch_sensors(inverter_config):
                     {"name": "Timed Charge Enable 4", "bit_position": 3, "control": True},
                     {"name": "Timed Charge Enable 5", "bit_position": 4, "control": True},
                     {"name": "Timed Charge Enable 6", "bit_position": 5, "control": True},
-                    {"name": "Timed Charge Enable 7", "bit_position": 6, "control": True},
-                    {"name": "Timed Charge Enable 8", "bit_position": 7, "control": True},
-                    {"name": "Timed Charge Enable 9", "bit_position": 8, "control": True},
-                    {"name": "Timed Charge Enable 10", "bit_position": 9, "control": True},
-                    {"name": "Timed Charge Enable 11", "bit_position": 10, "control": True},
-                    {"name": "Timed Charge Enable 12", "bit_position": 11, "control": True},
+                    {"name": "Timed Discharge Enable 1", "bit_position": 6, "control": True},
+                    {"name": "Timed Discharge Enable 2", "bit_position": 7, "control": True},
+                    {"name": "Timed Discharge Enable 3", "bit_position": 8, "control": True},
+                    {"name": "Timed Discharge Enable 4", "bit_position": 9, "control": True},
+                    {"name": "Timed Discharge Enable 5", "bit_position": 10, "control": True},
+                    {"name": "Timed Discharge Enable 6", "bit_position": 11, "control": True},
                 ],
             },
         ]

custom_components/solisconnect/sensor_data/time_sensors.py

@@ -25,28 +25,28 @@ def get_time_sensors(inverter_config):
             [
                 {"name": "Timed Charge Start 1", "register": 43711, "enabled": True, "control": True},
                 {"name": "Timed Charge End 1", "register": 43713, "enabled": True, "control": True},
-                {"name": "Timed Charge Start 7", "register": 43753, "enabled": True, "control": True},
-                {"name": "Timed Charge End 7", "register": 43755, "enabled": True, "control": True},
+                {"name": "Timed Discharge Start 1", "register": 43753, "enabled": True, "control": True},
+                {"name": "Timed Discharge End 1", "register": 43755, "enabled": True, "control": True},
                 {"name": "Timed Charge Start 2", "register": 43718, "enabled": True, "control": True},
                 {"name": "Timed Charge End 2", "register": 43720, "enabled": True, "control": True},
-                {"name": "Timed Charge Start 8", "register": 43760, "enabled": True, "control": True},
-                {"name": "Timed Charge End 8", "register": 43762, "enabled": True, "control": True},
+                {"name": "Timed Discharge Start 2", "register": 43760, "enabled": True, "control": True},
+                {"name": "Timed Discharge End 2", "register": 43762, "enabled": True, "control": True},
                 {"name": "Timed Charge Start 3", "register": 43725, "enabled": True, "control": True},
                 {"name": "Timed Charge End 3", "register": 43727, "enabled": True, "control": True},
-                {"name": "Timed Charge Start 9", "register": 43767, "enabled": True, "control": True},
-                {"name": "Timed Charge End 9", "register": 43769, "enabled": True, "control": True},
+                {"name": "Timed Discharge Start 3", "register": 43767, "enabled": True, "control": True},
+                {"name": "Timed Discharge End 3", "register": 43769, "enabled": True, "control": True},
                 {"name": "Timed Charge Start 4", "register": 43732, "enabled": True, "control": True},
                 {"name": "Timed Charge End 4", "register": 43734, "enabled": True, "control": True},
-                {"name": "Timed Charge Start 10", "register": 43774, "enabled": True, "control": True},
-                {"name": "Timed Charge End 10", "register": 43776, "enabled": True, "control": True},
+                {"name": "Timed Discharge Start 4", "register": 43774, "enabled": True, "control": True},
+                {"name": "Timed Discharge End 4", "register": 43776, "enabled": True, "control": True},
                 {"name": "Timed Charge Start 5", "register": 43739, "enabled": True, "control": True},
                 {"name": "Timed Charge End 5", "register": 43741, "enabled": True, "control": True},
-                {"name": "Timed Charge Start 11", "register": 43781, "enabled": True, "control": True},
-                {"name": "Timed Charge End 11", "register": 43783, "enabled": True, "control": True},
+                {"name": "Timed Discharge Start 5", "register": 43781, "enabled": True, "control": True},
+                {"name": "Timed Discharge End 5", "register": 43783, "enabled": True, "control": True},
                 {"name": "Timed Charge Start 6", "register": 43746, "enabled": True, "control": True},
                 {"name": "Timed Charge End 6", "register": 43748, "enabled": True, "control": True},
-                {"name": "Timed Charge Start 12", "register": 43788, "enabled": True, "control": True},
-                {"name": "Timed Charge End 12", "register": 43790, "enabled": True, "control": True},
+                {"name": "Timed Discharge Start 6", "register": 43788, "enabled": True, "control": True},
+                {"name": "Timed Discharge End 6", "register": 43790, "enabled": True, "control": True},
             ]
         )
 

custom_components/solisconnect/sensors/solis_select_entity.py

@@ -68,6 +68,8 @@ async def async_select_option(self, option: str) -> None:
                     break
                 else:
                     await self.set_register_bit(on_value, bit_position, conflicts_with, requires)
+                    self.async_write_ha_state()
+                    break
 
     @property
     def device_info(self):

tests/test_ac_coupling.py

@@ -139,27 +139,28 @@ def test_timed_charge_enable_switch_names():
 
     names = [entity["name"] for entity in _switch_entities(config, 43707)]
 
-    assert names == [f"Timed Charge Enable {slot}" for slot in range(1, 13)]
+    # Bits 0-5 are the 6 charge slots, bits 6-11 the 6 discharge slots.
+    assert names == [f"Timed Charge Enable {slot}" for slot in range(1, 7)] + [f"Timed Discharge Enable {slot}" for slot in range(1, 7)]
 
 
-def test_timed_charge_number_names_are_normalized():
+def test_timed_slot_number_names_are_normalized():
     from custom_components.solisconnect.sensor_data.hybrid_sensors import hybrid_sensors
 
     entities = {int(entity["register"][0]): entity for group in hybrid_sensors for entity in group.get("entities", []) if entity.get("register")}
 
     assert entities[43708]["name"] == "Timed Charge SOC 1"
     assert entities[43709]["name"] == "Timed Charge Current 1"
     assert entities[43710]["name"] == "Timed Charge Voltage 1"
-    assert entities[43750]["name"] == "Timed Charge SOC 7"
-    assert entities[43751]["name"] == "Timed Charge Current 7"
-    assert entities[43752]["name"] == "Timed Charge Voltage 7"
-    assert entities[43785]["name"] == "Timed Charge SOC 12"
-    assert entities[43786]["name"] == "Timed Charge Current 12"
-    assert entities[43787]["name"] == "Timed Charge Voltage 12"
+    assert entities[43750]["name"] == "Timed Discharge SOC 1"
+    assert entities[43751]["name"] == "Timed Discharge Current 1"
+    assert entities[43752]["name"] == "Timed Discharge Voltage 1"
+    assert entities[43785]["name"] == "Timed Discharge SOC 6"
+    assert entities[43786]["name"] == "Timed Discharge Current 6"
+    assert entities[43787]["name"] == "Timed Discharge Voltage 6"
     assert entities[43024]["name"] == "Backup Mode SOC"
 
 
-def test_timed_charge_time_names_are_normalized():
+def test_timed_slot_time_names_are_normalized():
     from custom_components.solisconnect.sensor_data.time_sensors import get_time_sensors
 
     template = next(inv for inv in SOLIS_INVERTERS if inv.model == "S6-EH1P")
@@ -168,10 +169,10 @@ def test_timed_charge_time_names_are_normalized():
 
     assert entities[43711]["name"] == "Timed Charge Start 1"
     assert entities[43713]["name"] == "Timed Charge End 1"
-    assert entities[43753]["name"] == "Timed Charge Start 7"
-    assert entities[43755]["name"] == "Timed Charge End 7"
-    assert entities[43788]["name"] == "Timed Charge Start 12"
-    assert entities[43790]["name"] == "Timed Charge End 12"
+    assert entities[43753]["name"] == "Timed Discharge Start 1"
+    assert entities[43755]["name"] == "Timed Discharge End 1"
+    assert entities[43788]["name"] == "Timed Discharge Start 6"
+    assert entities[43790]["name"] == "Timed Discharge End 6"
 
 
 def test_parallel_feature_disabled_by_default():

tests/test_solis_select_entity.py

@@ -246,3 +246,65 @@ async def test_companion_writes_skipped_when_cache_empty(mock_hass, mock_control
         await entity.async_select_option("Solis RC Force Battery Charge")
 
         mock_controller.async_write_holding_register.assert_awaited_once_with(primary_register, 1)
+
+
+def _work_mode_definition():
+    from custom_components.solisconnect.data.solis_config import SOLIS_INVERTERS, InverterOptions
+    from custom_components.solisconnect.sensor_data.select_sensors import get_select_sensors
+
+    template = next(inv for inv in SOLIS_INVERTERS if inv.model == "S6-EH1P")
+    config = template.clone_with_options(InverterOptions(), "S2_WL_ST")
+    return next(group for group in get_select_sensors(config) if group["name"] == "Work Mode")
+
+
+def _apply_option_bits(current: int, option: dict) -> int:
+    """Mirror SolisSelectEntity.set_register_bit's bit arithmetic for one option."""
+    value = current
+    for bit in option.get("conflicts_with", []):
+        value = set_bit(value, bit, False)
+    for bit in option.get("requires", []):
+        value = set_bit(value, bit, True)
+    return set_bit(value, option["bit_position"], True)
+
+
+def _resolve_option(register_value: int, options: list[dict]) -> str | None:
+    """Mirror SolisSelectEntity.current_option's resolution logic."""
+    sorted_options = sorted(options, key=lambda e: len(e.get("requires", [])) if "requires" in e else 0, reverse=True)
+    for e in sorted_options:
+        bit_position = e.get("bit_position")
+        requires = e.get("requires")
+        if bit_position is not None and (register_value >> bit_position) & 1:
+            if requires:
+                if all((register_value >> rbit) & 1 for rbit in requires):
+                    return e["name"]
+            else:
+                return e["name"]
+    return None
+
+
+@pytest.mark.parametrize("start_bits", [0b1, 0b11, 0b100, 0b1000010, 0b100000000001, 0b100011])
+def test_work_mode_options_round_trip_from_any_mode(start_bits):
+    """Every Work Mode option must land on a register state that resolves back to itself,
+    regardless of the mode the inverter was in before (the old definitions left stale mode
+    bits set, e.g. picking "Self-Use" never cleared TOU so it read back as "Self-Use + TOU",
+    and cross-mode transitions produced combos the firmware rejects)."""
+    definition = _work_mode_definition()
+    options = definition["entities"]
+
+    for option in options:
+        result = _apply_option_bits(start_bits, option)
+        assert _resolve_option(result, options) == option["name"], (
+            f"Selecting {option['name']!r} from {start_bits:#014b} produced {result:#014b}, which resolves to {_resolve_option(result, options)!r}"
+        )
+
+
+def test_work_mode_plain_options_clear_tou_bit():
+    """Picking a mode without "+ TOU" must actually turn the Timed Charge bit off."""
+    definition = _work_mode_definition()
+    by_name = {e["name"]: e for e in definition["entities"]}
+
+    tou_on = 0b11  # Self-Use + TOU active
+    assert (_apply_option_bits(tou_on, by_name["Self-Use"]) >> 1) & 1 == 0
+    assert (_apply_option_bits(tou_on, by_name["Feed-in Priority"]) >> 1) & 1 == 0
+    assert (_apply_option_bits(tou_on, by_name["Peak Shaving"]) >> 1) & 1 == 0
+    assert (_apply_option_bits(tou_on, by_name["Off-Grid Operation"]) >> 1) & 1 == 0

tests/test_switch_conflicts_requires.py

@@ -75,6 +75,33 @@ async def test_requires_tou(mock_hass, controller):
         controller.async_write_holding_register.assert_awaited_once_with(43110, expected)
 
 
+@pytest.mark.asyncio
+async def test_timed_charge_switch_clears_incompatible_modes(mock_hass, controller):
+    """Turning on Mode - Timed Charge while in Peak Shaving must clear bit 11 and fall
+    back to Self-Use, not write an invalid combo the firmware rejects (the bounce)."""
+    from custom_components.solisconnect.data.solis_config import SOLIS_INVERTERS, InverterOptions
+    from custom_components.solisconnect.sensor_data.switch_sensors import get_switch_sensors
+
+    template = next(inv for inv in SOLIS_INVERTERS if inv.model == "S6-EH1P")
+    config = template.clone_with_options(InverterOptions(), "S2_WL_ST")
+    entity_def = next(
+        entity for group in get_switch_sensors(config) if group.get("register") == 43110 for entity in group["entities"] if entity["bit_position"] == 1
+    )
+    entity_def = {**entity_def, "register": 43110, "write_register": None}
+
+    initial = set_bit(0, 11, True)  # Peak Shaving active, no Self-Use/Feed-In
+
+    with (
+        patch("custom_components.solisconnect.sensors.solis_binary_sensor.cache_get", return_value=initial),
+        patch("custom_components.solisconnect.sensors.solis_binary_sensor.cache_save"),
+    ):
+        entity = SolisBinaryEntity(mock_hass, controller, entity_def)
+        await entity.set_register_bit(True)
+
+        expected = set_bit(set_bit(0, 0, True), 1, True)  # Peak Shaving cleared, Self-Use + TOU set
+        controller.async_write_holding_register.assert_awaited_once_with(43110, expected)
+
+
 @pytest.mark.asyncio
 async def test_conflicts_and_requires_combined(mock_hass, controller):
     entity_def = {