up

internal/cat/cat.go

@@ -57,6 +57,7 @@ type RigState struct {
 // Manager owns the active backend and runs the polling loop.
 type Manager struct {
 	mu      sync.RWMutex
+	startMu sync.Mutex // serializes Start/Stop so concurrent calls can't leak a poller
 	state   RigState
 	emit    func(RigState)
 	backend Backend
@@ -115,7 +116,13 @@ func (m *Manager) State() RigState {
 // state.Error rather than returned, so the UI can keep retrying via the
 // poll loop on next reconnect attempt.
 func (m *Manager) Start(b Backend) {
-	m.Stop()
+	// Serialize the whole stop-old-then-start-new sequence. Two concurrent
+	// Start (or Start+Stop) calls could otherwise interleave and leave the
+	// previous poll goroutine alive — two pollers then fight, e.g. flipping
+	// OmniRig Rig1/Rig2 endlessly when the user reselects a rig.
+	m.startMu.Lock()
+	defer m.startMu.Unlock()
+	m.stopLocked()
 	m.mu.Lock()
 	m.stopCh = make(chan struct{})
 	m.doneCh = make(chan struct{})
@@ -134,6 +141,18 @@ func (m *Manager) Start(b Backend) {
 
 // Stop signals the CAT goroutine to disconnect and waits for it to exit.
 func (m *Manager) Stop() {
+	m.startMu.Lock()
+	defer m.startMu.Unlock()
+	m.stopLocked()
+	m.mu.Lock()
+	m.state = RigState{Enabled: false}
+	m.mu.Unlock()
+	m.emitState()
+}
+
+// stopLocked tears down any running poller and blocks until it exits. The
+// caller must hold startMu so it can't race a concurrent Start.
+func (m *Manager) stopLocked() {
 	m.mu.Lock()
 	stop := m.stopCh
 	done := m.doneCh
@@ -148,10 +167,6 @@ func (m *Manager) Stop() {
 	if done != nil {
 		<-done
 	}
-	m.mu.Lock()
-	m.state = RigState{Enabled: false}
-	m.mu.Unlock()
-	m.emitState()
 }
 
 // SetFrequency dispatches a SetFreq call to the CAT goroutine.

internal/cat/omnirig.go

@@ -3,11 +3,21 @@ package cat
 import (
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/go-ole/go-ole"
 	"github.com/go-ole/go-ole/oleutil"
 )
 
+// OmniRig Split is an enum, not a boolean: PM_SPLITON vs PM_SPLITOFF — both
+// non-zero, so it must be compared to PM_SPLITON (testing "!= 0" reads OFF as
+// split). Values confirmed empirically from real rigs (FT-710, SmartSDR):
+// split ON = 0x8000, split OFF = 0x10000.
+const (
+	pmSplitOn  = 0x8000  // PM_SPLITON
+	pmSplitOff = 0x10000 // PM_SPLITOFF
+)
+
 // OmniRig talks to the user's installed OmniRig server over COM.
 //
 // All methods MUST be called from the same OS thread (the one Manager.run
@@ -21,6 +31,15 @@ type OmniRig struct {
 
 	omnirig *ole.IDispatch
 	rig     *ole.IDispatch
+	lastSig string // last logged Split/VFO signature — only log on change
+
+	// lastSetFreq is the frequency most recently COMMANDED via SetFrequency.
+	// SetMode uses it to pick USB vs LSB for "SSB" instead of reading OmniRig's
+	// async Freq property, which still reports the OLD band for a poll or two
+	// after a QSY — that lag is why a clicked spot needed a second click to fix
+	// the sideband (freq moved, but mode read the old band → wrong sideband).
+	lastSetFreq   int64
+	lastSetFreqAt time.Time
 }
 
 // NewOmniRig creates a non-connected backend. Call Connect before use.
@@ -107,13 +126,19 @@ func (o *OmniRig) ReadState() (RigState, error) {
 	if modeVar, err := oleutil.GetProperty(o.rig, "Mode"); err == nil {
 		s.Mode = omniRigMode(modeVar.Val)
 	}
+	rawVfo := int64(0)
 	if vfoVar, err := oleutil.GetProperty(o.rig, "Vfo"); err == nil {
+		rawVfo = vfoVar.Val
 		s.Vfo = omniRigVfo(vfoVar.Val)
 	}
 
-	// Read both VFO frequencies separately so we can expose split TX/RX.
-	// Fall back to generic Freq if the rig only exposes the merged property.
-	freqA, freqB := int64(0), int64(0)
+	// Read the active/displayed frequency (generic Freq) AND both VFOs. The
+	// generic Freq is what the rig is operating on — the reliable source for the
+	// main/TX frequency. FreqA/FreqB are only needed to expose a genuine split.
+	freqMain, freqA, freqB := int64(0), int64(0), int64(0)
+	if v, err := oleutil.GetProperty(o.rig, "Freq"); err == nil {
+		freqMain = v.Val
+	}
 	if v, err := oleutil.GetProperty(o.rig, "FreqA"); err == nil {
 		freqA = v.Val
 	}
@@ -121,38 +146,58 @@ func (o *OmniRig) ReadState() (RigState, error) {
 		freqB = v.Val
 	}
 
-	// Split detection: trust the explicit Split property when it's set,
-	// BUT only call it a real split if both VFO frequencies are non-zero
-	// and distinct. Bridges like SmartSDR-OmniRig report Split=ON by
-	// default (raw bit PM_SPLITON = 65536) with FreqB=0 because the Flex's
-	// slice model doesn't map to VFO A/B — that would yield a useless
-	// permanent SPLIT badge.
-	if v, err := oleutil.GetProperty(o.rig, "Split"); err == nil && v.Val != 0 {
-		s.Split = true
-	}
-	if s.Split && (freqB == 0 || freqA == freqB) {
-		s.Split = false
-		s.RxFreqHz = 0
+	// Split is an enum (PM_SPLITON / PM_SPLITOFF) — both non-zero, so it must be
+	// compared to PM_SPLITON, not "!= 0".
+	splitRaw := int64(0)
+	if v, err := oleutil.GetProperty(o.rig, "Split"); err == nil {
+		splitRaw = v.Val
 	}
 
-	// OmniRig's Vfo enum is RX-letter then TX-letter (AB = RX A, TX B).
-	// We follow ADIF: FreqHz = TX, RxFreqHz = RX (only meaningful in split).
-	switch s.Vfo {
-	case "AB":
-		s.FreqHz = freqB   // TX
-		s.RxFreqHz = freqA // RX
-	case "BA":
-		s.FreqHz = freqA   // TX
-		s.RxFreqHz = freqB // RX
-	case "B", "BB":
-		s.FreqHz = freqB
-	default: // "A", "AA", "" — single VFO on A or unknown
-		s.FreqHz = freqA
+	// Diagnostic logged ONLY when Split or VFO changes (not on a timer), so
+	// normal operation stays quiet but toggling split on the radio is captured —
+	// needed to pin down this rig's PM_SPLITON value.
+	if sig := fmt.Sprintf("%x:%x", splitRaw, rawVfo); sig != o.lastSig {
+		o.lastSig = sig
+		debugLog.Printf("OmniRig Rig%d raw: Freq=%d FreqA=%d FreqB=%d Vfo=%q(raw=0x%X) Split=0x%X status=%d",
+			o.RigNum, freqMain, freqA, freqB, s.Vfo, rawVfo, splitRaw, func() int64 {
+				if v, e := oleutil.GetProperty(o.rig, "Status"); e == nil {
+					return v.Val
+				}
+				return -1
+			}())
 	}
-	if s.FreqHz == 0 {
-		// Last resort — some rigs only update generic Freq.
-		if v, err := oleutil.GetProperty(o.rig, "Freq"); err == nil {
-			s.FreqHz = v.Val
+
+	// A genuine split: the rig explicitly flags PM_SPLITON, the two VFOs are
+	// distinct and non-zero, AND they're in the same band. The same-band test
+	// kills the common false positive where VFO B just holds a leftover from
+	// another band (a "28 MHz / 7 MHz split" is nonsensical), which on the
+	// FT-710 / TS-570 otherwise froze the main/TX freq on the wrong VFO.
+	genuineSplit := splitRaw == pmSplitOn &&
+		freqA != 0 && freqB != 0 && freqA != freqB &&
+		BandFromHz(freqA) == BandFromHz(freqB)
+
+	if genuineSplit {
+		// OmniRig's Vfo enum is RX-letter then TX-letter (AB = RX A, TX B).
+		// ADIF: FreqHz = TX, RxFreqHz = RX.
+		s.Split = true
+		switch s.Vfo {
+		case "BA":
+			s.FreqHz, s.RxFreqHz = freqA, freqB // TX A, RX B
+		default: // "AB" and the usual "TX on the other VFO" case
+			s.FreqHz, s.RxFreqHz = freqB, freqA // TX B, RX A
+		}
+	} else {
+		// Simplex: the operating frequency is OmniRig's generic Freq (the active
+		// VFO), like Log4OM. Fall back to the per-VFO value only if Freq is 0.
+		s.Split = false
+		s.RxFreqHz = 0
+		s.FreqHz = freqMain
+		if s.FreqHz == 0 {
+			if s.Vfo == "B" || s.Vfo == "BB" {
+				s.FreqHz = freqB
+			} else {
+				s.FreqHz = freqA
+			}
 		}
 	}
 	return s, nil
@@ -169,6 +214,10 @@ func (o *OmniRig) SetFrequency(hz int64) error {
 		return fmt.Errorf("frequency out of OmniRig int32 range")
 	}
 	hz32 := int32(hz)
+	// Remember the commanded frequency so a mode change moments later (a clicked
+	// spot sets freq then mode) picks the sideband from the TARGET band, not the
+	// not-yet-updated OmniRig Freq property.
+	o.lastSetFreq, o.lastSetFreqAt = hz, time.Now()
 
 	// Log the rig's writable-params, status and VFO state up front so a
 	// friend's session shows exactly what OmniRig reports for their rig.
@@ -274,9 +323,15 @@ func (o *OmniRig) SetMode(mode string) error {
 	case "CW":
 		bit, bitName = pmCWU, "PM_CW_U"
 	case "SSB":
-		// Read current freq to decide USB vs LSB.
+		// Decide USB vs LSB from the frequency. Prefer the freq we just COMMANDED
+		// (a clicked spot sets freq then mode ~150ms later): OmniRig's Freq
+		// property still reports the OLD band for a poll or two after a QSY, so
+		// reading it here picked the wrong sideband and the user had to click a
+		// second time. Fall back to the live read for a standalone mode change.
 		var freq int64
-		if freqVar, err := oleutil.GetProperty(o.rig, "Freq"); err == nil {
+		if o.lastSetFreq > 0 && time.Since(o.lastSetFreqAt) < 5*time.Second {
+			freq = o.lastSetFreq
+		} else if freqVar, err := oleutil.GetProperty(o.rig, "Freq"); err == nil {
 			freq = freqVar.Val
 		}
 		if freq > 0 && freq < 10_000_000 {
@@ -396,20 +451,22 @@ func omniRigMode(m int64) string {
 	return ""
 }
 
+// omniRigVfo maps the OmniRig Vfo RigParamX enum to a short label, using the
+// documented PM_VFO* constants.
 func omniRigVfo(v int64) string {
 	switch {
-	case v&1024 != 0:
-		return "A"
-	case v&2048 != 0:
-		return "B"
-	case v&64 != 0:
+	case v&0x40 != 0: // PM_VFOAA
 		return "AA"
-	case v&128 != 0:
+	case v&0x80 != 0: // PM_VFOAB
 		return "AB"
-	case v&256 != 0:
+	case v&0x100 != 0: // PM_VFOBA
 		return "BA"
-	case v&512 != 0:
+	case v&0x200 != 0: // PM_VFOBB
 		return "BB"
+	case v&0x400 != 0: // PM_VFOA
+		return "A"
+	case v&0x800 != 0: // PM_VFOB
+		return "B"
 	}
 	return ""
 }