refclock: stop requiring 4 samples in median filter

Reduce the minimum number of samples required by the filter from
min(4, length) to 1.

This makes the filtering less confusing. The sample lifetime is limited
to one poll and the default filtering of the SOCK refclock (where the
maximum number of samples per poll is unknown) is identical to the other
refclocks.

A concern with potential variability in number of samples per poll below
4 is switching between different calculations of dispersion in
combine_selected_samples() in samplefilt.c.

The 106-refclock test shows how the order of refclocks in the config can
impact the first filtered sample and selection. If the PPS refclock
follows SHM, a single low-quality PPS sample is accepted in the same
poll where SHM is selected and the initial clock correction started,
which causes larger skew later and delays the first selection of the PPS
refclock.

diff --git a/doc/chrony.conf.adoc b/doc/chrony.conf.adoc
index 95c7f0f817257f1f70512e7f3964dfdefdc58fcb..9879b44c8cbe8ba6d0f5b45c02c11ca27778b99b 100644 (file)
--- a/doc/chrony.conf.adoc
+++ b/doc/chrony.conf.adoc
@@ -661,14 +661,13 @@ value is the estimated precision of the system clock.
 Maximum allowed dispersion for filtered samples (in seconds). Samples with
 larger estimated dispersion are ignored. By default, this limit is disabled.
 *filter* _samples_:::
-This option sets the length of the median filter which is used to reduce the
-noise in the measurements. With each poll about 40 percent of the stored
-samples are discarded and one final sample is calculated as an average of the
-remaining samples. If the length is 4 or more, at least 4 samples have to be
-collected between polls. For lengths below 4, the filter has to be full. The
-default is 64. With drivers that perform their own polling (PPS, PHC, SHM), the
-maximum value is adjusted to the number of driver polls per source poll, i.e.
-2^(_poll_ - _dpoll_).
+This option sets the maximum number of samples that can be stored in the median
+filter between polls of the source. The filter combines about 60 percent of the
+samples closest to the median offset into one sample. If more samples are
+received by the driver between polls, the oldest samples will be dropped. One
+sample per poll is sufficient for the source to be selectable. The default
+is 64. Note that the PHC driver has additional filtering based on the reading
+delay.
 *prefer*:::
 Prefer this source over other selectable sources without the *prefer* option.
 *noselect*:::

diff --git a/refclock.c b/refclock.c
index d14560fae98663308ed0e81269afe6b02f2ec978..4a466bd524fd1c413c0b527932ea7836ad9be16e 100644 (file)
--- a/refclock.c
+++ b/refclock.c
@@ -233,12 +233,10 @@ RCL_AddRefclock(RefclockParameters *params)
     if (inst->driver_poll > inst->poll)
       inst->driver_poll = inst->poll;
 
+    /* Adjust the filter length to save memory if the expected number
+       of samples is smaller */
     max_samples = 1 << (inst->poll - inst->driver_poll);
     if (max_samples < params->filter_length) {
-      if (max_samples < 4) {
-        LOG(LOGS_WARN, "Setting filter length for %s to %d",
-            UTI_RefidToString(inst->ref_id), max_samples);
-      }
       params->filter_length = max_samples;
     }
   }
@@ -246,11 +244,9 @@ RCL_AddRefclock(RefclockParameters *params)
   if (inst->driver->init && !inst->driver->init(inst))
     LOG_FATAL("refclock %s initialisation failed", params->driver_name);
 
-  /* Require the filter to have at least 4 samples to produce a filtered
-     sample, or be full for shorter lengths, and combine 60% of samples
-     closest to the median */
-  inst->filter = SPF_CreateInstance(MIN(params->filter_length, 4), params->filter_length,
-                                    params->max_dispersion, 0.6);
+  /* Don't require more than one sample per poll and combine 60% of the
+     samples closest to the median offset */
+  inst->filter = SPF_CreateInstance(1, params->filter_length, params->max_dispersion, 0.6);
 
   inst->source = SRC_CreateNewInstance(inst->ref_id, SRC_REFCLOCK, 0, params->sel_options,
                                        NULL, params->min_samples, params->max_samples,

diff --git a/test/simulation/106-refclock b/test/simulation/106-refclock
index 3793bd86ce60b6c5fa021d793eb730f308afa411..46baee021824ca228abace87719cef156268f4ff 100755 (executable)
--- a/test/simulation/106-refclock
+++ b/test/simulation/106-refclock
@@ -89,9 +89,9 @@ Root delay      : 0\.000000001 seconds
        check_file_messages "20.* PPS1.*- N " 60 63 refclocks.log || test_fail
        rm -f tmp/refclocks.log
 
-       min_sync_time=80
-       max_sync_time=180
-       chronyc_start=220
+       min_sync_time=180
+       max_sync_time=260
+       chronyc_start=270
        client_conf="
 refclock SHM 0 refid NMEA offset 0.35 delay 0.1
 refclock PPS /dev/pps0
@@ -112,6 +112,30 @@ Root delay      : 0\.000000001 seconds
        check_file_messages "20.* PPS1.*[0-9] N " 800 960  refclocks.log || test_fail
        check_file_messages "20.* PPS1.*- N " 50 63 refclocks.log || test_fail
        rm -f tmp/refclocks.log
+
+       min_sync_time=80
+       max_sync_time=180
+       chronyc_start=220
+       # Swapped order of SHM and PPS impacts first accepted sample
+       client_conf="
+refclock PPS /dev/pps0
+refclock SHM 0 refid NMEA offset 0.35 delay 0.1
+logdir tmp
+log refclocks
+maxupdateskew 10000"
+
+       run_test || test_fail
+       check_chronyd_exit || test_fail
+       check_source_selection || test_fail
+       check_sync || test_fail
+       check_chronyc_output "^Reference ID.*50505330 \(PPS0\)
+Stratum.*: 1
+.*
+Root delay      : 0\.000000001 seconds
+.*$" || test_fail
+
+       check_file_messages "20.* PPS0.*[0-9] N " 800 960  refclocks.log || test_fail
+       check_file_messages "20.* PPS0.*- N " 50 63 refclocks.log || test_fail
 fi
 
 export CLKNETSIM_PHC_JITTER_OFF=$[2 * 25 * 492]