fix(tci): one marker per callsign on the panorama, not one per spotter

Spot mirroring drew the same station two or three times, a few hertz apart. Not a loop in the spot pipeline — one cluster line produces exactly one SendSpot. It is that a popular DX station IS spotted two or three times, by different operators inside the same minute, and no two of them agree on the frequency to better than a few tens of hertz. This backend assumed ExpertSDR replaced a spot bearing a callsign it already held, and said so in a comment; it keys on the frequency too, so each of those became its own marker and stayed. The FlexRadio backend has always done this properly (spot remove before spot add, "one live spot per call"). TCI now does the same: the frequency drawn for each call is remembered, a re-spot within 500 Hz sends nothing at all, and one further away deletes the old marker before drawing the new one — so a station that really moves still moves, instead of collecting markers. SPOT_DELETE was not in the protocol document this backend was written from. Confirmed before use against ars-ka0s/eesdr-tci, an independent TCI library that lists SPOT with 5 arguments, SPOT_DELETE with 1 and SPOT_CLEAR with 0 — the other two matching exactly what already works here, which is what makes the third trustworthy. The memory is cleared where Connect sends spot_clear. Left standing, it would suppress the next spot for every remembered call as "already drawn" onto a panorama the radio had just emptied — spots would quietly stop appearing after a reconnect until each station changed frequency. Five tests on the decision, which is split out from SendSpot so it can be checked without a radio.
2026-08-17 01:57:11 +02:00
parent 91b21a4a36
commit 491546c7f4
3 changed files with 168 additions and 4 deletions
@@ -10,7 +10,8 @@
      "Selecting a QSO shows the entity the QSO records, not one re-derived from its callsign — a 3Y0K contact logged as Bouvet showed the Antarctica matrix.",
      "Back-entering a QSO resolves the ClubLog exception at the CONTACT’S date, so a DXpedition entered months later gets the entity it had then.",
      "QRZ.com sends an island reference for an operator on one, and OpsLog read past it — it now fills the IOTA award reference before the QSO is logged.",
-      "Sync across PCs: point every OpsLog at one folder you already synchronise and your contacts follow you between machines."
+      "Sync across PCs: point every OpsLog at one folder you already synchronise and your contacts follow you between machines.",
+      "TCI: a station spotted by several operators is drawn once on the panorama instead of two or three times a few hertz apart."
    ],
    "fr": [
      "Ouvrir le panneau Awards ne tire plus plusieurs fois le journal entier en même temps — un gros log occupait brièvement des gigaoctets de mémoire.",
@@ -20,7 +21,8 @@
      "Sélectionner un QSO affiche l’entité que le QSO enregistre, pas une recalculée depuis l’indicatif — un 3Y0K logué Bouvet montrait la matrice Antarctique.",
      "Saisir un QSO a posteriori résout l’exception ClubLog à la date DU CONTACT : une DXpedition entrée des mois après retrouve l’entité qu’elle avait alors.",
      "QRZ.com envoie la référence d’île d’un opérateur sur une île, et OpsLog l’ignorait — elle remplit désormais la référence IOTA avant l’enregistrement du QSO.",
-      "Synchro entre PC : fais pointer chaque OpsLog vers un dossier déjà synchronisé et tes contacts te suivent d’une machine à l’autre."
+      "Synchro entre PC : fais pointer chaque OpsLog vers un dossier déjà synchronisé et tes contacts te suivent d’une machine à l’autre.",
+      "TCI : une station spottée par plusieurs opérateurs n’est tracée qu’une fois sur le panorama, au lieu de deux ou trois fois."
    ]
  },
  {
@@ -48,6 +48,19 @@ type TCI struct {
 	tx     bool

 	lastSig string // last logged state signature (log only on change)
+
+	// spotFreq is the frequency of the marker currently on the panorama for each
+	// callsign — the panadapter's own state, which TCI never reports back. It is
+	// what makes one spot per call possible: without it there is no way to know
+	// there is an older marker to delete.
+	spotFreq map[string]int64
+}
+
+func absInt64(v int64) int64 {
+	if v < 0 {
+		return -v
+	}
+	return v
 }

 const tciDefaultPort = 40001
@@ -100,14 +113,65 @@ func (t *TCI) Connect() error {
 	debugLog.Printf("TCI: connected to %s", url)
 	go t.reader(conn)
 	if t.spotsEnabled {
+		// Forget what we thought was on the panorama at the same moment the radio
+		// is told to drop it. Kept, the memory would suppress the next spot for
+		// each of those calls as "already drawn" onto a panorama now empty.
+		t.mu.Lock()
+		t.spotFreq = map[string]int64{}
+		t.mu.Unlock()
 		_ = t.send("spot_clear;") // drop any leftover spots from a previous session
 	}
 	return nil
 }

+// spotFreqTolHz is how far a re-spot of the same callsign may sit from the one
+// already on the panorama before it is treated as a move rather than the same
+// spot said again.
+//
+// Two spotters hearing the same CW station rarely agree to better than a couple
+// of hundred hertz, and every one of them produces a cluster line. Below this
+// they are the same spot and nothing is sent at all; above it the marker is
+// deleted and redrawn where the station now is.
+const spotFreqTolHz = 500
+
+// noteSpot records what the panorama is about to hold for a callsign and says
+// what has to be sent: whether to draw at all, and whether an older marker for
+// the same call must be deleted first.
+//
+// Separate from SendSpot so the rule can be tested without a radio — and
+// because the lock must be released before anything is sent: t.send takes t.mu
+// itself, Go mutexes are not reentrant, and sending while holding it would
+// deadlock the backend and take the rig offline.
+func (t *TCI) noteSpot(call string, freqHz int64) (draw, deletePrev bool) {
+	key := strings.ToUpper(strings.TrimSpace(call))
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	prev, had := t.spotFreq[key]
+	if had && absInt64(prev-freqHz) <= spotFreqTolHz {
+		return false, false
+	}
+	if t.spotFreq == nil {
+		t.spotFreq = map[string]int64{}
+	}
+	if len(t.spotFreq) > 4000 {
+		t.spotFreq = map[string]int64{} // bound memory on a long session
+		had = false                     // forgotten: nothing left to delete by name
+	}
+	t.spotFreq[key] = freqHz
+	return true, had
+}
+
 // SendSpot mirrors a cluster spot onto the TCI panorama (implements Spotter).
-// The radio replaces a spot that has the same callsign, so re-spotting updates
-// it in place. No-op when spot mirroring is disabled.
+// No-op when spot mirroring is disabled.
+//
+// ONE MARKER PER CALLSIGN. This code assumed the radio replaced a spot carrying
+// a callsign it already had; it does not. ExpertSDR keys a spot on its
+// frequency too, so a DX station spotted by three operators — 14025.00,
+// 14025.12, 14024.90, which is an ordinary minute on a cluster — was drawn
+// three times, a few pixels apart, and stayed that way.
+//
+// So the previous spot for the call is deleted before the new one is sent,
+// which is what the FlexRadio backend has always done (spot remove / spot add).
 func (t *TCI) SendSpot(s SpotInfo) error {
 	if !t.spotsEnabled {
 		return nil
@@ -116,6 +180,17 @@ func (t *TCI) SendSpot(s SpotInfo) error {
 	if call == "" || s.FreqHz <= 0 {
 		return nil
 	}
+	draw, deletePrev := t.noteSpot(call, s.FreqHz)
+	if !draw {
+		return nil // the same station said again by another spotter
+	}
+	if deletePrev {
+		// SPOT_DELETE takes the callsign alone. Not in the protocol PDF this
+		// backend was written from; confirmed against ars-ka0s/eesdr-tci, which
+		// lists SPOT (5 arguments), SPOT_DELETE (1) and SPOT_CLEAR (0) — the
+		// other two matching what already works here.
+		_ = t.send(fmt.Sprintf("spot_delete:%s;", call))
+	}
 	// TCI's SPOT command wants the colour as a signed 32-bit DECIMAL integer in
 	// 0xAARRGGBB order — NOT a "0x…" hex string (e.g. "spot:UN7GK,cw,14025000,
 	// -16776961,test;"). ExpertSDR silently drops a spot whose colour field it
@@ -0,0 +1,87 @@
+//go:build windows
+
+package cat
+
+import "testing"
+
+// The reported symptom: with spot mirroring on, the same station appeared two
+// or three times on the panorama.
+//
+// Its cause is not in OpsLog's spot pipeline — one cluster line produces one
+// SendSpot. It is that a popular DX station IS spotted two or three times, by
+// different operators within the same minute, and no two of them agree on the
+// frequency to better than a few tens of hertz. The backend assumed ExpertSDR
+// replaced a spot bearing a callsign it already had; it keys on the frequency
+// too, so each of those became its own marker.
+func TestSameStationSpottedBySeveralOperatorsIsDrawnOnce(t *testing.T) {
+	tci := &TCI{spotsEnabled: true}
+
+	draw, del := tci.noteSpot("UN7GK", 14025000)
+	if !draw || del {
+		t.Fatalf("first spot: draw=%v delete=%v, want draw and nothing to delete", draw, del)
+	}
+	// The same station, two more spotters, a few tens of hertz apart.
+	for _, hz := range []int64{14025120, 14024900} {
+		if draw, del := tci.noteSpot("UN7GK", hz); draw || del {
+			t.Errorf("re-spot at %d Hz: draw=%v delete=%v, want nothing sent — this is the duplicate marker", hz, draw, del)
+		}
+	}
+}
+
+// A station that really moves must still move on the panorama, and the marker
+// left where it was must go. Deleting first is the whole difference between
+// "the spot follows the station" and "the station collects markers".
+func TestAStationThatMovesReplacesItsMarker(t *testing.T) {
+	tci := &TCI{spotsEnabled: true}
+	tci.noteSpot("UN7GK", 14025000)
+
+	draw, del := tci.noteSpot("UN7GK", 14032000) // 7 kHz up: a real QSY
+	if !draw || !del {
+		t.Fatalf("after a QSY: draw=%v delete=%v, want the old marker deleted and a new one drawn", draw, del)
+	}
+	// And the new position becomes the reference, so spotters agreeing with it
+	// are quiet again.
+	if draw, _ := tci.noteSpot("UN7GK", 14032100); draw {
+		t.Error("a spot at the station's new frequency was drawn again")
+	}
+}
+
+// Case matters nowhere in ham radio, and the cluster is not consistent about it.
+func TestSpotMemoryIgnoresCase(t *testing.T) {
+	tci := &TCI{spotsEnabled: true}
+	tci.noteSpot("un7gk", 14025000)
+	if draw, _ := tci.noteSpot("UN7GK", 14025000); draw {
+		t.Error("the same call in another case was treated as a different station")
+	}
+}
+
+// Two different stations are two markers — the whole point of the panorama.
+func TestDifferentStationsEachGetAMarker(t *testing.T) {
+	tci := &TCI{spotsEnabled: true}
+	tci.noteSpot("UN7GK", 14025000)
+	draw, del := tci.noteSpot("ZD7BG", 14025050) // 50 Hz away, a different operator
+	if !draw {
+		t.Error("a second station near the first was swallowed as a duplicate")
+	}
+	if del {
+		t.Error("deleting by callsign would have removed a spot this station never had")
+	}
+}
+
+// The connection drops and comes back: Connect sends spot_clear, so the
+// panorama is empty. If the memory survived that, the next spot for each of
+// those calls would be suppressed as "already drawn" onto an empty panorama —
+// the operator's spots would simply stop appearing until they changed
+// frequency.
+func TestReconnectingForgetsWhatWasDrawn(t *testing.T) {
+	tci := &TCI{spotsEnabled: true}
+	tci.noteSpot("UN7GK", 14025000)
+
+	tci.mu.Lock()
+	tci.spotFreq = map[string]int64{} // what Connect does alongside spot_clear
+	tci.mu.Unlock()
+
+	if draw, del := tci.noteSpot("UN7GK", 14025000); !draw || del {
+		t.Errorf("after a reconnect: draw=%v delete=%v, want it drawn again and nothing deleted", draw, del)
+	}
+}