diff --git a/cmd/server/advert_intervals_test.go b/cmd/server/advert_intervals_test.go index 46761e5c8..f0fb696d7 100644 --- a/cmd/server/advert_intervals_test.go +++ b/cmd/server/advert_intervals_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "os" "strings" "testing" "time" @@ -204,6 +205,15 @@ func TestEstimateAdvertInterval_RaisedInterval(t *testing.T) { // Lowered (47 h -> 12 h): 12 h explains the old 47 h gaps as 4x // once it is a candidate, as before. {"flood 47 h -> 12 h", advertIntervalFlood, series(aiRepeat(47*H, 14), aiRepeat(12*H, 5)), 12 * 3600, advertConfidenceHigh, 20, 19}, + // A gap at the interval itself among the newest 3 that fit breaks + // the run (review N1 on #247): 24, 12, 24, 24 h is missed adverts + // on 12 h, not a raised setting since the 12 h gap. + {"flood 12 h, last 4 gaps 2x 1x 2x 2x", advertIntervalFlood, series(aiRepeat(12*H, 12), []time.Duration{24 * H, 12 * H, 24 * H, 24 * H}), 12 * 3600, advertConfidenceHigh, 17, 16}, + // Raised twice (review N2 on #247): the cut repeats on the adverts + // since the first change until no run is left, so the estimate is + // the final setting, not the one in between. + {"zero-hop 60 -> 120 -> 240 min", advertIntervalZeroHop, series(aiRepeat(60*M, 6), aiRepeat(120*M, 6), aiRepeat(240*M, 6)), 240 * 60, advertConfidenceHigh, 7, 6}, + {"flood 12 h -> 24 h -> 47 h", advertIntervalFlood, series(aiRepeat(12*H, 6), aiRepeat(24*H, 6), aiRepeat(47*H, 6)), 47 * 3600, advertConfidenceHigh, 7, 6}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -228,6 +238,77 @@ func TestEstimateAdvertInterval_TimerCandidates(t *testing.T) { aiCheck(t, estimateAdvertInterval(aiGaps(aiRepeat(60*M, 8)...), advertIntervalFlood), 0, advertConfidenceNone, 9, 0) } +// The zero-hop 2 min candidate band is 120 s +- 10 % (review N5 on #247), +// the same tolerance the snap uses: gaps of 108-132 s give the 2 min +// default, gaps just outside it are no timer interval. The other edges: +// zero-hop 54 min (60 less 10 %), flood 2.7 h (3 h less 10 %). +func TestEstimateAdvertInterval_CandidateBandEdges(t *testing.T) { + S, M := time.Second, time.Minute + for _, c := range []struct { + gap time.Duration + want int64 + conf string + gaps int + }{ + {108 * S, 120, advertConfidenceHigh, 19}, + {132 * S, 120, advertConfidenceHigh, 19}, + {107 * S, 0, advertConfidenceNone, 0}, + {133 * S, 0, advertConfidenceNone, 0}, + } { + t.Run(c.gap.String(), func(t *testing.T) { + aiCheck(t, estimateAdvertInterval(aiGaps(aiRepeat(c.gap, 19)...), advertIntervalZeroHop), c.want, c.conf, 20, c.gaps) + }) + } + for _, c := range []struct { + class advertIntervalClass + s float64 + want bool + }{ + {advertIntervalZeroHop, 108, true}, + {advertIntervalZeroHop, 132, true}, + {advertIntervalZeroHop, 107, false}, + {advertIntervalZeroHop, 133, false}, + {advertIntervalZeroHop, (54 * M).Seconds(), true}, + {advertIntervalZeroHop, (54 * M).Seconds() - 1, false}, + {advertIntervalFlood, 2.7 * 3600, true}, + {advertIntervalFlood, 2.7*3600 - 1, false}, + } { + if got := advertIntervalCandidateOK(c.s, c.class); got != c.want { + t.Errorf("advertIntervalCandidateOK(%v s, class %d) = %v, want %v", c.s, c.class, got, c.want) + } + } +} + +// Known trade-off of the raised-interval rule (review N3 on #247): with an +// unchanged interval, when the newest 3 heard gaps are the same multiple k +// (every 2nd or 3rd advert lost), the estimate is k x at medium confidence +// on 4 adverts, until the next gap at the interval itself breaks the run. +// docs/api-spec.md documents it next to the sparse-coverage limitation. +func TestEstimateAdvertInterval_FalseChange(t *testing.T) { + H, M := time.Hour, time.Minute + cases := []struct { + name string + class advertIntervalClass + gaps []time.Duration + interval int64 + }{ + {"zero-hop 60 min, last 3 gaps 2x", advertIntervalZeroHop, append(aiRepeat(60*M, 10), aiRepeat(120*M, 3)...), 120 * 60}, + {"flood 47 h, last 3 gaps 2x", advertIntervalFlood, append(aiRepeat(47*H, 10), aiRepeat(94*H, 3)...), 94 * 3600}, + {"zero-hop 120 min, last 3 gaps 3x", advertIntervalZeroHop, append(aiRepeat(120*M, 10), aiRepeat(360*M, 3)...), 360 * 60}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + aiCheck(t, estimateAdvertInterval(aiGaps(c.gaps...), c.class), c.interval, advertConfidenceMedium, 4, 3) + }) + } + // A gap that fits no multiple (an outage over 4x) does not break the run. + s := aiGaps(append(aiRepeat(60*M, 10), 120*M, 120*M, 300*M, 120*M)...) + aiCheck(t, estimateAdvertInterval(s, advertIntervalZeroHop), 120*60, advertConfidenceMedium, 5, 3) + // One more gap at the interval itself ends it. + s = aiGaps(append(append(aiRepeat(60*M, 10), aiRepeat(120*M, 3)...), 60*M)...) + aiCheck(t, estimateAdvertInterval(s, advertIntervalZeroHop), 3600, advertConfidenceHigh, 15, 14) +} + // Known limitation (review F2 on #247): when coverage is so sparse that the // interval itself is never heard twice in a row, it is no candidate and a // multiple of it is estimated. A 47 h flood heard only 2x and 3x apart @@ -384,7 +465,7 @@ func TestEstimateAdvertInterval_ConfidenceTiers(t *testing.T) { } } -// The interval is the median of the fitting gaps, not their mean: one late +// The interval is the median of gap/k over the fitting gaps, not their mean: one late // first_seen (no sender clock) moves a mean, not the median. func TestEstimateAdvertInterval_MedianNotMean(t *testing.T) { s := aiSeries(12*time.Hour, aiInts(0, 7)...) @@ -557,3 +638,40 @@ func TestNodeDetail_AdvertIntervals(t *testing.T) { t.Fatalf("window = %d", body.Intervals.Window) } } + +// The docs describe what the estimator does (review N3 and N5 on #247): the +// value is the median of gap/k, not of the raw fitting gaps, and the +// false-change trade-off pinned by _FalseChange sits next to the +// sparse-coverage limitation in docs/api-spec.md. +func TestAdvertIntervals_DocsMatchCode(t *testing.T) { + schemas := asMap(t, asMap(t, fetchSpec(t)["components"], "components")["schemas"], "schemas") + desc := fmt.Sprint(asMap(t, schemas["NodeAdvertIntervals"], "NodeAdvertIntervals")["description"]) + if !strings.Contains(desc, "median of gap/k") || strings.Contains(desc, "median of the fitting gaps") { + t.Errorf("OpenAPI NodeAdvertIntervals must say the median of gap/k: %s", desc) + } + + raw, err := os.ReadFile("../../docs/api-spec.md") + if err != nil { + t.Fatal(err) + } + doc := string(raw) + start := strings.Index(doc, "#### Estimated advert intervals") + if start < 0 { + t.Fatal("docs/api-spec.md: no Estimated advert intervals section") + } + section := doc[start:] + if end := strings.Index(section[1:], "\n#"); end >= 0 { + section = section[:end+1] + } + sparse := strings.Index(section, "**Known limitation: sparse coverage.**") + falseChange := strings.Index(section, "**Known trade-off: false change.**") + if sparse < 0 || falseChange < 0 { + t.Fatalf("docs/api-spec.md must document sparse coverage and the false change (sparse %d, false change %d)", sparse, falseChange) + } + // The examples are the _FalseChange cases. + for _, want := range []string{"60 min reads as 120 min", "47 h as 94 h", "120 min with 3× gaps as 360 min", "medium confidence on 4 adverts"} { + if !strings.Contains(section[falseChange:], want) { + t.Errorf("false-change limitation lacks %q", want) + } + } +} diff --git a/cmd/server/openapi.go b/cmd/server/openapi.go index d379bc60a..2a059615e 100644 --- a/cmd/server/openapi.go +++ b/cmd/server/openapi.go @@ -261,7 +261,7 @@ func nodeAdvertRouteSchemas() map[string]*openAPISchema { }, "NodeAdvertIntervals": { Type: "object", - Description: "Node detail with include=advertRoutes only (#245): the node's estimated flood and zero-hop advert intervals, from the gaps between the adverts listed in recentAdvertsByRoute.flood / .zero_hop (mixed and unknown adverts are not used). A gap uses the adverts' own (sender) timestamps when both are plausible - not ahead of first_seen by more than 10 min, positive, and within max(10 min, 10 %) of the first_seen gap - else first_seen. The interval must be seen directly in at least two gaps and a quarter of them, and be one the class's timer can run at (flood 3 h or more, zero-hop 2 min or 60 min or more, each less 10 %); gaps of 2-4x it count as missed adverts, shorter gaps (manual adverts, reboots) are dropped, longer gaps that are no multiple are irregular. When the newest 3 gaps that fit are all the same multiple k > 1 the interval was raised, and the estimate is redone on the adverts since the change. It is the median of the fitting gaps, snapped to the firmware's settable values: flood.advert.interval whole hours 3-168, advert.interval even minutes 60-240 or the 2-minute new-install default. No zero-hop adverts can mean the node's zero-hop interval is 0 (off) or that no observer hears it directly. Absent without include=advertRoutes and when the node's identity is hidden; cached with recentAdvertsByRoute.", + Description: "Node detail with include=advertRoutes only (#245): the node's estimated flood and zero-hop advert intervals, from the gaps between the adverts listed in recentAdvertsByRoute.flood / .zero_hop (mixed and unknown adverts are not used). A gap uses the adverts' own (sender) timestamps when both are plausible - not ahead of first_seen by more than 10 min, positive, and within max(10 min, 10 %) of the first_seen gap - else first_seen. The interval must be seen directly in at least two gaps and a quarter of them, and be one the class's timer can run at (flood 3 h or more, zero-hop 2 min or 60 min or more, each less 10 %); gaps of 2-4x it count as missed adverts, shorter gaps (manual adverts, reboots) are dropped, longer gaps that are no multiple are irregular. When the newest 3 gaps that fit are all the same multiple k > 1 the interval was raised, and the estimate is redone on the adverts since the change. It is the median of gap/k over the gaps that fit k x the interval, snapped to the firmware's settable values: flood.advert.interval whole hours 3-168, advert.interval even minutes 60-240 or the 2-minute new-install default. No zero-hop adverts can mean the node's zero-hop interval is 0 (off) or that no observer hears it directly. Absent without include=advertRoutes and when the node's identity is hidden; cached with recentAdvertsByRoute.", Properties: map[string]*openAPISchema{ "window": {Type: "integer", Description: "Most adverts per class considered (the recentAdvertsByRoute limit, 20)."}, "flood": openAPIRef("AdvertIntervalEstimate"), diff --git a/docs/api-spec.md b/docs/api-spec.md index 481d3b72b..d84a2d51b 100644 --- a/docs/api-spec.md +++ b/docs/api-spec.md @@ -636,6 +636,15 @@ The firmware settings it maps to (MeshCore `src/helpers/CommonCLI.cpp`): heard twice in a row (a distant node heard every second or third time), it is no candidate, and a multiple of it is reported: a 47 h flood heard 94 h and 141 h apart reads as 94 h or 141 h at medium confidence. +- **Known trade-off: false change.** The raised-interval rule can read an + unchanged interval as raised. When the newest 3 heard gaps are all the + same multiple k (every 2nd or 3rd advert lost), the result is k× at + medium confidence on 4 adverts: 60 min reads as 120 min, 47 h as 94 h, + and 120 min with 3× gaps as 360 min. Gaps that fit no multiple (an + outage over 4×, a reboot advert between the 2× gaps) do not break the + run. The next gap heard at the interval itself does, and the estimate + returns to the interval. The opposite choice kept a raised interval at the + old value, at high confidence, for weeks (review F1 on #247). - **No zero-hop adverts.** A zero-hop advert is only recorded when an observer hears the node directly. "None observed" can therefore mean that the interval is 0 (off), or that no observer is in direct range.