diff --git a/changelog.json b/changelog.json index e384b47..aac818b 100644 --- a/changelog.json +++ b/changelog.json @@ -7,14 +7,16 @@ "CAT sharing can now speak TCI instead of Hamlib, so a TCI-only program reaches whatever radio OpsLog is on.", "Lookup cache: a TTL of 0 switches it off, so a callbook record you are correcting is re-read every time.", "TCI: when the radio forbids transmitting, PTT now says so instead of doing nothing silently.", - "TCI sharing: the server now announces transmit permission, without which a client such as MSHV never keys at all." + "TCI sharing: the server now announces transmit permission, without which a client such as MSHV never keys at all.", + "TCI sharing: split is armed on the frequency the client asked for, whichever order it sent the two commands in." ], "fr": [ "Une entité qui est un seul groupe d’îles remplit désormais la référence IOTA toute seule, sans abonnement callbook.", "Le partage CAT peut désormais parler TCI au lieu de Hamlib : un logiciel TCI atteint la radio, quelle qu’elle soit.", "Cache des recherches : un TTL à 0 le désactive, pour relire à chaque fois une fiche callbook en cours de correction.", "TCI : quand la radio interdit l’émission, le PTT le dit désormais au lieu de ne rien faire en silence.", - "Partage TCI : le serveur annonce désormais l’autorisation d’émettre, sans laquelle un client comme MSHV ne passe jamais en émission." + "Partage TCI : le serveur annonce désormais l’autorisation d’émettre, sans laquelle un client comme MSHV ne passe jamais en émission.", + "Partage TCI : le split s’arme sur la fréquence demandée par le logiciel, quel que soit l’ordre de ses deux commandes." ] }, { diff --git a/internal/tciserver/tciserver.go b/internal/tciserver/tciserver.go index 22bb6d0..ab858ed 100644 --- a/internal/tciserver/tciserver.go +++ b/internal/tciserver/tciserver.go @@ -75,6 +75,27 @@ type Server struct { conns map[*client]struct{} closed bool + // pendingTxHz is a transmit frequency a client set on channel B while the rig + // was still simplex. + // + // It must be REMEMBERED, not discarded. A client working split sends two + // commands and is free to send them in either order; when the frequency comes + // first, throwing it away means the split is then armed on whatever the + // transmit VFO happened to hold — the receive frequency — and the operator + // transmits straight onto the DX while their software shows exactly what they + // asked for. rigctld learned this the same way, and pairs set_split_vfo with + // set_split_freq for the same reason. + pendingTxHz int64 + + // ptt mirrors the last PTT state a client commanded, so a repeat can be + // recognised. A client is free to restate PTT as often as it likes, and one + // does: through the rigctl server Nexus sent set_ptt 0 about sixteen times a + // second, and the Flex's own "xmit 1" landed between two of them and was + // overwritten inside a millisecond — a transmit request that simply did + // nothing. The same radio sits behind this server. + ptt bool + pttKnown bool + // last is what the clients have been told, so only changes are sent. TCI // clients redraw on every command they receive; re-sending an unchanged // frequency four times a second makes a VFO readout flicker and, in some @@ -416,11 +437,15 @@ func (s *Server) handle(c *client, cmd string) string { return "" } if arg(1) == "1" { - // Channel B is the transmit frequency, and it only means anything - // with split armed. Setting it while simplex would silently move the - // rig's only VFO — the client asked to prepare a split TX frequency, - // not to QSY. + // Channel B is the transmit frequency. Setting it while simplex must + // not move the rig's only VFO — the client asked to prepare a split + // transmit frequency, not to QSY — but it must not be thrown away + // either: it is where the split will be armed a moment from now. + s.mu.Lock() + s.pendingTxHz = hz + s.mu.Unlock() if !split { + s.broadcast(fmt.Sprintf("vfo:0,1,%d;", hz)) return "" } if err := s.rig.SetSplit(true, hz); err != nil { @@ -454,10 +479,22 @@ func (s *Server) handle(c *client, cmd string) string { return reply("trx:0,false;") } on := strings.EqualFold(arg(1), "true") + // Only touch the radio on a CHANGE — restating a state is not a request + // to change it. The first command always goes through, since there is no + // knowing how the radio was left. + s.mu.Lock() + known, prev := s.pttKnown, s.ptt + s.ptt, s.pttKnown = on, true + s.mu.Unlock() + if known && prev == on { + s.broadcast(fmt.Sprintf("trx:0,%t;", on)) + return "" + } if err := s.rig.SetPTT(on); err != nil { s.log("tci server: PTT %v refused: %v", on, err) return "" } + s.log("tci server: PTT %s", map[bool]string{true: "ON", false: "off"}[on]) s.broadcast(fmt.Sprintf("trx:0,%t;", on)) return "" @@ -466,13 +503,34 @@ func (s *Server) handle(c *client, cmd string) string { return reply(fmt.Sprintf("split_enable:0,%t;", split)) } on := strings.EqualFold(arg(1), "true") - if err := s.rig.SetSplit(on, tx); err != nil { + // Already in the state asked for? Then it is done, and nothing goes to + // the radio. This is the lesson the rigctl server paid for: JTDX in "Fake + // It" uses no split but still says so to be sure, and a backend that + // cannot set split answered an error to a request that was already true. + // JTDX read that as rig control failing and abandoned the transmission a + // second into the frame. A refusal is only honest when something actually + // needed doing. + if on == split { + s.broadcast(fmt.Sprintf("split_enable:0,%t;", on)) + return "" + } + // Arm on the frequency the client gave for channel B, which it is free to + // have sent before this command rather than after. + s.mu.Lock() + pending := s.pendingTxHz + s.mu.Unlock() + txHz := tx + if on && pending > 0 { + txHz = pending + } + if err := s.rig.SetSplit(on, txHz); err != nil { // The refusal is the useful part: a backend that cannot split says // so, and the client can tell the operator instead of transmitting // on the wrong frequency believing all is well. s.log("tci server: split %v refused: %v", on, err) return "" } + s.log("tci server: split %s, TX %d Hz", map[bool]string{true: "ON", false: "off"}[on], txHz) s.broadcast(fmt.Sprintf("split_enable:0,%t;", on)) return "" diff --git a/internal/tciserver/tciserver_test.go b/internal/tciserver/tciserver_test.go index 5e009b5..323468f 100644 --- a/internal/tciserver/tciserver_test.go +++ b/internal/tciserver/tciserver_test.go @@ -248,3 +248,83 @@ func TestUnknownCommandsAreQuiet(t *testing.T) { } } } + +// Split, with the client sending the two commands in the order it prefers. +// +// A client working split has to say two things: where to transmit, and that +// split is on. Nothing obliges it to say them in that order, and the frequency +// arriving first is the dangerous case: discarding it and then arming split +// leaves the transmit VFO on whatever it held — the RECEIVE frequency — so the +// operator transmits straight onto the DX while their software shows exactly +// what they asked for. +func TestSplitIsArmedOnTheFrequencyTheClientGaveWhicheverOrderItCame(t *testing.T) { + // Frequency first, then split — the order that used to lose the frequency. + r := &fakeRig{freq: 14025000, rxFreq: 14025000, mode: "CW"} + s := srv(r) + ask(t, s, "vfo:0,1,14027000") + ask(t, s, "split_enable:0,true") + if len(r.calls) != 1 || r.calls[0] != "split=true,14027000" { + t.Errorf("frequency first: the radio was told %v, want split armed on 14027000", r.calls) + } + + // Split first, then the frequency — the order that always worked. + r2 := &fakeRig{freq: 14025000, rxFreq: 14025000, mode: "CW"} + s2 := srv(r2) + ask(t, s2, "split_enable:0,true") + ask(t, s2, "vfo:0,1,14027000") + if len(r2.calls) == 0 || r2.calls[len(r2.calls)-1] != "split=true,14027000" { + t.Errorf("split first: the radio was told %v, want it to end on 14027000", r2.calls) + } +} + +// "Fake It" uses no split at all: the client shifts the DIAL at the start of +// transmit and shifts it back at the end. All it needs is channel A, and it +// must reach the radio both ways. +func TestFakeItIsJustTheDialMoving(t *testing.T) { + r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB"} + s := srv(r) + ask(t, s, "vfo:0,0,14075300") // up for the over + ask(t, s, "vfo:0,0,14074000") // and back + want := []string{"freq=14075300", "freq=14074000"} + if strings.Join(r.calls, " ") != strings.Join(want, " ") { + t.Errorf("the radio was told %v, want %v", r.calls, want) + } +} + +// A client in Fake It still says "split off" to be sure. The rig is already +// simplex, so there is nothing to do — and saying so beats asking a backend +// that may not be able to set split at all. +// +// This is what broke JTDX through the rigctl server: an error answered to a +// request that was already true, read as rig control failing, and the +// transmission abandoned a second into the frame. +func TestSayingSplitOffWhenAlreadySimplexTouchesNothing(t *testing.T) { + r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB", + splitErr: fmt.Errorf("this backend cannot split")} + s := srv(r) + ask(t, s, "split_enable:0,false") + if len(r.calls) != 0 { + t.Errorf("the radio was told %v for a state it was already in", r.calls) + } +} + +// A client restating PTT must not re-command the radio. Through the rigctl +// server, one sent set_ptt 0 sixteen times a second and the Flex's own transmit +// request was overwritten between two of them inside a millisecond. +func TestRepeatedPTTIsNotResentToTheRadio(t *testing.T) { + r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB"} + s := srv(r) + for i := 0; i < 5; i++ { + ask(t, s, "trx:0,false") + } + if len(r.calls) != 1 || r.calls[0] != "ptt=false" { + // The FIRST one always goes through: there is no knowing how the radio + // was left. + t.Errorf("the radio was told %v, want one unkey and no repeats", r.calls) + } + ask(t, s, "trx:0,true") + ask(t, s, "trx:0,true") + if len(r.calls) != 2 || r.calls[1] != "ptt=true" { + t.Errorf("the radio was told %v, want the change through and the repeat dropped", r.calls) + } +}