From c1ef5ac52398171768a154459160863efb0f5237 Mon Sep 17 00:00:00 2001 From: Pierre Rouanet Date: Tue, 15 Sep 2026 17:50:50 +0200 Subject: [PATCH] duck-ble: the wire contract, extracted now that a second client wants it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three modules moved out of `btd` — `gatt`, `framing` and `adv`. Each was already marked as wire contract where it lived, each says why in its own header, and what they have in common is that a second implementation would agree only with itself: a client that chunked differently, or decoded the advertisement's address by hand, works until it does not and the failure looks like a robot problem. They stayed in `btd` because there was one client and it did not matter. Two things changed that. **`duckctl` was pulling a daemon to reach them** — and on Linux, `bluer` and a vendored libdbus with it, for a tool whose whole point is talking to a robot rather than being one. It now depends on `duck-ble` and not on `btd` at all. **And the phone app carried `clap` and `tracing-subscriber` into an iPhone binary** for the sake of one module. `mobile-app.md` §3 records that as a known cost and §8 as an open item to settle "before there is a second consumer". There is now, and this is the settling. The new crate has one dependency, `uuid`, because the GATT UUIDs are values of that type. No radio, no runtime, no logging, no argument parser: it is bytes and arithmetic, so it builds for a phone, a laptop and the board alike. Adding anything to it adds it to all three, which is the bar its manifest states. No behaviour changes. `btd`'s 63 tests are now 43 in `btd` and 20 in `duck-ble`, which is the same 63. Assisted-by: Claude:claude-opus-5 --- Cargo.lock | 11 +++++++++-- Cargo.toml | 4 ++-- btd/Cargo.toml | 6 ++++-- btd/src/bluez.rs | 18 +++++++++--------- btd/src/chorale.rs | 18 +++++++++--------- btd/src/lib.rs | 3 --- btd/src/link.rs | 2 +- btd/src/session.rs | 2 +- docs/design/app-path-design.md | 2 +- duck-ble/Cargo.toml | 22 ++++++++++++++++++++++ {btd => duck-ble}/src/adv.rs | 0 {btd => duck-ble}/src/framing.rs | 0 {btd => duck-ble}/src/gatt.rs | 0 duck-ble/src/lib.rs | 28 ++++++++++++++++++++++++++++ duckctl/Cargo.toml | 7 ++++++- duckctl/examples/advwatch.rs | 2 +- duckctl/src/main.rs | 12 ++++++------ 17 files changed, 99 insertions(+), 38 deletions(-) create mode 100644 duck-ble/Cargo.toml rename {btd => duck-ble}/src/adv.rs (100%) rename {btd => duck-ble}/src/framing.rs (100%) rename {btd => duck-ble}/src/gatt.rs (100%) create mode 100644 duck-ble/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 9c70a4ec..c97e8db1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -502,6 +502,7 @@ dependencies = [ "bluer", "clap", "dbus", + "duck-ble", "duck-ipc-proto", "futures", "serde_json", @@ -509,7 +510,6 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", - "uuid", ] [[package]] @@ -1107,6 +1107,13 @@ dependencies = [ "litrs", ] +[[package]] +name = "duck-ble" +version = "0.13.0" +dependencies = [ + "uuid", +] + [[package]] name = "duck-control" version = "0.13.0" @@ -1165,9 +1172,9 @@ dependencies = [ name = "duckctl" version = "0.13.0" dependencies = [ - "btd", "btleplug", "clap", + "duck-ble", "duck-ipc-proto", "futures", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index db6fef52..4c19bc64 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ # docs/design/architecture.md §1. `robotd`, `btd` and `mediad` are all siblings now. [workspace] resolver = "3" -members = ["btd", "configd", "duck-control", "duck-detect", "duck-ether", "duck-ipc-proto", "duckctl", "kinematics", "mediad", "odometry", "pad-imu", "padd", "pet-detect", "sounds", "tof", "updater", "robotctl", "robotd", "robotd-params", "test-support", "uyvy", "xtask"] +members = ["btd", "configd", "duck-ble", "duck-control", "duck-detect", "duck-ether", "duck-ipc-proto", "duckctl", "kinematics", "mediad", "odometry", "pad-imu", "padd", "pet-detect", "sounds", "tof", "updater", "robotctl", "robotd", "robotd-params", "test-support", "uyvy", "xtask"] # Everything except `duckctl`, and that exception is the whole reason this key exists. # @@ -18,7 +18,7 @@ members = ["btd", "configd", "duck-control", "duck-detect", "duck-ether", "duck- # writing down. One list here, and a new daemon is picked up by both without anybody remembering. # # `--workspace` is unaffected, so CI still lints and tests `duckctl` exactly as before. -default-members = ["btd", "configd", "duck-control", "duck-detect", "duck-ipc-proto", "kinematics", "mediad", "odometry", "pad-imu", "padd", "pet-detect", "sounds", "tof", "updater", "robotctl", "robotd", "robotd-params", "test-support", "uyvy", "xtask"] +default-members = ["btd", "configd", "duck-ble", "duck-ble", "duck-control", "duck-detect", "duck-ipc-proto", "kinematics", "mediad", "odometry", "pad-imu", "padd", "pet-detect", "sounds", "tof", "updater", "robotctl", "robotd", "robotd-params", "test-support", "uyvy", "xtask"] # ONNX Runtime, which robotd dlopens to run a policy. One source of truth: `xtask package` # bakes these into the release's preinstall hook, and a test asserts scripts/setup-board.sh diff --git a/btd/Cargo.toml b/btd/Cargo.toml index 987b881d..40a36754 100644 --- a/btd/Cargo.toml +++ b/btd/Cargo.toml @@ -14,13 +14,15 @@ description = "BLE transport adapter — a GATT front door onto the robot's JSON # stays unprivileged while `configd` is the one running as root. [dependencies] duck-ipc-proto = { path = "../duck-ipc-proto" } +# The wire contract, shared with every client rather than exported to them. +# It used to live here, which made a phone app depend on a daemon to agree +# about chunking — see that crate's manifest. +duck-ble = { path = "../duck-ble" } serde_json.workspace = true tokio = { workspace = true, features = ["rt-multi-thread", "macros", "net", "io-util", "time", "sync", "signal"] } clap.workspace = true tracing.workspace = true tracing-subscriber = { workspace = true, features = ["env-filter"] } -# The GATT UUIDs are a wire contract every client needs, not a Linux detail — see src/gatt.rs. -uuid = "1" # BlueZ, reached over bluetoothd's D-Bus API. Only on Linux, and this only ever *runs* on the # Radxa — but the crate must still *build* on a macOS laptop, because `cargo test` there is the diff --git a/btd/src/bluez.rs b/btd/src/bluez.rs index e9dde6c9..6580b524 100644 --- a/btd/src/bluez.rs +++ b/btd/src/bluez.rs @@ -50,11 +50,11 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use tokio::sync::mpsc; -use crate::framing::{FLOOR_MTU, notification_payload}; -use crate::gatt::{RPC_UUID, SERVICE_UUID}; use crate::link::Link; use crate::session; use crate::upstream::{NameChoice, Sockets}; +use duck_ble::framing::{FLOOR_MTU, notification_payload}; +use duck_ble::gatt::{RPC_UUID, SERVICE_UUID}; /// How many notifications to queue before pausing to let the radio drain. /// @@ -271,7 +271,7 @@ async fn serve_on_an_adapter( // `bd_addr` rather than `address`, because the advertisement now carries an IPv4 one too and a // journal with both spelled `address` reads as one field contradicting itself. // - // `max_adv_len` is logged because it is the budget `crate::adv` is written against: the payload + // `max_adv_len` is logged because it is the budget `duck_ble::adv` is written against: the payload // fits 31 bytes, and a controller that reports less is the one place that assumption fails. It // is the first thing to read if a robot ever advertises its name but no address. tracing::warn!( @@ -657,11 +657,11 @@ async fn notify_chunk(notifier: &mut CharacteristicNotifier, chunk: Vec) -> /// /// One struct rather than two arguments threaded through the reconcile loop, so that "has anything /// moved" is one comparison. Adding a third field would otherwise mean finding every place that -/// compares the pair — and `crate::adv` explains why there is no room for a third field anyway. +/// compares the pair — and `duck_ble::adv` explains why there is no room for a third field anyway. #[derive(Debug, Clone, PartialEq, Eq)] struct Advertised { name: String, - /// `None` is a robot with no IPv4 address, which goes out as `0.0.0.0` — see [`crate::adv`] for + /// `None` is a robot with no IPv4 address, which goes out as `0.0.0.0` — see [`duck_ble::adv`] for /// why the field is broadcast either way. address: Option, } @@ -703,7 +703,7 @@ impl std::fmt::Display for Advertised { /// all, and returning an error here would take the advertisement down with it. /// /// **The address field is dropped rather than allowed to fail the registration.** The arithmetic in -/// [`crate::adv`] says the payload fits, but the byte that overflows a legacy advertisement is the +/// [`duck_ble::adv`] says the payload fits, but the byte that overflows a legacy advertisement is the /// controller's to count, not ours — and BlueZ refuses the whole registration when it does not fit. /// On a robot whose only front door may be BLE, that trade is not close: an advertisement with no /// address is a robot someone can still reach, and a refused one is a robot that has gone dark. Same @@ -722,7 +722,7 @@ async fn advertise( let advertisement = |address: Option>| Advertisement { service_uuids: [SERVICE_UUID].into_iter().collect(), manufacturer_data: address - .map(|data| [(crate::adv::COMPANY_ID, data)].into_iter().collect()) + .map(|data| [(duck_ble::adv::COMPANY_ID, data)].into_iter().collect()) .unwrap_or_default(), discoverable: Some(true), local_name: Some(name.to_owned()), @@ -731,7 +731,7 @@ async fn advertise( ..Default::default() }; - let with_address = advertisement(Some(crate::adv::address_data(advertised.address))); + let with_address = advertisement(Some(duck_ble::adv::address_data(advertised.address))); match adapter.advertise(with_address).await { Ok(handle) => Ok(handle), Err(e) => { @@ -837,7 +837,7 @@ async fn ask_name(sockets: &Sockets, fallback: &str) -> String { /// watching the address blink. So an outage keeps the last known address, exactly as [`ask_name`] /// keeps the last known name. /// -/// Only IPv4, because only IPv4 fits — see [`crate::adv`]. +/// Only IPv4, because only IPv4 fits — see [`duck_ble::adv`]. /// /// `debug` rather than `warn` for the same reason as [`ask_name`]: this runs every few seconds. async fn ask_address(sockets: &Sockets, last: Option) -> Option { diff --git a/btd/src/chorale.rs b/btd/src/chorale.rs index 5e936c6b..6fc48916 100644 --- a/btd/src/chorale.rs +++ b/btd/src/chorale.rs @@ -12,7 +12,7 @@ //! ## Out: a second advertising instance //! //! The board reports five advertising instances with one in use, so the beacon gets its own and -//! **the existing advertisement is not touched.** That is not tidiness. `crate::adv` documents a +//! **the existing advertisement is not touched.** That is not tidiness. `duck_ble::adv` documents a //! 31-byte budget, and the controller here reports a 251-byte one — so BlueZ would happily accept //! a bigger payload on the existing instance and, because it picks legacy against extended PDUs by //! size, would switch it to extended and make the robot invisible to a legacy-only scanner. Phone @@ -67,10 +67,10 @@ use std::collections::HashMap; use duck_ipc_proto::ChoraleBeacon; -/// The company id chorale beacons ride under — the same one [`crate::adv`] uses, for the same +/// The company id chorale beacons ride under — the same one [`duck_ble::adv`] uses, for the same /// reason: `0xFFFF` is the id the SIG reserves for testing and is the correct choice for a project /// that has not been assigned one. -pub const COMPANY_ID: u16 = crate::adv::COMPANY_ID; +pub const COMPANY_ID: u16 = duck_ble::adv::COMPANY_ID; /// The advertising interval for the beacon. /// @@ -91,7 +91,7 @@ pub fn scan_pattern() -> Vec { /// A beacon as the advertisement's manufacturer-data payload. /// -/// The payload rather than the map, mirroring [`crate::adv::address_data`] — and because the two +/// The payload rather than the map, mirroring [`duck_ble::adv::address_data`] — and because the two /// halves want different containers: `bluer` advertises from a `BTreeMap` and reports a scanned /// device's data as a `HashMap`. pub fn beacon_data(beacon: &ChoraleBeacon) -> Vec { @@ -412,7 +412,7 @@ mod tests { } /// The round trip the broadcasting half and the scanning half both depend on — the same - /// property `crate::adv` pins for the address field, and for the same reason. + /// property `duck_ble::adv` pins for the address field, and for the same reason. #[test] fn a_beacon_survives_the_advertisement() { assert_eq!(beacon_in(&advertised(&beacon())), Some(beacon())); @@ -425,15 +425,15 @@ mod tests { fn the_address_instance_is_not_heard_as_a_beat() { let address = HashMap::from([( COMPANY_ID, - crate::adv::address_data(Some(std::net::Ipv4Addr::new(192, 168, 1, 42))), + duck_ble::adv::address_data(Some(std::net::Ipv4Addr::new(192, 168, 1, 42))), )]); assert_eq!(beacon_in(&address), None); // And the reverse: a beacon is not read as an address, so a scanning `duck-btctl` does not // report a robot at some nonsense IP. let as_advertised = advertised(&beacon()); - assert_eq!(crate::adv::address_in(&as_advertised), None); + assert_eq!(duck_ble::adv::address_in(&as_advertised), None); assert!( - !crate::adv::has_address_field(&as_advertised), + !duck_ble::adv::has_address_field(&as_advertised), "a beacon is not four bytes, so it is not an address field" ); } @@ -454,7 +454,7 @@ mod tests { assert_eq!(pattern, vec![0xFF, 0xFF, ChoraleBeacon::TAG]); // An address advertisement must *not* match, or the filter buys nothing. let mut address_field = COMPANY_ID.to_le_bytes().to_vec(); - address_field.extend(crate::adv::address_data(None)); + address_field.extend(duck_ble::adv::address_data(None)); assert!(!address_field.starts_with(&pattern)); } diff --git a/btd/src/lib.rs b/btd/src/lib.rs index 135ac159..f9c6bf2a 100644 --- a/btd/src/lib.rs +++ b/btd/src/lib.rs @@ -30,12 +30,9 @@ //! the advertisement in step. [`adv`] is the layout of the address field, shared with the client //! that decodes it. -pub mod adv; #[cfg(target_os = "linux")] pub mod bluez; pub mod chorale; -pub mod framing; -pub mod gatt; pub mod link; pub mod pairing; pub mod route; diff --git a/btd/src/link.rs b/btd/src/link.rs index c4228931..e88365c1 100644 --- a/btd/src/link.rs +++ b/btd/src/link.rs @@ -103,7 +103,7 @@ impl Link { /// await, because a yield point between receiving a chunk and enqueueing it lets two chunks swap /// places, and a reordered chunk corrupts a request rather than failing it. const _: () = assert!( - QUEUE * 20 >= crate::framing::MAX_LINE, + QUEUE * 20 >= duck_ble::framing::MAX_LINE, "QUEUE * 20 must be at least framing::MAX_LINE, or a full-length request can fill the \ inbound queue and be refused" ); diff --git a/btd/src/session.rs b/btd/src/session.rs index b1bf1e40..da03cefe 100644 --- a/btd/src/session.rs +++ b/btd/src/session.rs @@ -13,11 +13,11 @@ use duck_ipc_proto as proto; use tokio::sync::mpsc; -use crate::framing::{self, Reassembler}; use crate::link::{Link, QUEUE}; use crate::pairing; use crate::route::{self, Route}; use crate::upstream::{Pool, Sockets}; +use duck_ble::framing::{self, Reassembler}; /// How many wrong PINs a session may offer before it is closed. /// diff --git a/docs/design/app-path-design.md b/docs/design/app-path-design.md index 6fa304f0..1d04df91 100644 --- a/docs/design/app-path-design.md +++ b/docs/design/app-path-design.md @@ -606,7 +606,7 @@ links libdbus built from vendored source by `zig cc`. `duckctl` (`cargo run -p duckctl`) is the phone's stand-in and the only way to exercise the radio. An **example, not a binary**, so `btleplug` never reaches the robot; `btleplug` rather -than `bluer` because it must run on a developer's Mac. It reuses `btd::framing`, so the chunking is +than `bluer` because it must run on a developer's Mac. It reuses `duck_ble::framing`, so the chunking is genuinely the client half of the robot's own code rather than a reimplementation free to agree with itself. diff --git a/duck-ble/Cargo.toml b/duck-ble/Cargo.toml new file mode 100644 index 00000000..6adb1eab --- /dev/null +++ b/duck-ble/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "duck-ble" +version.workspace = true +edition.workspace = true +license.workspace = true +description = "The BLE wire contract — GATT UUIDs, NDJSON chunking, and the advertisement's payload" + +# **Deliberately tiny, and that is the feature.** Every client of the robot's +# BLE surface needs these three modules and none of them needs a Bluetooth +# stack: the phone app builds this for `aarch64-apple-ios`, `duckctl` builds it +# for three desktop platforms, and `btd` builds it for the board. Adding a +# dependency here is adding it to all three, so the bar is high. +# +# They lived in `btd` until a second consumer appeared. That cost the app +# `clap`, `tracing-subscriber` and a tokio feature set it never used, because +# `btd` is a daemon and needs them — and it cost `duckctl` a `bluer` and a +# vendored libdbus on Linux, for a tool that talks to a robot rather than being +# one. +[dependencies] +# The GATT UUIDs are values of this type, so a client cannot receive them +# without it. Nothing else here needs a crate at all. +uuid = "1" diff --git a/btd/src/adv.rs b/duck-ble/src/adv.rs similarity index 100% rename from btd/src/adv.rs rename to duck-ble/src/adv.rs diff --git a/btd/src/framing.rs b/duck-ble/src/framing.rs similarity index 100% rename from btd/src/framing.rs rename to duck-ble/src/framing.rs diff --git a/btd/src/gatt.rs b/duck-ble/src/gatt.rs similarity index 100% rename from btd/src/gatt.rs rename to duck-ble/src/gatt.rs diff --git a/duck-ble/src/lib.rs b/duck-ble/src/lib.rs new file mode 100644 index 00000000..70b620a1 --- /dev/null +++ b/duck-ble/src/lib.rs @@ -0,0 +1,28 @@ +//! The BLE wire contract, and nothing that serves it. +//! +//! Three things a client and the robot must agree on exactly, extracted from +//! `btd` so that agreeing does not require depending on the daemon: +//! +//! | | | +//! |---|---| +//! | [`gatt`] | the service and characteristic UUIDs | +//! | [`framing`] | how a line is cut into notifications and put back together | +//! | [`adv`] | what the advertisement carries besides the name | +//! +//! Each was already marked as wire contract where it lived, and each says why +//! in its own header. What they have in common is that **a second +//! implementation would agree only with itself** — a client that chunked +//! differently, or decoded the advertisement's address by hand, would work +//! until it did not, and the failure would look like a robot problem. +//! +//! **Nothing here touches a radio.** No `bluer`, no `btleplug`, no async +//! runtime, no logging, no argument parser: this crate is bytes and +//! arithmetic, so it builds for a phone, a laptop and the board alike. That is +//! the whole reason it exists apart from `btd`, which needs all of those and +//! only runs on Linux — `mobile-app.md` §3 records the app carrying `clap` and +//! `tracing-subscriber` into an iPhone binary for the sake of one module, and +//! this is that being fixed rather than noted. + +pub mod adv; +pub mod framing; +pub mod gatt; diff --git a/duckctl/Cargo.toml b/duckctl/Cargo.toml index 8c41cc56..e2c59566 100644 --- a/duckctl/Cargo.toml +++ b/duckctl/Cargo.toml @@ -26,7 +26,12 @@ description = "The robot from a laptop — over BLE today, over whatever reaches # the *client* side of the module the robot chunks with, so an asymmetry between them would show # up as this tool not working — which makes it a test of the protocol rather than a second # implementation free to agree with itself. -btd = { path = "../btd" } +# +# `duck-ble` rather than `btd`, since a second consumer arrived and the split +# became worth making: this tool wanted three platform-independent modules and +# was pulling a Linux daemon — and on Linux, `bluer` and a vendored libdbus with +# it — to reach them. +duck-ble = { path = "../duck-ble" } # `API_VERSION`, and the `semver` re-export it is compared with. The version handshake is a # contract between this client and every daemon, so it is read from the crate that defines it diff --git a/duckctl/examples/advwatch.rs b/duckctl/examples/advwatch.rs index 98051ee7..2f2a23dc 100644 --- a/duckctl/examples/advwatch.rs +++ b/duckctl/examples/advwatch.rs @@ -19,9 +19,9 @@ use std::time::{Duration, Instant}; -use btd::gatt::SERVICE_UUID; use btleplug::api::{Central, CentralEvent, Manager as _, Peripheral as _, ScanFilter}; use btleplug::platform::Manager; +use duck_ble::gatt::SERVICE_UUID; use futures::StreamExt; /// How long to watch. diff --git a/duckctl/src/main.rs b/duckctl/src/main.rs index c5cfd98e..7622fd8c 100644 --- a/duckctl/src/main.rs +++ b/duckctl/src/main.rs @@ -16,7 +16,7 @@ //! macOS, BlueZ on Linux, WinRT on Windows. `bluer` would restrict the client to Linux, which //! defeats the point. //! -//! It reuses `btd::framing` deliberately. The chunking here is the *client* half of the same +//! It reuses `duck_ble::framing` deliberately. The chunking here is the *client* half of the same //! module the robot uses, so if the framing were asymmetric this would not work — which makes //! it a real test of the protocol rather than a reimplementation that could agree with itself. //! @@ -41,15 +41,15 @@ use std::net::Ipv4Addr; use std::time::{Duration, Instant}; -use btd::adv; -use btd::framing::{self, Reassembler}; -use btd::gatt::{RPC_UUID, SERVICE_UUID}; use btleplug::api::{ Central, CharPropFlags, Characteristic, Manager as _, Peripheral as _, PeripheralProperties, ScanFilter, WriteType, }; use btleplug::platform::{Manager, Peripheral}; use clap::{Parser, Subcommand}; +use duck_ble::adv; +use duck_ble::framing::{self, Reassembler}; +use duck_ble::gatt::{RPC_UUID, SERVICE_UUID}; use futures::StreamExt; /// How long to look for a robot before giving up. @@ -135,7 +135,7 @@ struct Seen { /// Whether this advertisement carried the duck service UUID, which is the strongest evidence a /// listing has: anything better needs a connection, and `scan` deliberately makes none. duck: bool, - /// What the robot broadcast about its place on the network — see [`Address`], and `btd::adv` + /// What the robot broadcast about its place on the network — see [`Address`], and `duck_ble::adv` /// for why four bytes of IPv4 and not the SSID too. address: Address, } @@ -753,7 +753,7 @@ async fn listing(seen: &[Seen], verbose: bool, target: &Target) -> String { /// the radio — while a list the robot is missing from points at the robot. /// /// And the robot can be *in* that list, unrecognisable. `btd` advertises flags (3 bytes), a 128-bit -/// service UUID (18) and the address field (8, see `btd::adv`), which is 29 of the 31 bytes a legacy +/// service UUID (18) and the address field (8, see `duck_ble::adv`), which is 29 of the 31 bytes a legacy /// advertisement holds — so the name never travels in it. It goes in the scan response, a second /// exchange that can be missed on its own. A device reported with no name and no services is /// therefore a plausible robot, which is why the unnamed ones are listed rather than filtered out.