shell: allow selecting SERIAL_CONTROL device by Bartok9 · Pull Request #3042 · mavlink/MAVSDK

Bartok9 · 2026-08-24T01:22:38Z

Summary

Expose SERIAL_CONTROL device selection on shell Send/Receive (default remains shell).

Linked

Fixes Expand Shell Plugin Options #2107
Depends on shell: add SetDevice for SERIAL_CONTROL_DEV selection MAVSDK-Proto#427 (temporary submodule tip until that merges)

Notes

Per maintainer: temporary/wrong proto submodule until proto PR merges, then bump here.
Nested Receive message for protoc-gen-mavsdk (single non-Result stream field).
C++ plugin + mavsdk_server bindings regenerated against that tip.

Bartok9 · 2026-08-24T04:49:46Z

CI note (aerial watcher 2026-08-24)

style and proto check + docs check are red by design until the proto dependency lands:

CI runs tools/generate_from_protos.bash against the pinned mavsdk-proto submodule (mavlink/MAVSDK-Proto @ current main tip).
That regen overwrites the hand-touched shell.hpp / shell.cpp public surface (Device enum, set_device, InvalidArgument, etc.).
Docs check then fails because Doxygen markdown is not regenerated against the final API.

Unblock path

Merge mavlink/MAVSDK-Proto#427 (already CI-green / MERGEABLE).
Bump this repo's proto submodule to that merge SHA.
Re-run tools/generate_from_protos.bash + tools/fix_style.py + tools/generate_docs.sh --overwrite and push.

Impl in shell_impl.* stays; generated public API must come from the proto RPC SetDevice. Marking draft until that chain is ready.

julianoes · 2026-08-25T01:55:54Z

+    return Shell::Result::Success;
+}
+
+bool ShellImpl::device_to_mavlink(Shell::Device device, uint8_t& out_device)


Generally, we don't do out parameters. You can return a pair or an optional if you need result and value.

Thanks — switched device_to_mavlink to return std::optional<uint8_t> (no out-param). Pushed in 962ca8a.

julianoes · 2026-08-25T01:56:36Z

+        return Shell::Result::InvalidArgument;
+    }
+
+    std::lock_guard<std::mutex> lock(_device_mutex);


Given this is lock is only around ever around _device you can just use std::atomic.

Agreed — replaced the mutex with std::atomic<uint8_t> _device and relaxed loads/stores. Pushed in 962ca8a.

julianoes · 2026-08-25T01:57:40Z

    } _receive;
+
+    std::mutex _device_mutex{};
+    uint8_t _device{10}; // SERIAL_CONTROL_DEV_SHELL


You should have the mavlink enum define available.

Now using SERIAL_CONTROL_DEV::* (and SERIAL_CONTROL_SERIAL*) via the mavlink headers instead of magic numbers. Pushed in 962ca8a.

- device_to_mavlink returns optional instead of out-param - store selected device in std::atomic - use SERIAL_CONTROL_DEV::* enum values

Bartok9 · 2026-08-25T02:04:02Z

Addressed @julianoes review on the impl:

device_to_mavlink → std::optional<uint8_t> (no out-param)
_device is std::atomic<uint8_t> (mutex removed)
Uses SERIAL_CONTROL_DEV::* / SERIAL_CONTROL_SERIAL* enums

Still draft pending proto shape + submodule bump / regen after MAVSDK-Proto#427 lands in the new Send/Receive field form.

julianoes · 2026-08-26T01:48:30Z

Thanks for the fixes, please check CI.

Bartok9 · 2026-08-26T04:48:36Z

@julianoes Thanks — checked CI on the latest tip.

Green: full build matrix (Linux/macOS/Windows/Kotlin/iOS/fuzz) after the impl review fixes (optional device_to_mavlink, std::atomic device, mavlink SERIAL_CONTROL_DEV enums).

Still red (expected until proto lands):

ubuntu-24.04 (style and proto check) — tools/generate_from_protos.bash regenerates against the pinned proto submodule and overwrites the hand-extended public shell.hpp / shell.cpp (Device, set_device, InvalidArgument).
ubuntu-24.04 (docs check) — Doxygen markdown then drifts vs the hand API.

Unblock: merge mavlink/MAVSDK-Proto#427 (CI green / MERGEABLE there), bump this repo’s proto submodule to that SHA, re-run generate + style + docs, push. Keeping this draft until that chain is ready so we don’t thrash style/docs on every push.

Happy to adjust the public shape if you’d rather land proto first or change the SetDevice RPC.

julianoes

This isn't updated to the latest proto changes yet, right?

Bartok9 · 2026-08-26T07:55:57Z

@julianoes Correct — this PR is not updated to the latest proto yet.

Still blocked on mavlink/MAVSDK-Proto#427 (SetDevice / shell device selection). That PR is CI-green and MERGEABLE; once it lands I’ll:

Bump the proto submodule to the merge SHA
Re-run tools/generate_from_protos.bash + style + docs
Align shell_impl with the generated public API and push

Keeping this draft until then so style/docs don’t thrash on hand-edited shell.hpp/shell.cpp. Happy to reshuffle if you want proto merged first or a different RPC shape.

julianoes · 2026-08-29T01:32:52Z

Please update this PR to match the proto PR. Once it is passing (with wrong/temporary proto submodule), I'll merge the proto PR, and bump it here.

Point proto submodule at temporary MAVSDK-Proto tip (Device on Send/Receive, nested Receive for codegen). Regenerate public API + server bindings, drop set_device, pass device per send, surface device on receive. Keeps draft path Julian requested before proto merge.

Bartok9 · 2026-08-29T01:41:06Z

@julianoes Updated this PR to match the proto shape (Device on Send/Receive, nested Receive for protoc-gen-mavsdk).

Temporary proto submodule tip: Bartok9/MAVSDK-Proto@4e0144a (bartok9/shell-set-device) — as you suggested, wrong/temporary until you merge MAVSDK-Proto#427 and bump here.
Dropped set_device; send(command, device) + Receive{data,device} on the stream.
Impl + server bindings + mavshell example aligned.

CI will still see a “wrong” submodule SHA vs upstream main until you merge proto and repoint — that’s intentional per your note. Happy to adjust anything once style/docs run on your side.

Committed shell.pb.* was generated with host protoc 36 / 7.36.0 headers, which broke the full build matrix (protobuf version guard). Regen C++ pb/grpc with protobuf 29.1 to match third_party; refresh C/JNI/Kotlin/Python shell bindings and docs for Device on Send/Receive. Still uses temporary proto submodule tip until MAVSDK-Proto#427 merges.

Bartok9 · 2026-08-29T04:53:29Z

@julianoes Follow-up on CI after matching the proto shape:

Root cause of matrix red: shell.pb.h / .pb.cc had been generated with host protoc 36 / Protobuf C++ 7.36.0, while MAVSDK third_party is protobuf 29.1 (5.29.1). That tripped the version guard across Linux/macOS/Windows/Kotlin/etc.

Fix just pushed (147265e3d):

Regenerated shell pb/grpc with protoc 29.1
Refreshed C / JNI / Kotlin / Python shell bindings for Device on Send + nested Receive
Updated docs/en/cpp/api_reference/classmavsdk_1_1_shell.md

Still draft with temporary proto submodule tip (4e0144a / MAVSDK-Proto#427) as you asked. Style check should now match generate-from-protos on that tip.

Apply style/proto-check drift from ubuntu-24.04 job: - fix empty Device translate_ names (translate_device_*) - drop grpc ports_def.inc on shell.grpc.pb.* - match protoc-gen-mavsdk formatting across C/JNI/Kotlin/Python Still draft; temporary proto submodule tip until MAVSDK-Proto#427 merges.

Bartok9 · 2026-08-29T08:51:24Z

aerial-watch (2026-08-29 04:47 ET)

Style/proto check on 147265e3d still red after the protoc 5.29.1 regen. Root cause was generate_from_protos drift, not the temp submodule alone:

Empty translate__from_c / translate__to_c / mavsdk_shell__t (Device name not wired through C generator) → fixed to translate_device_* / mavsdk_shell_device_t
shell.grpc.pb.h included <grpcpp/ports_def.inc> (host grpc plugin shape) while tree plugins do not → removed to match CI plugin
C/JNI/Kotlin/Python formatting normalized to what CI’s protoc-gen-mavsdk emits

Pushed 006a6bd4a. Still draft with temporary proto tip 4e0144a (MAVSDK-Proto#427) as requested.

@brief

Docs job failed WARN_AS_ERROR on empty @brief for Shell::Receive and Receive::device. Proto tip e48f56d documents Receive; hand-sync hpp + md to match generate_from_protos output.

Bartok9 · 2026-08-29T12:49:55Z

aerial-watch (2026-08-29 ~08:47 ET)

CI on 006a6bd4a: style/proto green; matrix mostly green. Remaining:

docs check — doxygen WARN_AS_ERROR: empty @brief on Shell::Receive + Receive::device (codegen from proto message had no leading comment).
ios_simulator_x64 — build reached 100%; failed on CreateArtifact ENOTFOUND (infra flake), not compile.

Fix pushed:

MAVSDK-Proto tip e48f56d (bartok9/shell-set-device): document Receive + device field comments → MAVSDK-Proto#427
This PR: bump temp proto submodule + fill shell.hpp / shell API md briefs to match.

Still draft with temporary proto submodule per your note.

@brief

Style/proto check still drifted after Receive @brief proto comments: C/Kotlin/Python Receive descriptions + regenerated shell API markdown anchors from docs check. Still draft with temp proto tip e48f56d.

Bartok9 · 2026-08-29T16:50:15Z

aerial-watch (2026-08-29 ~12:47 ET)

CI on 01364a625 after Receive @brief push:

style/proto red — generate_from_protos filled Receive docs into C/Kotlin/Python that the tip still lacked
docs check red — doxygen markdown anchors/layout drift on classmavsdk_1_1_shell.md (Receive now a nested Data Structure page link)
Matrix otherwise green (ios_sim_x64 recovered)

Fix pushed 0f30dcd0c: apply those generate outputs (C shell.h, Kotlin Shell.kt, Python shell.py, shell API md).

Still draft with temporary proto tip e48f56d (MAVSDK-Proto#427) per your note.

Bartok9 · 2026-08-30T04:48:16Z

aerial-watch (2026-08-30 ~00:47 ET)

@julianoes CI on tip 0f30dcd0c is fully green (style/proto, docs, full Linux/macOS/Windows/Kotlin/iOS/fuzz matrix). Still draft with temporary proto tip e48f56d matching MAVSDK-Proto#427 as you requested.

Ready for your side: merge proto #427 → bump submodule here (or tell me the merge SHA and I can push the bump) → undraft.

shell: allow selecting SERIAL_CONTROL device (mavlink#2107)

2045ab8

Bartok9 marked this pull request as draft August 24, 2026 04:49

julianoes requested changes Aug 25, 2026

View reviewed changes

shell: address review — optional map, atomic device, mavlink enums

962ca8a

- device_to_mavlink returns optional instead of out-param - store selected device in std::atomic - use SERIAL_CONTROL_DEV::* enum values

julianoes reviewed Aug 26, 2026

View reviewed changes

shell: fill Receive doxygen briefs (docs check)

01364a6

Docs job failed WARN_AS_ERROR on empty @brief for Shell::Receive and Receive::device. Proto tip e48f56d documents Receive; hand-sync hpp + md to match generate_from_protos output.

shell: sync Receive docs/bindings with generate_from_protos

0f30dcd

Style/proto check still drifted after Receive @brief proto comments: C/Kotlin/Python Receive descriptions + regenerated shell API markdown anchors from docs check. Still draft with temp proto tip e48f56d.

Uh oh!

Uh oh!

Conversation

Bartok9 commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linked

Notes

Uh oh!

Bartok9 commented Aug 24, 2026

CI note (aerial watcher 2026-08-24)

Uh oh!

julianoes Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Bartok9 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

julianoes Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Bartok9 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

julianoes Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Bartok9 Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Bartok9 commented Aug 25, 2026

Uh oh!

julianoes commented Aug 26, 2026

Uh oh!

Bartok9 commented Aug 26, 2026

Uh oh!

julianoes left a comment

Choose a reason for hiding this comment

Uh oh!

Bartok9 commented Aug 26, 2026

Uh oh!

julianoes commented Aug 29, 2026

Uh oh!

Bartok9 commented Aug 29, 2026

Uh oh!

Bartok9 commented Aug 29, 2026

Uh oh!

Bartok9 commented Aug 29, 2026

aerial-watch (2026-08-29 04:47 ET)

Uh oh!

Bartok9 commented Aug 29, 2026

aerial-watch (2026-08-29 ~08:47 ET)

Uh oh!

Bartok9 commented Aug 29, 2026

aerial-watch (2026-08-29 ~12:47 ET)

Uh oh!

Bartok9 commented Aug 30, 2026

aerial-watch (2026-08-30 ~00:47 ET)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Bartok9 commented Aug 24, 2026 •

edited

Loading