Add non-spawning create variants for TCP/TLS/serial clients#180
Merged
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new ClientTask type and corresponding create_* functions (create_tcp_client_task_with_options, create_rtu_client_task, and create_tls_client_task_with_options) to allow creating Modbus client tasks without spawning them immediately. This allows callers to apply custom instrumentation before spawning. The review feedback suggests optimizing spawn_tcp_channel and spawn_tls_channel to avoid cloning host (which can cause heap allocations) by creating the tracing span first and then moving host into the channel creation functions.
Adds create_tcp_client_task_with_options, create_tls_client_task_with_options, and create_rtu_client_task, each returning (Channel, ClientTask) so the caller can spawn the task on their own runtime and attach their own tracing instrumentation. ClientTask is a single opaque type shared across all three transports (an internal enum over TcpChannelTask/SerialChannelTask) exposing only `async fn run(self)`. The default tracing spans now live in the spawn_* functions rather than the create_*_channel helpers, so created tasks carry no span. Also fixes the TLS path span name (was "Modbus-Client-TCP").
7c7d42a to
b197780
Compare
Merged
jadamcrain
added a commit
that referenced
this pull request
May 28, 2026
* release 1.5.0-RC2 Bump workspace version 1.5.0-RC1 -> 1.5.0-RC2 across all crates, bindings, and the guide, and refresh Cargo.lock. Collapse the 1.5.0 and 1.5.0-RC1 changelog sections into a single 1.5.0-RC2 section and add the non-spawning client task entry (#180). * derive rodbus-client CLI version from CARGO_PKG_VERSION Avoids manually bumping the hardcoded version string on every release. * changelog: add TCP client performance optimizations (#177) Also note the ReadBuffer shift-condition fix from the same PR. These landed after 1.5.0-RC1 and were missing from the release notes. * changelog: clarify ReadBuffer fix impact (rare spurious disconnect)
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.
Summary
Adds non-spawning (
create_*) variants of the client channel constructors alongside the existingspawn_*functions, so callers can spawn the channel task on their own runtime and attach their own tracing instrumentation.New public functions:
create_tcp_client_task_with_options(host, retry, listener, client_options) -> (Channel, ClientTask)create_tls_client_task_with_options(host, retry, tls_config, listener, client_options) -> (Channel, ClientTask)(enable-tls)create_rtu_client_task(path, serial_settings, max_queued_requests, retry, decode, listener) -> (Channel, ClientTask)(serial)Design
ClientTaskis a single opaque public type shared across all three transports — internally an enum overTcpChannelTask/SerialChannelTask— exposing onlypub async fn run(self). No boxing.create_*_channelhelpers and into thespawn_*functions, so created tasks carry no span and the caller is free to wrapClientTask::runwith their own instrumentation, e.g.tokio::spawn(task.run().instrument(my_span)).create_*rather than inside anasync moveblock). This is a pure struct build with no I/O; connection state transitions still happen inrun().Bug fix
The TLS connection path was labeling its tracing span
"Modbus-Client-TCP"; it is now correctly"Modbus-Client-TLS".Verification
cargo fmt,cargo check, andcargo clippyall pass clean with--features serial,enable-tls. Reviewed by Codex with no correctness findings.