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
71.89k stars 30.12k forks source link

restful sensor availability template is not evaluated #119992

Open litinoveweedle opened 3 months ago

litinoveweedle commented 3 months ago

The problem

With configuration as bellow:

rest:
  - authentication: basic
    username: "user"
    password: "pass"
    scan_interval: 60
    timeout: 20
    resource: http://IP/url
    sensor:
      - name: "tigo_a01_vin"
        value_template: "{{ value_json.data.A1.Vin | float }}"
        availability: "{{ value_json is mapping and 'active' in value_json and value_json.active and 'data' in value_json and value_json.data is mapping and 'A1' in value_json.data and value_json.data.A1.Vin is defined }}"
        device_class: "voltage"
        unit_of_measurement: "V"

it should prevent any error in value template evaluation when key 'A1' is missing in the value_json.data.

Unfortuantelly is doesn't:

2024-06-19 21:16:09.062 ERROR (MainThread) [homeassistant.helpers.template] Template variable error: 'dict object' has no attribute 'A1' when rendering '{{ value_json.data.A1.Vin | float }}'

When put such availability template content into Developer Tools Template checker it is evaluated correctly as false, but evaluation inside of HA it seems to be obviously returning true and not preventing value template from being executed and throwing error.

As it is kind a know fact, that Jinja templates are evaluated differently in the template checker and the HA core, this points to a bug in the HA core as the behavior of the template checker is correct one here:

{% set value_json ={"active": true, "data": {}} %}
{{ value_json is mapping and 'active' in value_json and value_json.active and 'data' in value_json and value_json.data is mapping and 'A1' in value_json.data and value_json.data.A1.Vin is defined }}

Result type: boolean
false
{% set value_json ={"active": true, "data": { "A1": { "Vin": 10 }}} %}
{{ value_json is mapping and 'active' in value_json and value_json.active and 'data' in value_json and value_json.data is mapping and 'A1' in value_json.data and value_json.data.A1.Vin is defined }}

Result type: boolean
true

What version of Home Assistant Core has the issue?

2024.6.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Supervised

Integration causing the issue

restful

Link to integration documentation on our website

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

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 3 months ago

rest documentation rest source

litinoveweedle commented 3 months ago

Base on the quick look at the code I do not see any place where availability template is being evaluated!

core/homeassistant/components/rest/sensor.py:

    def _update_from_rest_data(self) -> None:
        """Update state from the rest data."""
        try:
            value = self.rest.data_without_xml()
        except ExpatError as err:
            _LOGGER.warning(
                "REST xml result could not be parsed and converted to JSON: %s", err
            )
            value = self.rest.data

        if self._json_attrs:
            self._attr_extra_state_attributes = parse_json_attributes(
                value, self._json_attrs, self._json_attrs_path
            )

        raw_value = value

        if value is not None and self._value_template is not None:
            value = self._value_template.async_render_with_possible_json_value(
                value, None
            )

        if value is None or self.device_class not in (
            SensorDeviceClass.DATE,
            SensorDeviceClass.TIMESTAMP,
        ):
            self._attr_native_value = value
            self._process_manual_data(raw_value)
            self.async_write_ha_state()
            return

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

        self._process_manual_data(raw_value)
        self.async_write_ha_state()

If availability template should be evaluated it has to be before this:

        if value is not None and self._value_template is not None:
            value = self._value_template.async_render_with_possible_json_value(
                value, None
            )

so this is obvious mistake as documentation clearly mentions availability template in the rest sensor configuration, but it is not implemented!

https://www.home-assistant.io/integrations/sensor.rest#configuration-variables

availability template (optional)
Defines a template if the entity state is available or not.
litinoveweedle commented 3 months ago

Roughly something like this could do the trick (+ config option for availability)

    def _update_from_rest_data(self) -> None:
        """Update state from the rest data."""
        try:
            value = self.rest.data_without_xml()
        except ExpatError as err:
            _LOGGER.warning(
                "REST xml result could not be parsed and converted to JSON: %s", err
            )
            value = self.rest.data

+        if self._availability is not None:
+            if not self._availability.async_render_with_possible_json_value(
+                value, None
+            ):
+                self._attr_native_value = STATE_UNAVAILABLE
+                self.async_write_ha_state()
+                return

        if self._json_attrs:
            self._attr_extra_state_attributes = parse_json_attributes(
                value, self._json_attrs, self._json_attrs_path
            )

        raw_value = value

        if value is not None and self._value_template is not None:
            value = self._value_template.async_render_with_possible_json_value(
                value, None
            )

        if value is None or self.device_class not in (
            SensorDeviceClass.DATE,
            SensorDeviceClass.TIMESTAMP,
        ):
            self._attr_native_value = value
            self._process_manual_data(raw_value)
            self.async_write_ha_state()
            return

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

        self._process_manual_data(raw_value)
        self.async_write_ha_state()
timr49 commented 3 months ago

This issue appears to be the same as "Rest sensor not updating (non-numeric value: 'None') #99942". As discussed there, the availability template is evaluated but not until after the state is evaluated. The value_template needs to deal with data that does not conform with what it expects (unless/until changes such as suggested by @litinoveweedle above). To put it another way, the availability template does not filter the data for the benefit of value_template; rather, it tells the state machine and hence the rest of HA whether the entity should be considered available.

litinoveweedle commented 3 months ago

Hello, thank you for the reference to the other issue. Based on the information discussed in #99942, I believe that the current implementation of the availability and value templates is not optimal and shall be improved.

Reasoning:

The proposal would be to either evaluate availability template before value template to prevent from needs to duplicate all checks and only evaluate value template if True is returned by availability template - I can provide PR for such proposal for all affected rest entities.

It is also possible to implement better handling for None value returned by value template to be interpreted as a either STATE_UNKNOWN or STATE_UNAVAILABLE.

timr49 commented 3 months ago

Hello, thank you for the reference to the other issue. Based on the information discussed in #99942, I believe that the current implementation of the availability and value templates is not optimal and shall be improved. ...

By the way @litinoveweedle, there is a closed/stale PR "Manual trigger template handle faulty state #100079" that references issue 99942. I don't know why that PR was not progressed.

issue-triage-workflows[bot] commented 1 week 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.

litinoveweedle commented 1 week ago

Error is still present at HA 2024.09, it also affects multiple users as current behaviour is country intuitive to using templates entities.

The root cause of the issue is that value template is executed completely without any dependency to the availability template. In such case one could not use availability template to decide, if the value template should be executed at all.

Proposed solution should be to execute value template only in case when availability template returns true.

This solution would also decrease load to HA, where value template doesn't need to be executed at all, when availability template returns false.