Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double-print of "New SDK_client connected" #7522

Open
jleibs opened this issue Sep 26, 2024 · 0 comments
Open

Double-print of "New SDK_client connected" #7522

jleibs opened this issue Sep 26, 2024 · 0 comments
Labels
😤 annoying Something in the UI / SDK is annoying to use 📺 re_viewer affects re_viewer itself

Comments

@jleibs
Copy link
Member

jleibs commented Sep 26, 2024

When using:

  rr.init("rerun_example", spawn=True)
  rr.connect()

The SDK prints out the "New SDK client connected" message TWICE which is unexpected for users:

For exampole:

[2024-09-26T00:08:33Z INFO  re_sdk_comms::server] Hosting a SDK server over TCP at 0.0.0.0:9876. Connect with the Rerun logging SDK.
[2024-09-26T00:08:33Z INFO  re_sdk_comms::server] New SDK client connected from: 127.0.0.1:63884
[2024-09-26T00:08:33Z INFO  re_sdk_comms::server] New SDK client connected from: 127.0.0.1:63885

The rationale comes back to the fact that after spawning we wait for the spawned server to be able to be connected to, which is helpful to prevents other connection-timeout messages down the line. See:

// Give the newly spawned Rerun Viewer some time to bind.
//
// NOTE: The timeout only covers the TCP handshake: if no process is bound to that address
// at all, the connection will fail immediately, irrelevant of the timeout configuration.
// For that reason we use an extra loop.
for i in 0..5 {
re_log::debug!("connection attempt {}", i + 1);
if TcpStream::connect_timeout(&connect_addr, Duration::from_secs(1)).is_ok() {
break;
}
std::thread::sleep(Duration::from_millis(100));
}
}

This "test connection" causes the spurious connection message from the viewer instance.

Now that we have a more defined connection protocol we should be able to include a payload indicating this is a "test" connection and skip the client connection printout in that case.

@jleibs jleibs added 📺 re_viewer affects re_viewer itself 😤 annoying Something in the UI / SDK is annoying to use labels Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😤 annoying Something in the UI / SDK is annoying to use 📺 re_viewer affects re_viewer itself
Projects
None yet
Development

No branches or pull requests

1 participant