Repository navigation
fix(serial): SerialClient greets every connection with hello (#443) - #447
Merged
Merged
Conversation
A UART never closes, so a server learns that the client behind it changed only from a hello. SerialClient never sent one: a host process restarted while the board kept running reused subscription ids the old session still held, the board answered Denied, and mirrored records went silent until a board reset. aimdb-core: - EnvelopeCodec::hello (default None) names the request a client opens each connection with; AimxCodec returns AimX's hello. - ClientConfig::sends_hello sends that hello when the codec has one (else the old Ping/Pong). It skips frames of the previous session that arrive before the reply, resends an unanswered hello every HELLO_RESEND_MS, and stops the engine on a version_mismatch instead of redialing. aimdb-serial-connector: - SerialClient::new and over_port turn sends_hello on. - SerialPortDialer writes a frame delimiter on open, so a frame a previous process left half-written cannot swallow the hello. - tests/client_restart.rs: three host processes in turn on one live line keep mirroring (fails at the second without the fix), plus a lost hello with stray frames, and a refused version. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WGkDJ6wpoRhHZ8DEGbXbf3
Frame order already puts the hello ahead of every command, so waiting for the Welcome bought little and added failure modes: the handshake loop did not watch the command channel, so a silent line kept it resending after every handle was dropped, and a Welcome over the receive limit was dropped by the framer and left the client resending forever. - sends_hello with a codec hello now sends it once per connection and goes straight on. Its reply (id 0, which no ClientHandle issues) is dropped in the demux loop; a version_mismatch reply is logged as HelloRefused and the client keeps running. The Ping/Pong path is unchanged. - Drop the resend timer, HELLO_RESEND_MS, Ended::Refused and the HelloUnanswered event. - Tests: the resend and version-refusal tests become one showing the hello goes first, the subscribe follows without a Welcome, and late id-0 replies (ok and refused) leave the subscription mirroring. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WGkDJ6wpoRhHZ8DEGbXbf3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A UART never closes, so a
SerialServerlearns that the client behind it changed only from ahello.SerialClientnever sent one. When a host process restarted while the board kept running, the new process numbered its subscriptions from 1 again. The old session still held those ids, so the board answeredDenied. Mirrored records went silent until the board was reset, while writes kept working.aimdb-core
EnvelopeCodec::hellois a new provided method that defaults toNone. It names the request a client sends to open each connection.AimxCodecreturns("hello", {"client","version": PROTOCOL_VERSION}).ClientConfig::sends_hellonow sends the codec's hello when the codec has one:hellobefore the subscriptions and writes that follow it.0, whichClientHandlenever issues. The demux loop drops its reply when it arrives. Aversion_mismatchreply is logged as the newHelloRefusedevent and the client keeps running.Pong.aimdb-serial-connector
SerialClient::newandSerialClient::over_portnow setsends_hello: true.SerialPortDialeropens the port, it writes a single0x00delimiter. Without it, a frame left half-written by the previous process could merge with the newhello, and the board would drop both. The receiver skips an extra delimiter as an empty chunk.Why the client doesn't wait for the
WelcomeThe first version of this PR waited for the reply and resent the
helloevery second until one came. That opened failure modes that order alone avoids:Welcomelarger than the receive limit was dropped by the COBS reader. The client then resent forever without ever moving on.Accepted limits
hellolost on the line is not resent. The client stays in the old session until it reconnects or restarts. The leading delimiter covers the likely cause, a half-written frame left by the previous process.helloeither.hello, only gets greeted by the first upgraded client'shello. Resetting the board once clears this.Related Issue
Tests
aimdb-serial-connector/tests/client_restart.rs:a_restarted_client_keeps_mirroring: the board produces every 5 ms on a line that stays open. Three host processes (SerialClient::new+link_from) run one after another and are killed without a goodbye. Each must mirror values newer than when it started. Withsends_helloturned off, this test fails at host process 2, which reproduces the issue.the_hello_goes_first_and_its_reply_is_not_awaited: a fake board receives ahello(id 0) first, then a subscribe with noWelcomesent. The subscription's events mirror. A lateokreply and a lateversion_mismatchreply, both with id 0, leave the subscription mirroring.Checklist
make check). I did not run the fullmake check. What I ran passed:cargo fmt --check;cargo testforaimdb-core(std,connector-session,remote,tracing,log),aimdb-serial-connector(std) andaimdb-client(all features); clippy-D warningson the core and serial legs, includingthumbv7em-none-eabihfwithdefmtand_test-embassy; and rustdoc-D warningsfor both crates.🤖 Generated with Claude Code
https://claude.ai/code/session_01WGkDJ6wpoRhHZ8DEGbXbf3