Skip to content

RSDK-654 - refactor message handling for streaming#184

Merged
Ethan (stuqdog) merged 2 commits into
viamrobotics:mainfrom
stuqdog:RSDK-654-bidi-fix
Jul 6, 2026
Merged

RSDK-654 - refactor message handling for streaming#184
Ethan (stuqdog) merged 2 commits into
viamrobotics:mainfrom
stuqdog:RSDK-654-bidi-fix

Conversation

@stuqdog

Copy link
Copy Markdown
Member

Refactors message handling to send messages asynchronously as soon as they come in, defer sending EOS until client messages are demonstrably completed, and only as its own frame.

@stuqdog
Ethan (stuqdog) marked this pull request as ready for review July 1, 2026 15:08
@stuqdog
Ethan (stuqdog) requested a review from a team as a code owner July 1, 2026 15:08
@stuqdog
Ethan (stuqdog) marked this pull request as draft July 1, 2026 15:08
@acmorrow

Copy link
Copy Markdown
Member

Ethan (@stuqdog) - I haven't dug into the code, but I have pulled this PR locally and confirmed that in my repro environment, I no longer see the arm wait until all the points have been sent on the stream. So this looks promising!

@stuqdog
Ethan (stuqdog) marked this pull request as ready for review July 2, 2026 20:05

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a pass through this, but it has been over a year since I wrote any production rust. You might want Nicolas Menard (@npmenard) to also take a look, especially since he did the BiDi work in the MicroRDK and knows the async rust gRPC interaction quite well.

Comment thread src/rpc/client_channel.rs

/// Signals the end of the client's send stream (the gRPC CloseSend equivalent).
// note(ethan): the variable that used to carry this signal was named
// `it_was_all_a_stream` — the best variable name I've ever shipped into production.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that this is a memorial to a memorial.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just couldn't let it go haha

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand completely! Long ago, in a closed source system, I managed to slip in the following haiku, which I now have an opportunity to bring back to life:

Exception rising
On the signal handler stack
Portending sorrow

@npmenard Nicolas Menard (npmenard) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stuqdog
Ethan (stuqdog) merged commit 60db4e1 into viamrobotics:main Jul 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants