From: Timo Kokkonen Date: Fri, 28 Oct 2016 17:38:43 +0000 (+0300) Subject: onewire_parser: Implement glitch removal X-Git-Url: http://git.itanic.dy.fi/?p=rrdd;a=commitdiff_plain;h=037c08bff3634581ac11e07ed7f9bbc4838be3b8 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 --- diff --git a/onewire_parser.c b/onewire_parser.c index effd097..fe29aa4 100644 --- a/onewire_parser.c +++ b/onewire_parser.c @@ -4,6 +4,7 @@ #include #include #include +#include #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);