fix(mavlink2-bridge): honor ?proto= query on the serial transport

[phil-opp]

Issue

The MAVLink transport builder (libraries/extensions/mavlink2-bridge/src/transport.rs) accepts an optional ?proto= query parameter to select the MAVLink wire version. connect_tcp and connect_udp both read it via parse_proto_query(url).unwrap_or(MavlinkVersion::V2), but connect_serial routed through the open_mavlink wrapper, which hardcoded MavlinkVersion::V2:

fn connect_serial(url: &Url) -> BridgeResult<...> {
    ...
    open_mavlink(&format!("serial:{device}:{baud}"))   // always V2
}

fn open_mavlink(addr: &str) -> BridgeResult<...> {
    open_mavlink_versioned(addr, MavlinkVersion::V2)   // proto ignored
}

So a user connecting with serial:///dev/ttyUSB0?baud=57600&proto=v1 silently got a MAVLink V2 connection — no error, no warning — while the exact same override works on tcp:// and udp://. The proto parameter was also completely undocumented (the module docs cover baud but never mention proto).

Fix

• Route connect_serial through parse_proto_query(url).unwrap_or(MavlinkVersion::V2) and open_mavlink_versioned, exactly like the tcp/udp paths.
• Delete the now-dead open_mavlink wrapper (its only caller was connect_serial).
• Document the proto query parameter (accepted spellings + default) in the module docs.

No behavior change for tcp/udp or for serial connections that omit proto; the default remains MAVLink V2.

Validation

• Added unit tests: parses_proto_query_variants, parses_proto_query_missing_or_garbage, and serial_url_honors_proto_query (regression guard for the serial branch).
• cargo +1.97.1 fmt -p dora-mavlink2-bridge -- --check — clean.
• cargo +1.97.1 clippy -p dora-mavlink2-bridge -- -D warnings — clean.
• cargo +1.97.1 test -p dora-mavlink2-bridge — all pass (incl. doctests).

---

⚠️ This is a machine-generated pull request authored by Claude (Claude Code). A human should review before merging.

🤖 Generated with Claude Code

---

Generated by Claude Code

[trunk-io]

Merging to main in this repository is managed by Trunk.

• To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

[phil-opp]

Automated review run — summary

This is the summary for a batch of machine-generated PRs opened by Claude (Claude Code) from an in-depth review of the repo against a freshly-fetched origin/main. Each PR is standalone, branches off origin/main, and is clearly attributed + marked machine-generated. Per-branch verification: fmt + clippy -D warnings + affected-crate tests (toolchain 1.97.1). The combined diff additionally passed the full workspace test suite (cargo test --all minus the Python/examples excludes) and the /review + /simplify gates.

PRs by category

Correctness

• fix(mavlink2-bridge): honor ?proto= query on the serial transport #3098 — fix(mavlink2-bridge): honor the ?proto= query on the serial transport (tcp/udp already did; serial silently forced V2). (this PR)
• fix(cli): reject a malformed coordinator address/port in dora up #3100 — fix(cli): dora up now rejects a malformed DORA_COORDINATOR_ADDR/PORT instead of silently falling back to the default (diverged from every other command's clap env=).
• fix(core): make ${_param.…} substitution single-pass and order-independent #3101 — fix(core): ${_param.…} substitution is now single-pass and order-independent (was BTreeMap-order-dependent and transitively expanded param values).
• fix(node): record zenoh-delivered inputs in the write_events_to log #3103 — fix(node): record zenoh-delivered inputs in the write_events_to log (they surface as Event::Input but were dropped from recordings). Includes a timestamp-underflow guard surfaced by the /review gate (a remote HLC timestamp predating node start would panic/wrap get_diff_duration).

Documentation

• docs(cli): attach --env help to the right field in dora start #3099 — docs(cli): attach the --env help text to the right field in dora start (it was mis-attributed to --exit-when-nodes-finish, leaving --env undocumented).

Optimization

• perf(daemon): drop the per-log-line String reallocation in the log task #3102 — perf(daemon): drop the per-log-line String reallocation in the log task (hot path: one alloc+copy per log line per node).

Rejected / not-pursued findings (one-line reasons)

• InputMapping::Logs Display drops node_filter when min_level is None — unreachable: FromStr never constructs that state and no other construction site exists, so no demonstrable failure.
• dora logs --follow degrades to a one-shot dump when the coordinator is unreachable — real but niche; deferred (per-run PR cap).
• restart.rs request/reply duplication — valid simplification (send_control_request+expect_reply!); deferred (per-run PR cap).
• core::resolve_path missing doctest — genuine doc gap, but a deterministic doctest depends on uv/$PATH availability; deferred.
• daemon::truncate_log_line off-by-one at an exact 1 MB line — cosmetic and only at the exact boundary; too weak.
• message::auth::generate_token per-byte format! — cold startup path; too weak to justify a PR.
• Metadata::open_telemetry_context always allocates — fix is an API-signature change needing a call-site audit; minor.
• descriptor.rs pattern_metadata_keys() doc reference — the function does exist (in dora-core, cross-crate); weak.
• event_stream Stream::poll_next bypasses recording — plausibly intentional (recording may be scheduler-path-only); medium-confidence, not pursued.
• Dead coordinator_addr branch in open_zenoh_session_with_listen — effectively dead (all in-repo callers pass None); removing it touches a pub signature; deferred.
• mavlink MavlinkArrow::schema() rebuilt (with format!) per message — real per-frame optimization, but the clean fix needs a trait-signature change (SchemaRef); larger scope, deferred.
• Altitude (/simplify): flatten CoordinatorOptions into dora up — the deeper fix for fix(cli): reject a malformed coordinator address/port in dora up #3100, but it adds new --coordinator-addr/--coordinator-port flags (a CLI-surface change beyond the bug fix); skipped as out-of-scope, noted here as a follow-up. The in-scope /simplify cleanup (fold the env read into the helper) was applied to fix(cli): reject a malformed coordinator address/port in dora up #3100.

---

Generated by Claude Code

[phil-opp]

🤖 Automated review by Claude Code — fully automated review, not vetted by a human.

No blocking issues found. The fix is correct: connect_serial now reads parse_proto_query(url).unwrap_or(MavlinkVersion::V2) and routes through open_mavlink_versioned, matching the tcp/udp paths exactly; the dead open_mavlink wrapper is removed along with its only caller. Behavior is unchanged when proto is omitted (defaults to V2).

Two smaller notes, neither blocking:

• The connect() rustdoc still asserts "The returned connection is pre-configured for MAVLink V2," which is now only the default rather than a guarantee — worth softening now that all three schemes can select V1.
• serial_url_honors_proto_query exercises parse_baud/parse_proto_query on the serial URL rather than connect_serial itself (which needs real hardware), so it guards the parsing but not the wiring into open_mavlink_versioned. Acceptable given the constraint, and it does prevent a regression to the hardcoded-V2 parse.

---

Generated by Claude Code