Repository navigation
Issue #181 solved: report TCP connect and send status to the GUI - #234
PRINCEKK122 wants to merge 2 commits into
Conversation
FromTcpThreadMessage declared four variants and update.rs handled all four, but the TCP thread only ever constructed Disconnected, added by oss-slu#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 oss-slu#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 oss-slu#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.
Sanjana-Siddani
left a comment
There was a problem hiding this comment.
Nice fix! All tests pass, and I also tested iMotions closing the connection mid-stream. The banner showed the error, the event log got just one entry instead of a flood, and reconnecting worked right away without pressing Disconnect first. Approving
# Conflicts: # relay/src/message.rs # relay/src/state.rs
|
@Sanjana-Siddani this was approved on Oct 18, but #247 landed in the meantime and I pushed a merge commit to resolve the conflict in relay/src/state.rs, which dismissed the approval. The only new content is the conflict resolution: the Send arm now mirrors every packet to the Python mirror regardless of whether the iMotions write succeeded, since the mirror is a separate destination and shouldn't lose a copy because iMotions went away. The #181 changes themselves are unchanged. Could you take another look? |
|



Closes #181.
Problem
FromTcpThreadMessagedeclares four variants andupdate.rshandles all four, but the TCP thread only ever constructedDisconnected, added by #179.Connected,SentandSendErrorwere dead arms.Two consequences, both from the issue:
on_tcp_packet_sentis reachable only from theSentarm, soshow_metricsstayed false forever andmetrics_blockearly-returned an empty widget every frame. The whole metrics subsystem —packets_last_60s,bps,sent_packet_times,human_bps— was unreachable, so Add a status indicator to the Relay GUI that shows packet transfer rate over TCP #109's acceptance criteria were never actually met in shipped code.tcp_connectedwas only ever assigned by theCheck TCP Connection Statusbutton, so the GUI could readfalsewhile packets were actively reaching the listener.Change
Connectedis reported from theOkarm ofTcpStream::connect, mirroring what theErrarm already does withDisconnected.Sent(bytes)is reported fromtry_send, the single funnel all three packet-shape branches (4-field, 8-field, paired) pass through, so every path is covered without duplication.send_packet_and_debugwrites exactlypacket.as_bytes(), sopkt.len()is the honest byte count.SendRawreports too — a test packet is still a packet on the wire.let _ = try_send(...), eight times). It now uses?, and a failure reportsDisconnectedand exits the thread.Why a write failure is a disconnect
A broken connection doesn't fail one packet, it fails all of them: six to eight per baton tick at 20 Hz. Reporting each one would write ~120 entries/sec into the event log and bury everything else in the five-line GUI panel — the bug #183 just fixed. Treating it as a disconnect instead reuses the recovery path #179 established, which calls
reap_tcp_thread()soConnect TCPworks again without pressingDisconnect TCPfirst.SendErroris left in the enum, documented as unconstructed, for a future transient-failure case that can be reported without tearing the connection down.Testing
Two tests added to
update.rs, using the existingpump_updateharness so they drive the sameMessage::Updatethe GUI subscription sends:connecting_reports_status_without_a_manual_check— binds a real listener, connects, accepts, and assertstcp_connectedbecomes true with no button press.sent_packets_make_the_throughput_block_appear— sends a packet over a real socket and assertspackets_last_60s > 0andshow_metrics, the flag that gates the throughput block.cargo testpasses, 10/10. Warning count is unchanged frommain.Also verified manually on Linux against
src/server/with the baton driven at the plugin's 20 Hz: the status flips on its own and the throughput block appears.