Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Climate offline, temperature sensor still reports value #43897

Closed
rr326 opened this issue Dec 3, 2020 · 10 comments · Fixed by #51873
Closed

Climate offline, temperature sensor still reports value #43897

rr326 opened this issue Dec 3, 2020 · 10 comments · Fixed by #51873

Comments

@rr326
Copy link

rr326 commented Dec 3, 2020

The problem

I have some thermostat devices - some ecobees and a Nuheat. They create climate.* and sensor.xxx_temperature entities.

When the thermostat is offline, the climate will properly show state==None. But the temperature sensor will continue to show the last valid value - which is definitely no longer valid. This means I'm relying on a value that is incorrect. Further, I can't, easily, just use the "current temperature" value of the climate entity in my use case. (In my case, I use a custom card that doesn't handle null values well.)

I think the proper behavior should be when offline, the sensor should read None, or maybe numpy.Nan

Environment

  • Home Assistant Core release with the issue: 0.188.4
  • Last working Home Assistant Core release (if known):
  • Operating environment (OS/Container/Supervised/Core): Docker container, armv71, 5.4.72-v71+
  • Integration causing this issue: ecobee
  • Link to integration documentation on our website:

Problem-relevant configuration.yaml

# Configure a default setup of Home Assistant (frontend, api, etc)
default_config:

# Text to speech
tts:
  - platform: google_translate

group: !include groups.yaml
script: !include scripts.yaml
scene: !include scenes.yaml
automation: !include_dir_merge_list automations

homeassistant:
  name: Home
  latitude: !secret latitude
  longitude: !secret longitude
  elevation: !secret elevation
  unit_system: imperial
  temperature_unit: F
  time_zone: America/Los_Angeles
  internal_url: http://localhost:8123
  customize: !include customize.yaml
  packages:
    package_inputs: !include package_inputs.yaml

lovelace:
  mode: yaml
  resources:
    - url: /local/button-card.js?v=1
      type: module
    - url: /local/simple-thermostat.js?v=1
      type: module
    - url: /local/banner-card.js?v=1
      type: module
    - url: /local/card-mod.js?v=1
      type: module
    - url: /local/bignumber_fork/bignumber-card.js?v=1
      type: module
    - url: /local/stack-in-card.js?v=1
      type: module
    - url: /local/auto-entities.js?v=1
      type: module

  dashboards:
    dashboards-all:
      mode: yaml
      title: Dashboards
      # icon: mdi:script
      show_in_sidebar: true
      filename: dashboards/main_dashboard.yaml

zwave:
  network_key: !secret zwave_network_key
  
# Turn on http: needed by api
http:
# Turn on api: needed by MQtt
api:

logger:
  default: info

mqtt:
  broker: 127.0.0.1
  port: 1883

notify:
  - name: event_log
    platform: file
    filename: /config/event_log.log

  # See DEVNOTES.md for details
  - name: gmail
    platform: smtp
    server: smtp.gmail.com
    port: 587
    encryption: starttls
    timeout: 30
    sender: !secret gmail_un
    username: !secret gmail_un
    password: !secret gmail_app_password
    recipient: !secret email_recipient
    sender_name: !secret email_sender_name

Traceback/Error logs

Additional information

If this is indeed a bug (and not a side effect of something needed or a design decision), if you point me a the likely solution, I'll try to fix and issue a PR.

@frenck
Copy link
Member

frenck commented Dec 5, 2020

So which integration is this bug report for? We can only target one integration per issue...

@rr326
Copy link
Author

rr326 commented Dec 5, 2020

I'd start with the ecobee integration. I know that is a problem. But I'm pretty sure it is in the NuHeat integration as well. So either both integrations copied the same code (and bug), or could it take place outside the integration?

@frenck
Copy link
Member

frenck commented Dec 5, 2020

No, this behavior is defined by the integration itself.

@probot-home-assistant
Copy link

ecobee documentation
ecobee source
(message by IssueLinks)

@probot-home-assistant
Copy link

Hey there @marthoc, mind taking a look at this issue as its been labeled with an integration (ecobee) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@hakusaro
Copy link

hakusaro commented Dec 7, 2020

There's probably a lot of good value in at least publishing an announcement about how users should re-add their integrations. This is my first real experience with HA, and it seems like some parts of the stack don't report errors back in a noticeable way, particularly when it comes to automations. The only way I knew it was as simple as re-adding the integration was finding this thread. Granted, the ecobee API is not intuitive and should probably offer some scheme for users to be able to continue to use existing apps -- but the UX here is pretty meh given that the underlying everything still technically works, but you need to do work to re-add it for existing setups to continue working.

Is it possible to add something like this to release notes / changelogs / etc easily?

@rr326
Copy link
Author

rr326 commented Dec 16, 2020

@marthoc I've been looking in the code at components/ecobee/sensor.py. It's all new to me, but I think the logic is:

  • Wait for an update from the EcobeeData object
  • If the capability is 'temperature' (or 'humidity'), then update the state
  • Otherwise ignore and move on

This would definitely cause the problem I'm seeing since when the ecobee is offline, it doesn't have a 'temperature' capability.

The relevant code is here:

    async def async_update(self):
        """Get the latest state of the sensor."""
        await self.data.update()
        for sensor in self.data.ecobee.get_remote_sensors(self.index):
            if sensor["name"] != self.sensor_name:
                continue
            for item in sensor["capability"]:
                if item["type"] != self.type: # <-- If offline, there will be no 'temperature' / self.type
                    continue
                self._state = item["value"]
                break

You'd probably need to add a:

if self.type not in sensor["capability"]:
   # Offline
   self._state = math.nan # Can't set state to None

It looks to me like this is generic code. It probably affects all the sensors in ecobee, and quite likely other components that copied this logic.

If that's the case, this could be a big fix across many components that would need to be thought through. Definitely above my pay grade for a PR!

@github-actions
Copy link

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.

@github-actions github-actions bot added the stale label Mar 16, 2021
@rr326
Copy link
Author

rr326 commented Mar 16, 2021

@frenck (or someone else) - I think someone with good overview of the HA system and feature / bug roadmap should look into this. I do not need it for me, but if I'm right, then many or most integrations are publishing meaningfully incorrect data. It seems a pretty big and fundamental error, if I'm right. Someone should look into and either disprove, or put in the appropriate place on the roadmap.

@github-actions github-actions bot removed the stale label Mar 16, 2021
@github-actions
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants