-
Notifications
You must be signed in to change notification settings - Fork 62
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
WIP: Warn about unusual sensor readings #78
base: master
Are you sure you want to change the base?
Conversation
As in my comment on the issue: It won't work this way. Some implementation-related remarks presupposing you actually want to go through with the mentioned noise estimation:
|
Giving it some more thought: If we want to warn/fail only if the noise is actually 0, the noise estimation can (and should) be rather cheap. |
Sound reasonable. But just to be on the same side, what exactly do you mean by noise here? |
Well, the noise in the temperature readings. If there is no noise in a sensor, we can safely assume that sensor isn't measuring anything. That said, we don't need a "correct" noise measure. We could e.g. just sum up the temperature changes during the last N cycles. If that sum is 0 for a large enough N, we know something must be wrong. |
Ok, that sound fairly simple and I'll try to implement it. |
@@ -41,6 +41,7 @@ | |||
#include "config.h" | |||
#include "message.h" | |||
|
|||
const int NOISE_MEASUREMENT = 10; |
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.
constexpr int NOISE_MEASURE_COUNT = 10;
@@ -299,6 +303,20 @@ void TemperatureState::add_temp(int t) | |||
int diff = t - *temp_; | |||
*temp_ = t; | |||
|
|||
if (diff == 0) { | |||
if (*noise_counter < NOISE_MEASUREMENT) { |
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.
Could you explain why do we need the whole vector noise_counters
when we only use the first element?
If I understand your logic, then you just need to keep one static count, increase that count every time diff==0
and compare that count with a fix threshold. In this case, you don't even need noise_counter
and noise_counters
as member variables.
Oh just realize this is from 2019. Sorry for digging it up. Do we still need this feature though? |
Related issue: #77
This PR attempts to warn user about unusual sensors readings by log messages. In current state it is more like crude attempt and I am open to suggestions, how to make this PR better.
There certainly is room for improvements, just to mention few:
SensorDriver
instead of impletement it in every driverread_temps()
?#define
in header file?