Skip to content

[RSDK-13658] Plumb force_relay/force_p2p/turn_uri through C FFI#176

Open
danielbotros wants to merge 3 commits intomainfrom
APP-coturn-ffi-plumbing
Open

[RSDK-13658] Plumb force_relay/force_p2p/turn_uri through C FFI#176
danielbotros wants to merge 3 commits intomainfrom
APP-coturn-ffi-plumbing

Conversation

@danielbotros
Copy link
Copy Markdown
Member

@danielbotros danielbotros commented Apr 28, 2026

Summary

  • Exposes the DialBuilder options landed in [RSDK-13658] Add force relay and force p2p dial flags #170 (force_relay, force_p2p, turn_uri) through viam_dial so non-Rust SDKs (Python) can drive ICE relay/P2P testing and TURN-server filtering.
  • New params are appended after the existing trailing rt_ptr so rt_ptr keeps its original positional slot — required for the C++ source-compat scheme below.
  • The deprecated dial continues to provide the original 7-arg ABI by trampolining into viam_dial with the new args defaulted (rt_ptr, false, false, NULL).

C++ SDK source compatibility

The C++ SDK calls viam_dial(uri, entity, type, payload, allow_insec, timeout, rt_ptr) (7 args). To avoid breaking that call site, cbindgen.toml now emits a C++-only extern "C" re-declaration in the generated header trailer that adds defaults to the three new trailing params:

#ifdef __cplusplus
extern "C" char *viam_dial(...,
                           struct viam_dial_ffi *rt_ptr,
                           bool c_force_relay = false,
                           bool c_force_p2p = false,
                           const char *c_turn_uri = nullptr);
#endif

C++ allows a later declaration of an extern "C" function to add default arguments; the compiler merges them. So:

  • C callers must pass all 10 args (defaults are invisible to C).
  • C++ callers can keep passing 7 — defaults fill in the rest.
  • Signature drift between Rust and the trailer surfaces as a C++ compile error at the call site.

Downstream PRs (do not merge until rust-utils releases)

Test plan

  • cargo build --release clean
  • cargo test --lib --release passes
  • Built dylib still exports both _viam_dial and (deprecated) _dial
  • Loaded the new dylib from a paired Python SDK build and confirmed viam_init_rust_runtime / viam_free_rust_runtime round-trip
  • Compile- and link-tested the literal C++ SDK call shape from dial.cpp:209-210 against the cbindgen-generated header + the new static lib — resolves via the C++ default-arg trailer, no warnings.
  • Also verified C 10-arg and C++ 10-arg explicit-override calls compile against the new header.
  • (CI) Existing integration tests pass

🤖 Generated with Claude Code

Exposes the DialBuilder methods added in #170 (force_relay, force_p2p,
turn_uri) through viam_dial so non-Rust SDKs can drive ICE relay/P2P
testing and TURN-server filtering.

The deprecated `dial` keeps its 7-arg ABI by trampolining into
viam_dial with `false, false, NULL` defaults.

BREAKING (C ABI): viam_dial now takes 10 args instead of 7. Any
non-Rust consumer linking against viam_dial must rebuild against the
new header. Consumers still using the deprecated `dial` are unaffected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@danielbotros danielbotros force-pushed the APP-coturn-ffi-plumbing branch from 55422d7 to 9c6b582 Compare April 28, 2026 20:05
@danielbotros danielbotros marked this pull request as ready for review May 4, 2026 17:29
Copy link
Copy Markdown
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

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

The logic changes all look reasonable but I'm pretty sure this will break existing C++ SDK instances. When C++ builds it looks for an existing rust-utils instance and if it doesn't find one it just downloads the latest release.

One possible fix would be to update the cbindgen process to generate a function overload signature for viam_dial that uses the old signature and puts default values in. Happy to chat more in person if you have questions or thoughts!

@danielbotros
Copy link
Copy Markdown
Member Author

danielbotros commented May 5, 2026

The logic changes all look reasonable but I'm pretty sure this will break existing C++ SDK instances. When C++ builds it looks for an existing rust-utils instance and if it doesn't find one it just downloads the latest release.

One possible fix would be to update the cbindgen process to generate a function overload signature for viam_dial that uses the old signature and puts default values in. Happy to chat more in person if you have questions or thoughts!

I went ahead and changed the cbindgen config to emit a C++ only header that fills in default values for the last 3 new dial options args so the C++ SDK can compile against the 10-arg header.

This requires a small follow up PR to the C++ SDK to update the viam_dial stub to 10-args for machines on a non-x86_64 Windows platform to compile against since they don't pull a rust-utils binary and can't dial but can still interface modules. Pretty sure this a breaking change for them, if anyone is even running that kind of setup...

@danielbotros danielbotros requested a review from stuqdog May 5, 2026 18:36
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.

2 participants