Repository: jrester/tesla_powerwall (language: python)

Issue:
Disabled battery causes traceback
This is a followup from the [Home Assistant core issue 110516](https://github.com/home-assistant/core/issues/110516)

I have 4 Powerwall batteries in my install and one of the batteries is inoperative and disabled.  This causes a traceback when attempting to call `power_wall.get_batteries()` (called from Home Assistant's [_call_base_info](https://github.com/home-assistant/core/blob/dev/homeassistant/components/powerwall/__init__.py#L221) function. The traceback causes my Home Assistant integration to fail to provision correctly and the Powerwall device is not registered in Home Assistant.

Here's the raw output for my disabled battery from inside [get_batteries](https://github.com/jrester/tesla_powerwall/blob/89a8ce242f9a9b8a96e8b29bfe5101ff7191413e/tesla_powerwall/powerwall.py#L121) in the module.

```
	{
		'Type': '',
		'PackagePartNumber': '3012170-05-E',
		'PackageSerialNumber': 'REDACTED',
		'disabled_reasons': ['DisabledExcessiveVoltageDrop'],
		'pinv_state': '',
		'pinv_grid_state': '',
		'nominal_energy_remaining': 0,
		'nominal_full_pack_energy': 14714,
		'p_out': None,
		'q_out': None,
		'v_out': None,
		'f_out': None,
		'i_out': None,
		'energy_charged': None,
		'energy_discharged': None,
		'off_grid': False,
		'vf_mode': False,
		'wobble_detected': False,
		'charge_power_clamped': False,
		'backup_ready': False,
		'OpSeqState': 'Standby',
		'version': 'eb113390162784'
	}
```

I can prevent the traceback by adding an empty key to the GridState enum in `const.py` like this:

```
class GridState(Enum):
    COMPLIANT = "Grid_Compliant"
    QUALIFYING = "Grid_Qualifying"
    UNCOMPLIANT = "Grid_Uncompliant"
    DISABLED = ""
```

I'm happy to turn this +1 line change into a PR if that's helpful, but I have no opinions on if this is the correct solution or not.  It definitely "fixes the glitch" on my end.  It might make more sense to probe `disabled_reasons` instead to bypass any attempt to parse the empty valued `pinv_grid_state` as an alternative solution.
Disabled battery causes traceback
This is a followup from the [Home Assistant core issue 110516](https://github.com/home-assistant/core/issues/110516)

I have 4 Powerwall batteries in my install and one of the batteries is inoperative and disabled.  This causes a traceback when attempting to call `power_wall.get_batteries()` (called from Home Assistant's [_call_base_info](https://github.com/home-assistant/core/blob/dev/homeassistant/components/powerwall/__init__.py#L221) function. The traceback causes my Home Assistant integration to fail to provision correctly and the Powerwall device is not registered in Home Assistant.

Here's the raw output for my disabled battery from inside [get_batteries](https://github.com/jrester/tesla_powerwall/blob/89a8ce242f9a9b8a96e8b29bfe5101ff7191413e/tesla_powerwall/powerwall.py#L121) in the module.

```
	{
		'Type': '',
		'PackagePartNumber': '3012170-05-E',
		'PackageSerialNumber': 'REDACTED',
		'disabled_reasons': ['DisabledExcessiveVoltageDrop'],
		'pinv_state': '',
		'pinv_grid_state': '',
		'nominal_energy_remaining': 0,
		'nominal_full_pack_energy': 14714,
		'p_out': None,
		'q_out': None,
		'v_out': None,
		'f_out': None,
		'i_out': None,
		'energy_charged': None,
		'energy_discharged': None,
		'off_grid': False,
		'vf_mode': False,
		'wobble_detected': False,
		'charge_power_clamped': False,
		'backup_ready': False,
		'OpSeqState': 'Standby',
		'version': 'eb113390162784'
	}
```

I can prevent the traceback by adding an empty key to the GridState enum in `const.py` like this:

```
class GridState(Enum):
    COMPLIANT = "Grid_Compliant"
    QUALIFYING = "Grid_Qualifying"
    UNCOMPLIANT = "Grid_Uncompliant"
    DISABLED = ""
```

I'm happy to turn this +1 line change into a PR if that's helpful, but I have no opinions on if this is the correct solution or not.  It definitely "fixes the glitch" on my end.  It might make more sense to probe `disabled_reasons` instead to bypass any attempt to parse the empty valued `pinv_grid_state` as an alternative solution.


Discussion / hints:
For the record, this is the traceback I receive without the patch above:

```
  File "/Users/nugget/src/python-tesla/tesla_powerwall/tesla_powerwall/powerwall.py", line 128, in get_batteries
    return [BatteryResponse.from_dict(battery) for battery in batteries]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/nugget/src/python-tesla/tesla_powerwall/tesla_powerwall/powerwall.py", line 128, in <listcomp>
    return [BatteryResponse.from_dict(battery) for battery in batteries]
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/nugget/src/python-tesla/tesla_powerwall/tesla_powerwall/responses.py", line 305, in from_dict
    grid_state=GridState(src["pinv_grid_state"]),
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.7/Frameworks/Python.framework/Versions/3.11/lib/python3.11/enum.py", line 712, in __call__
    return cls.__new__(cls, value)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.7/Frameworks/Python.framework/Versions/3.11/lib/python3.11/enum.py", line 1135, in __new__
    raise ve_exc
ValueError: '' is not a valid GridState
```
For the record, this is the traceback I receive without the patch above:

```
  File "/Users/nugget/src/python-tesla/tesla_powerwall/tesla_powerwall/powerwall.py", line 128, in get_batteries
    return [BatteryResponse.from_dict(battery) for battery in batteries]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/nugget/src/python-tesla/tesla_powerwall/tesla_powerwall/powerwall.py", line 128, in <listcomp>
    return [BatteryResponse.from_dict(battery) for battery in batteries]
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/nugget/src/python-tesla/tesla_powerwall/tesla_powerwall/responses.py", line 305, in from_dict
    grid_state=GridState(src["pinv_grid_state"]),
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.7/Frameworks/Python.framework/Versions/3.11/lib/python3.11/enum.py", line 712, in __call__
    return cls.__new__(cls, value)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.11/3.11.7/Frameworks/Python.framework/Versions/3.11/lib/python3.11/enum.py", line 1135, in __new__
    raise ve_exc
ValueError: '' is not a valid GridState
```

Fix patch:
diff --git a/CHANGELOG b/CHANGELOG
index 817dfee..20414e7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,7 @@
 ## [WIP]
 
 - Use yarl for URL parsing by @bdraco (https://github.com/jrester/tesla_powerwall/pull/62)
+- Correctly handle disabled battery packs (https://github.com/jrester/tesla_powerwall/pull/66/)
 
 ## [0.5.1]
 
diff --git a/README.md b/README.md
index f72a0a5..7fe2acd 100644
--- a/README.md
+++ b/README.md
@@ -171,6 +171,7 @@ await powerwall.get_capacity()
 
 Get information about the battery packs that are installed:
 
+Assuming that the battery is operational, you can retrive a number of values about each battery:
 ```python
 batteries = await powerwall.get_batteries()
 #=> [<Battery ...>, <Battery ...>]
@@ -200,6 +201,22 @@ batteries[0].i_out
 #=> -7.4
 batteries[0].grid_state
 #=> GridState.COMPLIANT
+batteries[0].disabled_reasons
+#=> []
+
+```
+
+If a battery is disabled it's `grid_state` will be `GridState.DISABLED` and some values will be `None`. The variable `disabled_reasons` might contain more information why the battery is disabled:
+```python
+...
+batteries[1].grid_state
+#=> GridState.DISABLED
+batteries[1].disabled_reasons
+#=> ["DisabledExcessiveVoltageDrop"]
+batteries[1].p_out
+#=> None
+batteries[1].energy_charged
+#=> None
 ```
 
 ### Powerwall Status
diff --git a/tesla_powerwall/const.py b/tesla_powerwall/const.py
index 896dab4..f2259a2 100644
--- a/tesla_powerwall/const.py
+++ b/tesla_powerwall/const.py
@@ -32,6 +32,7 @@ class IslandMode(Enum):
 
 
 class GridState(Enum):
+    DISABLED = "Disabled"
     COMPLIANT = "Grid_Compliant"
     QUALIFYING = "Grid_Qualifying"
     UNCOMPLIANT = "Grid_Uncompliant"
diff --git a/tesla_powerwall/responses.py b/tesla_powerwall/responses.py
index b72236a..3d50d72 100644
--- a/tesla_powerwall/responses.py
+++ b/tesla_powerwall/responses.py
@@ -272,22 +272,38 @@ def from_dict(src: dict) -> "SolarResponse":
 
 @dataclass
 class BatteryResponse(ResponseBase):
+    """
+    A battery pack as part of the system_status response.
+    """
+
     part_number: str
     serial_number: str
-    energy_charged: int
-    energy_discharged: int
+    wobble_detected: bool
     energy_remaining: int
     capacity: int
-    wobble_detected: bool
-    p_out: int
-    q_out: int
-    v_out: float
-    f_out: float
-    i_out: float
+    # Values might be None if this battery is in GridState.DISABLED
+    energy_charged: Optional[int]
+    energy_discharged: Optional[int]
+    p_out: Optional[int]
+    q_out: Optional[int]
+    v_out: Optional[float]
+    f_out: Optional[float]
+    i_out: Optional[float]
     grid_state: GridState
+    disabled_reasons: List[str]
 
     @staticmethod
     def from_dict(src: dict) -> "BatteryResponse":
+        # Check if the battery is disabled. A battery is considered disabled if:
+        # - there is at least one disabled reason present in the response,
+        # - or the pinv_grid_state is empty
+        disabled_reasons = src["disabled_reasons"]
+        raw_grid_state = src["pinv_grid_state"]
+        grid_state = (
+            GridState.DISABLED
+            if len(disabled_reasons) > 0 or len(raw_grid_state) == 0
+            else GridState(raw_grid_state)
+        )
         return BatteryResponse(
             src,
             part_number=src["PackagePartNumber"],
@@ -302,5 +318,6 @@ def from_dict(src: dict) -> "BatteryResponse":
             v_out=src["v_out"],
             f_out=src["f_out"],
             i_out=src["i_out"],
-            grid_state=GridState(src["pinv_grid_state"]),
+            grid_state=grid_state,
+            disabled_reasons=disabled_reasons,
         )


Test patch:
diff --git a/tests/unit/fixtures/system_status.json b/tests/unit/fixtures/system_status.json
index 7a1ed61..64eee63 100644
--- a/tests/unit/fixtures/system_status.json
+++ b/tests/unit/fixtures/system_status.json
@@ -50,6 +50,32 @@
       "version": "67f943cb05d12d",
       "vf_mode": false,
       "wobble_detected": false
+    },
+    {
+      "OpSeqState": "Standby",
+      "PackagePartNumber": "XXX-E",
+      "PackageSerialNumber": "XXX",
+      "Type": "",
+      "backup_ready": false,
+      "charge_power_clamped": false,
+      "disabled_reasons": [
+        "DisabledExcessiveVoltageDrop"
+      ],
+      "energy_charged": null,
+      "energy_discharged": null,
+      "f_out": null,
+      "i_out": null,
+      "nominal_energy_remaining": 0,
+      "nominal_full_pack_energy": 14714,
+      "off_grid": false,
+      "p_out": null,
+      "pinv_grid_state": "",
+      "pinv_state": "",
+      "q_out": null,
+      "v_out": null,
+      "version": "eb113390162784",
+      "vf_mode": false,
+      "wobble_detected": false
     }
   ],
   "battery_target_power": -3646.2544361664613,
diff --git a/tests/unit/test_powerwall.py b/tests/unit/test_powerwall.py
index 88bccee..63ab158 100644
--- a/tests/unit/test_powerwall.py
+++ b/tests/unit/test_powerwall.py
@@ -242,7 +242,7 @@ async def test_system_status(self):
 
         self.add_response("system_status", body=SYSTEM_STATUS_RESPONSE)
         batteries = await self.powerwall.get_batteries()
-        self.assertEqual(len(batteries), 2)
+        self.assertEqual(len(batteries), 3)
         self.assertEqual(batteries[0].part_number, "XXX-G")
         self.assertEqual(batteries[0].serial_number, "TGXXX")
         self.assertEqual(batteries[0].energy_remaining, 7378)
@@ -256,6 +256,13 @@ async def test_system_status(self):
         self.assertEqual(batteries[0].q_out, 30)
         self.assertEqual(batteries[0].v_out, 226.60000000000002)
         self.assertEqual(batteries[0].grid_state, GridState.COMPLIANT)
+        self.assertEqual(batteries[2].grid_state, GridState.DISABLED)
+        self.assertEqual(batteries[2].p_out, None)
+        self.assertEqual(batteries[2].i_out, None)
+        self.assertEqual(batteries[2].energy_charged, None)
+        self.assertEqual(
+            batteries[2].disabled_reasons, ["DisabledExcessiveVoltageDrop"]
+        )
         self.aresponses.assert_plan_strictly_followed()
 
     async def test_islanding_mode_offgrid(self):