Follow-up fixes for fyne-ui-v2 (PR #174 review comments)

gui_interface/fyne-ui/engine/mercury_bridge.c

@@ -177,3 +177,8 @@ void mercury_ui_set_waterfall(bool enabled)
 {
     ui_comm_set_waterfall(enabled);
 }
+
+void mercury_ui_get_tcp_ports(int *arq_base_port, int *broadcast_port)
+{
+    ui_comm_get_tcp_ports(arq_base_port, broadcast_port);
+}

gui_interface/fyne-ui/engine/mercury_bridge.h

@@ -79,6 +79,9 @@ int mercury_ui_get_input_channel(void);
 /* Enable / disable waterfall/spectrum at runtime.  Saves to mercury.ini. */
 void mercury_ui_set_waterfall(bool enabled);
 
+/* Read the TNC TCP ports the engine listens on. */
+void mercury_ui_get_tcp_ports(int *arq_base_port, int *broadcast_port);
+
 #ifdef __cplusplus
 }
 #endif

gui_interface/fyne-ui/link_engine.go

@@ -43,6 +43,11 @@ const (
 	engineSpectrumInterval = 20 * time.Millisecond
 	// Matches MODEM_STATS_NSPEC; the bridge clamps to its own size anyway.
 	engineSpectrumBins = 512
+	// After a config change that restarts a subsystem (audioio, hamlib),
+	// re-read the device lists several times so a slow restart is not
+	// reported as the stale pre-change selection.
+	refreshRetryInterval = 200 * time.Millisecond
+	refreshRetries       = 5
 )
 
 func newEngineLink() *engineLink {
@@ -119,9 +124,20 @@ func (l *engineLink) Start(ctx context.Context) (<-chan Event, error) {
 
 			case <-l.refresh:
 				// A device or radio change was just applied; re-read so the
-				// pickers show what the engine actually ended up using, which
-				// is not always what was asked for.
-				_ = l.emitDeviceLists(ctx, events)
+				// pickers show what the engine actually ended up using, which is
+				// not always what was asked for.  Retry a few times because the
+				// restart it triggered may take longer than one read to settle.
+				for i := 0; i <= refreshRetries; i++ {
+					_ = l.emitDeviceLists(ctx, events)
+					if i == refreshRetries {
+						break
+					}
+					select {
+					case <-ctx.Done():
+						return
+					case <-time.After(refreshRetryInterval):
+					}
+				}
 			}
 		}
 	}()
@@ -147,12 +163,12 @@ func (l *engineLink) Send(cmd Command) error {
 
 	switch cmd.Name {
 	case "set_audio_config", "set_radio_config":
-		time.AfterFunc(100*time.Millisecond, func() {
-			select {
-			case l.refresh <- struct{}{}:
-			default:
-			}
-		})
+		// The Start goroutine owns the retry timing (cancellable via ctx),
+		// so no bare time.AfterFunc here that would outlive Close().
+		select {
+		case l.refresh <- struct{}{}:
+		default:
+		}
 	}
 	return nil
 }
@@ -260,6 +276,14 @@ func (l *engineLink) SetWaterfall(enabled bool) {
 	C.mercury_ui_set_waterfall(cEn)
 }
 
+// TCPPorts returns the ARQ base and broadcast TCP ports the engine is
+// actually listening on (from its config).
+func (l *engineLink) TCPPorts() (arqBase, broadcast int) {
+	var a, b C.int
+	C.mercury_ui_get_tcp_ports(&a, &b)
+	return int(a), int(b)
+}
+
 func (l *engineLink) Close() {}
 
 // statusFromC converts the engine's status struct into the UI's own type. This

gui_interface/fyne-ui/link_engine_stub.go

@@ -31,3 +31,7 @@ func (l *engineLink) Send(cmd Command) error {
 func (l *engineLink) Close() {}
 
 func (l *engineLink) SetWaterfall(enabled bool) {}
+
+func (l *engineLink) TCPPorts() (arqBase, broadcast int) {
+	return 8300, 8100
+}

gui_interface/fyne-ui/main.go

@@ -827,10 +827,15 @@ func main() {
 	}
 
 	mercuryClientButton := widget.NewButton("Launch Mercury Client", func() {
-		state.mu.Lock()
+		state.mu.RLock()
 		tel := state.telemetry
-		state.mu.Unlock()
-		openMercuryClientWindow(myApp, tel)
+		link := state.link
+		state.mu.RUnlock()
+		arqPort, broadcastPort := 8300, 8100
+		if engLink, ok := link.(*engineLink); ok {
+			arqPort, broadcastPort = engLink.TCPPorts()
+		}
+		openMercuryClientWindow(myApp, tel, arqPort, broadcastPort)
 	})
 
 	topBar := container.NewHBox(

gui_interface/fyne-ui/mercury_chat_window.go

@@ -19,7 +19,7 @@ import (
 // transfers: each append would otherwise rebuild a growing string / prepend a
 // new widget forever, making the window unusable.
 const (
-	maxLogLines     = 1000
+	maxLogLines     = 200
 	maxChatMessages = 200
 )
 
@@ -28,13 +28,13 @@ const (
 // the first and tear down its ARQ session.  Reuse the window instead.
 var mercuryClientSingleton *chatWindow
 
-func openMercuryClientWindow(app fyne.App, telemetry telemetryState) {
+func openMercuryClientWindow(app fyne.App, telemetry telemetryState, arqPort, broadcastPort int) {
 	if mercuryClientSingleton != nil {
 		mercuryClientSingleton.win.RequestFocus()
 		return
 	}
 	cw := &chatWindow{}
-	cw.build(app, telemetry)
+	cw.build(app, telemetry, arqPort, broadcastPort)
 	mercuryClientSingleton = cw
 }
 
@@ -43,6 +43,9 @@ type chatWindow struct {
 	mc   *client.Client
 	done chan struct{}
 	log  *widget.Entry
+	// logLines is the bounded ring (newest first) backing the log Entry,
+	// so appending never re-splits the widget's own text.
+	logLines []string
 
 	arqBox      *fyne.Container
 	arqScroll   *container.Scroll
@@ -66,7 +69,7 @@ type chatWindow struct {
 	bcastMsg      *widget.Entry
 }
 
-func (cw *chatWindow) build(app fyne.App, telemetry telemetryState) {
+func (cw *chatWindow) build(app fyne.App, telemetry telemetryState, arqPort, broadcastPort int) {
 	cw.win = app.NewWindow("Mercury Client")
 
 	cw.myCall = widget.NewEntry()
@@ -76,9 +79,9 @@ func (cw *chatWindow) build(app fyne.App, telemetry telemetryState) {
 	cw.ip = widget.NewEntry()
 	cw.ip.SetText("127.0.0.1")
 	cw.arqPort = widget.NewEntry()
-	cw.arqPort.SetText("8300")
+	cw.arqPort.SetText(strconv.Itoa(arqPort))
 	cw.bcastPort = widget.NewEntry()
-	cw.bcastPort.SetText("8100")
+	cw.bcastPort.SetText(strconv.Itoa(broadcastPort))
 
 	cw.arqMsg = widget.NewEntry()
 	cw.arqMsg.SetPlaceHolder("Type message to be sent...")
@@ -176,17 +179,11 @@ func (cw *chatWindow) logMsg(format string, args ...any) {
 	fyne.Do(func() {
 		ts := time.Now().Format("15:04:05")
 		line := fmt.Sprintf("[%s] "+format, append([]any{ts}, args...)...)
-		cur := cw.log.Text
-		if cur == "" {
-			cw.log.SetText(line)
-		} else {
-			newText := fmt.Sprintf("%s\n%s", line, cur)
-			lines := strings.Split(newText, "\n")
-			if len(lines) > maxLogLines {
-				lines = lines[:maxLogLines]
-			}
-			cw.log.SetText(strings.Join(lines, "\n"))
+		cw.logLines = append([]string{line}, cw.logLines...)
+		if len(cw.logLines) > maxLogLines {
+			cw.logLines = cw.logLines[:maxLogLines]
 		}
+		cw.log.SetText(strings.Join(cw.logLines, "\n"))
 		cw.log.Refresh()
 	})
 }
@@ -263,8 +260,33 @@ func (cw *chatWindow) setARQ(on bool) {
 }
 
 func (cw *chatWindow) onConnect() {
-	arqPort, _ := strconv.Atoi(cw.arqPort.Text)
-	bcastPort, _ := strconv.Atoi(cw.bcastPort.Text)
+	// Synchronous guard: a double-tap (or key-repeat on a focused button)
+	// can fire this twice in one poll batch before setTCP's fyne.Do runs.
+	cw.connectBtn.Disable()
+
+	// Disconnect any existing client before opening a new one, so a stale
+	// control client is not left open to be evicted by the new connection.
+	if cw.mc != nil {
+		cw.mc.Disconnect()
+		cw.mc = nil
+	}
+	if cw.done != nil {
+		close(cw.done)
+		cw.done = nil
+	}
+
+	arqPort, err := strconv.Atoi(cw.arqPort.Text)
+	if err != nil {
+		dialog.ShowError(fmt.Errorf("invalid ARQ port: %v", err), cw.win)
+		cw.connectBtn.Enable()
+		return
+	}
+	bcastPort, err := strconv.Atoi(cw.bcastPort.Text)
+	if err != nil {
+		dialog.ShowError(fmt.Errorf("invalid broadcast port: %v", err), cw.win)
+		cw.connectBtn.Enable()
+		return
+	}
 	cfg := client.Config{
 		MyCallsign:     cw.myCall.Text,
 		TargetCallsign: cw.target.Text,
@@ -276,11 +298,9 @@ func (cw *chatWindow) onConnect() {
 	if err := mc.Connect(); err != nil {
 		dialog.ShowError(err, cw.win)
 		cw.logMsg("connect: %v", err)
+		cw.connectBtn.Enable()
 		return
 	}
-	if cw.done != nil {
-		close(cw.done)
-	}
 	cw.mc = mc
 	cw.setTCP(true)
 	cw.done = make(chan struct{})
@@ -316,17 +336,23 @@ func (cw *chatWindow) onARQConnect() {
 	go func() {
 		if err := mc.ConnectARQ(); err != nil {
 			cw.logMsg("ARQ connect: %v", err)
-			cw.setARQ(false)
+			// Only re-enable the button if the modem is still up: a
+			// disconnect mid-handshake leaves cw.mc nil.
+			if cw.mc == mc {
+				cw.setARQ(false)
+			}
 			return
 		}
 		cw.logMsg("ARQ connected.")
-		cw.setARQ(true)
+		if cw.mc == mc {
+			cw.setARQ(true)
+		}
 	}()
 }
 
 func (cw *chatWindow) onARQDisconnect() {
-	if cw.mc != nil {
-		cw.mc.DisconnectARQ()
+	if mc := cw.mc; mc != nil {
+		mc.DisconnectARQ()
 	}
 	cw.setARQ(false)
 	cw.logMsg("ARQ disconnected.")

gui_interface/mercury-client/client/client.go

@@ -237,6 +237,18 @@ func (c *Client) SendBroadcast(msg string) error {
 	return nil
 }
 
+// sendOrStop forwards v to ch unless done is closed first.  It returns false
+// when the goroutine should stop instead of blocking forever on a full channel
+// whose only drainer has already exited.
+func sendOrStop[T any](ch chan<- T, v T, done <-chan struct{}) bool {
+	select {
+	case ch <- v:
+		return true
+	case <-done:
+		return false
+	}
+}
+
 // updateLog forwards modem log lines to LogCh.
 func (c *Client) updateLog() {
 	c.mu.Lock()
@@ -252,7 +264,9 @@ func (c *Client) updateLog() {
 			if !ok {
 				return
 			}
-			c.LogCh <- logMsg
+			if !sendOrStop(c.LogCh, logMsg, done) {
+				return
+			}
 		case <-done:
 			return
 		}
@@ -274,9 +288,13 @@ func (c *Client) handleIncomingARQ() {
 			if !ok {
 				return
 			}
-			c.LogCh <- fmt.Sprintf("ARQ Control: %s", arqMsg)
+			if !sendOrStop(c.LogCh, fmt.Sprintf("ARQ Control: %s", arqMsg), done) {
+				return
+			}
 			if strings.HasPrefix(arqMsg, "CONNECTED") {
-				c.updateRemoteCall(arqMsg)
+				if !c.updateRemoteCall(arqMsg, done) {
+					return
+				}
 			}
 		case <-done:
 			return
@@ -286,10 +304,10 @@ func (c *Client) handleIncomingARQ() {
 
 // updateRemoteCall derives the remote callsign from a "CONNECTED <src> <dst> ..."
 // line by matching against the local callsign.
-func (c *Client) updateRemoteCall(connectedLine string) {
+func (c *Client) updateRemoteCall(connectedLine string, done <-chan struct{}) bool {
 	fields := strings.Fields(connectedLine)
 	if len(fields) < 3 {
-		return
+		return true
 	}
 	myCall := strings.ToUpper(strings.TrimSpace(c.cfg.MyCallsign))
 	callA := fields[1]
@@ -306,7 +324,7 @@ func (c *Client) updateRemoteCall(connectedLine string) {
 	c.mu.Lock()
 	c.remoteCall = call
 	c.mu.Unlock()
-	c.LogCh <- fmt.Sprintf("Remote ARQ callsign set to: %s", call)
+	return sendOrStop(c.LogCh, fmt.Sprintf("Remote ARQ callsign set to: %s", call), done)
 }
 
 // handleIncomingARQData buffers incoming ARQ data and emits complete
@@ -325,7 +343,9 @@ func (c *Client) handleIncomingARQData() {
 			if !ok {
 				return
 			}
-			c.LogCh <- fmt.Sprintf("ARQ Data RX: %d bytes: %q", len(data), string(data))
+			if !sendOrStop(c.LogCh, fmt.Sprintf("ARQ Data RX: %d bytes: %q", len(data), string(data)), done) {
+				return
+			}
 			c.mu.Lock()
 			c.chatRxBuffer += string(data)
 			var lines []ChatMessage
@@ -349,7 +369,9 @@ func (c *Client) handleIncomingARQData() {
 			}
 			c.mu.Unlock()
 			for _, msg := range lines {
-				c.ARQChatCh <- msg
+				if !sendOrStop(c.ARQChatCh, msg, done) {
+					return
+				}
 			}
 		case <-done:
 			return
@@ -374,7 +396,9 @@ func (c *Client) handleIncomingBroadcast() {
 			if !ok {
 				return
 			}
-			c.LogCh <- fmt.Sprintf("Broadcast RX (Decoded): %s", string(data))
+			if !sendOrStop(c.LogCh, fmt.Sprintf("Broadcast RX (Decoded): %s", string(data)), done) {
+				return
+			}
 			c.mu.Lock()
 			c.broadcastRxBuffer += string(data)
 			var lines []ChatMessage
@@ -394,7 +418,9 @@ func (c *Client) handleIncomingBroadcast() {
 			}
 			c.mu.Unlock()
 			for _, msg := range lines {
-				c.BroadcastChatCh <- msg
+				if !sendOrStop(c.BroadcastChatCh, msg, done) {
+					return
+				}
 			}
 		case <-done:
 			return
@@ -418,8 +444,12 @@ func (c *Client) handleStatus() {
 			if !ok {
 				return
 			}
-			c.LogCh <- fmt.Sprintf("TNC Status: %s", status)
-			c.StatusCh <- status
+			if !sendOrStop(c.LogCh, fmt.Sprintf("TNC Status: %s", status), done) {
+				return
+			}
+			if !sendOrStop(c.StatusCh, status, done) {
+				return
+			}
 			if status == "DISCONNECTED" {
 				c.mu.Lock()
 				c.remoteCall = ""