onewire_parser: Implement glitch removal

[rrdd] / onewire_parser.c

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);