onewire_parser: Implement glitch removal

Some times in noisy 1wire networks there might be random bit flips
during read, which slip through even from the CRC check done in
line. This might lead to errorneous readings that cause spikes to
graphs.

Implement a relatively straight forward logic for avoiding the
glitches. Here we keep a track of 20 delta values between samples and
the latest reading (so that we can calculate the new delta). If the
latest delta is significantly larger than biggest known delta within
the sample history, we will retry reading the sensor. If we get
exactly same value back we trust the reading. If the value keeps on
changing or we get too many failures, we get what we can.

This should allow recovery for most simple transient glitches.

Signed-off-by: Timo Kokkonen <timo.t.kokkonen@iki.fi>

diff --git a/onewire_parser.c b/onewire_parser.c
index effd097619f0f1421e2a0b2bc5f546c42198b692..fe29aa483c2f01749c029d0f7e74908ba3dbaa8e 100644 (file)
--- a/onewire_parser.c
+++ b/onewire_parser.c
@@ -4,6 +4,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <math.h>
 
 #include "parser.h"
 #include "debug.h"
@@ -11,8 +12,67 @@
 #include "utils.h"
 #include "plugin.h"
 #include "version.h"
+#include "utils.h"
+
+struct owparser_state {
+       double prev_delta[20];
+       double prev_data;
+};
+
+static struct owparser_state *allocate_parser_state(const char **datastr)
+{
+       int i;
+
+       /* Count how many sensor entries we need */
+       for (i = 0; datastr[i]; i++)
+               ;
+
+       /* The first entry belongs to server address or mount point */
+       i--;
+
+       return calloc(sizeof(struct owparser_state), i);
+}
+
+static int might_be_glitch(double data, const struct owparser_state *s)
+{
+       double max_delta = 0, delta;
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(s->prev_delta); i++)
+               max_delta = max(s->prev_delta[i], max_delta);
+
+       /* Probably no enough data yet, so no glitch detection */
+       if (!max_delta)
+               return 0;
+
+       /*
+        * Simple glitch detection. If delta to previous value is more
+        * than twice as larger as some older delta, we might have a
+        * glit
+        */
+       delta = fabs(data - s->prev_data);
+       return delta > max_delta * 2;
+}
+
+static void update_glitch_data(double data, struct owparser_state *s)
+{
+       double max_delta = 0;
+       int i;
+
+       for (i = 1; i < ARRAY_SIZE(s->prev_delta); i++) {
+               s->prev_delta[i - 1] = s->prev_delta[i];
+               max_delta = max(s->prev_delta[i], max_delta);
+       }
 
-static int parse_opts(const char *str, char *ow_path, size_t pathlen, double *offset)
+       /* Avoid storing the first incorrect delta value */
+       if (s->prev_data || max_delta)
+               s->prev_delta[--i] = fabs(data - s->prev_data);
+
+       s->prev_data = data;
+}
+
+static int parse_opts(const char *str, char *ow_path, size_t pathlen,
+               double *offset)
 {
        char *endptr;
        const char *start_str = str;
@@ -121,6 +181,7 @@ static int onewire_parser(char *rrd_data, const char **parser_data, void **s)
 {
        OWNET_HANDLE h;
        const char *server_addr, *mount_point;
+       struct owparser_state *state = *s;
        char buf[24], *tmp;
        int i = 1, ret;
        int max_str = RRD_DATA_MAX_LEN;
@@ -131,6 +192,9 @@ static int onewire_parser(char *rrd_data, const char **parser_data, void **s)
                return -1;
        }
 
+       if (!state)
+               *s = state = allocate_parser_state(parser_data);
+
        if (is_mountpoint) {
                mount_point = parser_data[0];
 
@@ -154,10 +218,11 @@ static int onewire_parser(char *rrd_data, const char **parser_data, void **s)
        }
 
        while (parser_data[i]) {
-               double offset = 0, data;
+               double offset = 0, data, prev_data = 85;
                char *endptr;
                char ow_path[1024];
                int retries = 0;
+               int glitches = 0;
 
                if (!strcmp("U", parser_data[i])) {
 undefined:
@@ -192,17 +257,47 @@ undefined:
                        else
                                fail = 1;
 
+                       if (ret <= 0 || fail)
+                               goto retry;
+
+
+                       /*
+                        * Older versions of OWNET_read did not NULL
+                        * terminate data.
+                        */
+                       memcpy(buf, tmp, min(ret, sizeof(buf) -1));
+                       buf[ret] = 0;
+
+                       data = strtod(buf, &endptr);
+
+                       free(tmp);
+                       tmp = NULL;
+
+                       /*
+                        * If we read the same value as previously,
+                        * it's not a glitch
+                        */
+                       if (glitches && prev_data == data)
+                               break;
+
+                       if (might_be_glitch(data, &state[i]) &&
+                               glitches < 2 && retries < 7) {
+                               glitches++;
+                               prev_data = data;
+                               pr_info("Retrying due to a glitch: %f\n", data);
+                               goto retry;
+                       }
+
+                       break;
+retry:
                        /*
                         * In case of failure, retry with uncached
                         * data. This is likely to help as it forces a
                         * retry even if the sensor is missing from
-                        * the cache. We thread "85" also as a failure
-                        * above, as temp sensors some times report 85
-                        * under faulty conditions.
+                        * the cache. We treat "85" also as a failure,
+                        * as temp sensors some times report 85 under
+                        * faulty conditions.
                         */
-                       if (ret > 0 && !fail)
-                               break;
-
                        ret = make_uncached(ow_path, sizeof(ow_path));
                        if (retries >= 10 || ret < 0) {
                                pr_err("Failed to read entry %s: %m\n",
@@ -214,12 +309,7 @@ undefined:
                                free(tmp);
                }
 
-               /* The data from OWNET_read appears to not be NULL terminated */
-               memcpy(buf, tmp, min(ret, sizeof(buf) -1));
-               free(tmp);
-               buf[ret] = 0;
-
-               data = strtod(buf, &endptr);
+               update_glitch_data(data, &state[i]);
 
                if (endptr == buf) {
                        pr_err("Failed to parse data %s\n", buf);