home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.71k stars 30.83k forks source link

SNMP doesn't honor correct template with device_class timestamp #114980

Open Petro31 opened 7 months ago

Petro31 commented 7 months ago

The problem

Using the SNMP integration, if you supply a device_class: timestamp SNMP sensor with a proper datetime object from value_template, the logs are flooded with errors on every template resolution.

- platform: snmp
  host: ...
  name: X
  device_class: timestamp
  baseoid: ...
  value_template: "{{ now() - timedelta(day=1) }}"

What version of Home Assistant Core has the issue?

2024.4.1

What was the last working version of Home Assistant Core?

Unknown

What type of installation are you running?

Home Assistant Supervised

Integration causing the issue

SNMP

Link to integration documentation on our website

https://www.home-assistant.io/integrations/snmp/

Diagnostics information

No response

Example YAML snippet

- platform: snmp
  host: ...
  name: X
  device_class: timestamp
  baseoid: ...
  value_template: "{{ now() - timedelta(day=1) }}"

### Anything in the logs that might be useful for us?

```txt
ValueError: Invalid datetime: sensor.synology_uptime has timestamp device class but provides state 2024-01-10 06:20:35.120629-05:00:<class 'str'> resulting in ''str' object has no attribute 'tzinfo''


### Additional information
home-assistant[bot] commented 7 months ago

snmp documentation snmp source

lextm commented 7 months ago
"""SNMP sensor tests."""

from unittest.mock import patch

from pysnmp.hlapi import TimeTicks
import pytest

from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er
from homeassistant.setup import async_setup_component

@pytest.fixture(autouse=True)
def hlapi_mock():
    """Mock out 3rd party API."""
    mock_data = TimeTicks(586867)
    with patch(
        "homeassistant.components.snmp.sensor.getCmd",
        return_value=(None, None, None, [[mock_data]]),
    ):
        yield

async def test_basic_config(hass: HomeAssistant) -> None:
    """Test basic entity configuration."""

    config = {
        SENSOR_DOMAIN: {
            "platform": "snmp",
            "host": "192.168.1.32",
            "baseoid": "1.3.6.1.4.1.2021.10.1.3.1",
        },
    }

    assert await async_setup_component(hass, SENSOR_DOMAIN, config)
    await hass.async_block_till_done()

    state = hass.states.get("sensor.snmp")
    assert state.state == ""
    assert state.attributes == {"friendly_name": "SNMP"}

async def test_entity_config(hass: HomeAssistant) -> None:
    """Test entity configuration."""

    config = {
        SENSOR_DOMAIN: {
            # SNMP configuration
            "platform": "snmp",
            "host": "192.168.1.32",
            "baseoid": "1.3.6.1.4.1.2021.10.1.3.1",
            # Entity configuration
            "icon": "{{'mdi:one_two_three'}}",
            "picture": "{{'blabla.png'}}",
            "device_class": "timestamp",
            "name": "{{'SNMP' + ' ' + 'Sensor'}}"
        },
    }

    assert await async_setup_component(hass, SENSOR_DOMAIN, config)
    await hass.async_block_till_done()

    entity_registry = er.async_get(hass)

    state = hass.states.get("sensor.snmp_sensor")
    assert state.state == ""
    assert state.attributes == {
        "device_class": "temperature",
        "entity_picture": "blabla.png",
        "friendly_name": "SNMP Sensor",
        "icon": "mdi:one_two_three",
        "state_class": "timestamp"
    }

We can reproduce this issue with the test case above, and the full error call stack is

INFO:homeassistant.loader:Loaded sensor from homeassistant.components.sensor
INFO:homeassistant.loader:Loaded snmp from homeassistant.components.snmp
INFO:homeassistant.setup:Setting up sensor
INFO:homeassistant.components.sensor:Setting up snmp.sensor
ERROR:homeassistant.components.sensor:Error adding entity sensor.snmp_sensor for domain sensor with platform snmp
Traceback (most recent call last):
  File "/srv/core/homeassistant/components/sensor/__init__.py", line 600, in state
    if value.tzinfo is None:
       ^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'tzinfo'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/srv/core/homeassistant/helpers/entity_platform.py", line 580, in _async_add_entities
    await coro
  File "/srv/core/homeassistant/helpers/entity_platform.py", line 881, in _async_add_entity
    await entity.add_to_platform_finish()
  File "/srv/core/homeassistant/helpers/entity.py", line 1333, in add_to_platform_finish
    await self.async_added_to_hass()
  File "/srv/core/homeassistant/components/snmp/sensor.py", line 214, in async_added_to_hass
    await self.async_update()
  File "/srv/core/homeassistant/components/snmp/sensor.py", line 230, in async_update
    self._process_manual_data(raw_value)
  File "/srv/core/homeassistant/helpers/trigger_template_entity.py", line 239, in _process_manual_data
    self.async_write_ha_state()
  File "/srv/core/homeassistant/helpers/entity.py", line 998, in async_write_ha_state
    self._async_write_ha_state()
  File "/srv/core/homeassistant/helpers/entity.py", line 1119, in _async_write_ha_state
    state, attr, capabilities, shadowed_attr = self.__async_calculate_state()
                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/srv/core/homeassistant/helpers/entity.py", line 1056, in __async_calculate_state
    state = self._stringify_state(available)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/srv/core/homeassistant/helpers/entity.py", line 1004, in _stringify_state
    if (state := self.state) is None:
                 ^^^^^^^^^^
  File "/srv/core/homeassistant/components/sensor/__init__.py", line 611, in state
    raise ValueError(
ValueError: Invalid datetime: sensor.snmp_sensor has timestamp device class but provides state 586867:<class 'str'> resulting in ''str' object has no attribute 'tzinfo''

Not sure what's the actual meaning of "timestamp device class", but that seems to be beyond the SNMP/PySNMP scope and more related to HA sensor API itself. Maybe another device class should be used instead of timestamp.

Petro31 commented 7 months ago
"""SNMP sensor tests."""

from unittest.mock import patch

from pysnmp.hlapi import TimeTicks
import pytest

from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN
from homeassistant.core import HomeAssistant
from homeassistant.helpers import entity_registry as er
from homeassistant.setup import async_setup_component

@pytest.fixture(autouse=True)
def hlapi_mock():
    """Mock out 3rd party API."""
    mock_data = TimeTicks(586867)
    with patch(
        "homeassistant.components.snmp.sensor.getCmd",
        return_value=(None, None, None, [[mock_data]]),
    ):
        yield

async def test_basic_config(hass: HomeAssistant) -> None:
    """Test basic entity configuration."""

    config = {
        SENSOR_DOMAIN: {
            "platform": "snmp",
            "host": "192.168.1.32",
            "baseoid": "1.3.6.1.4.1.2021.10.1.3.1",
        },
    }

    assert await async_setup_component(hass, SENSOR_DOMAIN, config)
    await hass.async_block_till_done()

    state = hass.states.get("sensor.snmp")
    assert state.state == ""
    assert state.attributes == {"friendly_name": "SNMP"}

async def test_entity_config(hass: HomeAssistant) -> None:
    """Test entity configuration."""

    config = {
        SENSOR_DOMAIN: {
            # SNMP configuration
            "platform": "snmp",
            "host": "192.168.1.32",
            "baseoid": "1.3.6.1.4.1.2021.10.1.3.1",
            # Entity configuration
            "icon": "{{'mdi:one_two_three'}}",
            "picture": "{{'blabla.png'}}",
            "device_class": "timestamp",
            "name": "{{'SNMP' + ' ' + 'Sensor'}}"
        },
    }

    assert await async_setup_component(hass, SENSOR_DOMAIN, config)
    await hass.async_block_till_done()

    entity_registry = er.async_get(hass)

    state = hass.states.get("sensor.snmp_sensor")
    assert state.state == ""
    assert state.attributes == {
        "device_class": "temperature",
        "entity_picture": "blabla.png",
        "friendly_name": "SNMP Sensor",
        "icon": "mdi:one_two_three",
        "state_class": "timestamp"
    }

We can reproduce this issue with the test case above, and the full error call stack is

INFO:homeassistant.loader:Loaded sensor from homeassistant.components.sensor
INFO:homeassistant.loader:Loaded snmp from homeassistant.components.snmp
INFO:homeassistant.setup:Setting up sensor
INFO:homeassistant.components.sensor:Setting up snmp.sensor
ERROR:homeassistant.components.sensor:Error adding entity sensor.snmp_sensor for domain sensor with platform snmp
Traceback (most recent call last):
  File "/srv/core/homeassistant/components/sensor/__init__.py", line 600, in state
    if value.tzinfo is None:
       ^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'tzinfo'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/srv/core/homeassistant/helpers/entity_platform.py", line 580, in _async_add_entities
    await coro
  File "/srv/core/homeassistant/helpers/entity_platform.py", line 881, in _async_add_entity
    await entity.add_to_platform_finish()
  File "/srv/core/homeassistant/helpers/entity.py", line 1333, in add_to_platform_finish
    await self.async_added_to_hass()
  File "/srv/core/homeassistant/components/snmp/sensor.py", line 214, in async_added_to_hass
    await self.async_update()
  File "/srv/core/homeassistant/components/snmp/sensor.py", line 230, in async_update
    self._process_manual_data(raw_value)
  File "/srv/core/homeassistant/helpers/trigger_template_entity.py", line 239, in _process_manual_data
    self.async_write_ha_state()
  File "/srv/core/homeassistant/helpers/entity.py", line 998, in async_write_ha_state
    self._async_write_ha_state()
  File "/srv/core/homeassistant/helpers/entity.py", line 1119, in _async_write_ha_state
    state, attr, capabilities, shadowed_attr = self.__async_calculate_state()
                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/srv/core/homeassistant/helpers/entity.py", line 1056, in __async_calculate_state
    state = self._stringify_state(available)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/srv/core/homeassistant/helpers/entity.py", line 1004, in _stringify_state
    if (state := self.state) is None:
                 ^^^^^^^^^^
  File "/srv/core/homeassistant/components/sensor/__init__.py", line 611, in state
    raise ValueError(
ValueError: Invalid datetime: sensor.snmp_sensor has timestamp device class but provides state 586867:<class 'str'> resulting in ''str' object has no attribute 'tzinfo''

Not sure what's the actual meaning of "timestamp device class", but that seems to be beyond the SNMP/PySNMP scope and more related to HA sensor API itself. Maybe another device class should be used instead of timestamp.

No. SNMP should honor device_classes. Here's how template sensor does it to handle this case:

    @callback
    def _update_state(self, result):
        super()._update_state(result)
        if isinstance(result, TemplateError):
            self._attr_native_value = None
            return

        if result is None or self.device_class not in (
            SensorDeviceClass.DATE,
            SensorDeviceClass.TIMESTAMP,
        ):
            self._attr_native_value = result
            return

        self._attr_native_value = async_parse_date_datetime(
            result, self.entity_id, self.device_class
        )

It specifically calls out this device class to handle the returned template value. SNMP needs to incorporate this. This is done in other integrations as well.

issue-triage-workflows[bot] commented 4 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

Petro31 commented 4 months ago

Not stale

ptr727 commented 1 month ago

Ran into this, reporting battery time remaining for UPS, would like to use timestamp, resorted to reporting seconds due to this issue.