-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
Clear miflora sensor state on exception #29276
Conversation
Clear state if querying the device fails. The state is then set to unknown, so it can be tracked if a miflora device isn't responding any more.
Hey there @Danielhiversen, @ChristianKuehnel, mind taking a look at this pull request as its been labeled with a integration ( |
@@ -158,9 +158,11 @@ def update(self): | |||
data = self.poller.parameter_value(self.parameter) | |||
except OSError as ioerr: | |||
_LOGGER.info("Polling error %s", ioerr) | |||
self._state = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of self._state = None
, we should return False
from the available
property.
@@ -158,9 +158,11 @@ def update(self): | |||
data = self.poller.parameter_value(self.parameter) | |||
except OSError as ioerr: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can combine the except clause.
except OSError as ioerr: | |
except (OSError, BluetoothBackendException) as ioerr: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use available property.
Signal valid data via available()
I think "available()" and "except merge" should be changed in mitemp_bt, too. (?)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good! Please update the PR description according to latest changes. Then we can merge.
@ferbar: Great idea! We have quite a few users who's sensors are at the edge of what's feasible with regards to the range of the Bluetooth devices. If you fail on the first exception, this will cause a lot of errors, as the Bluetooth communication is quite unreliable. So they would get tons of errors this way. I would recommend something like: "fail if no new data for x hours", where x can be configured. Rationale: |
@ChristianKuehnel: I'm with you. I was playing around with last_updated and last_changed before. However last_updated is changed even if update() was unsuccessful. I think that a timeout for values to get unavailable should be implemented in Entity or a derived class since this would affect many types of sensors and would produce a lot of copy&paste otherwise.
Do you have any idea how to prevent last_updated get updated in an error case? |
No I don't.
What about creating your own stopwatch: remember the time of the last
successful update. If it's older than your threshold: null the values.
Am So., 1. Dez. 2019 um 18:43 Uhr schrieb Christian Ferbar <
[email protected]>:
… @ChristianKuehnel <https://github.com/ChristianKuehnel>: I'm with you. I
was playing around with last_updated and last_changed before. However
last_updated is changed even if update() was unsuccessful. I think that a
timeout for values to get unavailable should be implemented in Entity or a
derived class since this would affect many types of sensors and would
produce a lot of copy&paste otherwise.
- I didn't find out why last_updated is changed (I was searching for a
long time)
- I tried to override last_update with device_state_attributes.
Worked, but I think this is really ugly :-D IMHO the current solution is
better.
- I don't like to track an additional last_update timestamp.
Do you have any idea how to prevent last_updated get updated in an error
case?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#29276>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEYJMCCPBH2P6JH3MSTWDBDQWPZS3ANCNFSM4JTIV64A>
.
|
Breaking Change:
Values for a unresponsive device aren't reported any more as if they were valid, so it's possible to send an alert.
Description:
Report avaiable() = False if querying the device fails. The state is then set to unavailable, so it can be tracked if a miflora device isn't responding any more.
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>
Example entry for
automations.yaml
:Checklist:
tox
. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
python3 -m script.hassfest
.requirements_all.txt
by runningpython3 -m script.gen_requirements_all
..coveragerc
.If the code does not interact with devices:
Updates:
changed unknown to unavailable in automations.yaml