From 2c70e16325d2091f550840147ae703a258a10af5 Mon Sep 17 00:00:00 2001 From: Prince Karikari Date: Wed, 14 Oct 2026 22:52:10 -0500 Subject: [PATCH] Issue #181 solved: report TCP connect and send status to the GUI FromTcpThreadMessage declared four variants and update.rs handled all four, but the TCP thread only ever constructed Disconnected, added by #179. Connected, Sent and SendError were dead arms. Two things were broken by that. tcp_connected was only ever assigned by the Check TCP Connection Status button, so the status line showed stale data while packets were flowing. And on_tcp_packet_sent was reachable only from the Sent arm, so show_metrics stayed false forever and metrics_block early-returned every frame: the throughput indicator added in #109 has never once displayed. Report Connected from the connect Ok arm, and Sent from try_send, which is the single funnel all three packet-shape branches pass through. SendRaw reports too, since a test packet is still a packet on the wire. A failed write now propagates instead of being discarded: the fan-out uses ? and the thread reports Disconnected and exits, reusing the recovery path from #179 that reaps the handle so Connect TCP works again. Reporting each failed packet instead would write ~120 entries a second into the event log and bury everything else in the panel. SendError is left in place, documented as unconstructed. Tests cover both halves: the status flips without a manual check, and a sent packet makes the throughput block appear. --- relay/src/message.rs | 7 +- relay/src/state.rs | 171 +++++++++++++++++++++++++++---------------- relay/src/update.rs | 43 +++++++++++ 3 files changed, 156 insertions(+), 65 deletions(-) diff --git a/relay/src/message.rs b/relay/src/message.rs index 28882aa..fc70595 100644 --- a/relay/src/message.rs +++ b/relay/src/message.rs @@ -64,6 +64,11 @@ pub(crate) enum FromTcpThreadMessage { Disconnected(String), /// A packet was sent to remote (In bytes) Sent(usize), - /// Error reported + /// Error reported. + /// + /// Nothing constructs this today: a failed write means the socket is gone, + /// so the TCP thread reports `Disconnected` and exits rather than emitting + /// a send error per dropped packet. Kept for a future transient-failure + /// case that can be reported without tearing the connection down. SendError(String), } diff --git a/relay/src/state.rs b/relay/src/state.rs index 18976ce..0d27336 100644 --- a/relay/src/state.rs +++ b/relay/src/state.rs @@ -580,6 +580,9 @@ impl State { Ok(stream) => { human_log("TCP", "Successfully connected to iMotions server."); let _ = child_bichannel.set_is_conn_to_endpoint(true); + // Report it so the status line follows the real socket instead + // of waiting for a manual Check TCP Connection Status press. + let _ = child_bichannel.send_to_parent(FromTcpThreadMessage::Connected); stream } Err(e) => { @@ -609,76 +612,116 @@ impl State { continue; } - let mut try_send = |pkt: String| -> Result<()> { - let res = send_packet_and_debug(&mut stream, &pkt); - if res.is_err() { - let _ = child_bichannel.set_is_conn_to_endpoint(false); - } else { - let _ = child_bichannel.set_is_conn_to_endpoint(true); + // A write failure means the socket is gone, so stop at the + // first one and report it as a disconnect. Reporting each + // failed packet instead would write ~120 entries/sec to the + // event log and bury everything else in the GUI panel. + let send_outcome = (|| -> Result<()> { + let mut try_send = |pkt: String| -> Result<()> { + let bytes = pkt.len(); + match send_packet_and_debug(&mut stream, &pkt) { + Ok(()) => { + let _ = child_bichannel.set_is_conn_to_endpoint(true); + // Report the write so the throughput block has + // something to count. + let _ = child_bichannel + .send_to_parent(FromTcpThreadMessage::Sent(bytes)); + Ok(()) + } + Err(e) => { + let _ = child_bichannel.set_is_conn_to_endpoint(false); + Err(e) + } + } + }; + + if fields.len() == 4 { + let alt = fields[0].clone(); + let air = fields[1].clone(); + let head = fields[2].clone(); + let vv = fields[3].clone(); + + try_send(build_imotions_packet("AltitudeSync", &[alt.clone(), alt.clone()]))?; + try_send(build_imotions_packet("AirspeedSync", &[air.clone(), air.clone()]))?; + try_send(build_imotions_packet("VerticalVelocitySync", &[vv.clone(), vv.clone()]))?; + try_send(build_imotions_packet("HeadingSync", &[head.clone(), head.clone()]))?; + return Ok(()); } - res - }; - - if fields.len() == 4 { - let alt = fields[0].clone(); - let air = fields[1].clone(); - let head = fields[2].clone(); - let vv = fields[3].clone(); - - let _ = try_send(build_imotions_packet("AltitudeSync", &[alt.clone(), alt.clone()])); - let _ = try_send(build_imotions_packet("AirspeedSync", &[air.clone(), air.clone()])); - let _ = try_send(build_imotions_packet("VerticalVelocitySync", &[vv.clone(), vv.clone()])); - let _ = try_send(build_imotions_packet("HeadingSync", &[head.clone(), head.clone()])); - continue; - } - - if fields.len() == 8 { - let alt = fields[0].clone(); - let air = fields[1].clone(); - let head = fields[2].clone(); - let vv = fields[3].clone(); - let roll = fields[4].clone(); - let pitch = fields[5].clone(); - let yaw = fields[6].clone(); - let gforce = fields[7].clone(); - - let _ = try_send(build_imotions_packet("AltitudeSync", &[alt.clone(), alt.clone()])); - let _ = try_send(build_imotions_packet("AirspeedSync", &[air.clone(), air.clone()])); - let _ = try_send(build_imotions_packet("VerticalVelocitySync", &[vv.clone(), vv.clone()])); - let _ = try_send(build_imotions_packet("HeadingSync", &[head.clone(), head.clone()])); - let _ = try_send(build_imotions_packet("RollSync", &[roll.clone(), roll.clone()])); - let _ = try_send(build_imotions_packet("PitchSync", &[pitch.clone(), pitch.clone()])); - let _ = try_send(build_imotions_packet("YawSync", &[yaw.clone(), yaw.clone()])); - let _ = try_send(build_imotions_packet("GForceSync", &[gforce.clone(), gforce.clone()])); - continue; - } - // Paired mapping - let mut idx = 0usize; - let mut send_pair_if_present = |name: &str, idx: &mut usize| -> Result<()> { - if *idx + 1 < fields.len() { - let payload = vec![fields[*idx].clone(), fields[*idx + 1].clone()]; - let pkt = build_imotions_packet(name, &payload); - *idx += 2; - try_send(pkt)?; + if fields.len() == 8 { + let alt = fields[0].clone(); + let air = fields[1].clone(); + let head = fields[2].clone(); + let vv = fields[3].clone(); + let roll = fields[4].clone(); + let pitch = fields[5].clone(); + let yaw = fields[6].clone(); + let gforce = fields[7].clone(); + + try_send(build_imotions_packet("AltitudeSync", &[alt.clone(), alt.clone()]))?; + try_send(build_imotions_packet("AirspeedSync", &[air.clone(), air.clone()]))?; + try_send(build_imotions_packet("VerticalVelocitySync", &[vv.clone(), vv.clone()]))?; + try_send(build_imotions_packet("HeadingSync", &[head.clone(), head.clone()]))?; + try_send(build_imotions_packet("RollSync", &[roll.clone(), roll.clone()]))?; + try_send(build_imotions_packet("PitchSync", &[pitch.clone(), pitch.clone()]))?; + try_send(build_imotions_packet("YawSync", &[yaw.clone(), yaw.clone()]))?; + try_send(build_imotions_packet("GForceSync", &[gforce.clone(), gforce.clone()]))?; + return Ok(()); } + + // Paired mapping + let mut idx = 0usize; + let mut send_pair_if_present = |name: &str, idx: &mut usize| -> Result<()> { + if *idx + 1 < fields.len() { + let payload = vec![fields[*idx].clone(), fields[*idx + 1].clone()]; + let pkt = build_imotions_packet(name, &payload); + *idx += 2; + try_send(pkt)?; + } + Ok(()) + }; + + send_pair_if_present("AltitudeSync", &mut idx)?; + send_pair_if_present("AirspeedSync", &mut idx)?; + send_pair_if_present("VerticalVelocitySync", &mut idx)?; + send_pair_if_present("HeadingSync", &mut idx)?; + send_pair_if_present("RollSync", &mut idx)?; + send_pair_if_present("PitchSync", &mut idx)?; + send_pair_if_present("YawSync", &mut idx)?; + send_pair_if_present("GForceSync", &mut idx)?; Ok(()) - }; - - let _ = send_pair_if_present("AltitudeSync", &mut idx); - let _ = send_pair_if_present("AirspeedSync", &mut idx); - let _ = send_pair_if_present("VerticalVelocitySync", &mut idx); - let _ = send_pair_if_present("HeadingSync", &mut idx); - let _ = send_pair_if_present("RollSync", &mut idx); - let _ = send_pair_if_present("PitchSync", &mut idx); - let _ = send_pair_if_present("YawSync", &mut idx); - let _ = send_pair_if_present("GForceSync", &mut idx); + })(); + + if let Err(e) = send_outcome { + let reason = format!("TCP send failed: {}", e); + human_log("TCP", &reason); + // Same path #179 established for a failed connect: the + // GUI shows it and reap_tcp_thread clears the handle, so + // Connect TCP works again without Disconnect TCP first. + let _ = child_bichannel + .send_to_parent(FromTcpThreadMessage::Disconnected(reason.clone())); + bail!(reason); + } } ToTcpThreadMessage::SendRaw(packet) => { - let res = send_packet_and_debug(&mut stream, &packet); - let _ = child_bichannel.set_is_conn_to_endpoint(res.is_ok()); - if let Err(e) = res { - human_log("TCP", &format!("Test packet failed: {}", e)); + match send_packet_and_debug(&mut stream, &packet) { + Ok(()) => { + let _ = child_bichannel.set_is_conn_to_endpoint(true); + // A test packet is still a packet on the wire, so it + // counts toward throughput like any other. + let _ = child_bichannel.send_to_parent( + FromTcpThreadMessage::Sent(packet.len()), + ); + } + Err(e) => { + let reason = format!("Test packet failed: {}", e); + human_log("TCP", &reason); + let _ = child_bichannel.set_is_conn_to_endpoint(false); + let _ = child_bichannel.send_to_parent( + FromTcpThreadMessage::Disconnected(reason.clone()), + ); + bail!(reason); + } } } } diff --git a/relay/src/update.rs b/relay/src/update.rs index b8acc74..634a1eb 100644 --- a/relay/src/update.rs +++ b/relay/src/update.rs @@ -426,6 +426,49 @@ mod tests { let _ = state.tcp_disconnect(); } + #[test] + fn connecting_reports_status_without_a_manual_check() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind failed"); + let addr = listener.local_addr().expect("local_addr failed").to_string(); + + let mut state = State::default(); + state.tcp_connect(addr).expect("tcp_connect failed"); + let (_stream, _) = listener.accept().expect("accept failed"); + + pump_update(&mut state, 50); + + // Before #181 the TCP thread never sent Connected, so this stayed false + // until Check TCP Connection Status was pressed. + assert!( + state.tcp_connected, + "status never flipped on its own: {:?}", + state.event_log + ); + + let _ = state.tcp_disconnect(); + } + + #[test] + fn sent_packets_make_the_throughput_block_appear() { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind failed"); + let addr = listener.local_addr().expect("local_addr failed").to_string(); + + let mut state = State::default(); + state.tcp_connect(addr).expect("tcp_connect failed"); + let (_stream, _) = listener.accept().expect("accept failed"); + assert!(wait_until(|| state.is_tcp_connected()), "TCP never connected"); + + let _ = update(&mut state, Message::SendPacket); + pump_update(&mut state, 50); + + // show_metrics gates metrics_block in view.rs. Nothing ever set it + // before #181, so the throughput indicator could never render. + assert!(state.packets_last_60s > 0, "no packets counted"); + assert!(state.show_metrics, "throughput block still hidden"); + + let _ = state.tcp_disconnect(); + } + #[test] fn send_packet_with_every_toggle_off_reports_an_error() { let mut state = State::default();