Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions btd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 9 additions & 9 deletions btd/src/bluez.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -657,11 +657,11 @@ async fn notify_chunk(notifier: &mut CharacteristicNotifier, chunk: Vec<u8>) ->
///
/// 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<Ipv4Addr>,
}
Expand Down Expand Up @@ -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
Expand All @@ -722,7 +722,7 @@ async fn advertise(
let advertisement = |address: Option<Vec<u8>>| 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()),
Expand All @@ -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) => {
Expand Down Expand Up @@ -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<Ipv4Addr>) -> Option<Ipv4Addr> {
Expand Down
18 changes: 9 additions & 9 deletions btd/src/chorale.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
///
Expand All @@ -91,7 +91,7 @@ pub fn scan_pattern() -> Vec<u8> {

/// 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<u8> {
Expand Down Expand Up @@ -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()));
Expand All @@ -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"
);
}
Expand All @@ -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));
}

Expand Down
3 changes: 0 additions & 3 deletions btd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion btd/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
2 changes: 1 addition & 1 deletion btd/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
2 changes: 1 addition & 1 deletion docs/design/app-path-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
22 changes: 22 additions & 0 deletions duck-ble/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
File renamed without changes.
File renamed without changes.
File renamed without changes.
28 changes: 28 additions & 0 deletions duck-ble/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 6 additions & 1 deletion duckctl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion duckctl/examples/advwatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions duckctl/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//!
Expand All @@ -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.
Expand Down Expand 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,
}
Expand Down Expand Up @@ -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.
Expand Down
Loading