jrester / tesla_powerwall

Python API for Tesla Powerwall
MIT License
73 stars 25 forks source link

Disabled battery causes traceback #65

Closed nugget closed 9 months ago

nugget commented 9 months ago

This is a followup from the Home Assistant core issue 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 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 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.

nugget commented 9 months ago

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
jrester commented 9 months ago

Hey @nugget,

thank you for your bug report!

As home-assistant is using the GridStates string value in the interface, I think it makes more sense to include a more descriptive value for it and instead rely - as you suggested - on checking disabled_reasons.

The fix is part of the v0.5.2 release and can be included in home-assistant to fix the bug there.