vlebourl / custom_vesync

Custom VeSync component for Home Assistant
97 stars 66 forks source link

LV131 air_quality is an Enum, not AQI #147

Closed gdgib closed 1 year ago

gdgib commented 1 year ago

I'm not sure this change works across all devices, but it definitely works for the LV131. I've made the change in my fork, and I'd be HAPPY to learn how to limit this to the specific devices it applies to, but I figured I'd open the PR and see if I could get a little help with that.

The problem is that this integration has been using SensorDeviceClass.AQI to report the air quality from the LV131. However, AIR must now be numerical, whereas the LV131 returns a string, rather than an actual AQI number. I think it's only recently that HA has started enforcing numerical values (not string) in https://github.com/home-assistant/core/blob/dev/homeassistant/components/sensor/__init__.py#L465, because there used to be a warning and now it's just broken. Without the change in my PR, the LV131 air quality sensor is disabled, and the HA logs include ValueError with a message that it can't accept a string as a numerical value.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

vlebourl commented 1 year ago

Thanks for the submission! good catch. Can you fix linting in your code?

Also, at the end of the native_value property, we have some logic to handle string vs numeric type for the value itslef. Could you do something similar for the class and state_class? A rough idea would be for instance

    return SensorDeviceClass.ENUM if isinstance(self.native_value, str) else SensorDeviceClass.AQI

There might be an initialization issue if we look at the type of self. native_value but there may be something usable in the diagnostics.

gdgib commented 1 year ago

Thanks for the suggestions. I'll get on it this weekend.

In the meantime, I hope you'll pardon my ignorance, but can you point me in the rough direction of the diagnostics? From context it sounds like there's perhaps an endpoint or a dump of information I can get from the actual device. I can figure most of it out for myself, but if there's any kind of link or reference you can send me it'll save some confusion on my end.

dylanlive commented 1 year ago

I noticed today I've been receiving the below error related to fetching the air quality, and I'm guessing this PR may resolve it. Will look into opening an issue later. v2023.6.3, LV-PUR131S, v1.0.1 of this plugin.

ValueError: Sensor sensor.levoit_131s_air_purifier_air_quality has device class 'aqi', state class 'measurement' unit 'None' and suggested precision 'None' thus indicating it has a numeric value; however, it has the non-numeric value: 'Excellent' (<class 'str'>)
2023-07-06 15:13:48.884 ERROR (MainThread) [homeassistant.components.switch] vesync: Error on device update!
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 537, in _async_add_entity
    await entity.async_device_update(warning=False)
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 784, in async_device_update
    await self.async_update()
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 469, in async_update
    await self.coordinator.async_request_refresh()
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 236, in async_request_refresh
    await self._debounced_refresh.async_call()
  File "/usr/src/homeassistant/homeassistant/helpers/debounce.py", line 95, in async_call
    await task
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 262, in async_refresh
    await self._async_refresh(log_failures=True)
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 374, in _async_refresh
    self.async_update_listeners()
  File "/usr/src/homeassistant/homeassistant/helpers/update_coordinator.py", line 164, in async_update_listeners
    update_callback()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 590, in async_write_ha_state
    self._async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 654, in _async_write_ha_state
    state = self._stringify_state(available)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 596, in _stringify_state
    if (state := self.state) is None:
                 ^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1073, in state
    if (is_on := self.is_on) is None:
                 ^^^^^^^^^^
  File "/config/custom_components/vesync/switch.py", line 177, in is_on
    return self.device.details["display"]
           ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^

Reinstalled the base VeSync integration and not seeing this error.

Screenshot of Base Vesync

base_vesync

Screenshot of Custom Vesync (this plugin)

custom_vesync