From ec51e004f9da75f6da31d23873d242bc4e05ad9a Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 14:42:28 +0200 Subject: [PATCH 01/13] feat: freeze the BLE shim transport seam and document the wire protocol SimBleLink.h is the interface between the socket transport and the GATT model. Both halves are built in parallel against it, so it is frozen before either starts. docs/ble-shim.md seeds the wire protocol, the threading model and the four fidelity items the shim has to reproduce. --- docs/ble-shim.md | 129 +++++++++++++++++++++++++++++++++++++++++++++++ src/SimBleLink.h | 73 +++++++++++++++++++++++++++ 2 files changed, 202 insertions(+) create mode 100644 docs/ble-shim.md create mode 100644 src/SimBleLink.h diff --git a/docs/ble-shim.md b/docs/ble-shim.md new file mode 100644 index 0000000..25e5aca --- /dev/null +++ b/docs/ble-shim.md @@ -0,0 +1,129 @@ +# The BLE peripheral shim + +The simulator fakes a BLE peripheral. Firmware BLE code runs unchanged; only +the radio is replaced. A client speaks a line protocol over a TCP socket and +plays the part of the central (the phone). + +**Shim** here means a header-compatible fake: it declares the same C++ API as +the real NimBLE library and implements it differently. No NimBLE source is +compiled. + +Status: **seed**. This file was created before the implementation, so it states +the frozen contract, not measured behaviour. Every claim below is marked +`[contract]` (what the shim must do) or `[verified]` (observed running, with +the command that showed it). Nothing is `[verified]` yet. + +## What it cannot answer + +Write this down first, because it is the part that gets forgotten. + +- **Heap.** The real NimBLE host and BT controller are the biggest single RAM + consumer on a device. The simulator's `esp_get_free_heap_size()` returns a + flat 1000000 (`src/esp_system.h:4`), so a feature that fits here can still + fail to fit on hardware. `[verified]` by reading `src/esp_system.h`. +- **The radio.** Range, RSSI (faked, see the `rssi` op), interference, and + whatever coexistence rules the target platform has. +- **The peer's real GATT stack.** A python client agreeing with the firmware + proves the firmware self-consistent, not interoperable with a phone's stack. + +## Turning it on + +``` +CROSSPOINT_SIM_BLE_PORT=8765 # absent or 0 = feature off +``` + +Off by default. A simulator run with no BLE client behaves exactly as it did +before the shim existed. The `CROSSPOINT_SIM_*` prefix matches the existing +simulator env vars (`CROSSPOINT_SIM_INPUT_SCRIPT`, `CROSSPOINT_SIM_SCREENSHOTS`). + +## Wire protocol `[contract]` + +One TCP listener on loopback. The simulator is the server. Newline-delimited +JSON, one object per line, UTF-8. One client at a time; a second connecting +client is refused with an `error` line. + +Binary payloads are lowercase hex strings. UUIDs are the full 36-char form the +firmware uses. + +### Client to simulator + +| op | fields | effect in the shim | +|---|---|---| +| `connect` | `mtu` (default 23), `interval` (units, default 24), `latency`, `timeout` | fires `onConnect`, then `onMTUChange`, then `onConnParamsUpdate`; stops advertising | +| `disconnect` | `reason` (default 0x13) | fires `onDisconnect`; the shim does **not** resume advertising by itself, because NimBLE does not | +| `write` | `uuid`, `hex`, `response` (default true) | sets the characteristic value, fires `onWrite` | +| `subscribe` | `uuid`, `value` (0 off, 1 notify, 2 indicate, 3 both) | fires `onSubscribe` with that `subValue` | +| `confirm` | `uuid` | fires `onStatus` with `BLE_HS_EDONE` for the pending indication | +| `mtu` | `mtu` | fires `onMTUChange` | +| `connparams` | `interval`, `latency`, `timeout` | fires `onConnParamsUpdate`. This is the central's answer to a device request | +| `rssi` | `value` | what `ble_gap_conn_rssi()` returns from now on | +| `auto_confirm` | `enabled` (default **true**), `delay_ms` (default 10) | confirm every indication automatically after the delay. Set false to drive confirms by hand or to test a timeout | + +`auto_confirm` defaults on so a simple client does not have to know the confirm +dance exists. A timeout test turns it off. + +### Simulator to client + +| ev | fields | when | +|---|---|---| +| `stack` | `state`: `up`/`down` | `NimBLEDevice::init` / `deinit` | +| `gatt` | `service`, `chars`: `[{uuid, props}]` | after the firmware builds the table | +| `advertising` | `up`, `interval_min`, `interval_max`, `name`, `service` | every `start()`/`stop()`, including an interval change | +| `indicate` | `uuid`, `hex` | `indicate()` accepted a payload into the pending slot | +| `clobber` | `uuid`, `dropped_hex` | a new `indicate()` overwrote an unconfirmed one. **Not a real BLE event**: it exists to make the clobber observable instead of silent | +| `connparams_request` | `min`, `max`, `latency`, `timeout` | firmware called `updateConnParams` | +| `error` | `msg` | a client op the real stack would refuse | + +### Rules the shim enforces, because the real stack does `[contract]` + +- `write` or `subscribe` with no central connected: `error`, no callback. +- `indicate()` with nobody subscribed to that characteristic: returns false. +- A subscription belongs to a connection. On `disconnect` the shim clears + subscription state itself, because NimBLE fires no unsubscribe callback. +- Advertising stops on connect and is not restarted by the shim. + +## Threading model `[contract]` + +- `NimBLEDevice::init()` starts **two** threads: a socket reader and a **host + thread**. `deinit()` joins both. +- The reader parses lines and pushes events onto the host thread's queue. It + never calls firmware code. +- The host thread dispatches every firmware callback. It is the simulator's + stand-in for the NimBLE host task. +- `indicate()` is called from the activity thread. It fills a single pending + slot, emits `indicate` (plus `clobber` if it overwrote one), and returns. The + confirm arrives later as an event and is dispatched on the host thread. +- The `portENTER_CRITICAL` shim is a real `std::mutex` + (`src/freertos/FreeRTOS.h:27-29`), so existing critical sections keep working + across these threads. `[verified]` by reading that file. + +`SimBleLink.h` is the frozen seam between the transport and the GATT model, and +its header comment restates this split. + +## Fidelity: four things that must be right, or the simulator lies + +A shim that gets these wrong hides exactly the bugs a real device already +showed. All four are `[contract]` until a run demonstrates them. + +1. **Callbacks run on the host thread**, never inline on the caller's thread. + Inline dispatch makes a whole class of deadlock impossible to reproduce. +2. **Indication confirm is out of band, and withholdable.** `indicate()` + returns true when the single pending slot accepted the payload, not when the + peer got it. The confirm arrives later through `onStatus`. A shim that + confirms synchronously never executes the firmware's timeout path. +3. **A second `indicate()` before a confirm clobbers the first.** Real and + measured on hardware: back-to-back calls all returned true, the peer saw the + first and the last. The shim must reproduce the clobber, not queue politely. + The `clobber` event is how that stays visible. +4. **The client sets the MTU.** MTU drives the firmware's payload arithmetic + and chunk counts, so a wrong default tests different arithmetic than a + device runs. 23 is the pessimistic default; 517 is the fast path. + +## Fault injection + +The point of a shim over hardware. All `[contract]`: + +- Withhold a confirm (`auto_confirm` false, then never send `confirm`). +- Drop the link mid-transfer (`disconnect` while a transfer is running). +- Send a malformed frame (trailing bytes, bad path, oversized length). +- Send a transfer `begin` without subscribing to the status characteristic. diff --git a/src/SimBleLink.h b/src/SimBleLink.h new file mode 100644 index 0000000..ced825e --- /dev/null +++ b/src/SimBleLink.h @@ -0,0 +1,73 @@ +#pragma once + +// SimBleLink -- the transport seam of the simulator's fake BLE peripheral. +// +// FROZEN INTERFACE. Two agents share this file's consumers and neither may +// change it: the GATT/API side calls it, the socket side implements it. A +// change here breaks the other half silently, so a change is a report, not an +// edit. +// +// Split of duty: +// - This class owns the socket, the listener, the reader thread and the +// line framing. It never calls firmware code. +// - The sink it invokes (setSink) is the GATT model's entry point. The sink +// is called ON THE READER THREAD and must not run firmware callbacks +// itself: it queues them for the host thread. See docs/ble-shim.md, +// "Threading model". +// +// Wire format is one JSON object per line, UTF-8, newline delimited. The +// decode happens below this seam: a SimBleEvent is already parsed. + +#include +#include +#include +#include + +// One decoded client op. Deliberately flat: the ops carry at most four small +// integers, and a tagged union would buy nothing but ceremony. +// +// Field meaning per op (docs/ble-shim.md has the table): +// connect a=mtu, b=interval, c=latency, d=timeout +// disconnect a=reason +// write uuid, data, flag=response +// subscribe uuid, a=subValue +// confirm uuid +// mtu a=mtu +// connparams b=interval, c=latency, d=timeout +// rssi a=value (cast to int8_t by the consumer) +// auto_confirm flag=enabled, a=delay_ms +struct SimBleEvent { + std::string op, uuid; + std::vector data; + uint32_t a = 0, b = 0, c = 0, d = 0; // mtu/interval/latency/timeout/subValue + bool flag = true; +}; + +class SimBleLink { + public: + static SimBleLink& get(); + + // Binds a loopback TCP listener and starts the reader thread. + // Returns false if port is 0 or the bind fails. A false return is not an + // error the caller recovers from: it means the feature stays off and the + // simulator behaves as it did before BLE existed. + bool start(uint16_t port); + + // Closes the socket, joins the reader thread. Safe to call when not running. + void stop(); + + bool running() const; + + // Registers the decoded-op sink. Called on the reader thread, so the sink + // must be cheap and must not block. Passing nullptr clears it. + void setSink(void (*fn)(void*, const SimBleEvent&), void* ctx); + + // Writes one JSON line to the connected client. Thread safe: the activity + // thread and the host thread both emit. A no-op when nothing is connected. + void emit(const char* json); + + private: + SimBleLink() = default; + SimBleLink(const SimBleLink&) = delete; + SimBleLink& operator=(const SimBleLink&) = delete; +}; From 620f2133d379deb173ccc6cc8489fc563cf1137e Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 14:58:09 +0200 Subject: [PATCH 02/13] feat: implement the BLE shim transport and wire codec SimBleLink.cpp is the socket half of the frozen seam: a loopback TCP listener, one reader thread that lives in poll(), line framing over recv boundaries, and a thread safe emit(). SimBleProtocol.{h,cpp} is the hand rolled JSON codec for the ten client ops. No JSON library is added: the ops are flat and this branch must not grow a dependency. Transport decisions worth naming: - The listener binds 127.0.0.1, never INADDR_ANY. This process runs firmware command handling. - stop() wakes the reader with a self pipe byte. shutdown() on a listening socket does not portably wake accept(), and closing a fd another thread is polling races a new socket onto that number. - One line caps at 65536 bytes. Past it the buffer is dropped, one error event goes out, and bytes up to the next newline are discarded. - The reader alone closes the client fd. emit() shuts it down on a write failure so teardown stays in one place, and carries a 5 s send timeout so a wedged client cannot hang a firmware thread. - A lost socket synthesizes a disconnect op, so the GATT model does not keep believing a central is connected. tests/sim_ble_link_selftest.{cpp,py} is the gate. It builds against those two sources alone, so the transport is provable before the GATT model exists: 65 checks covering every op explicit and defaulted, 20 malformed lines, the cap, the second client refusal, prompt stop() and concurrent emit. Clean under ASan+UBSan and under TSan. --- src/SimBleLink.cpp | 434 ++++++++++++++++++++ src/SimBleProtocol.cpp | 687 ++++++++++++++++++++++++++++++++ src/SimBleProtocol.h | 69 ++++ tests/sim_ble_link_selftest.cpp | 116 ++++++ tests/sim_ble_link_selftest.py | 468 ++++++++++++++++++++++ 5 files changed, 1774 insertions(+) create mode 100644 src/SimBleLink.cpp create mode 100644 src/SimBleProtocol.cpp create mode 100644 src/SimBleProtocol.h create mode 100644 tests/sim_ble_link_selftest.cpp create mode 100644 tests/sim_ble_link_selftest.py diff --git a/src/SimBleLink.cpp b/src/SimBleLink.cpp new file mode 100644 index 0000000..82049ac --- /dev/null +++ b/src/SimBleLink.cpp @@ -0,0 +1,434 @@ +// SimBleLink -- socket, reader thread and line framing for the fake BLE +// peripheral. The interface is frozen in SimBleLink.h; this file is the +// transport half of it and knows nothing about GATT. +// +// Design notes that are not obvious from the code: +// +// - **Loopback only.** The listener binds 127.0.0.1, never 0.0.0.0. This +// process runs firmware command handling, so a LAN-reachable port would let +// anything on the network drive the device model. +// - **One reader thread, always in poll().** The thread never blocks in +// accept() or recv(): it polls the listener, the client and a self-pipe, +// and only calls accept/recv on a fd poll already reported readable. That +// is what makes stop() prompt -- see wakeReader(). +// - **The reader owns the client fd's lifetime.** Only the reader closes it. +// emit() takes the mutex, writes the whole line, and on a write failure +// shuts the fd down instead of closing it, which makes poll() return and +// lets the reader do the teardown in one place. + +#include "SimBleLink.h" + +#include "SimBleProtocol.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +namespace ble = crosspoint_simulator::ble; + +// Bytes pulled from the socket per recv. Small on purpose: a line is split +// across recv boundaries either way, so the framer has to handle it. +constexpr size_t kRecvChunk = 4096; + +// A wedged client must not hang the firmware thread inside emit(). After this +// long the send fails, the link is dropped and the simulator carries on. +constexpr int kSendTimeoutSeconds = 5; + +struct LinkState { + // Guards clientFd, sink and sinkCtx. Held across a whole emit() so two + // threads cannot interleave halves of two lines. + std::mutex mtx; + // Serialises start() and stop() against each other. Separate from mtx + // because stop() joins the reader while the reader takes mtx. + std::mutex lifecycle; + + int listenFd = -1; + int clientFd = -1; + int wakeRead = -1; + int wakeWrite = -1; + + std::thread reader; + std::atomic up{false}; + std::atomic stopping{false}; + + void (*sink)(void *, const SimBleEvent &) = nullptr; + void *sinkCtx = nullptr; + + // Reader thread only. No lock. + std::string rx; + bool skippingLongLine = false; +}; + +LinkState &state() { + static LinkState s; + return s; +} + +void closeFd(int &fd) { + if (fd >= 0) { + ::close(fd); + fd = -1; + } +} + +// Writes one byte into the self-pipe. This is the only wake that is reliable: +// shutdown() on a listening socket is not portable, and closing a fd another +// thread is polling is a use-after-free waiting to happen. +void wakeReader(LinkState &s) { + if (s.wakeWrite < 0) + return; + const char byte = 1; + while (::write(s.wakeWrite, &byte, 1) < 0 && errno == EINTR) { + } +} + +void drainWake(LinkState &s) { + char scratch[64]; + while (::read(s.wakeRead, scratch, sizeof(scratch)) > 0) { + } +} + +// Sends every byte or gives up. Caller holds s.mtx. +bool sendAll(int fd, const char *data, size_t len) { + size_t sent = 0; + while (sent < len) { + const ssize_t n = ::send(fd, data + sent, len - sent, MSG_NOSIGNAL); + if (n > 0) { + sent += static_cast(n); + continue; + } + if (n < 0 && errno == EINTR) + continue; + return false; + } + return true; +} + +// Best effort single line to a fd the state does not own yet. Used to tell a +// second client it is not welcome. +void sendRefusal(int fd, const std::string &line) { + const std::string framed = line + "\n"; + timeval tv{}; + tv.tv_sec = 1; + ::setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)); + sendAll(fd, framed.data(), framed.size()); +} + +void emitLine(const std::string &line); + +// Drops the client. With `notify` set and a sink installed it also +// synthesizes a disconnect op: a socket that goes away is a link that went +// away, and without this the GATT model would keep believing a central is +// connected. `reason` 0x13 is the same remote-terminated code the disconnect +// op defaults to. The teardown path passes false, because stop() is not a +// link event and the model is being destroyed anyway. +void dropClient(LinkState &s, bool notify) { + void (*sink)(void *, const SimBleEvent &) = nullptr; + void *ctx = nullptr; + bool had = false; + { + std::lock_guard lock(s.mtx); + if (s.clientFd >= 0) { + ::shutdown(s.clientFd, SHUT_RDWR); + closeFd(s.clientFd); + had = true; + } + sink = s.sink; + ctx = s.sinkCtx; + } + s.rx.clear(); + s.skippingLongLine = false; + if (!notify || !had || !sink) + return; + SimBleEvent ev; + ev.op = "disconnect"; + ev.a = 0x13; + sink(ctx, ev); +} + +void handleLine(LinkState &s, const char *line, size_t len) { + if (len == 0) + return; // A blank line is not an op and not an error. + + SimBleEvent ev; + std::string err; + if (!ble::parseLine(line, len, ev, err)) { + emitLine(ble::errorLine(err.empty() ? "malformed line" : err)); + return; + } + + void (*sink)(void *, const SimBleEvent &) = nullptr; + void *ctx = nullptr; + { + std::lock_guard lock(s.mtx); + sink = s.sink; + ctx = s.sinkCtx; + } + if (sink) + sink(ctx, ev); +} + +// Line framing. Buffers partial reads, splits on '\n', tolerates "\r\n", and +// caps one line at ble::kMaxLineBytes: past that the buffer is thrown away, +// one error goes out, and everything up to the next newline is discarded. +void feedBytes(LinkState &s, const char *data, size_t len) { + s.rx.append(data, len); + + size_t start = 0; + while (true) { + const size_t nl = s.rx.find('\n', start); + if (nl == std::string::npos) + break; + size_t end = nl; + if (end > start && s.rx[end - 1] == '\r') + --end; + if (s.skippingLongLine) { + // This "line" is only the tail of the one already dropped. + s.skippingLongLine = false; + } else { + handleLine(s, s.rx.data() + start, end - start); + } + start = nl + 1; + } + s.rx.erase(0, start); + + if (s.rx.size() > ble::kMaxLineBytes) { + if (!s.skippingLongLine) { + emitLine(ble::errorLine("line longer than 65536 bytes, dropped")); + s.skippingLongLine = true; + } + s.rx.clear(); + } +} + +void acceptClient(LinkState &s) { + sockaddr_in peer{}; + socklen_t peerLen = sizeof(peer); + const int fd = + ::accept(s.listenFd, reinterpret_cast(&peer), &peerLen); + if (fd < 0) + return; + + bool busy = false; + { + std::lock_guard lock(s.mtx); + busy = (s.clientFd >= 0); + } + if (busy) { + sendRefusal(fd, ble::errorLine("busy: the shim takes one client at a time")); + ::shutdown(fd, SHUT_RDWR); + ::close(fd); + return; + } + + int yes = 1; + ::setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &yes, sizeof(yes)); + timeval tv{}; + tv.tv_sec = kSendTimeoutSeconds; + ::setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)); + + s.rx.clear(); + s.skippingLongLine = false; + { + std::lock_guard lock(s.mtx); + s.clientFd = fd; + } +} + +void readerLoop() { + LinkState &s = state(); + while (!s.stopping.load()) { + int cfd = -1; + { + std::lock_guard lock(s.mtx); + cfd = s.clientFd; + } + + pollfd fds[3]; + int count = 0; + const int listenSlot = count; + fds[count++] = {s.listenFd, POLLIN, 0}; + const int wakeSlot = count; + fds[count++] = {s.wakeRead, POLLIN, 0}; + int clientSlot = -1; + if (cfd >= 0) { + clientSlot = count; + fds[count++] = {cfd, POLLIN, 0}; + } + + const int ready = ::poll(fds, static_cast(count), -1); + if (ready < 0) { + if (errno == EINTR) + continue; + break; + } + + if (fds[wakeSlot].revents != 0) { + drainWake(s); + if (s.stopping.load()) + break; + } + + if (clientSlot >= 0 && fds[clientSlot].revents != 0) { + char buffer[kRecvChunk]; + const ssize_t n = ::recv(cfd, buffer, sizeof(buffer), MSG_DONTWAIT); + if (n > 0) { + feedBytes(s, buffer, static_cast(n)); + } else if (n == 0) { + dropClient(s, true); + } else if (errno != EINTR && errno != EAGAIN && errno != EWOULDBLOCK) { + dropClient(s, true); + } + } + + if (fds[listenSlot].revents != 0) + acceptClient(s); + } + dropClient(state(), false); +} + +} // namespace + +SimBleLink &SimBleLink::get() { + static SimBleLink instance; + return instance; +} + +bool SimBleLink::start(uint16_t port) { + LinkState &s = state(); + std::lock_guard life(s.lifecycle); + if (port == 0) + return false; + if (s.up.load()) + return true; + + int wake[2] = {-1, -1}; + if (::pipe(wake) != 0) + return false; + // The reader drains the pipe in a loop, so its read end must not block. + ::fcntl(wake[0], F_SETFL, ::fcntl(wake[0], F_GETFL, 0) | O_NONBLOCK); + ::fcntl(wake[0], F_SETFD, FD_CLOEXEC); + ::fcntl(wake[1], F_SETFD, FD_CLOEXEC); + + const int fd = ::socket(AF_INET, SOCK_STREAM, 0); + if (fd < 0) { + ::close(wake[0]); + ::close(wake[1]); + return false; + } + int yes = 1; + ::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(yes)); + + sockaddr_in addr{}; + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); // 127.0.0.1, never a LAN nic. + addr.sin_port = htons(port); + if (::bind(fd, reinterpret_cast(&addr), sizeof(addr)) != 0 || + ::listen(fd, 4) != 0) { + ::close(fd); + ::close(wake[0]); + ::close(wake[1]); + return false; + } + + s.listenFd = fd; + s.wakeRead = wake[0]; + s.wakeWrite = wake[1]; + s.rx.clear(); + s.skippingLongLine = false; + s.stopping.store(false); + s.up.store(true); + s.reader = std::thread(readerLoop); + return true; +} + +void SimBleLink::stop() { + LinkState &s = state(); + std::lock_guard life(s.lifecycle); + if (!s.up.load()) { + // Safe when never started, and safe twice in a row. + closeFd(s.listenFd); + closeFd(s.wakeRead); + closeFd(s.wakeWrite); + return; + } + + s.stopping.store(true); + wakeReader(s); + { + // Shuts a connected client down so a reader mid-recv sees EOF at once. + // The listener is left alone: the reader is in poll(), not accept(). + std::lock_guard lock(s.mtx); + if (s.clientFd >= 0) + ::shutdown(s.clientFd, SHUT_RDWR); + } + if (s.reader.joinable()) + s.reader.join(); + + { + std::lock_guard lock(s.mtx); + closeFd(s.clientFd); + } + closeFd(s.listenFd); + closeFd(s.wakeRead); + closeFd(s.wakeWrite); + s.rx.clear(); + s.skippingLongLine = false; + s.up.store(false); + s.stopping.store(false); +} + +bool SimBleLink::running() const { return state().up.load(); } + +void SimBleLink::setSink(void (*fn)(void *, const SimBleEvent &), void *ctx) { + LinkState &s = state(); + std::lock_guard lock(s.mtx); + s.sink = fn; + s.sinkCtx = fn ? ctx : nullptr; +} + +void SimBleLink::emit(const char *json) { + if (!json || !*json) + return; + emitLine(std::string(json)); +} + +namespace { + +void emitLine(const std::string &line) { + LinkState &s = state(); + std::string framed; + framed.reserve(line.size() + 1); + for (const char ch : line) { + // An embedded newline would inject a second frame, so it cannot survive. + framed.push_back((ch == '\n' || ch == '\r') ? ' ' : ch); + } + while (!framed.empty() && framed.back() == ' ') + framed.pop_back(); + if (framed.empty()) + return; + framed.push_back('\n'); + + std::lock_guard lock(s.mtx); + if (s.clientFd < 0) + return; + if (!sendAll(s.clientFd, framed.data(), framed.size())) { + // Do not close here: the reader owns the fd's lifetime. A shutdown makes + // its poll() return and the teardown happens in one place. + ::shutdown(s.clientFd, SHUT_RDWR); + } +} + +} // namespace diff --git a/src/SimBleProtocol.cpp b/src/SimBleProtocol.cpp new file mode 100644 index 0000000..05be334 --- /dev/null +++ b/src/SimBleProtocol.cpp @@ -0,0 +1,687 @@ +#include "SimBleProtocol.h" + +#include +#include +#include + +namespace crosspoint_simulator::ble { + +namespace { + +// --------------------------------------------------------------------------- +// Hex +// --------------------------------------------------------------------------- + +int hexDigit(char c) { + if (c >= '0' && c <= '9') + return c - '0'; + if (c >= 'a' && c <= 'f') + return c - 'a' + 10; + if (c >= 'A' && c <= 'F') + return c - 'A' + 10; + return -1; +} + +// --------------------------------------------------------------------------- +// A parsed JSON value. Client ops are flat, so only these four types are +// kept; a nested object or array is skipped and remembered as Other so a +// wrong-type field reports as a type error instead of a missing field. +// --------------------------------------------------------------------------- + +enum class VType { Str, Num, Bool, Null, Other }; + +struct Value { + VType type = VType::Null; + std::string str; + double num = 0.0; + bool boolean = false; +}; + +struct Member { + std::string key; + Value value; +}; + +class Parser { +public: + Parser(const char *data, size_t len) : p_(data), end_(data + len) {} + + // Parses one top level object into `out`. Returns false and fills `err` on + // anything that is not exactly one object followed by whitespace. + bool parseTopObject(std::vector &out, std::string &err) { + skipWs(); + if (!parseObject(out, err)) + return false; + skipWs(); + if (p_ != end_) { + err = "trailing bytes after the object"; + return false; + } + return true; + } + +private: + const char *p_; + const char *end_; + + bool atEnd() const { return p_ >= end_; } + char peek() const { return *p_; } + + void skipWs() { + while (!atEnd() && (*p_ == ' ' || *p_ == '\t' || *p_ == '\r' || *p_ == '\n')) + ++p_; + } + + bool expect(char c, std::string &err) { + if (atEnd() || *p_ != c) { + err = std::string("expected '") + c + "'"; + return false; + } + ++p_; + return true; + } + + bool parseObject(std::vector &out, std::string &err) { + if (!expect('{', err)) + return false; + skipWs(); + if (!atEnd() && peek() == '}') { + ++p_; + return true; + } + while (true) { + skipWs(); + Member m; + if (!parseString(m.key, err)) + return false; + skipWs(); + if (!expect(':', err)) + return false; + skipWs(); + if (!parseValue(m.value, 0, err)) + return false; + if (out.size() >= kMaxObjectKeys) { + err = "too many keys"; + return false; + } + out.push_back(std::move(m)); + skipWs(); + if (atEnd()) { + err = "unterminated object"; + return false; + } + if (peek() == ',') { + ++p_; + continue; + } + if (peek() == '}') { + ++p_; + return true; + } + err = "expected ',' or '}'"; + return false; + } + } + + // Appends `cp` to `out` as UTF-8. Rejects NUL so every parsed string stays + // usable as a C string by the consumer. + bool appendUtf8(uint32_t cp, std::string &out, std::string &err) { + if (cp == 0) { + err = "NUL in a string"; + return false; + } + if (cp < 0x80) { + out.push_back(static_cast(cp)); + } else if (cp < 0x800) { + out.push_back(static_cast(0xC0 | (cp >> 6))); + out.push_back(static_cast(0x80 | (cp & 0x3F))); + } else if (cp < 0x10000) { + out.push_back(static_cast(0xE0 | (cp >> 12))); + out.push_back(static_cast(0x80 | ((cp >> 6) & 0x3F))); + out.push_back(static_cast(0x80 | (cp & 0x3F))); + } else { + out.push_back(static_cast(0xF0 | (cp >> 18))); + out.push_back(static_cast(0x80 | ((cp >> 12) & 0x3F))); + out.push_back(static_cast(0x80 | ((cp >> 6) & 0x3F))); + out.push_back(static_cast(0x80 | (cp & 0x3F))); + } + return true; + } + + bool parseHex4(uint32_t &out, std::string &err) { + if (end_ - p_ < 4) { + err = "truncated \\u escape"; + return false; + } + uint32_t v = 0; + for (int i = 0; i < 4; ++i) { + const int d = hexDigit(p_[i]); + if (d < 0) { + err = "bad \\u escape"; + return false; + } + v = (v << 4) | static_cast(d); + } + p_ += 4; + out = v; + return true; + } + + bool parseString(std::string &out, std::string &err) { + if (!expect('"', err)) + return false; + out.clear(); + while (true) { + if (atEnd()) { + err = "unterminated string"; + return false; + } + const unsigned char c = static_cast(*p_); + if (c == '"') { + ++p_; + return true; + } + if (c < 0x20) { + // A raw control byte, a raw NUL included, is not valid JSON. Rejecting + // here is what keeps a binary blob on the socket from being parsed as + // half an op. + err = "raw control byte in a string"; + return false; + } + if (c != '\\') { + out.push_back(static_cast(c)); + ++p_; + continue; + } + ++p_; + if (atEnd()) { + err = "truncated escape"; + return false; + } + const char esc = *p_++; + switch (esc) { + case '"': + out.push_back('"'); + break; + case '\\': + out.push_back('\\'); + break; + case '/': + out.push_back('/'); + break; + case 'b': + out.push_back('\b'); + break; + case 'f': + out.push_back('\f'); + break; + case 'n': + out.push_back('\n'); + break; + case 'r': + out.push_back('\r'); + break; + case 't': + out.push_back('\t'); + break; + case 'u': { + uint32_t cp = 0; + if (!parseHex4(cp, err)) + return false; + if (cp >= 0xD800 && cp <= 0xDBFF) { + // High surrogate. A low surrogate must follow or the string is bad. + if (end_ - p_ < 6 || p_[0] != '\\' || p_[1] != 'u') { + err = "lone surrogate"; + return false; + } + p_ += 2; + uint32_t low = 0; + if (!parseHex4(low, err)) + return false; + if (low < 0xDC00 || low > 0xDFFF) { + err = "bad surrogate pair"; + return false; + } + cp = 0x10000 + ((cp - 0xD800) << 10) + (low - 0xDC00); + } else if (cp >= 0xDC00 && cp <= 0xDFFF) { + err = "lone surrogate"; + return false; + } + if (!appendUtf8(cp, out, err)) + return false; + break; + } + default: + err = "unknown escape"; + return false; + } + } + } + + bool parseNumber(Value &out, std::string &err) { + const char *start = p_; + if (!atEnd() && *p_ == '-') + ++p_; + if (atEnd() || *p_ < '0' || *p_ > '9') { + err = "bad number"; + return false; + } + if (*p_ == '0') { + ++p_; + } else { + while (!atEnd() && *p_ >= '0' && *p_ <= '9') + ++p_; + } + if (!atEnd() && *p_ == '.') { + ++p_; + if (atEnd() || *p_ < '0' || *p_ > '9') { + err = "bad fraction"; + return false; + } + while (!atEnd() && *p_ >= '0' && *p_ <= '9') + ++p_; + } + if (!atEnd() && (*p_ == 'e' || *p_ == 'E')) { + ++p_; + if (!atEnd() && (*p_ == '+' || *p_ == '-')) + ++p_; + if (atEnd() || *p_ < '0' || *p_ > '9') { + err = "bad exponent"; + return false; + } + while (!atEnd() && *p_ >= '0' && *p_ <= '9') + ++p_; + } + // strtod needs a terminated buffer, and the line is not terminated. + const std::string token(start, static_cast(p_ - start)); + errno = 0; + char *stop = nullptr; + const double v = std::strtod(token.c_str(), &stop); + if (stop != token.c_str() + token.size()) { + err = "bad number"; + return false; + } + out.type = VType::Num; + out.num = v; + return true; + } + + bool parseLiteral(const char *text, Value &out, VType type, bool boolean, + std::string &err) { + const size_t n = std::strlen(text); + if (static_cast(end_ - p_) < n || std::memcmp(p_, text, n) != 0) { + err = "bad literal"; + return false; + } + p_ += n; + out.type = type; + out.boolean = boolean; + return true; + } + + // Consumes a nested object or array without keeping it. Depth bounded. + bool skipNested(int depth, std::string &err) { + if (depth >= kMaxNestDepth) { + err = "nesting too deep"; + return false; + } + const char open = *p_; + const char close = (open == '{') ? '}' : ']'; + ++p_; + while (true) { + skipWs(); + if (atEnd()) { + err = "unterminated nested value"; + return false; + } + if (peek() == close) { + ++p_; + return true; + } + if (peek() == ',' || peek() == ':') { + ++p_; + continue; + } + Value ignored; + if (!parseValue(ignored, depth + 1, err)) + return false; + } + } + + bool parseValue(Value &out, int depth, std::string &err) { + if (atEnd()) { + err = "value expected"; + return false; + } + switch (peek()) { + case '"': { + out.type = VType::Str; + return parseString(out.str, err); + } + case 't': + return parseLiteral("true", out, VType::Bool, true, err); + case 'f': + return parseLiteral("false", out, VType::Bool, false, err); + case 'n': + return parseLiteral("null", out, VType::Null, false, err); + case '{': + case '[': + out.type = VType::Other; + return skipNested(depth, err); + default: + return parseNumber(out, err); + } + } +}; + +// --------------------------------------------------------------------------- +// Field access. Every getter reports a wrong type as an error instead of +// falling back to the default, because a client sending "mtu": "517" has a +// bug worth seeing. +// --------------------------------------------------------------------------- + +const Value *find(const std::vector &members, const char *key) { + for (const Member &m : members) { + if (m.key == key) + return &m.value; + } + return nullptr; +} + +bool getInt(const std::vector &members, const char *key, long def, + long lo, long hi, long &out, std::string &err) { + const Value *v = find(members, key); + if (!v || v->type == VType::Null) { + out = def; + return true; + } + if (v->type != VType::Num) { + err = std::string(key) + ": expected a number"; + return false; + } + // Range first. A cast of 1e300 to long is undefined behaviour, so the + // bounds check has to happen while the value is still a double. + const double d = v->num; + if (!(d >= static_cast(lo) && d <= static_cast(hi))) { + err = std::string(key) + ": out of range"; + return false; + } + const long got = static_cast(d); + if (static_cast(got) != d) { + err = std::string(key) + ": expected a whole number"; + return false; + } + out = got; + return true; +} + +bool getBool(const std::vector &members, const char *key, bool def, + bool &out, std::string &err) { + const Value *v = find(members, key); + if (!v || v->type == VType::Null) { + out = def; + return true; + } + if (v->type == VType::Bool) { + out = v->boolean; + return true; + } + // 0 and 1 are accepted, because a hand written client sends them. + if (v->type == VType::Num && (v->num == 0.0 || v->num == 1.0)) { + out = (v->num != 0.0); + return true; + } + err = std::string(key) + ": expected true or false"; + return false; +} + +bool getUuid(const std::vector &members, std::string &out, + std::string &err) { + const Value *v = find(members, "uuid"); + if (!v || v->type == VType::Null) { + err = "uuid: missing"; + return false; + } + if (v->type != VType::Str) { + err = "uuid: expected a string"; + return false; + } + if (v->str.empty() || v->str.size() > kMaxUuidChars) { + err = "uuid: bad length"; + return false; + } + out = v->str; + return true; +} + +bool getHex(const std::vector &members, std::vector &out, + std::string &err) { + const Value *v = find(members, "hex"); + if (!v || v->type == VType::Null) { + out.clear(); + return true; + } + if (v->type != VType::Str) { + err = "hex: expected a string"; + return false; + } + if (!decodeHex(v->str, out)) { + err = "hex: not an even run of hex digits"; + return false; + } + return true; +} + +// Connection parameter ranges are the Bluetooth spec ones. Out of range is an +// error, not a clamp: the real stack refuses these too. +constexpr long kMtuMin = 23, kMtuMax = 517; +constexpr long kIntervalMin = 6, kIntervalMax = 3200; +constexpr long kLatencyMax = 499; +constexpr long kTimeoutMin = 10, kTimeoutMax = 3200; + +bool readConnParams(const std::vector &members, SimBleEvent &ev, + std::string &err) { + long interval = 0, latency = 0, timeout = 0; + if (!getInt(members, "interval", 24, kIntervalMin, kIntervalMax, interval, + err)) + return false; + if (!getInt(members, "latency", 0, 0, kLatencyMax, latency, err)) + return false; + if (!getInt(members, "timeout", 400, kTimeoutMin, kTimeoutMax, timeout, err)) + return false; + ev.b = static_cast(interval); + ev.c = static_cast(latency); + ev.d = static_cast(timeout); + return true; +} + +} // namespace + +// --------------------------------------------------------------------------- +// Public codec +// --------------------------------------------------------------------------- + +bool decodeHex(const std::string &hex, std::vector &out) { + out.clear(); + if ((hex.size() % 2) != 0) + return false; + out.reserve(hex.size() / 2); + for (size_t i = 0; i < hex.size(); i += 2) { + const int hi = hexDigit(hex[i]); + const int lo = hexDigit(hex[i + 1]); + if (hi < 0 || lo < 0) { + out.clear(); + return false; + } + out.push_back(static_cast((hi << 4) | lo)); + } + return true; +} + +std::string encodeHex(const uint8_t *data, size_t len) { + static const char kDigits[] = "0123456789abcdef"; + std::string out; + if (!data || len == 0) + return out; + out.reserve(len * 2); + for (size_t i = 0; i < len; ++i) { + out.push_back(kDigits[data[i] >> 4]); + out.push_back(kDigits[data[i] & 0x0F]); + } + return out; +} + +std::string encodeHex(const std::vector &data) { + return encodeHex(data.data(), data.size()); +} + +std::string escapeJson(const std::string &raw) { + static const char kDigits[] = "0123456789abcdef"; + std::string out; + out.reserve(raw.size() + 8); + for (const char ch : raw) { + const unsigned char c = static_cast(ch); + switch (c) { + case '"': + out += "\\\""; + break; + case '\\': + out += "\\\\"; + break; + case '\b': + out += "\\b"; + break; + case '\f': + out += "\\f"; + break; + case '\n': + out += "\\n"; + break; + case '\r': + out += "\\r"; + break; + case '\t': + out += "\\t"; + break; + default: + if (c < 0x20) { + out += "\\u00"; + out.push_back(kDigits[c >> 4]); + out.push_back(kDigits[c & 0x0F]); + } else { + out.push_back(ch); + } + break; + } + } + return out; +} + +std::string errorLine(const std::string &msg) { + return "{\"ev\":\"error\",\"msg\":\"" + escapeJson(msg) + "\"}"; +} + +bool parseLine(const char *line, size_t len, SimBleEvent &ev, + std::string &err) { + err.clear(); + if (!line) { + err = "empty line"; + return false; + } + if (len > kMaxLineBytes) { + err = "line too long"; + return false; + } + + std::vector members; + members.reserve(8); + Parser parser(line, len); + if (!parser.parseTopObject(members, err)) + return false; + + const Value *opValue = find(members, "op"); + if (!opValue) { + err = "no op field"; + return false; + } + if (opValue->type != VType::Str || opValue->str.empty()) { + err = "op: expected a non-empty string"; + return false; + } + + SimBleEvent out; + out.op = opValue->str; + + if (out.op == "connect") { + long mtu = 0; + if (!getInt(members, "mtu", 23, kMtuMin, kMtuMax, mtu, err)) + return false; + if (!readConnParams(members, out, err)) + return false; + out.a = static_cast(mtu); + } else if (out.op == "disconnect") { + long reason = 0; + if (!getInt(members, "reason", 0x13, 0, 0xFF, reason, err)) + return false; + out.a = static_cast(reason); + } else if (out.op == "write") { + if (!getUuid(members, out.uuid, err)) + return false; + if (!getHex(members, out.data, err)) + return false; + if (!getBool(members, "response", true, out.flag, err)) + return false; + } else if (out.op == "subscribe") { + if (!getUuid(members, out.uuid, err)) + return false; + long value = 0; + if (!getInt(members, "value", 1, 0, 3, value, err)) + return false; + out.a = static_cast(value); + } else if (out.op == "confirm") { + if (!getUuid(members, out.uuid, err)) + return false; + } else if (out.op == "mtu") { + long mtu = 0; + if (!getInt(members, "mtu", 23, kMtuMin, kMtuMax, mtu, err)) + return false; + out.a = static_cast(mtu); + } else if (out.op == "connparams") { + if (!readConnParams(members, out, err)) + return false; + } else if (out.op == "rssi") { + long value = 0; + if (!getInt(members, "value", -60, -128, 127, value, err)) + return false; + // The consumer reads this back as int8_t, so store the low byte. + out.a = static_cast(static_cast(value & 0xFF)); + } else if (out.op == "auto_confirm") { + if (!getBool(members, "enabled", true, out.flag, err)) + return false; + long delay = 0; + if (!getInt(members, "delay_ms", 10, 0, 60000, delay, err)) + return false; + out.a = static_cast(delay); + } else { + err = "unknown op: " + out.op; + return false; + } + + ev = std::move(out); + return true; +} + +uint16_t portFromEnv() { + const char *configured = std::getenv("CROSSPOINT_SIM_BLE_PORT"); + if (!configured || !*configured) + return 0; + errno = 0; + char *stop = nullptr; + const long parsed = std::strtol(configured, &stop, 10); + if (errno != 0 || stop == configured || *stop != '\0' || parsed <= 0 || + parsed > 65535) + return 0; + return static_cast(parsed); +} + +} // namespace crosspoint_simulator::ble diff --git a/src/SimBleProtocol.h b/src/SimBleProtocol.h new file mode 100644 index 0000000..9cd4dcb --- /dev/null +++ b/src/SimBleProtocol.h @@ -0,0 +1,69 @@ +#pragma once + +// SimBleProtocol -- the wire codec of the simulator's fake BLE peripheral. +// +// One JSON object per line, UTF-8, newline delimited. Client lines carry an +// "op" key; simulator lines carry an "ev" key. docs/ble-shim.md is the +// authoritative field table. +// +// The parse is hand rolled on purpose. The simulator's dependency list is +// small and this code is bound for upstream, so no JSON library is added for +// ten flat object shapes. +// +// Nothing here touches a socket or a thread: SimBleLink.cpp owns those. This +// is pure functions over bytes, so the codec can be tested without a link. + +#include +#include +#include +#include + +#include "SimBleLink.h" + +namespace crosspoint_simulator::ble { + +// Longest single wire line accepted, in bytes, newline excluded. A longer +// line is dropped whole and answered with one `error` event. 64 KiB holds a +// 32 KiB hex payload, which is far more than any firmware GATT write, and it +// bounds what a hostile or wedged client can make the reader buffer. +constexpr size_t kMaxLineBytes = 65536; + +// Longest accepted UUID string. The firmware uses the 36-char form; short +// 16-bit and 32-bit forms are accepted too. The cap only stops nonsense. +constexpr size_t kMaxUuidChars = 64; + +// Most keys read from one object. A longer object is a malformed line. +constexpr size_t kMaxObjectKeys = 32; + +// Deepest nested value skipped while scanning. Client ops are flat, so +// anything deeper is either a mistake or an attempt to blow the stack. +constexpr int kMaxNestDepth = 8; + +// Lowercase-hex string to bytes. Rejects odd length and any non-hex +// character. Uppercase input is accepted; output elsewhere is lowercase. +bool decodeHex(const std::string &hex, std::vector &out); + +// Bytes to lowercase hex. Empty input gives an empty string. +std::string encodeHex(const uint8_t *data, size_t len); +std::string encodeHex(const std::vector &data); + +// JSON string body for `raw`, quotes excluded. Escapes the two mandatory +// characters and every control character. +std::string escapeJson(const std::string &raw); + +// A complete `error` event line, newline excluded. +std::string errorLine(const std::string &msg); + +// Parses one line (newline already stripped) into `ev`. +// +// Returns true when the line was a well formed client op with usable fields; +// `ev` is then fully populated, defaults applied. Returns false on anything +// else and puts a short reason in `err`; the caller answers with an `error` +// event and drops the line. Never reads outside [line, line + len). +bool parseLine(const char *line, size_t len, SimBleEvent &ev, std::string &err); + +// Port from CROSSPOINT_SIM_BLE_PORT. Returns 0 when the variable is absent, +// empty, zero or unparseable, which means the feature stays off. +uint16_t portFromEnv(); + +} // namespace crosspoint_simulator::ble diff --git a/tests/sim_ble_link_selftest.cpp b/tests/sim_ble_link_selftest.cpp new file mode 100644 index 0000000..ec31c63 --- /dev/null +++ b/tests/sim_ble_link_selftest.cpp @@ -0,0 +1,116 @@ +// sim_ble_link_selftest -- standalone driver for SimBleLink and +// SimBleProtocol. It builds from those two files alone, with no simulator and +// no firmware, so the transport can be proven before the GATT model exists. +// +// Build: +// g++ -std=c++17 -Wall -Wextra -O1 -Isrc +// src/SimBleLink.cpp src/SimBleProtocol.cpp +// tests/sim_ble_link_selftest.cpp -o /tmp/sim_ble_link_selftest -lpthread +// +// Run: the port comes from argv[1]. Commands arrive on stdin, one per line: +// emit emit one line to the client +// emitstress two threads emit n lines each, concurrently +// stop time SimBleLink::stop() and exit +// Every decoded op is printed as one SINK line on stdout. The python driver +// tests/sim_ble_link_selftest.py reads those. + +#include "SimBleLink.h" +#include "SimBleProtocol.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +std::mutex outMutex; + +void say(const std::string &line) { + std::lock_guard lock(outMutex); + std::cout << line << "\n"; + std::cout.flush(); +} + +void recordEvent(void *ctx, const SimBleEvent &ev) { + (void)ctx; + std::string line = "SINK op=" + ev.op + " uuid=" + (ev.uuid.empty() ? "-" : ev.uuid) + + " data=" + + (ev.data.empty() + ? std::string("-") + : crosspoint_simulator::ble::encodeHex(ev.data)) + + " a=" + std::to_string(ev.a) + " b=" + std::to_string(ev.b) + + " c=" + std::to_string(ev.c) + " d=" + std::to_string(ev.d) + + " flag=" + (ev.flag ? "1" : "0"); + say(line); +} + +void emitStress(int perThread) { + auto worker = [perThread](char tag) { + for (int i = 0; i < perThread; ++i) { + // A long payload so an interleave would be obvious in the received line. + std::string payload(200, tag); + const std::string json = "{\"ev\":\"stress\",\"tag\":\"" + + std::string(1, tag) + "\",\"n\":" + + std::to_string(i) + ",\"pad\":\"" + payload + + "\"}"; + SimBleLink::get().emit(json.c_str()); + } + }; + std::thread a(worker, 'A'); + std::thread b(worker, 'B'); + a.join(); + b.join(); + say("STRESS done"); +} + +} // namespace + +int main(int argc, char **argv) { + // A zero port must be refused, and the refusal must not be fatal. + if (SimBleLink::get().start(0)) + say("FAIL start(0) returned true"); + else + say("OK start(0) refused"); + say(std::string("OK running after start(0) = ") + + (SimBleLink::get().running() ? "true" : "false")); + + // stop() before any start must be a no-op, not a hang or a crash. + SimBleLink::get().stop(); + say("OK stop() before start returned"); + + const uint16_t port = static_cast(argc > 1 ? std::atoi(argv[1]) : 0); + SimBleLink::get().setSink(&recordEvent, nullptr); + if (!SimBleLink::get().start(port)) { + say("FAIL start failed"); + return 1; + } + say("READY " + std::to_string(port)); + + std::string command; + while (std::getline(std::cin, command)) { + if (command.rfind("emitstress ", 0) == 0) { + emitStress(std::atoi(command.c_str() + 11)); + } else if (command.rfind("emit ", 0) == 0) { + SimBleLink::get().emit(command.c_str() + 5); + } else if (command == "stop") { + const auto t0 = std::chrono::steady_clock::now(); + SimBleLink::get().stop(); + const auto ms = std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0) + .count(); + say("STOPPED in " + std::to_string(ms) + " ms, running=" + + (SimBleLink::get().running() ? "true" : "false")); + break; + } + } + + SimBleLink::get().setSink(nullptr, nullptr); + SimBleLink::get().stop(); + say("EXIT"); + return 0; +} diff --git a/tests/sim_ble_link_selftest.py b/tests/sim_ble_link_selftest.py new file mode 100644 index 0000000..9f08582 --- /dev/null +++ b/tests/sim_ble_link_selftest.py @@ -0,0 +1,468 @@ +#!/usr/bin/env python3 +"""Gate for the BLE shim transport. + +Builds tests/sim_ble_link_selftest.cpp against src/SimBleLink.cpp and +src/SimBleProtocol.cpp, then drives the listener over TCP: + + - every client op, once with explicit fields and once with defaults + - a second client is refused + - malformed lines do not crash the process + - a line over the 65536 byte cap is dropped, framing recovers + - stop() returns promptly with a client connected + - two threads emitting concurrently produce intact, non-interleaved lines + +Run from the repo root: python3 tests/sim_ble_link_selftest.py +Exit code 0 means every check passed. +""" + +import json +import os +import re +import socket +import subprocess +import sys +import tempfile +import threading +import time + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +PORT = int(os.environ.get("SELFTEST_PORT", "18765")) + +failures = [] +checks = 0 + + +def check(ok, label, detail=""): + global checks + checks += 1 + if ok: + print(f" ok {label}") + else: + failures.append(label) + print(f" FAIL {label}: {detail}") + + +def build(binary): + cmd = [ + "g++", "-std=c++17", "-Wall", "-Wextra", "-O1", "-I", os.path.join(ROOT, "src"), + ] + sanitize = os.environ.get("SELFTEST_SANITIZE", "") + if sanitize == "1": + cmd += ["-fsanitize=address,undefined", "-fno-omit-frame-pointer", "-g"] + elif sanitize == "thread": + cmd += ["-fsanitize=thread", "-fno-omit-frame-pointer", "-g"] + cmd += [ + os.path.join(ROOT, "src", "SimBleLink.cpp"), + os.path.join(ROOT, "src", "SimBleProtocol.cpp"), + os.path.join(ROOT, "tests", "sim_ble_link_selftest.cpp"), + "-o", binary, "-lpthread", + ] + print("$ " + " ".join(cmd)) + proc = subprocess.run(cmd, capture_output=True, text=True) + if proc.stdout.strip(): + print(proc.stdout) + if proc.stderr.strip(): + print(proc.stderr) + if proc.returncode != 0: + print("build failed") + sys.exit(1) + print("build clean, no warnings\n") + + +class Harness: + """The C++ selftest process. stdout lines are collected by a thread.""" + + def __init__(self, binary): + argv = [binary, str(PORT)] + if os.environ.get("SELFTEST_SANITIZE") == "thread": + # ThreadSanitizer needs ASLR off on current kernels, otherwise it + # dies with "unexpected memory mapping" before main(). + argv = ["setarch", "-R"] + argv + self.proc = subprocess.Popen( + argv, stdin=subprocess.PIPE, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, text=True, bufsize=1, cwd=ROOT, + ) + self.lines = [] + self.lock = threading.Lock() + self.pump = threading.Thread(target=self._pump, daemon=True) + self.pump.start() + + def _pump(self): + for line in self.proc.stdout: + with self.lock: + self.lines.append(line.rstrip("\n")) + + def wait_for(self, prefix, timeout=5.0): + deadline = time.time() + timeout + while time.time() < deadline: + with self.lock: + for line in self.lines: + if line.startswith(prefix): + return line + time.sleep(0.01) + return None + + def sink_lines(self): + with self.lock: + return [ln for ln in self.lines if ln.startswith("SINK ")] + + def all_lines(self): + with self.lock: + return list(self.lines) + + def command(self, text): + self.proc.stdin.write(text + "\n") + self.proc.stdin.flush() + + +def connect(): + sock = socket.create_connection(("127.0.0.1", PORT), timeout=5) + sock.settimeout(5) + return sock + + +class LineReader: + def __init__(self, sock): + self.sock = sock + self.buf = b"" + + def read_line(self, timeout=2.0): + self.sock.settimeout(timeout) + while b"\n" not in self.buf: + try: + chunk = self.sock.recv(4096) + except socket.timeout: + return None + if not chunk: + return None + self.buf += chunk + line, _, self.buf = self.buf.partition(b"\n") + return line.decode("utf-8", "replace") + + def read_all(self, seconds=0.4): + out = [] + deadline = time.time() + seconds + while time.time() < deadline: + line = self.read_line(timeout=max(0.05, deadline - time.time())) + if line is None: + break + out.append(line) + return out + + +def parse_sink(line): + fields = {} + for part in line.split()[1:]: + key, _, value = part.partition("=") + fields[key] = value + return fields + + +# Every op, explicit then defaulted, with the SINK line each must produce. +# op / json sent / expected decoded fields. +OPS = [ + ("connect explicit", + {"op": "connect", "mtu": 517, "interval": 12, "latency": 3, "timeout": 500}, + {"a": "517", "b": "12", "c": "3", "d": "500"}), + ("connect defaults", + {"op": "connect"}, + {"a": "23", "b": "24", "c": "0", "d": "400"}), + ("disconnect explicit", + {"op": "disconnect", "reason": 8}, + {"a": "8"}), + ("disconnect defaults", + {"op": "disconnect"}, + {"a": "19"}), + ("write explicit", + {"op": "write", "uuid": "0000ffe1-0000-1000-8000-00805f9b34fb", + "hex": "DEADbeef00", "response": False}, + {"uuid": "0000ffe1-0000-1000-8000-00805f9b34fb", "data": "deadbeef00", + "flag": "0"}), + ("write defaults", + {"op": "write", "uuid": "2a19"}, + {"uuid": "2a19", "data": "-", "flag": "1"}), + ("subscribe explicit", + {"op": "subscribe", "uuid": "2a19", "value": 2}, + {"uuid": "2a19", "a": "2"}), + ("subscribe defaults", + {"op": "subscribe", "uuid": "2a19"}, + {"uuid": "2a19", "a": "1"}), + ("confirm", + {"op": "confirm", "uuid": "2a19"}, + {"uuid": "2a19"}), + ("mtu explicit", + {"op": "mtu", "mtu": 247}, + {"a": "247"}), + ("mtu defaults", + {"op": "mtu"}, + {"a": "23"}), + ("connparams explicit", + {"op": "connparams", "interval": 80, "latency": 4, "timeout": 600}, + {"b": "80", "c": "4", "d": "600"}), + ("connparams defaults", + {"op": "connparams"}, + {"b": "24", "c": "0", "d": "400"}), + ("rssi explicit", + {"op": "rssi", "value": -95}, + {"a": "161"}), # 0xA1, the low byte of int8_t(-95) + ("rssi defaults", + {"op": "rssi"}, + {"a": "196"}), # 0xC4, the low byte of int8_t(-60) + ("auto_confirm explicit", + {"op": "auto_confirm", "enabled": False, "delay_ms": 250}, + {"flag": "0", "a": "250"}), + ("auto_confirm defaults", + {"op": "auto_confirm"}, + {"flag": "1", "a": "10"}), +] + +MALFORMED = [ + ("not json at all", b"hello world"), + ("truncated object", b'{"op":"mtu"'), + ("unterminated string", b'{"op":"mt'), + ("no op field", b'{"mtu":100}'), + ("op not a string", b'{"op":42}'), + ("unknown op", b'{"op":"launch_missiles"}'), + ("wrong type", b'{"op":"mtu","mtu":"517"}'), + ("out of range mtu", b'{"op":"mtu","mtu":99999}'), + ("huge number", b'{"op":"mtu","mtu":1e300}'), + ("odd length hex", b'{"op":"write","uuid":"2a19","hex":"abc"}'), + ("non hex hex", b'{"op":"write","uuid":"2a19","hex":"zzzz"}'), + ("missing uuid", b'{"op":"write","hex":"00"}'), + ("embedded raw NUL", b'{"op":"write","uuid":"2a\x0019","hex":"00"}'), + ("escaped NUL", b'{"op":"write","uuid":"a\\u0000b","hex":"00"}'), + ("nested too deep", b'{"op":"mtu","x":' + b"[" * 20 + b"]" * 20 + b"}"), + ("trailing bytes", b'{"op":"mtu"} garbage'), + ("empty object", b"{}"), + ("bare bracket", b"["), + ("lone brace", b"}"), + ("too many keys", b'{"op":"mtu",' + b",".join( + b'"k%d":%d' % (i, i) for i in range(40)) + b"}"), +] + + +def main(): + with tempfile.TemporaryDirectory() as tmp: + binary = os.path.join(tmp, "sim_ble_link_selftest") + build(binary) + harness = Harness(binary) + + print("== startup ==") + check(harness.wait_for("OK start(0) refused") is not None, + "start(0) returns false") + check(harness.wait_for("OK running after start(0) = false") is not None, + "running() false after a refused start") + check(harness.wait_for("OK stop() before start returned") is not None, + "stop() before start is a no-op") + check(harness.wait_for(f"READY {PORT}") is not None, "listener up") + + # Loopback only: the listener must not answer on a routable address. + print("\n== loopback only ==") + host_ip = None + try: + probe = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + probe.connect(("192.0.2.1", 9)) + host_ip = probe.getsockname()[0] + probe.close() + except OSError: + pass + if host_ip and not host_ip.startswith("127."): + try: + s = socket.create_connection((host_ip, PORT), timeout=1) + s.close() + check(False, "refuses a connect to the host LAN address", + f"{host_ip}:{PORT} accepted the connection") + except OSError as exc: + check(True, f"refuses {host_ip}:{PORT} ({exc.__class__.__name__})") + else: + print(" skip no routable address on this host") + + sock = connect() + reader = LineReader(sock) + + print("\n== every op, explicit and defaulted ==") + for label, payload, expected in OPS: + before = len(harness.sink_lines()) + sock.sendall((json.dumps(payload) + "\n").encode()) + deadline = time.time() + 2 + got = None + while time.time() < deadline: + lines = harness.sink_lines() + if len(lines) > before: + got = lines[before] + break + time.sleep(0.005) + if got is None: + check(False, label, "no SINK line") + continue + fields = parse_sink(got) + bad = {k: (v, fields.get(k)) for k, v in expected.items() + if fields.get(k) != v} + op_ok = fields.get("op") == payload["op"] + check(op_ok and not bad, f"{label} -> {got[5:]}", + f"op={fields.get('op')} mismatches={bad}") + + print("\n== framing ==") + before = len(harness.sink_lines()) + # \r\n, blank lines, two ops in one write, and one op split in half. + sock.sendall(b'{"op":"mtu","mtu":100}\r\n\n') + sock.sendall(b'{"op":"mtu","mtu":101}\n{"op":"mtu","mtu":102}\n') + sock.sendall(b'{"op":"mtu",') + time.sleep(0.15) + sock.sendall(b'"mtu":103}\n') + time.sleep(0.3) + got = [parse_sink(l).get("a") for l in harness.sink_lines()[before:]] + check(got == ["100", "101", "102", "103"], + "CRLF, blank line, batched writes and a split line", str(got)) + + print("\n== malformed lines ==") + for label, raw in MALFORMED: + sock.sendall(raw + b"\n") + line = reader.read_line(timeout=2) + ok = line is not None and '"ev":"error"' in line + check(ok, f"malformed: {label} -> {line}", str(line)) + before = len(harness.sink_lines()) + sock.sendall(b'{"op":"mtu","mtu":123}\n') + time.sleep(0.3) + after = harness.sink_lines() + check(len(after) > before and parse_sink(after[before]).get("a") == "123", + "still alive and framing after every malformed line") + + print("\n== over-long line ==") + reader.read_all(0.2) + huge = b'{"op":"write","uuid":"2a19","hex":"' + b"ab" * 40000 + b'"}' + check(len(huge) > 65536, f"test line is {len(huge)} bytes") + before = len(harness.sink_lines()) + sock.sendall(huge + b"\n") + line = reader.read_line(timeout=3) + check(line is not None and "line longer than 65536" in line, + f"over-long line answered with one error -> {line}", str(line)) + check(len(harness.sink_lines()) == before, + "over-long line produced no decoded op") + sock.sendall(b'{"op":"mtu","mtu":42}\n') + time.sleep(0.4) + after = harness.sink_lines() + check(len(after) > before and parse_sink(after[before]).get("a") == "42", + "framing recovered after the drop", str(after[before:])) + # A line of exactly the cap must be accepted: the cap is inclusive. + head = b'{"op":"write","uuid":"2a19","hex":"' + tail = b'"}' + pad = 65536 - len(head) - len(tail) + spaces = b"" + if pad % 2: # hex must stay an even run of digits + pad -= 1 + spaces = b" " # legal JSON whitespace, keeps the size + line = head + b"a" * pad + b'"' + spaces + b"}" + assert len(line) == 65536, len(line) + before = len(harness.sink_lines()) + sock.sendall(line + b"\n") + time.sleep(0.6) + after = harness.sink_lines() + fields = parse_sink(after[before]) if len(after) > before else {} + check(fields.get("op") == "write" and len(fields.get("data", "")) == pad, + f"a line of exactly {len(line)} bytes (the cap) is accepted", + str(fields.get("op")) + " datalen=" + str(len(fields.get("data", "")))) + + print("\n== second client refused ==") + second = connect() + second_reader = LineReader(second) + line = second_reader.read_line(timeout=2) + check(line is not None and '"ev":"error"' in line and "busy" in line, + f"second client gets an error line -> {line}", str(line)) + check(second_reader.read_line(timeout=2) is None, + "second client socket is then closed") + second.close() + before = len(harness.sink_lines()) + sock.sendall(b'{"op":"mtu","mtu":200}\n') + time.sleep(0.3) + after = harness.sink_lines() + check(len(after) > before and parse_sink(after[before]).get("a") == "200", + "the first client still works") + + print("\n== the client drops its socket ==") + before = len(harness.sink_lines()) + sock.close() + deadline = time.time() + 2 + got = None + while time.time() < deadline: + lines = harness.sink_lines() + if len(lines) > before: + got = parse_sink(lines[before]) + break + time.sleep(0.01) + check(got is not None and got.get("op") == "disconnect" and got.get("a") == "19", + f"a lost socket synthesizes disconnect reason 0x13 -> {got}", str(got)) + sock = connect() + reader = LineReader(sock) + before = len(harness.sink_lines()) + sock.sendall(b'{"op":"mtu","mtu":300}\n') + time.sleep(0.3) + after = harness.sink_lines() + check(len(after) > before and parse_sink(after[before]).get("a") == "300", + "the client slot is free again and a new client works") + + print("\n== concurrent emit ==") + reader.read_all(0.2) + per_thread = 300 + harness.command(f"emitstress {per_thread}") + collected = [] + deadline = time.time() + 15 + while time.time() < deadline and len(collected) < per_thread * 2: + line = reader.read_line(timeout=2) + if line is None: + break + collected.append(line) + check(len(collected) == per_thread * 2, + f"received {len(collected)} of {per_thread * 2} emitted lines") + broken = [l for l in collected if not re.fullmatch( + r'\{"ev":"stress","tag":"[AB]","n":\d+,"pad":"(A{200}|B{200})"\}', l)] + check(not broken, "every emitted line arrived intact and unspliced", + f"{len(broken)} broken, first: {broken[0][:120] if broken else ''}") + tags = {} + for line in collected: + obj = json.loads(line) + tags.setdefault(obj["tag"], []).append(obj["n"]) + check(sorted(tags.keys()) == ["A", "B"], f"both threads got through: " + f"{ {k: len(v) for k, v in tags.items()} }") + check(all(v == sorted(v) for v in tags.values()), + "each thread's lines stayed in its own order") + interleaved = any( + collected[i].startswith('{"ev":"stress","tag":"A"') != + collected[i + 1].startswith('{"ev":"stress","tag":"A"') + for i in range(len(collected) - 1)) + print(f" note the two threads did{'' if interleaved else ' not'} " + f"interleave at line granularity") + + print("\n== stop() with a client connected ==") + harness.command("stop") + line = harness.wait_for("STOPPED", timeout=5) + check(line is not None, "stop() returned", "no STOPPED line in 5 s") + if line: + ms = int(re.search(r"in (\d+) ms", line).group(1)) + check(ms < 1000, f"stop() took {ms} ms with a client connected") + check("running=false" in line, "running() false after stop()") + check(reader.read_line(timeout=2) is None, + "the client socket is closed by stop()") + sock.close() + check(harness.wait_for("EXIT", timeout=5) is not None, "clean exit") + try: + code = harness.proc.wait(timeout=5) + except subprocess.TimeoutExpired: + harness.proc.kill() + code = "timeout" + check(code == 0, f"process exit code {code}") + crashes = [l for l in harness.all_lines() + if "Segmentation" in l or "Aborted" in l or "sanitizer" in l + or "WARNING: ThreadSanitizer" in l or "runtime error" in l] + check(not crashes, "no crash output", str(crashes)) + + print(f"\n{checks - len(failures)}/{checks} checks passed") + if failures: + print("FAILED: " + "; ".join(failures)) + return 1 + print("ALL PASS") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 6467125e1082b27660312d514ac9408030c2a51e Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 14:58:18 +0200 Subject: [PATCH 03/13] docs: record the BLE shim transport specifics as verified Adds a "Transport specifics" section with the settled numbers: the 65536 byte line cap and what a longer line does, how stop() wakes a blocked reader and why the self pipe beats the alternatives, the loopback only bind and its reason, the second client refusal, the malformed line answer, emit()'s line atomicity, the synthetic disconnect on socket loss, and the full default and range table for every op field. Flips those claims to [verified] and names the command that showed them. The GATT half stays [contract]. Every claim carries a file:line. --- docs/ble-shim.md | 181 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 180 insertions(+), 1 deletion(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index 25e5aca..544f52b 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -11,7 +11,8 @@ compiled. Status: **seed**. This file was created before the implementation, so it states the frozen contract, not measured behaviour. Every claim below is marked `[contract]` (what the shim must do) or `[verified]` (observed running, with -the command that showed it). Nothing is `[verified]` yet. +the command that showed it). The transport half is `[verified]`: see +"Transport specifics" below. The GATT half is still `[contract]`. ## What it cannot answer @@ -82,6 +83,184 @@ dance exists. A timeout test turns it off. subscription state itself, because NimBLE fires no unsubscribe callback. - Advertising stops on connect and is not restarted by the shim. +## Transport specifics `[verified]` + +The socket, the reader thread and the line framing live in +`src/SimBleLink.cpp`. The codec lives in `src/SimBleProtocol.h` and +`src/SimBleProtocol.cpp`. Neither touches GATT or firmware code. + +Everything in this section was demonstrated by + +``` +python3 tests/sim_ble_link_selftest.py # 65 checks, all pass +SELFTEST_SANITIZE=1 python3 tests/sim_ble_link_selftest.py # ASan + UBSan +SELFTEST_SANITIZE=thread python3 tests/sim_ble_link_selftest.py # TSan +``` + +The gate builds `tests/sim_ble_link_selftest.cpp` against `SimBleLink.cpp` and +`SimBleProtocol.cpp` and nothing else: no simulator, no firmware, no GATT +model. So the transport is provable before the GATT half exists. The driver +sends every client op twice, once with explicit fields and once with all +fields absent, and the harness prints the decoded `SimBleEvent` for each. All +three runs come back clean. ThreadSanitizer dies on this kernel unless ASLR is +off, so the driver wraps the binary in `setarch -R` +(`tests/sim_ble_link_selftest.py:75-79`). + +### Loopback only + +The listener binds `INADDR_LOOPBACK` (`src/SimBleLink.cpp:336`), never +`INADDR_ANY`. This is a hazard decision, not a style one: the process on the +other end of this socket runs firmware command handling, so a LAN-reachable +port would hand the device model to anything on the network. The gate connects +to the host's own routable address and requires a refusal. + +Backlog is 4 (`src/SimBleLink.cpp:339`). A second client has to complete its +connect before it can be told to go away. + +### stop() wakes a blocked reader with a self-pipe + +The reader thread never blocks in `accept()` or `recv()`. It sits in `poll()` +over three fds: the listener, the connected client, and the read end of a +self-pipe (`src/SimBleLink.cpp:250-299`). `accept()` and `recv()` run only on +a fd `poll()` already reported readable, and the `recv()` uses `MSG_DONTWAIT` +(`src/SimBleLink.cpp:286`). + +`stop()` sets a stop flag, writes one byte into the self-pipe +(`src/SimBleLink.cpp:89-95`), shuts a connected client down, then joins +(`src/SimBleLink.cpp:368-378`). Measured: 0 to 2 ms with a client connected. + +Why the self-pipe and not the alternatives: + +- `shutdown()` on a **listening** socket is not portable. On Linux it does not + reliably wake an `accept()`. +- Closing a fd another thread is polling is a use after free waiting to + happen: the number can be handed to a new socket between the close and the + wake. +- A poll timeout loop would work but would either burn wakeups or add latency + to `stop()`. The pipe costs two fds and is exact. + +`stop()` is safe when the link was never started and safe twice in a row +(`src/SimBleLink.cpp:360-366`). `start(0)` returns false and leaves the +feature off. + +### The reader owns the client fd + +Only the reader thread closes the client socket. `emit()` writes under the +mutex and, on a write failure, calls `shutdown()` rather than `close()` +(`src/SimBleLink.cpp:427-431`). That makes the reader's `poll()` return and +keeps the teardown in one place. + +The client socket carries `SO_SNDTIMEO` of 5 s (`src/SimBleLink.cpp:47`, +`src/SimBleLink.cpp:236-241`). A client that stops reading cannot hang a +firmware thread inside `emit()` forever: the send fails, the link is dropped, +the simulator carries on. + +### emit() is one line, whole, under one mutex + +`emit()` frames and writes the whole line while holding the state mutex +(`src/SimBleLink.cpp:410-433`), so two threads cannot interleave halves of two +lines. Verified: two threads emitting 300 lines each, 600 lines received, every +one intact, each thread's lines in its own order, the two threads interleaved +at line granularity. + +An embedded `\n` or `\r` in the caller's JSON would inject a second frame, so +`emit()` replaces both with a space (`src/SimBleLink.cpp:414-417`). Callers do +not have to be careful. + +### A lost socket synthesizes a disconnect + +A client that drops its socket is a link that went away. The reader delivers +`op="disconnect"`, `a=0x13` to the sink so the GATT model does not keep +believing a central is connected (`src/SimBleLink.cpp:131-159`). The teardown +inside `stop()` does **not** synthesize one: `stop()` is not a link event and +the model is being destroyed anyway. + +### Line framing and the 65536 byte cap + +Bytes are buffered, split on `\n`, and a trailing `\r` is stripped, so +`\r\n` works (`src/SimBleLink.cpp:186-214`). A blank line is neither an op +nor an error. A line split across `recv` boundaries is reassembled. Several +lines in one `recv` are all handled. + +One line is capped at **65536 bytes**, newline excluded +(`src/SimBleProtocol.h:29`). The cap is inclusive: a line of exactly 65536 +bytes is accepted. Past it the buffer is thrown away, one `error` event goes +out, and every byte up to the next newline is discarded. Framing then +recovers: the next line parses normally. The cap is what bounds how much a +hostile or wedged client can make the reader buffer. + +Three smaller caps stop nonsense earlier: 32 keys per object +(`src/SimBleProtocol.h:36`), 64 characters per UUID +(`src/SimBleProtocol.h:33`), and 8 levels of nesting when skipping a value +that a client op should not have had (`src/SimBleProtocol.h:40`). + +### A malformed line answers with `error` and is dropped + +The parse is hand rolled: no JSON library is added for ten flat shapes. On any +failure the line produces one `error` event with a short reason and no sink +call (`src/SimBleLink.cpp:161-181`, `src/SimBleProtocol.cpp:584`). The link +stays up and the next line parses normally. + +Twenty malformed inputs are in the gate and each one answers with `error`, not +a crash: not JSON at all, a truncated object, an unterminated string, no `op` +field, `op` not a string, an unknown op, a wrong field type, a value out of +range, `1e300`, odd-length hex, non-hex hex, a missing `uuid`, a raw NUL inside +a string, an escaped `\u0000`, twenty levels of nesting, trailing bytes after +the object, `{}`, a bare `[`, a bare `}`, and forty keys. + +Two decisions worth naming: + +- **A raw control byte inside a string is rejected** + (`src/SimBleProtocol.cpp:185-189`). That is strict JSON, and it is what keeps + a binary blob arriving on the socket from being parsed as half an op. +- **A NUL is rejected even when escaped** (`src/SimBleProtocol.cpp:128-132`), + so every parsed string stays usable as a C string by the consumer. + +A wrong type is an error, never a silent fall back to the default: a client +sending `"mtu": "517"` has a bug worth seeing. + +### Defaults and accepted ranges + +The op table above names the fields. These are the values the parser applies +when a field is absent or `null`, and the ranges it accepts +(`src/SimBleProtocol.cpp:615-667`, ranges at `src/SimBleProtocol.cpp:477-480`). + +| field | default | accepted range | +|---|---|---| +| `mtu` | 23 | 23 to 517 | +| `interval` | 24 | 6 to 3200 | +| `latency` | 0 | 0 to 499 | +| `timeout` | 400 | 10 to 3200 | +| `reason` (disconnect) | 0x13 | 0 to 255 | +| `hex` (write) | empty | even run of hex digits, either case | +| `response` (write) | true | `true`/`false`, or 0/1 | +| `value` (subscribe) | 1 (notify) | 0 to 3 | +| `value` (rssi) | -60 | -128 to 127 | +| `enabled` (auto_confirm) | true | `true`/`false`, or 0/1 | +| `delay_ms` (auto_confirm) | 10 | 0 to 60000 | + +Out of range is an `error`, not a clamp, because the real stack refuses these +too. `rssi` is stored as the low byte of the `int8_t` +(`src/SimBleProtocol.cpp:657`), which is what the header's `a=value (cast to +int8_t by the consumer)` mapping expects: -60 arrives as `a=196`. + +Hex output is always lowercase; hex input accepts either case. + +### The second client is refused, then closed + +While a client is connected, the next one that connects is accepted, sent one +`error` line, and closed (`src/SimBleLink.cpp:224-234`). The first client is +untouched and keeps working. When the first client goes away the slot frees and +the next connect is served. + +### The port + +`crosspoint_simulator::ble::portFromEnv()` reads +`CROSSPOINT_SIM_BLE_PORT` and returns 0 when the variable is absent, empty, +zero or unparseable (`src/SimBleProtocol.cpp:674-685`). `start(0)` returns +false, so a bad value is the same as the feature being off, never a crash and +never a default port nobody asked for. + ## Threading model `[contract]` - `NimBLEDevice::init()` starts **two** threads: a socket reader and a **host From 6c77186fea2ace2ef8753be1720fe51d420c2296 Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 15:01:49 +0200 Subject: [PATCH 04/13] feat: add the NimBLE peripheral shim and its GATT model Header-compatible fake for NimBLE-Arduino's peripheral API, so the firmware's BLE code runs on the host. No NimBLE source is compiled. The NimBLE* classes forward; SimBleGatt holds the model, the host thread, the single per-connection indication slot and the outgoing event JSON. Callbacks are dispatched on the host thread, never inline on the caller's, because the firmware is built around the disconnect callback and the sync event it waits for sharing one task. Three fidelity points the shim reproduces rather than smooths over: the indication confirm is out of band and can be withheld, a second indicate() before a confirm clobbers the first and still returns true, and the client owns the MTU (default 23). Self-test: 40 checks, 0 failures, clean under ThreadSanitizer. The real firmware translation unit compiles against these headers. --- src/NimBLEAttValue.h | 39 ++ src/NimBLECharacteristic.h | 91 ++++ src/NimBLEConnInfo.h | 37 ++ src/NimBLEDevice.cpp | 106 +++++ src/NimBLEDevice.h | 155 +++++++ src/SimBleGatt.cpp | 761 +++++++++++++++++++++++++++++++++ src/SimBleGatt.h | 184 ++++++++ src/SimBleGattSelfTest.cpp | 311 ++++++++++++++ src/SimBleGattSelfTest.h | 27 ++ src/SimBleGattSelfTestStub.cpp | 76 ++++ src/host/ble_gap.h | 50 +++ 11 files changed, 1837 insertions(+) create mode 100644 src/NimBLEAttValue.h create mode 100644 src/NimBLECharacteristic.h create mode 100644 src/NimBLEConnInfo.h create mode 100644 src/NimBLEDevice.cpp create mode 100644 src/NimBLEDevice.h create mode 100644 src/SimBleGatt.cpp create mode 100644 src/SimBleGatt.h create mode 100644 src/SimBleGattSelfTest.cpp create mode 100644 src/SimBleGattSelfTest.h create mode 100644 src/SimBleGattSelfTestStub.cpp create mode 100644 src/host/ble_gap.h diff --git a/src/NimBLEAttValue.h b/src/NimBLEAttValue.h new file mode 100644 index 0000000..a56ceca --- /dev/null +++ b/src/NimBLEAttValue.h @@ -0,0 +1,39 @@ +#pragma once + +// Shim for NimBLE's NimBLEAttValue -- the value of one GATT attribute. +// +// The real class is a small heap buffer with a capacity policy and a pile of +// conversion helpers. The firmware uses three things from it: it takes one by +// value out of getValue(), then reads data() and size(). So that is what this +// is: a vector with those names on it. +// +// Returning by value is deliberate, not an oversight of the shim. The real +// getValue() copies too, and the firmware's transfer path is written around +// that copy costing one malloc/memcpy/free per chunk. + +#include +#include +#include +#include + +class NimBLEAttValue { + public: + NimBLEAttValue() = default; + NimBLEAttValue(const uint8_t *data, size_t len) { + if (data != nullptr && len > 0) m_value.assign(data, data + len); + } + explicit NimBLEAttValue(std::vector value) + : m_value(std::move(value)) {} + + const uint8_t *data() const { return m_value.data(); } + size_t size() const { return m_value.size(); } + size_t length() const { return m_value.size(); } + bool empty() const { return m_value.empty(); } + + const uint8_t *begin() const { return m_value.data(); } + const uint8_t *end() const { return m_value.data() + m_value.size(); } + uint8_t operator[](size_t i) const { return m_value[i]; } + + private: + std::vector m_value; +}; diff --git a/src/NimBLECharacteristic.h b/src/NimBLECharacteristic.h new file mode 100644 index 0000000..43e19cf --- /dev/null +++ b/src/NimBLECharacteristic.h @@ -0,0 +1,91 @@ +#pragma once + +// Shim for NimBLE's NimBLECharacteristic and its callback interface. +// +// The characteristic is a value plus a subscription plus a callback pointer. +// All three live in this object; SimBleGatt is a friend and owns the mutex +// that guards them, because the reader thread, the host thread and the +// activity thread all touch them. +// +// **Callback dispatch is never inline.** onWrite, onStatus and onSubscribe are +// called by SimBleGatt's host thread, never by the client op that caused them +// and never by indicate(). See docs/ble-shim.md, "Threading model". + +#include +#include +#include +#include + +#include "NimBLEAttValue.h" +#include "NimBLEConnInfo.h" + +// Real NimBLE spells these as a namespace of constants, so a firmware +// expression like `WRITE | NOTIFY | INDICATE` is an integer. Values are +// NimBLE's own, so a props number in an emitted `gatt` event means the same +// thing here as in a NimBLE header. +namespace NIMBLE_PROPERTY { +static constexpr uint32_t READ = 0x0001; +static constexpr uint32_t WRITE = 0x0008; +static constexpr uint32_t NOTIFY = 0x0010; +static constexpr uint32_t INDICATE = 0x0020; +} // namespace NIMBLE_PROPERTY + +class NimBLECharacteristic; +class SimBleGatt; + +class NimBLECharacteristicCallbacks { + public: + virtual ~NimBLECharacteristicCallbacks() = default; + + // A central wrote this characteristic. Read the bytes with getValue(). + virtual void onWrite(NimBLECharacteristic *, NimBLEConnInfo &) {} + + // One indication finished. `code` is BLE_HS_EDONE when the peer confirmed. + // Fires once per accepted indicate(), out of band, on the host thread. + virtual void onStatus(NimBLECharacteristic *, NimBLEConnInfo &, int) {} + + // A central changed its subscription. bit0 is notify, bit1 is indicate. + // Never fired for a disconnect -- NimBLE does not, so neither does this. + virtual void onSubscribe(NimBLECharacteristic *, NimBLEConnInfo &, + uint16_t) {} +}; + +class NimBLECharacteristic { + public: + NimBLECharacteristic(const char *uuid, uint32_t properties) + : m_uuid(uuid != nullptr ? uuid : ""), m_properties(properties) {} + + void setCallbacks(NimBLECharacteristicCallbacks *callbacks); + + // A copy of the last value a central wrote. Returned by value, same as the + // real API. + NimBLEAttValue getValue(); + + // Puts `len` bytes into the connection's single pending indication slot. + // + // **True means the slot accepted the payload, not that the peer got it.** + // The confirm arrives later through onStatus. A second call before that + // confirm overwrites the first and still returns true -- measured on real + // hardware, and the shim reproduces it rather than queueing politely. The + // overwrite emits a `clobber` event so it is observable instead of silent. + // + // False means the real stack would have refused: stack down, no central + // connected, nobody subscribed to this characteristic, this characteristic + // cannot notify or indicate, or an empty payload. + bool indicate(const uint8_t *data, size_t len); + + // Shim-only accessors. Not part of the NimBLE API; the emitted `gatt` event + // and the self-test read them. + const std::string &shimUuid() const { return m_uuid; } + uint32_t shimProperties() const { return m_properties; } + + private: + friend class SimBleGatt; + + std::string m_uuid; + uint32_t m_properties = 0; + NimBLECharacteristicCallbacks *m_callbacks = nullptr; + // Guarded by SimBleGatt's mutex. + std::vector m_value; + uint16_t m_subValue = 0; +}; diff --git a/src/NimBLEConnInfo.h b/src/NimBLEConnInfo.h new file mode 100644 index 0000000..76c09f3 --- /dev/null +++ b/src/NimBLEConnInfo.h @@ -0,0 +1,37 @@ +#pragma once + +// Shim for NimBLE's NimBLEConnInfo -- what one connection negotiated. +// +// Every firmware callback in the shim's contract takes one of these by +// reference. The real class wraps ble_gap_conn_desc and exposes peer address, +// bonding state and more; the firmware reads four numbers off it, so those +// four are what exist here. +// +// The interval, latency and timeout are the central's choices, not the +// peripheral's: the client sets them with the `connect` and `connparams` ops +// (docs/ble-shim.md, "Client to simulator"). Interval is in 1.25 ms units, +// timeout in 10 ms units -- the same units the firmware's logs assume. + +#include + +#include "host/ble_gap.h" + +class NimBLEConnInfo { + public: + NimBLEConnInfo() = default; + NimBLEConnInfo(uint16_t handle, uint16_t interval, uint16_t latency, + uint16_t timeout) + : m_handle(handle), m_interval(interval), m_latency(latency), + m_timeout(timeout) {} + + uint16_t getConnHandle() const { return m_handle; } + uint16_t getConnInterval() const { return m_interval; } + uint16_t getConnLatency() const { return m_latency; } + uint16_t getConnTimeout() const { return m_timeout; } + + private: + uint16_t m_handle = BLE_HS_CONN_HANDLE_NONE; + uint16_t m_interval = 0; + uint16_t m_latency = 0; + uint16_t m_timeout = 0; +}; diff --git a/src/NimBLEDevice.cpp b/src/NimBLEDevice.cpp new file mode 100644 index 0000000..6ddb6d7 --- /dev/null +++ b/src/NimBLEDevice.cpp @@ -0,0 +1,106 @@ +#include "NimBLEDevice.h" + +#include "SimBleGatt.h" + +// Every method here is a forwarder. The model, the threads and the wire +// protocol live in SimBleGatt; this file only exists so the firmware's +// #include compiles and links. + +// --- NimBLEDevice ----------------------------------------------------------- + +bool NimBLEDevice::init(const char *deviceName) { + return SimBleGatt::get().init(deviceName); +} + +void NimBLEDevice::deinit(bool clearAll) { + SimBleGatt::get().deinit(clearAll); +} + +bool NimBLEDevice::isInitialized() { return SimBleGatt::get().initialized(); } + +NimBLEServer *NimBLEDevice::createServer() { return SimBleGatt::get().server(); } + +NimBLEAdvertising *NimBLEDevice::getAdvertising() { + return SimBleGatt::get().advertising(); +} + +void NimBLEDevice::setSecurityAuth(bool bonding, bool mitm, bool sc) { + SimBleGatt::get().setSecurityAuth(bonding, mitm, sc); +} + +// --- NimBLEServer ----------------------------------------------------------- + +NimBLEService *NimBLEServer::createService(const char *uuid) { + return SimBleGatt::get().createService(uuid); +} + +void NimBLEServer::setCallbacks(NimBLEServerCallbacks *callbacks, bool) { + // The delete flag is ignored: the shim never owns the pointer. The firmware + // passes false and registers a static object, so nothing is lost. + SimBleGatt::get().setServerCallbacks(callbacks); +} + +bool NimBLEServer::start() { return SimBleGatt::get().startServer(); } + +void NimBLEServer::updateConnParams(uint16_t handle, uint16_t minInterval, + uint16_t maxInterval, uint16_t latency, + uint16_t timeout) { + SimBleGatt::get().requestConnParams(handle, minInterval, maxInterval, latency, + timeout); +} + +// --- NimBLEService ---------------------------------------------------------- + +NimBLECharacteristic *NimBLEService::createCharacteristic(const char *uuid, + uint32_t properties) { + return SimBleGatt::get().createCharacteristic(this, uuid, properties); +} + +bool NimBLEService::start() { return true; } + +// --- NimBLECharacteristic --------------------------------------------------- + +void NimBLECharacteristic::setCallbacks( + NimBLECharacteristicCallbacks *callbacks) { + SimBleGatt::get().setCharacteristicCallbacks(this, callbacks); +} + +NimBLEAttValue NimBLECharacteristic::getValue() { + return SimBleGatt::get().characteristicValue(this); +} + +bool NimBLECharacteristic::indicate(const uint8_t *data, size_t len) { + return SimBleGatt::get().indicate(this, data, len); +} + +// --- NimBLEAdvertising ------------------------------------------------------ + +bool NimBLEAdvertising::start() { return SimBleGatt::get().advertisingStart(); } + +void NimBLEAdvertising::stop() { SimBleGatt::get().advertisingStop(); } + +void NimBLEAdvertising::setName(const char *name) { + SimBleGatt::get().setAdvertisingName(name); +} + +void NimBLEAdvertising::addServiceUUID(const char *uuid) { + SimBleGatt::get().setAdvertisingServiceUuid(uuid); +} + +void NimBLEAdvertising::enableScanResponse(bool enable) { + SimBleGatt::get().setAdvertisingScanResponse(enable); +} + +void NimBLEAdvertising::setMinInterval(uint16_t interval) { + SimBleGatt::get().setAdvertisingMinInterval(interval); +} + +void NimBLEAdvertising::setMaxInterval(uint16_t interval) { + SimBleGatt::get().setAdvertisingMaxInterval(interval); +} + +// --- host/ble_gap.h --------------------------------------------------------- + +extern "C" int ble_gap_conn_rssi(uint16_t conn_handle, int8_t *out_rssi) { + return SimBleGatt::get().connRssi(conn_handle, out_rssi); +} diff --git a/src/NimBLEDevice.h b/src/NimBLEDevice.h new file mode 100644 index 0000000..40dcca5 --- /dev/null +++ b/src/NimBLEDevice.h @@ -0,0 +1,155 @@ +#pragma once + +// Shim for NimBLE-Arduino's NimBLEDevice.h. +// +// **Shim** means header-compatible fake: the same C++ API the firmware +// compiles against, implemented over a TCP line protocol instead of a radio. +// No NimBLE source is compiled. docs/ble-shim.md has the wire protocol, the +// threading model and what this cannot answer. +// +// Only the peripheral (GATT server) side exists. There is no NimBLEClient, no +// NimBLEScan and no central role, because the firmware has none. +// +// Every firmware callback runs on SimBleGatt's host thread. That is the whole +// point: the firmware is written around "the disconnect callback runs on the +// NimBLE host task, and the sync event it waits for is dispatched on that same +// task", and inline dispatch would make that class of deadlock impossible to +// reproduce. + +#include +#include +#include +#include + +#include "NimBLEAttValue.h" +#include "NimBLECharacteristic.h" +#include "NimBLEConnInfo.h" +#include "host/ble_gap.h" + +class NimBLEServer; +class SimBleGatt; + +class NimBLEServerCallbacks { + public: + virtual ~NimBLEServerCallbacks() = default; + + // A central connected. Advertising is already down by the time this runs: + // NimBLE stops it for the duration of the connection and does not resume it. + virtual void onConnect(NimBLEServer *, NimBLEConnInfo &) {} + + // The link dropped. `reason` is the client's `disconnect` reason field. + virtual void onDisconnect(NimBLEServer *, NimBLEConnInfo &, int) {} + + // The central changed the connection parameters, or answered a request the + // firmware made with updateConnParams(). + virtual void onConnParamsUpdate(NimBLEConnInfo &) {} + + // The central set the MTU. It drives the firmware's payload arithmetic, so + // the client owns this number; the default is the pessimistic 23. + virtual void onMTUChange(uint16_t, NimBLEConnInfo &) {} +}; + +class NimBLEService { + public: + explicit NimBLEService(const char *uuid) + : m_uuid(uuid != nullptr ? uuid : "") {} + + NimBLECharacteristic *createCharacteristic(const char *uuid, + uint32_t properties); + + // Deprecated in NimBLE 2.x and unused by the firmware: services start when + // the server starts. Kept because the shim's contract names it. Always true. + bool start(); + + const std::string &shimUuid() const { return m_uuid; } + + private: + friend class SimBleGatt; + + std::string m_uuid; + std::vector m_characteristics; +}; + +class NimBLEServer { + public: + NimBLEService *createService(const char *uuid); + + // `deleteCallbacks` is honoured to the extent the shim can: it never deletes + // the pointer. The firmware passes false and registers a static object. + void setCallbacks(NimBLEServerCallbacks *callbacks, + bool deleteCallbacks = true); + + // Builds the GATT table and emits the `gatt` event. False when the stack is + // down. Not in the shim's original contract list -- the firmware calls it + // (BlePositionServer.cpp:314) in place of the deprecated + // NimBLEService::start(). + bool start(); + + // Asks the central for new connection parameters. A request, not a command: + // it emits `connparams_request` and fires no callback. The central answers + // with a `connparams` op, which is what fires onConnParamsUpdate. + void updateConnParams(uint16_t handle, uint16_t minInterval, + uint16_t maxInterval, uint16_t latency, + uint16_t timeout); + + private: + friend class SimBleGatt; + NimBLEServer() = default; +}; + +class NimBLEAdvertising { + public: + // True once advertising is up. A start() while already advertising is a + // no-op success and emits nothing -- real NimBLE behaves that way + // (NimBLEAdvertising.cpp:194-197), which is why the firmware's slow-interval + // switch does stop() before start(). + bool start(); + void stop(); + + void setName(const char *name); + void addServiceUUID(const char *uuid); + void enableScanResponse(bool enable); + + // Both in 0.625 ms units. 0 on both means "let the host pick", and the host + // picks BLE_GAP_ADV_FAST_INTERVAL1. + void setMinInterval(uint16_t interval); + void setMaxInterval(uint16_t interval); + + private: + friend class SimBleGatt; + NimBLEAdvertising() = default; + + // Guarded by SimBleGatt's mutex. + std::string m_name; + std::string m_serviceUuid; + bool m_scanResponse = false; + uint16_t m_minInterval = 0; + uint16_t m_maxInterval = 0; +}; + +class NimBLEDevice { + public: + // Starts the host thread and the transport reader thread, and emits + // `stack up`. True even with no client and no listener: a simulator run + // without BLE must behave exactly as it did before the shim existed. + static bool init(const char *deviceName); + + // Stops advertising, emits `stack down`, joins both threads. `clearAll` + // deletes the server, services and characteristics, same as the real API -- + // which is why the firmware nulls its own pointers before calling this. + static void deinit(bool clearAll = false); + + static bool isInitialized(); + + // One server per stack. Repeated calls return the same object. + static NimBLEServer *createServer(); + + // One advertising object per stack, created on first use. It outlives a + // start()/stop() pair, so interval bounds set on it persist -- the firmware + // depends on that. + static NimBLEAdvertising *getAdvertising(); + + // Recorded and otherwise ignored. There is no pairing in the shim, and the + // firmware asks for none. + static void setSecurityAuth(bool bonding, bool mitm, bool sc); +}; diff --git a/src/SimBleGatt.cpp b/src/SimBleGatt.cpp new file mode 100644 index 0000000..bcd1bf3 --- /dev/null +++ b/src/SimBleGatt.cpp @@ -0,0 +1,761 @@ +#include "SimBleGatt.h" + +#include + +#include "NimBLEDevice.h" +#include "SimBleProtocol.h" + +// JSON is hand-rolled on purpose: the shim emits seven event shapes and adding +// a JSON dependency to the simulator to write them would cost more than it +// saves. Every field is a number, a bool, a hex string or a UUID. +namespace { + +std::string jsonEscape(const std::string &in) { + std::string out; + out.reserve(in.size() + 2); + for (const char c : in) { + switch (c) { + case '"': + out += "\\\""; + break; + case '\\': + out += "\\\\"; + break; + case '\n': + out += "\\n"; + break; + case '\r': + out += "\\r"; + break; + case '\t': + out += "\\t"; + break; + default: + if (static_cast(c) < 0x20) { + char buf[8]; + snprintf(buf, sizeof(buf), "\\u%04x", static_cast(c)); + out += buf; + } else { + out += c; + } + } + } + return out; +} + +std::string toHex(const std::vector &bytes) { + static const char *digits = "0123456789abcdef"; + std::string out; + out.reserve(bytes.size() * 2); + for (const uint8_t b : bytes) { + out += digits[b >> 4]; + out += digits[b & 0x0f]; + } + return out; +} + +void emitLine(const std::string &json) { SimBleLink::get().emit(json.c_str()); } + +} // namespace + +SimBleGatt &SimBleGatt::get() { + static SimBleGatt instance; + return instance; +} + +// --- stack lifecycle -------------------------------------------------------- + +bool SimBleGatt::init(const char *deviceName) { + { + std::lock_guard lock(m_mutex); + // Real NimBLE returns true for a second init. The firmware self-heals a + // partial teardown by deiniting first anyway, so this path is a safety net, + // not the normal route. + if (m_initialized) return true; + + m_deviceName = deviceName != nullptr ? deviceName : ""; + m_initialized = true; + m_advertisingUp = false; + clearConnectionLocked(); + m_rssi = 0; + m_autoConfirm = true; + m_autoConfirmDelayMs = kDefaultAutoConfirmDelayMs; + m_queue.clear(); + m_hostRunning = true; + m_hostThread = std::thread(&SimBleGatt::hostThreadMain, this); + } + + // Sink first, listener second: a client op must not arrive before there is + // somewhere to put it. + SimBleLink::get().setSink(&SimBleGatt::sinkTrampoline, this); + // A false return means the feature is off (no port set, or the bind failed). + // That is not an init failure: a simulator run with no BLE client must behave + // exactly as it did before the shim existed. + // The port comes from CROSSPOINT_SIM_BLE_PORT, decoded by the transport's own + // helper. One reader of that variable, not two. + SimBleLink::get().start(crosspoint_simulator::ble::portFromEnv()); + + emitLine("{\"ev\":\"stack\",\"state\":\"up\"}"); + return true; +} + +void SimBleGatt::deinit(bool clearAll) { + std::thread hostThread; + { + std::lock_guard lock(m_mutex); + if (!m_initialized) return; + // Joining the host thread from the host thread is a self-join and would + // abort. Real NimBLE deinit from the host task is equally broken; the + // firmware never does it. Say so rather than crash. + if (m_hostRunning && std::this_thread::get_id() == m_hostThreadId) { + emitErrorLocked("deinit called from the host thread; ignored"); + return; + } + } + + advertisingStop(); + emitLine("{\"ev\":\"stack\",\"state\":\"down\"}"); + + SimBleLink::get().setSink(nullptr, nullptr); + SimBleLink::get().stop(); + + { + std::lock_guard lock(m_mutex); + m_hostRunning = false; + hostThread = std::move(m_hostThread); + } + m_queueCv.notify_all(); + if (hostThread.joinable()) hostThread.join(); + + std::lock_guard lock(m_mutex); + m_queue.clear(); + clearConnectionLocked(); + m_serverCallbacks = nullptr; + m_initialized = false; + if (!clearAll) return; + + // clearAll deletes the table, same as the real API -- which is why the + // firmware nulls its own characteristic pointers before calling this. + for (NimBLECharacteristic *characteristic : m_characteristics) { + delete characteristic; + } + m_characteristics.clear(); + for (NimBLEService *service : m_services) delete service; + m_services.clear(); + delete m_server; + m_server = nullptr; + delete m_advertising; + m_advertising = nullptr; +} + +bool SimBleGatt::initialized() const { + std::lock_guard lock(m_mutex); + return m_initialized; +} + +void SimBleGatt::setSecurityAuth(bool, bool, bool) { + // Recorded nowhere and acted on nowhere: there is no pairing here, and the + // firmware asks for none (bonding, mitm and sc all false). +} + +// --- the table -------------------------------------------------------------- + +NimBLEServer *SimBleGatt::server() { + std::lock_guard lock(m_mutex); + if (m_server == nullptr) m_server = new NimBLEServer(); + return m_server; +} + +NimBLEAdvertising *SimBleGatt::advertising() { + std::lock_guard lock(m_mutex); + if (m_advertising == nullptr) m_advertising = new NimBLEAdvertising(); + return m_advertising; +} + +NimBLEService *SimBleGatt::createService(const char *uuid) { + std::lock_guard lock(m_mutex); + NimBLEService *service = new NimBLEService(uuid); + m_services.push_back(service); + return service; +} + +NimBLECharacteristic *SimBleGatt::createCharacteristic(NimBLEService *service, + const char *uuid, + uint32_t properties) { + std::lock_guard lock(m_mutex); + NimBLECharacteristic *characteristic = + new NimBLECharacteristic(uuid, properties); + m_characteristics.push_back(characteristic); + if (service != nullptr) service->m_characteristics.push_back(characteristic); + return characteristic; +} + +void SimBleGatt::setServerCallbacks(NimBLEServerCallbacks *callbacks) { + std::lock_guard lock(m_mutex); + m_serverCallbacks = callbacks; +} + +void SimBleGatt::setCharacteristicCallbacks( + NimBLECharacteristic *characteristic, + NimBLECharacteristicCallbacks *callbacks) { + std::lock_guard lock(m_mutex); + if (characteristic != nullptr) characteristic->m_callbacks = callbacks; +} + +void SimBleGatt::setAdvertisingName(const char *name) { + std::lock_guard lock(m_mutex); + if (m_advertising != nullptr) m_advertising->m_name = name != nullptr ? name : ""; +} + +void SimBleGatt::setAdvertisingServiceUuid(const char *uuid) { + std::lock_guard lock(m_mutex); + if (m_advertising != nullptr) { + m_advertising->m_serviceUuid = uuid != nullptr ? uuid : ""; + } +} + +void SimBleGatt::setAdvertisingScanResponse(bool enable) { + std::lock_guard lock(m_mutex); + if (m_advertising != nullptr) m_advertising->m_scanResponse = enable; +} + +void SimBleGatt::setAdvertisingMinInterval(uint16_t interval) { + std::lock_guard lock(m_mutex); + if (m_advertising != nullptr) m_advertising->m_minInterval = interval; +} + +void SimBleGatt::setAdvertisingMaxInterval(uint16_t interval) { + std::lock_guard lock(m_mutex); + if (m_advertising != nullptr) m_advertising->m_maxInterval = interval; +} + +NimBLEAttValue +SimBleGatt::characteristicValue(NimBLECharacteristic *characteristic) { + std::lock_guard lock(m_mutex); + if (characteristic == nullptr) return NimBLEAttValue(); + return NimBLEAttValue(characteristic->m_value); +} + +bool SimBleGatt::startServer() { + std::lock_guard lock(m_mutex); + if (!m_initialized) return false; + // One `gatt` line per service. The firmware builds one; a second would get + // its own line rather than being folded into the first. + for (const NimBLEService *service : m_services) { + std::string line = "{\"ev\":\"gatt\",\"service\":\""; + line += jsonEscape(service->m_uuid); + line += "\",\"chars\":["; + bool first = true; + for (const NimBLECharacteristic *characteristic : + service->m_characteristics) { + if (!first) line += ","; + first = false; + line += "{\"uuid\":\""; + line += jsonEscape(characteristic->m_uuid); + line += "\",\"props\":"; + line += std::to_string(characteristic->m_properties); + line += "}"; + } + line += "]}"; + emitLine(line); + } + return true; +} + +// --- a live link ------------------------------------------------------------ + +void SimBleGatt::requestConnParams(uint16_t, uint16_t minInterval, + uint16_t maxInterval, uint16_t latency, + uint16_t timeout) { + std::lock_guard lock(m_mutex); + if (!m_initialized || !m_connected) { + emitErrorLocked("updateConnParams with no connection"); + return; + } + // A request, not a change. Nothing moves until the central answers with a + // `connparams` op, which is what fires onConnParamsUpdate. + std::string line = "{\"ev\":\"connparams_request\",\"min\":"; + line += std::to_string(minInterval); + line += ",\"max\":" + std::to_string(maxInterval); + line += ",\"latency\":" + std::to_string(latency); + line += ",\"timeout\":" + std::to_string(timeout); + line += "}"; + emitLine(line); +} + +bool SimBleGatt::indicate(NimBLECharacteristic *characteristic, + const uint8_t *data, size_t len) { + bool clobbered = false; + std::string clobberedUuid; + std::vector clobberedPayload; + std::string uuid; + std::vector payload; + bool autoConfirm = false; + uint32_t autoConfirmDelayMs = 0; + uint32_t seq = 0; + + { + std::lock_guard lock(m_mutex); + if (!m_initialized || characteristic == nullptr) return false; + if (data == nullptr || len == 0) return false; + // Refusals the real stack makes. Each one is a false return with no event + // and no callback. + if (!m_connected) return false; + if (characteristic->m_subValue == 0) return false; + if ((characteristic->m_properties & + (NIMBLE_PROPERTY::INDICATE | NIMBLE_PROPERTY::NOTIFY)) == 0) { + return false; + } + + // One slot per connection, not per characteristic: the firmware's transfer + // status channel parks a line precisely because the command channel can be + // holding it (BlePositionServer.cpp:956-958). + // + // An overwrite is what real hardware does. 18 back-to-back indicate() + // calls all returned true and the peer saw the first and the last, so a + // second call before a confirm takes the slot and the previous payload is + // gone. Not queued -- queueing would hide the bug this shim exists to + // reproduce. + clobbered = m_pending; + if (clobbered) { + clobberedUuid = m_pendingUuid; + clobberedPayload = m_pendingPayload; + } + m_pending = true; + m_pendingUuid = characteristic->m_uuid; + m_pendingPayload.assign(data, data + len); + ++m_pendingSeq; + + uuid = m_pendingUuid; + payload = m_pendingPayload; + seq = m_pendingSeq; + autoConfirm = m_autoConfirm; + autoConfirmDelayMs = m_autoConfirmDelayMs; + } + + if (clobbered) { + std::string line = "{\"ev\":\"clobber\",\"uuid\":\""; + line += jsonEscape(clobberedUuid); + line += "\",\"dropped_hex\":\"" + toHex(clobberedPayload) + "\"}"; + emitLine(line); + } + std::string line = "{\"ev\":\"indicate\",\"uuid\":\""; + line += jsonEscape(uuid); + line += "\",\"hex\":\"" + toHex(payload) + "\"}"; + emitLine(line); + + if (autoConfirm) { + // The confirm is out of band even when the shim generates it: it goes + // through the host thread's queue with a delay, so indicate() has always + // returned before onStatus can run. + HostEvent event; + event.kind = HostEvent::Kind::Confirm; + event.uuid = uuid; + event.seq = seq; + event.flag = true; // shim-generated, so a stale one is dropped silently + event.due = std::chrono::steady_clock::now() + + std::chrono::milliseconds(autoConfirmDelayMs); + enqueue(std::move(event)); + } + return true; +} + +bool SimBleGatt::advertisingStart() { + std::lock_guard lock(m_mutex); + if (!m_initialized) return false; + // A no-op success while already advertising, same as real NimBLE + // (NimBLEAdvertising.cpp:194-197). Nothing is emitted, because nothing + // changed -- which is why the firmware's slow-interval switch stops first. + if (m_advertisingUp) return true; + m_advertisingUp = true; + emitAdvertisingLocked(); + return true; +} + +void SimBleGatt::advertisingStop() { + std::lock_guard lock(m_mutex); + if (!m_advertisingUp) return; + m_advertisingUp = false; + emitAdvertisingLocked(); +} + +int SimBleGatt::connRssi(uint16_t handle, int8_t *out) const { + std::lock_guard lock(m_mutex); + if (!m_connected || handle != m_connHandle) return BLE_HS_ENOTCONN; + if (out != nullptr) *out = m_rssi; + return 0; +} + +// --- the reader thread ------------------------------------------------------ + +void SimBleGatt::sinkTrampoline(void *ctx, const SimBleEvent &event) { + static_cast(ctx)->onReaderEvent(event); +} + +void SimBleGatt::onReaderEvent(const SimBleEvent &event) { + // Runs on the reader thread. Enqueue only -- not one firmware callback, and + // not one state change, so ordering is whatever the client sent and nothing + // races the host thread. + HostEvent out; + out.uuid = event.uuid; + out.op = event.op; + out.data = event.data; + out.a = event.a; + out.b = event.b; + out.c = event.c; + out.d = event.d; + out.flag = event.flag; + + if (event.op == "connect") { + out.kind = HostEvent::Kind::Connect; + } else if (event.op == "disconnect") { + out.kind = HostEvent::Kind::Disconnect; + } else if (event.op == "write") { + out.kind = HostEvent::Kind::Write; + } else if (event.op == "subscribe") { + out.kind = HostEvent::Kind::Subscribe; + } else if (event.op == "confirm") { + out.kind = HostEvent::Kind::Confirm; + out.flag = false; // client-driven: confirms whatever is pending + } else if (event.op == "mtu") { + out.kind = HostEvent::Kind::Mtu; + } else if (event.op == "connparams") { + out.kind = HostEvent::Kind::ConnParams; + } else if (event.op == "rssi") { + out.kind = HostEvent::Kind::Rssi; + } else if (event.op == "auto_confirm") { + out.kind = HostEvent::Kind::AutoConfirm; + } else { + out.kind = HostEvent::Kind::Unknown; + } + enqueue(std::move(out)); +} + +void SimBleGatt::enqueue(HostEvent event) { + { + std::lock_guard lock(m_mutex); + if (!m_hostRunning) return; + m_queue.push_back(std::move(event)); + } + m_queueCv.notify_all(); +} + +// --- the host thread -------------------------------------------------------- + +void SimBleGatt::hostThreadMain() { + { + std::lock_guard lock(m_mutex); + m_hostThreadId = std::this_thread::get_id(); + } + + std::unique_lock lock(m_mutex); + while (m_hostRunning) { + const auto now = std::chrono::steady_clock::now(); + // First event that is due. Events with a future due time (an auto-confirm + // delay) do not block the ones behind them -- in a real stack the confirm + // genuinely arrives late while other traffic keeps flowing. + auto ready = m_queue.end(); + auto earliest = std::chrono::steady_clock::time_point::max(); + for (auto it = m_queue.begin(); it != m_queue.end(); ++it) { + if (it->due <= now) { + ready = it; + break; + } + if (it->due < earliest) earliest = it->due; + } + + if (ready == m_queue.end()) { + if (earliest == std::chrono::steady_clock::time_point::max()) { + m_idleCv.notify_all(); + m_queueCv.wait(lock); + } else { + m_queueCv.wait_until(lock, earliest); + } + continue; + } + + HostEvent event = std::move(*ready); + m_queue.erase(ready); + m_dispatching = true; + lock.unlock(); + // The mutex is down for the whole dispatch. Every callback below calls + // back into this object -- the firmware's onDisconnect calls + // advertising->start() -- and holding it would deadlock on the first one. + dispatch(event); + lock.lock(); + m_dispatching = false; + m_idleCv.notify_all(); + } + m_idleCv.notify_all(); +} + +void SimBleGatt::waitIdle() { + std::unique_lock lock(m_mutex); + m_idleCv.wait(lock, [this] { + return (m_queue.empty() && !m_dispatching) || !m_hostRunning; + }); +} + +void SimBleGatt::dispatch(HostEvent &event) { + // Snapshot what the callback needs under the lock, drop the lock, call. + NimBLEServerCallbacks *serverCallbacks = nullptr; + NimBLECharacteristicCallbacks *charCallbacks = nullptr; + NimBLECharacteristic *characteristic = nullptr; + NimBLEServer *serverObject = nullptr; + NimBLEConnInfo info; + int intArg = 0; + uint16_t shortArg = 0; + bool alsoMtu = false; + bool alsoConnParams = false; + + { + std::lock_guard lock(m_mutex); + if (!m_initialized) { + emitErrorLocked("client op while the stack is down: " + event.op); + return; + } + serverObject = m_server; + + switch (event.kind) { + case HostEvent::Kind::Connect: { + if (m_connected) { + emitErrorLocked("connect while already connected"); + return; + } + m_connected = true; + m_connHandle = m_nextConnHandle++; + if (m_nextConnHandle == BLE_HS_CONN_HANDLE_NONE) m_nextConnHandle = 1; + m_mtu = event.a != 0 ? static_cast(event.a) : kDefaultMtu; + m_interval = + event.b != 0 ? static_cast(event.b) : kDefaultInterval; + m_latency = static_cast(event.c); + m_timeout = static_cast(event.d); + // Advertising stops on connect and the shim does not resume it, because + // NimBLE does not. The firmware restarts it from onDisconnect. + if (m_advertisingUp) { + m_advertisingUp = false; + emitAdvertisingLocked(); + } + serverCallbacks = m_serverCallbacks; + info = connInfoLocked(); + shortArg = m_mtu; + alsoMtu = true; + alsoConnParams = true; + break; + } + case HostEvent::Kind::Disconnect: { + if (!m_connected) { + emitErrorLocked("disconnect with no connection"); + return; + } + // The dying connection's info, as the real callback gets it. + info = connInfoLocked(); + intArg = static_cast(event.a); + serverCallbacks = m_serverCallbacks; + clearConnectionLocked(); + break; + } + case HostEvent::Kind::Write: { + if (!m_connected) { + emitErrorLocked("write with no connection"); + return; + } + characteristic = findCharLocked(event.uuid); + if (characteristic == nullptr) { + emitErrorLocked("write to unknown characteristic " + event.uuid); + return; + } + if ((characteristic->m_properties & NIMBLE_PROPERTY::WRITE) == 0) { + emitErrorLocked("write to a characteristic that is not writable " + + event.uuid); + return; + } + characteristic->m_value = event.data; + charCallbacks = characteristic->m_callbacks; + info = connInfoLocked(); + break; + } + case HostEvent::Kind::Subscribe: { + if (!m_connected) { + emitErrorLocked("subscribe with no connection"); + return; + } + characteristic = findCharLocked(event.uuid); + if (characteristic == nullptr) { + emitErrorLocked("subscribe to unknown characteristic " + event.uuid); + return; + } + characteristic->m_subValue = static_cast(event.a); + shortArg = characteristic->m_subValue; + charCallbacks = characteristic->m_callbacks; + info = connInfoLocked(); + break; + } + case HostEvent::Kind::Confirm: { + // A shim-generated confirm whose payload has since been clobbered is + // dropped without a word: it is a timer, not something a client asked + // for. One clobbered burst therefore yields one confirm, which is what + // hardware showed. + if (event.flag && (!m_pending || event.seq != m_pendingSeq)) return; + if (!m_pending) { + emitErrorLocked("confirm with no indication pending"); + return; + } + if (!event.uuid.empty() && event.uuid != m_pendingUuid) { + emitErrorLocked("confirm for " + event.uuid + " but " + m_pendingUuid + + " is pending"); + return; + } + characteristic = findCharLocked(m_pendingUuid); + m_pending = false; + m_pendingUuid.clear(); + m_pendingPayload.clear(); + if (characteristic == nullptr) return; + charCallbacks = characteristic->m_callbacks; + intArg = BLE_HS_EDONE; + info = connInfoLocked(); + break; + } + case HostEvent::Kind::Mtu: { + if (!m_connected) { + emitErrorLocked("mtu with no connection"); + return; + } + m_mtu = event.a != 0 ? static_cast(event.a) : kDefaultMtu; + shortArg = m_mtu; + serverCallbacks = m_serverCallbacks; + info = connInfoLocked(); + alsoMtu = true; + break; + } + case HostEvent::Kind::ConnParams: { + if (!m_connected) { + emitErrorLocked("connparams with no connection"); + return; + } + if (event.b != 0) m_interval = static_cast(event.b); + m_latency = static_cast(event.c); + m_timeout = static_cast(event.d); + serverCallbacks = m_serverCallbacks; + info = connInfoLocked(); + alsoConnParams = true; + break; + } + case HostEvent::Kind::Rssi: + m_rssi = static_cast(static_cast(event.a)); + return; + case HostEvent::Kind::AutoConfirm: + m_autoConfirm = event.flag; + m_autoConfirmDelayMs = + event.a != 0 ? event.a : kDefaultAutoConfirmDelayMs; + return; + case HostEvent::Kind::Unknown: + emitErrorLocked("unknown op " + event.op); + return; + } + } + + // Lock is down. Everything below is firmware code. + switch (event.kind) { + case HostEvent::Kind::Connect: + if (serverCallbacks != nullptr) { + serverCallbacks->onConnect(serverObject, info); + if (alsoMtu) serverCallbacks->onMTUChange(shortArg, info); + if (alsoConnParams) serverCallbacks->onConnParamsUpdate(info); + } + break; + case HostEvent::Kind::Disconnect: + if (serverCallbacks != nullptr) { + serverCallbacks->onDisconnect(serverObject, info, intArg); + } + break; + case HostEvent::Kind::Write: + if (charCallbacks != nullptr) charCallbacks->onWrite(characteristic, info); + break; + case HostEvent::Kind::Subscribe: + if (charCallbacks != nullptr) { + charCallbacks->onSubscribe(characteristic, info, shortArg); + } + break; + case HostEvent::Kind::Confirm: + if (charCallbacks != nullptr) { + charCallbacks->onStatus(characteristic, info, intArg); + } + break; + case HostEvent::Kind::Mtu: + if (serverCallbacks != nullptr) { + serverCallbacks->onMTUChange(shortArg, info); + } + break; + case HostEvent::Kind::ConnParams: + if (serverCallbacks != nullptr) serverCallbacks->onConnParamsUpdate(info); + break; + case HostEvent::Kind::Rssi: + case HostEvent::Kind::AutoConfirm: + case HostEvent::Kind::Unknown: + break; + } +} + +// --- lock-held helpers ------------------------------------------------------ + +NimBLECharacteristic * +SimBleGatt::findCharLocked(const std::string &uuid) const { + for (NimBLECharacteristic *characteristic : m_characteristics) { + if (characteristic->m_uuid == uuid) return characteristic; + } + return nullptr; +} + +NimBLEConnInfo SimBleGatt::connInfoLocked() const { + return NimBLEConnInfo(m_connHandle, m_interval, m_latency, m_timeout); +} + +void SimBleGatt::clearConnectionLocked() { + m_connected = false; + m_connHandle = BLE_HS_CONN_HANDLE_NONE; + m_mtu = kDefaultMtu; + m_interval = 0; + m_latency = 0; + m_timeout = 0; + // A subscription belongs to a connection, and NimBLE fires no unsubscribe + // callback when the link drops. The shim clears it here, silently, because + // the firmware relies on there being no callback + // (BlePositionServer.cpp:713-716). + for (NimBLECharacteristic *characteristic : m_characteristics) { + characteristic->m_subValue = 0; + } + // The pending indication belonged to the peer that just left. + m_pending = false; + m_pendingUuid.clear(); + m_pendingPayload.clear(); +} + +void SimBleGatt::emitAdvertisingLocked() const { + std::string line = "{\"ev\":\"advertising\",\"up\":"; + line += m_advertisingUp ? "true" : "false"; + const uint16_t minInterval = + m_advertising != nullptr ? m_advertising->m_minInterval : 0; + const uint16_t maxInterval = + m_advertising != nullptr ? m_advertising->m_maxInterval : 0; + // 0/0 means "let the host pick", and the host picks the fast pair. Report + // what the radio would actually use, not the sentinel. + line += ",\"interval_min\":" + + std::to_string(minInterval != 0 ? minInterval + : BLE_GAP_ADV_FAST_INTERVAL1_MIN); + line += ",\"interval_max\":" + + std::to_string(maxInterval != 0 ? maxInterval + : BLE_GAP_ADV_FAST_INTERVAL1_MAX); + line += ",\"name\":\""; + line += jsonEscape(m_advertising != nullptr ? m_advertising->m_name + : m_deviceName); + line += "\",\"service\":\""; + line += jsonEscape(m_advertising != nullptr ? m_advertising->m_serviceUuid + : std::string()); + line += "\"}"; + emitLine(line); +} + +void SimBleGatt::emitErrorLocked(const std::string &message) const { + emitLine("{\"ev\":\"error\",\"msg\":\"" + jsonEscape(message) + "\"}"); +} diff --git a/src/SimBleGatt.h b/src/SimBleGatt.h new file mode 100644 index 0000000..d27f0f7 --- /dev/null +++ b/src/SimBleGatt.h @@ -0,0 +1,184 @@ +#pragma once + +// SimBleGatt -- the GATT model and the host thread behind the NimBLE shim. +// +// The NimBLE* classes are thin wrappers; every decision lives here. Three +// threads meet in this object and the split matters: +// +// - The **reader thread** (SimBleLink's) delivers decoded client ops to +// onReaderEvent(). It only enqueues. It never calls firmware code. +// - The **host thread** (this class's) drains that queue and dispatches +// every firmware callback. It is the stand-in for the NimBLE host task. +// - The **activity thread** (the firmware's own) calls indicate(), +// advertisingStart() and the rest of the API. Those return without ever +// running a callback. +// +// One mutex guards all state, including the fields inside the NimBLE* wrapper +// objects (this class is their friend). The mutex is **never held while a +// firmware callback runs**, because the callbacks call straight back in -- +// the firmware's disconnect handler calls advertising->start(). +// +// docs/ble-shim.md is the contract this implements. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "NimBLEAttValue.h" +#include "NimBLECharacteristic.h" +#include "NimBLEConnInfo.h" +#include "SimBleLink.h" + +class NimBLEAdvertising; +class NimBLEServer; +class NimBLEServerCallbacks; +class NimBLEService; + +class SimBleGatt { + public: + // The pessimistic ATT default. A client that says nothing gets this, and the + // firmware's chunk arithmetic is then the arithmetic a fresh link runs. + static constexpr uint16_t kDefaultMtu = 23; + // 1.25 ms units: 30 ms, a plausible negotiated interval. + static constexpr uint16_t kDefaultInterval = 24; + static constexpr uint32_t kDefaultAutoConfirmDelayMs = 10; + + static SimBleGatt &get(); + + // --- stack lifecycle, called from the activity thread ------------------- + bool init(const char *deviceName); + void deinit(bool clearAll); + bool initialized() const; + void setSecurityAuth(bool bonding, bool mitm, bool sc); + + // --- the objects the firmware builds its table out of ------------------- + NimBLEServer *server(); + NimBLEAdvertising *advertising(); + NimBLEService *createService(const char *uuid); + NimBLECharacteristic *createCharacteristic(NimBLEService *service, + const char *uuid, + uint32_t properties); + void setServerCallbacks(NimBLEServerCallbacks *callbacks); + void setCharacteristicCallbacks(NimBLECharacteristic *characteristic, + NimBLECharacteristicCallbacks *callbacks); + bool startServer(); + + // Everything a wrapper object stores goes through here, because the same + // mutex guards it. Advertising bounds in particular are written from two + // threads: the activity thread sets them on the slow-interval switch, and + // the firmware's onDisconnect resets them on the host thread. + void setAdvertisingName(const char *name); + void setAdvertisingServiceUuid(const char *uuid); + void setAdvertisingScanResponse(bool enable); + void setAdvertisingMinInterval(uint16_t interval); + void setAdvertisingMaxInterval(uint16_t interval); + NimBLEAttValue characteristicValue(NimBLECharacteristic *characteristic); + + // --- things the firmware does to a live link ---------------------------- + void requestConnParams(uint16_t handle, uint16_t minInterval, + uint16_t maxInterval, uint16_t latency, + uint16_t timeout); + bool indicate(NimBLECharacteristic *characteristic, const uint8_t *data, + size_t len); + bool advertisingStart(); + void advertisingStop(); + int connRssi(uint16_t handle, int8_t *out) const; + + // Blocks until the host thread's queue is empty and it is not mid-dispatch, + // including events not yet due (an auto-confirm delay is waited out). + // + // Not a NimBLE API. It exists so a test can assert that a callback did *not* + // fire without racing the host thread, and so a test can wait for one that + // should. The firmware never calls it. + void waitIdle(); + + private: + // One unit of work for the host thread. Client ops arrive as these, and so + // does the shim's own delayed auto-confirm. + struct HostEvent { + enum class Kind { + Connect, + Disconnect, + Write, + Subscribe, + Confirm, + Mtu, + ConnParams, + Rssi, + AutoConfirm, + Unknown + }; + Kind kind = Kind::Unknown; + std::string uuid; + std::string op; // Unknown only, for the error message + std::vector data; + uint32_t a = 0, b = 0, c = 0, d = 0; + bool flag = false; + // Confirm only: which pending payload this confirm is for. A confirm the + // shim generated for a payload that has since been clobbered is dropped + // silently rather than confirming its replacement. + uint32_t seq = 0; + std::chrono::steady_clock::time_point due{}; + }; + + SimBleGatt() = default; + SimBleGatt(const SimBleGatt &) = delete; + SimBleGatt &operator=(const SimBleGatt &) = delete; + + static void sinkTrampoline(void *ctx, const SimBleEvent &event); + void onReaderEvent(const SimBleEvent &event); + void enqueue(HostEvent event); + void hostThreadMain(); + void dispatch(HostEvent &event); + + // All of these want m_mutex held. + NimBLECharacteristic *findCharLocked(const std::string &uuid) const; + NimBLEConnInfo connInfoLocked() const; + void clearConnectionLocked(); + void emitAdvertisingLocked() const; + void emitErrorLocked(const std::string &message) const; + + mutable std::mutex m_mutex; + std::condition_variable m_queueCv; + std::condition_variable m_idleCv; + std::deque m_queue; + std::thread m_hostThread; + std::thread::id m_hostThreadId{}; + bool m_hostRunning = false; + bool m_dispatching = false; + + bool m_initialized = false; + std::string m_deviceName; + + NimBLEServer *m_server = nullptr; + NimBLEAdvertising *m_advertising = nullptr; + std::vector m_services; + std::vector m_characteristics; + NimBLEServerCallbacks *m_serverCallbacks = nullptr; + bool m_advertisingUp = false; + + // Connection state. Owned by the client: it sets the MTU and the timing. + bool m_connected = false; + uint16_t m_connHandle = BLE_HS_CONN_HANDLE_NONE; + uint16_t m_nextConnHandle = 1; + uint16_t m_mtu = kDefaultMtu; + uint16_t m_interval = 0; + uint16_t m_latency = 0; + uint16_t m_timeout = 0; + int8_t m_rssi = 0; + + // The connection's single indication slot, shared by every characteristic. + bool m_pending = false; + std::string m_pendingUuid; + std::vector m_pendingPayload; + uint32_t m_pendingSeq = 0; + + bool m_autoConfirm = true; + uint32_t m_autoConfirmDelayMs = kDefaultAutoConfirmDelayMs; +}; diff --git a/src/SimBleGattSelfTest.cpp b/src/SimBleGattSelfTest.cpp new file mode 100644 index 0000000..425fdc0 --- /dev/null +++ b/src/SimBleGattSelfTest.cpp @@ -0,0 +1,311 @@ +// Self-test for the NimBLE shim. No socket, no device, no firmware: it drives +// the shim's API the way BlePositionServer does and asserts the behaviour the +// contract calls for. +// +// Build and run (one line, no continuations): +// g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/simble_selftest +// src/NimBLEDevice.cpp src/SimBleGatt.cpp src/SimBleProtocol.cpp +// src/SimBleGattSelfTestStub.cpp src/SimBleGattSelfTest.cpp +// /tmp/simble_selftest +// +// SimBleProtocol.cpp is linked for portFromEnv() only. The transport itself is +// the stub: no socket is opened. +// +// Not part of the simulator build. It links the stub in +// SimBleGattSelfTestStub.cpp instead of the real transport. + +#include +#include +#include +#include +#include +#include + +#include "NimBLEDevice.h" +#include "SimBleGatt.h" +#include "SimBleGattSelfTest.h" +#include "host/ble_gap.h" + +namespace { + +// UUIDs shaped like the firmware's: one service, a write-only channel, a +// write+notify+indicate command channel, an indicate-only status channel. +const char *kServiceUuid = "5a1e6d00-73a4-4f1e-9b8f-2c6e1a8f0001"; +const char *kPosUuid = "5a1e6d00-73a4-4f1e-9b8f-2c6e1a8f0002"; +const char *kCmdUuid = "5a1e6d00-73a4-4f1e-9b8f-2c6e1a8f0003"; +const char *kStatusUuid = "5a1e6d00-73a4-4f1e-9b8f-2c6e1a8f0005"; + +int g_failures = 0; +int g_checks = 0; + +void check(bool ok, const char *what) { + ++g_checks; + if (!ok) ++g_failures; + printf("%s %s\n", ok ? "PASS" : "FAIL", what); +} + +std::thread::id g_mainThread; + +struct Counters { + int onWrite = 0; + int onStatus = 0; + int lastStatusCode = 0; + int onSubscribe = 0; + uint16_t lastSubValue = 0; + int onConnect = 0; + int onDisconnect = 0; + int lastDisconnectReason = 0; + int onConnParams = 0; + int onMtu = 0; + uint16_t lastMtu = 0; + uint16_t lastHandle = 0; + std::thread::id callbackThread{}; + std::vector lastWrite; +}; + +Counters g_counters; + +class CmdCallbacks : public NimBLECharacteristicCallbacks { + void onWrite(NimBLECharacteristic *characteristic, + NimBLEConnInfo &) override { + const NimBLEAttValue value = characteristic->getValue(); + g_counters.lastWrite.assign(value.data(), value.data() + value.size()); + ++g_counters.onWrite; + g_counters.callbackThread = std::this_thread::get_id(); + } + void onStatus(NimBLECharacteristic *, NimBLEConnInfo &, int code) override { + g_counters.lastStatusCode = code; + ++g_counters.onStatus; + g_counters.callbackThread = std::this_thread::get_id(); + } + void onSubscribe(NimBLECharacteristic *, NimBLEConnInfo &, + uint16_t subValue) override { + g_counters.lastSubValue = subValue; + ++g_counters.onSubscribe; + g_counters.callbackThread = std::this_thread::get_id(); + } +}; + +class ServerCallbacks : public NimBLEServerCallbacks { + void onConnect(NimBLEServer *, NimBLEConnInfo &info) override { + g_counters.lastHandle = info.getConnHandle(); + ++g_counters.onConnect; + g_counters.callbackThread = std::this_thread::get_id(); + } + void onDisconnect(NimBLEServer *, NimBLEConnInfo &, int reason) override { + g_counters.lastDisconnectReason = reason; + ++g_counters.onDisconnect; + g_counters.callbackThread = std::this_thread::get_id(); + } + void onConnParamsUpdate(NimBLEConnInfo &) override { + ++g_counters.onConnParams; + g_counters.callbackThread = std::this_thread::get_id(); + } + void onMTUChange(uint16_t mtu, NimBLEConnInfo &) override { + g_counters.lastMtu = mtu; + ++g_counters.onMtu; + g_counters.callbackThread = std::this_thread::get_id(); + } +}; + +CmdCallbacks g_cmdCallbacks; +ServerCallbacks g_serverCallbacks; + +SimBleEvent op(const char *name) { + SimBleEvent event; + event.op = name; + return event; +} + +bool sawEvent(const char *needle) { + for (const std::string &line : simble_selftest::emitted()) { + if (line.find(needle) != std::string::npos) return true; + } + return false; +} + +void feedAndSettle(const SimBleEvent &event) { + simble_selftest::feed(event); + SimBleGatt::get().waitIdle(); +} + +} // namespace + +int main() { + g_mainThread = std::this_thread::get_id(); + + // --- build the table, same order the firmware does -------------------- + check(NimBLEDevice::init("sim-selftest"), "init returns true"); + check(NimBLEDevice::isInitialized(), "isInitialized after init"); + + NimBLEServer *server = NimBLEDevice::createServer(); + check(server != nullptr, "createServer"); + server->setCallbacks(&g_serverCallbacks, false); + + NimBLEService *service = server->createService(kServiceUuid); + NimBLECharacteristic *posChar = + service->createCharacteristic(kPosUuid, NIMBLE_PROPERTY::WRITE); + NimBLECharacteristic *cmdChar = service->createCharacteristic( + kCmdUuid, NIMBLE_PROPERTY::WRITE | NIMBLE_PROPERTY::NOTIFY | + NIMBLE_PROPERTY::INDICATE); + NimBLECharacteristic *statusChar = + service->createCharacteristic(kStatusUuid, NIMBLE_PROPERTY::INDICATE); + cmdChar->setCallbacks(&g_cmdCallbacks); + check(server->start(), "server->start builds the table"); + check(sawEvent("\"ev\":\"gatt\""), "gatt event emitted"); + + NimBLEAdvertising *advertising = NimBLEDevice::getAdvertising(); + advertising->addServiceUUID(kServiceUuid); + advertising->enableScanResponse(true); + advertising->setName("sim-selftest"); + check(advertising->start(), "advertising->start"); + check(sawEvent("\"ev\":\"advertising\",\"up\":true"), + "advertising up event emitted"); + check(advertising->start(), "advertising->start again is a no-op success"); + + // --- 5: a write with no connection is refused, no callback ------------- + simble_selftest::clearEmitted(); + SimBleEvent write = op("write"); + write.uuid = kCmdUuid; + write.data = {'x'}; + feedAndSettle(write); + check(g_counters.onWrite == 0, "write with no connection fires no callback"); + check(sawEvent("\"ev\":\"error\""), "write with no connection emits error"); + + // --- 1: callbacks run on the host thread, not the caller's ------------- + simble_selftest::clearEmitted(); + SimBleEvent connect = op("connect"); + connect.a = 0; // mtu absent -> the pessimistic default + connect.b = 24; // 30 ms + connect.c = 0; + connect.d = 500; + feedAndSettle(connect); + check(g_counters.onConnect == 1, "connect fires onConnect once"); + check(g_counters.onMtu == 1 && g_counters.onConnParams == 1, + "connect also fires onMTUChange and onConnParamsUpdate"); + check(g_counters.callbackThread != g_mainThread && + g_counters.callbackThread != std::thread::id(), + "callback ran on a different thread than the caller"); + // --- 4: the client owns the MTU --------------------------------------- + check(g_counters.lastMtu == SimBleGatt::kDefaultMtu, + "MTU defaults to 23 when the client says nothing"); + check(sawEvent("\"ev\":\"advertising\",\"up\":false"), + "advertising goes down on connect"); + + // --- indicate with nobody subscribed returns false --------------------- + const uint8_t payloadA[] = {'l', 'i', 'n', 'e', '-', 'A', '\n'}; + const uint8_t payloadB[] = {'l', 'i', 'n', 'e', '-', 'B', '\n'}; + check(!cmdChar->indicate(payloadA, sizeof(payloadA)), + "indicate with nobody subscribed returns false"); + check(!statusChar->indicate(payloadA, sizeof(payloadA)), + "indicate on an unsubscribed second characteristic returns false"); + + SimBleEvent subscribe = op("subscribe"); + subscribe.uuid = kCmdUuid; + subscribe.a = 2; // indications + feedAndSettle(subscribe); + check(g_counters.onSubscribe == 1 && g_counters.lastSubValue == 2, + "subscribe fires onSubscribe with the subValue"); + + // --- 2: the confirm is out of band ------------------------------------ + // auto_confirm on, delayed. indicate() must return with the confirm still + // in the future. + SimBleEvent autoConfirmSlow = op("auto_confirm"); + autoConfirmSlow.flag = true; + autoConfirmSlow.a = 150; + feedAndSettle(autoConfirmSlow); + g_counters.onStatus = 0; + const auto started = std::chrono::steady_clock::now(); + const bool accepted = cmdChar->indicate(payloadA, sizeof(payloadA)); + const int statusRightAfter = g_counters.onStatus; + const auto elapsed = std::chrono::duration_cast( + std::chrono::steady_clock::now() - started) + .count(); + check(accepted, "indicate returns true when the slot accepts"); + check(statusRightAfter == 0 && elapsed < 100, + "indicate returned before any confirm arrived"); + SimBleGatt::get().waitIdle(); + check(g_counters.onStatus == 1, "the delayed confirm arrived later"); + check(g_counters.lastStatusCode == BLE_HS_EDONE, + "onStatus code is BLE_HS_EDONE"); + + // A withheld confirm never arrives. This is the firmware's timeout path. + SimBleEvent autoConfirmOff = op("auto_confirm"); + autoConfirmOff.flag = false; + feedAndSettle(autoConfirmOff); + g_counters.onStatus = 0; + check(cmdChar->indicate(payloadA, sizeof(payloadA)), + "indicate accepted with auto_confirm off"); + SimBleGatt::get().waitIdle(); + check(g_counters.onStatus == 0, "a withheld confirm never fires onStatus"); + + // --- 3: a second indicate clobbers the first --------------------------- + simble_selftest::clearEmitted(); + check(cmdChar->indicate(payloadB, sizeof(payloadB)), + "a second indicate before a confirm still returns true"); + check(sawEvent("\"ev\":\"clobber\""), "the second indicate emitted clobber"); + check(sawEvent("\"dropped_hex\":\"6c696e652d410a\""), + "clobber names the dropped payload (line-A)"); + check(g_counters.onStatus == 0, "the clobber fired no confirm of its own"); + + SimBleEvent confirm = op("confirm"); + confirm.uuid = kCmdUuid; + feedAndSettle(confirm); + check(g_counters.onStatus == 1, + "a client confirm confirms the surviving payload, once"); + + // --- a write on a live connection reaches the callback ------------------ + g_counters.onWrite = 0; + SimBleEvent liveWrite = op("write"); + liveWrite.uuid = kCmdUuid; + liveWrite.data = {'t', 'i', 'l', 'e', 's'}; + feedAndSettle(liveWrite); + check(g_counters.onWrite == 1, "write on a live connection fires onWrite"); + check(g_counters.lastWrite == liveWrite.data, + "getValue returns the bytes that were written"); + + // A characteristic with no WRITE property is refused. + simble_selftest::clearEmitted(); + SimBleEvent badWrite = op("write"); + badWrite.uuid = kStatusUuid; + badWrite.data = {'x'}; + feedAndSettle(badWrite); + check(sawEvent("\"ev\":\"error\""), + "write to an indicate-only characteristic emits error"); + + // --- rssi comes from the client ---------------------------------------- + SimBleEvent rssi = op("rssi"); + rssi.a = static_cast(static_cast(-72)); + feedAndSettle(rssi); + int8_t rssiOut = 0; + check(ble_gap_conn_rssi(g_counters.lastHandle, &rssiOut) == 0 && + rssiOut == -72, + "ble_gap_conn_rssi returns what the client set"); + + // --- 5: disconnect clears the subscription, with no callback ----------- + simble_selftest::clearEmitted(); + g_counters.onSubscribe = 0; + SimBleEvent disconnect = op("disconnect"); + disconnect.a = 0x13; + feedAndSettle(disconnect); + check(g_counters.onDisconnect == 1 && + g_counters.lastDisconnectReason == 0x13, + "disconnect fires onDisconnect with the reason"); + check(g_counters.onSubscribe == 0, + "disconnect fires no unsubscribe callback, same as NimBLE"); + check(!cmdChar->indicate(payloadA, sizeof(payloadA)), + "the subscription is gone after a disconnect"); + check(ble_gap_conn_rssi(g_counters.lastHandle, &rssiOut) != 0, + "ble_gap_conn_rssi fails with no connection"); + check(posChar != nullptr, "the position characteristic outlives the link"); + + // --- teardown ---------------------------------------------------------- + simble_selftest::clearEmitted(); + NimBLEDevice::deinit(true); + check(!NimBLEDevice::isInitialized(), "deinit clears the stack"); + check(sawEvent("\"ev\":\"stack\",\"state\":\"down\""), + "deinit emits stack down"); + + printf("\n%d checks, %d failures\n", g_checks, g_failures); + return g_failures == 0 ? 0 : 1; +} diff --git a/src/SimBleGattSelfTest.h b/src/SimBleGattSelfTest.h new file mode 100644 index 0000000..3a3a83e --- /dev/null +++ b/src/SimBleGattSelfTest.h @@ -0,0 +1,27 @@ +#pragma once + +// Test-only seam for the NimBLE shim's self-test. +// +// A3 owns the real SimBleLink implementation. Until it lands, and to keep the +// self-test free of sockets either way, SimBleGattSelfTestStub.cpp implements +// SimBleLink over these two calls: feed() plays the reader thread, emitted() +// captures what would have gone down the socket. +// +// Delete this pair once the shim is exercised through the real transport. + +#include +#include + +#include "SimBleLink.h" + +namespace simble_selftest { + +// Hands one decoded op to the sink SimBleGatt registered, exactly as the +// reader thread would. +void feed(const SimBleEvent &event); + +// Every JSON line the shim emitted, oldest first. +std::vector emitted(); +void clearEmitted(); + +} // namespace simble_selftest diff --git a/src/SimBleGattSelfTestStub.cpp b/src/SimBleGattSelfTestStub.cpp new file mode 100644 index 0000000..08c6d11 --- /dev/null +++ b/src/SimBleGattSelfTestStub.cpp @@ -0,0 +1,76 @@ +// A SimBleLink that keeps no socket. See SimBleGattSelfTest.h. +// +// This file is the self-test's stand-in for A3's transport. It is not part of +// the shim and must not be linked into the simulator. + +#include "SimBleGattSelfTest.h" + +#include + +namespace { + +std::mutex g_mutex; +void (*g_sink)(void *, const SimBleEvent &) = nullptr; +void *g_sinkCtx = nullptr; +bool g_running = false; +std::vector g_emitted; + +} // namespace + +SimBleLink &SimBleLink::get() { + static SimBleLink instance; + return instance; +} + +bool SimBleLink::start(uint16_t port) { + std::lock_guard lock(g_mutex); + g_running = port != 0; + return g_running; +} + +void SimBleLink::stop() { + std::lock_guard lock(g_mutex); + g_running = false; +} + +bool SimBleLink::running() const { + std::lock_guard lock(g_mutex); + return g_running; +} + +void SimBleLink::setSink(void (*fn)(void *, const SimBleEvent &), void *ctx) { + std::lock_guard lock(g_mutex); + g_sink = fn; + g_sinkCtx = ctx; +} + +void SimBleLink::emit(const char *json) { + if (json == nullptr) return; + std::lock_guard lock(g_mutex); + g_emitted.emplace_back(json); +} + +namespace simble_selftest { + +void feed(const SimBleEvent &event) { + void (*sink)(void *, const SimBleEvent &) = nullptr; + void *ctx = nullptr; + { + std::lock_guard lock(g_mutex); + sink = g_sink; + ctx = g_sinkCtx; + } + if (sink != nullptr) sink(ctx, event); +} + +std::vector emitted() { + std::lock_guard lock(g_mutex); + return g_emitted; +} + +void clearEmitted() { + std::lock_guard lock(g_mutex); + g_emitted.clear(); +} + +} // namespace simble_selftest diff --git a/src/host/ble_gap.h b/src/host/ble_gap.h new file mode 100644 index 0000000..6e9f165 --- /dev/null +++ b/src/host/ble_gap.h @@ -0,0 +1,50 @@ +#pragma once + +// Shim for NimBLE's host/ble_gap.h. +// +// The firmware includes this header directly for one function: +// ble_gap_conn_rssi(). No GAP wrapper exists on NimBLEServer or +// NimBLEConnInfo, so the firmware reaches past the C++ API to the C host +// (see the include comment in the firmware's BlePositionServer.cpp). +// +// The host error codes and the advertising interval defaults live here too. +// Real NimBLE spreads them over host/ble_hs.h and host/ble_gap.h; the shim +// mirrors the API surface, not the file layout, and NimBLEDevice.h includes +// this header so either include path sees every constant. +// +// Values match NimBLE's own so a firmware comparison means the same thing +// here as on a device. + +#include + +// 0xffff: "no connection". NimBLE host/ble_hs.h. +#define BLE_HS_CONN_HANDLE_NONE 0xffff + +// Host return codes, in NimBLE's order. Only the three the firmware names are +// defined; adding more is a contract change, not a convenience. +#define BLE_HS_ENOTCONN 7 +#define BLE_HS_ETIMEOUT 13 +// The onStatus code meaning "the peer confirmed this indication". +#define BLE_HS_EDONE 14 + +// Advertising interval defaults, in 0.625 ms units. 0x0030 = 30 ms, +// 0x0060 = 60 ms. The host substitutes this pair when a peripheral asks for +// 0/0 bounds, which is what the firmware's disconnect path relies on. +#define BLE_GAP_ADV_FAST_INTERVAL1_MIN 0x0030 +#define BLE_GAP_ADV_FAST_INTERVAL1_MAX 0x0060 +#define BLE_GAP_ADV_FAST_INTERVAL1 BLE_GAP_ADV_FAST_INTERVAL1_MIN + +#ifdef __cplusplus +extern "C" { +#endif + +// Reads the RSSI of a live connection. Returns 0 on success and writes +// out_rssi; returns non-zero and leaves out_rssi alone otherwise. +// +// There is no radio here. The value is whatever the client last set with the +// `rssi` op (docs/ble-shim.md, "Client to simulator"). +int ble_gap_conn_rssi(uint16_t conn_handle, int8_t *out_rssi); + +#ifdef __cplusplus +} +#endif From 4967b54cd1de7324091c77df2452532e191c9b84 Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 15:01:58 +0200 Subject: [PATCH 05/13] docs: record the BLE shim GATT model as verified Flips the threading model, the four fidelity items and the enforced refusals from contract to verified, each naming the self-test check that showed it. Adds what building it turned up: two NimBLE calls the frozen contract list missed (NimBLEServer::start and setCallbacks' second argument), the indication slot being per connection rather than per characteristic, and two FreeRTOS symbols the simulator still lacks that the firmware BLE file needs. Extends "What it cannot answer" with the one refusal this shim cannot produce: indicate() never returns false for a busy slot, so the firmware's park-and-flush path is not exercised here. --- docs/ble-shim.md | 181 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 172 insertions(+), 9 deletions(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index 544f52b..889b315 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -12,7 +12,8 @@ Status: **seed**. This file was created before the implementation, so it states the frozen contract, not measured behaviour. Every claim below is marked `[contract]` (what the shim must do) or `[verified]` (observed running, with the command that showed it). The transport half is `[verified]`: see -"Transport specifics" below. The GATT half is still `[contract]`. +"Transport specifics" below. The GATT half is `[verified]` too: see "GATT model +specifics" below. ## What it cannot answer @@ -26,6 +27,20 @@ Write this down first, because it is the part that gets forgotten. whatever coexistence rules the target platform has. - **The peer's real GATT stack.** A python client agreeing with the firmware proves the firmware self-consistent, not interoperable with a phone's stack. +- **A busy indication slot.** `indicate()` returns false only for a refusal -- + nothing connected, nobody subscribed, wrong properties, empty payload. It + never returns false for "the slot is full", because a full slot is clobbered + and the call still succeeds (`src/SimBleGatt.cpp:319`). So the firmware's + park-and-flush path for transfer status, which exists because the command + channel can be holding the connection's one slot + (`BlePositionServer.cpp:956-958`), is reachable here only through the + unsubscribed refusal. No retry-on-false loop is exercised by this shim. +- **Two FreeRTOS symbols the simulator still lacks.** + `BlePositionServer.cpp` uses `pdMS_TO_TICKS` and `xSemaphoreCreateBinary`, + and `src/freertos/` defines neither, so the firmware BLE file does not build + in-tree yet even though every NimBLE symbol it wants now exists. Outside this + shim's own files. `[verified]` by the firmware syntax check below, which had + to supply both. ## Turning it on @@ -75,13 +90,28 @@ dance exists. A timeout test turns it off. | `connparams_request` | `min`, `max`, `latency`, `timeout` | firmware called `updateConnParams` | | `error` | `msg` | a client op the real stack would refuse | -### Rules the shim enforces, because the real stack does `[contract]` +### Rules the shim enforces, because the real stack does - `write` or `subscribe` with no central connected: `error`, no callback. + `[verified]` for `write` -- "write with no connection fires no callback" and + "write with no connection emits error" (`src/SimBleGatt.cpp:547`). + `subscribe` takes the same branch (`src/SimBleGatt.cpp:559`) and is + `[contract]`. - `indicate()` with nobody subscribed to that characteristic: returns false. + `[verified]` -- "indicate with nobody subscribed returns false" and the same + for a second characteristic (`src/SimBleGatt.cpp:304`). - A subscription belongs to a connection. On `disconnect` the shim clears subscription state itself, because NimBLE fires no unsubscribe callback. -- Advertising stops on connect and is not restarted by the shim. + `[verified]` -- "disconnect fires no unsubscribe callback, same as NimBLE" + and "the subscription is gone after a disconnect" + (`src/SimBleGatt.cpp:726`). +- Advertising stops on connect and is not restarted by the shim. `[verified]` + -- "advertising goes down on connect" (`src/SimBleGatt.cpp:536`). Only the + firmware's own `onDisconnect` brings it back. +- Two more the real stack makes, added while building it: a `write` to a + characteristic without the WRITE property is an `error` (`[verified]`, + "write to an indicate-only characteristic emits error"), and a client op + arriving while the stack is down is an `error` with no callback. ## Transport specifics `[verified]` @@ -261,20 +291,143 @@ zero or unparseable (`src/SimBleProtocol.cpp:674-685`). `start(0)` returns false, so a bad value is the same as the feature being off, never a crash and never a default port nobody asked for. -## Threading model `[contract]` +## GATT model specifics `[verified]` + +The API the firmware compiles against lives in `src/NimBLEDevice.h`, +`src/NimBLECharacteristic.h`, `src/NimBLEConnInfo.h`, `src/NimBLEAttValue.h` +and `src/host/ble_gap.h`. Every method in them is a forwarder +(`src/NimBLEDevice.cpp`). The model, the host thread, the single indication +slot and the outgoing JSON are all in `src/SimBleGatt.h` and +`src/SimBleGatt.cpp`. No NimBLE source is compiled and no JSON library is +linked: seven event shapes are hand rolled (`src/SimBleGatt.cpp:11-59`). + +Everything in this section was demonstrated by + +``` +g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/simble_selftest \ + src/NimBLEDevice.cpp src/SimBleGatt.cpp src/SimBleProtocol.cpp \ + src/SimBleGattSelfTestStub.cpp src/SimBleGattSelfTest.cpp +/tmp/simble_selftest # 40 checks, 0 failures +``` + +`src/SimBleGattSelfTest.cpp` builds the same table the firmware builds, in the +same order, then drives it. `src/SimBleGattSelfTestStub.cpp` is a `SimBleLink` +that opens no socket: it hands decoded ops to the sink and captures the emitted +lines, so the GATT half is provable without the transport. `SimBleProtocol.cpp` +is linked for `portFromEnv()` alone. Rebuild with `-fsanitize=thread` and run +under `setarch $(uname -m) -R` for the race check. + +Both halves also link and run together, real transport and no stub: + +``` +g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/simble_integration \ + src/NimBLEDevice.cpp src/SimBleGatt.cpp src/SimBleLink.cpp \ + src/SimBleProtocol.cpp +``` + +### The firmware translation unit compiles against it + +This is what proves the surface complete rather than plausible: + +``` +g++ -std=c++17 -fsyntax-only -DFREEINK_CAP_BLE_PERIPHERAL=1 -Isrc \ + -I/lib/BlePositionServer/include -I/lib/Logging \ + -include Arduino.h -include freertos/FreeRTOS.h \ + -include freertos/task.h -include freertos/semphr.h \ + -include \ + /lib/BlePositionServer/src/BlePositionServer.cpp +``` + +Clean. Two NimBLE calls the seed contract did not list turned up doing it, and +both are real: + +- **`NimBLEServer::start()`** (`BlePositionServer.cpp:314`). The firmware calls + it, not the deprecated `NimBLEService::start()`, and builds the GATT table + with it. It is what emits the `gatt` event (`src/SimBleGatt.cpp:239-262`). + `NimBLEService::start()` exists anyway and returns true. +- **`NimBLEServer::setCallbacks()` takes a second argument** + (`BlePositionServer.cpp:275`, `deleteCallbacks=false`). The shim ignores it + and never owns the pointer; the firmware registers a static object, so + nothing leaks either way (`src/NimBLEDevice.cpp:37`). + +Nothing else was missing. + +### Decisions worth naming + +- **The indication slot is per connection, not per characteristic** + (`src/SimBleGatt.cpp:319`). That is what the firmware assumes: its transfer + status channel parks a line precisely because the command channel can be + holding the one slot (`BlePositionServer.cpp:956-958`). +- **A clobbered burst yields one confirm, not one per call.** The shim's + auto-confirm carries the sequence number of the payload it was created for, + and a confirm for a payload that has since been clobbered is dropped + silently (`src/SimBleGatt.cpp:599`). Two `indicate()` calls with one confirm + between them is the hardware behaviour: 18 calls, two payloads seen. +- **A client `confirm` op confirms whatever is pending**, regardless of which + payload the client thought it was confirming. It errors when nothing is + pending, or when its `uuid` names a different characteristic than the + pending one. +- **`advertising` is also emitted with `up:false` when a central connects** + (`src/SimBleGatt.cpp:536`). The event table above lists start/stop; this is + the third case, and without it a client's view of advertising would be wrong + for the whole connection. +- **`start()` while already advertising is a no-op success and emits nothing** + (`src/SimBleGatt.cpp:369`). Real NimBLE behaves that way + (`NimBLEAdvertising.cpp:194-197`), which is why the firmware's slow-interval + switch calls `stop()` first. A shim that emitted a fresh event here would + hide that. +- **0/0 interval bounds are reported as the fast pair.** The firmware sets + 0/0 to mean "let the host pick" (`BlePositionServer.cpp:216-218`), and the + host picks `BLE_GAP_ADV_FAST_INTERVAL1`. The `advertising` event reports what + the radio would use, not the sentinel (`src/SimBleGatt.cpp:743`). +- **`deinit(clearAll)` deletes the table when `clearAll` is true**, same as the + real API, which is why the firmware nulls its own characteristic pointers + first (`BlePositionServer.cpp:381-385`). With `false` the objects survive. +- **`deinit()` called from the host thread is refused with an `error`** + (`src/SimBleGatt.cpp:111`). It would be a self-join. Real NimBLE deinit from + the host task is equally broken; the firmware never does it, and a clear line + beats an abort. +- **The advertising name has no default in the shim.** The firmware passes it + in, and the shim never invents one, so no device name is baked in here. + +### `waitIdle()` is the one addition that is not NimBLE + +`SimBleGatt::waitIdle()` blocks until the host thread's queue is empty and it +is not mid-dispatch, including events not yet due +(`src/SimBleGatt.cpp:492`). It exists so a test can assert that a callback did +**not** fire without racing the host thread. The firmware never calls it, and a +client cannot reach it. + +## Threading model - `NimBLEDevice::init()` starts **two** threads: a socket reader and a **host - thread**. `deinit()` joins both. + thread**. `deinit()` joins both. `[verified]` -- the host thread is started + in `init()` (`src/SimBleGatt.cpp:85`) and joined in `deinit()` + (`src/SimBleGatt.cpp:128`); the reader is `SimBleLink`'s. - The reader parses lines and pushes events onto the host thread's queue. It - never calls firmware code. + never calls firmware code. `[verified]` by reading + `SimBleGatt::onReaderEvent` (`src/SimBleGatt.cpp:395`): it maps an op name to + a queue entry and returns. Even a state-only op (`rssi`, `auto_confirm`) is + queued rather than applied there, so ordering is whatever the client sent. - The host thread dispatches every firmware callback. It is the simulator's - stand-in for the NimBLE host task. + stand-in for the NimBLE host task. `[verified]` -- "callback ran on a + different thread than the caller" compares `std::this_thread::get_id()` + inside the callback against the thread that fed the event. - `indicate()` is called from the activity thread. It fills a single pending slot, emits `indicate` (plus `clobber` if it overwrote one), and returns. The confirm arrives later as an event and is dispatched on the host thread. + `[verified]` -- "indicate returned before any confirm arrived" and "the + delayed confirm arrived later". - The `portENTER_CRITICAL` shim is a real `std::mutex` (`src/freertos/FreeRTOS.h:27-29`), so existing critical sections keep working across these threads. `[verified]` by reading that file. +- One mutex guards the whole GATT model, including the fields inside the + `NimBLE*` wrapper objects, and it is **dropped before every firmware + callback** (`src/SimBleGatt.cpp:480`). It has to be: the firmware's + `onDisconnect` calls `advertising->start()`, which comes straight back in. + `[verified]` -- the self-test runs clean under ThreadSanitizer + (`-fsanitize=thread`, under `setarch -R` because TSan needs ASLR off on this + kernel). `SimBleLink.h` is the frozen seam between the transport and the GATT model, and its header comment restates this split. @@ -282,21 +435,31 @@ its header comment restates this split. ## Fidelity: four things that must be right, or the simulator lies A shim that gets these wrong hides exactly the bugs a real device already -showed. All four are `[contract]` until a run demonstrates them. +showed. All four are `[verified]`, each by the named self-test check. 1. **Callbacks run on the host thread**, never inline on the caller's thread. Inline dispatch makes a whole class of deadlock impossible to reproduce. + `[verified]` -- "callback ran on a different thread than the caller". 2. **Indication confirm is out of band, and withholdable.** `indicate()` returns true when the single pending slot accepted the payload, not when the peer got it. The confirm arrives later through `onStatus`. A shim that confirms synchronously never executes the firmware's timeout path. + `[verified]` twice: "indicate returned before any confirm arrived" (with + `auto_confirm` on and a 150 ms delay, `onStatus` had not fired when + `indicate()` returned) and "a withheld confirm never fires onStatus" (with + `auto_confirm` off, it never fires at all). Even the shim's own auto-confirm + goes through the host thread's queue with a delay + (`src/SimBleGatt.cpp:356`), so it cannot short-circuit. 3. **A second `indicate()` before a confirm clobbers the first.** Real and measured on hardware: back-to-back calls all returned true, the peer saw the first and the last. The shim must reproduce the clobber, not queue politely. - The `clobber` event is how that stays visible. + The `clobber` event is how that stays visible. `[verified]` -- "a second + indicate before a confirm still returns true", "the second indicate emitted + clobber" and "clobber names the dropped payload (line-A)". 4. **The client sets the MTU.** MTU drives the firmware's payload arithmetic and chunk counts, so a wrong default tests different arithmetic than a device runs. 23 is the pessimistic default; 517 is the fast path. + `[verified]` -- "MTU defaults to 23 when the client says nothing". ## Fault injection From 9904d922de3ee39582081b0231478b30dcd0130f Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 15:47:07 +0200 Subject: [PATCH 06/13] fix: move the GATT self-test out of the library archive The self-test defined a second main() and a second SimBleLink under src/. library.json has no srcFilter, so both landed in the library archive next to simulator_main.o and the linker satisfied main() from the test: the simulator built clean, never ran, printed one line from a global constructor and aborted in a destructor of code it had never entered. Moved to tests/, which is where the transport self-test already lives and which is why only this half broke. No srcFilter: an allowlist has to be remembered when the next file appears, a directory that is not compiled does not. The test now ends with fflush(stdout) and _exit() instead of returning, so its verdict cannot be lost to someone else's teardown. --- src/SimBleGattSelfTest.h | 27 ------------ .../sim_ble_gatt_selftest.cpp | 44 +++++++++++++++---- tests/sim_ble_gatt_selftest.h | 31 +++++++++++++ .../sim_ble_gatt_stub.cpp | 10 +++-- 4 files changed, 73 insertions(+), 39 deletions(-) delete mode 100644 src/SimBleGattSelfTest.h rename src/SimBleGattSelfTest.cpp => tests/sim_ble_gatt_selftest.cpp (87%) create mode 100644 tests/sim_ble_gatt_selftest.h rename src/SimBleGattSelfTestStub.cpp => tests/sim_ble_gatt_stub.cpp (80%) diff --git a/src/SimBleGattSelfTest.h b/src/SimBleGattSelfTest.h deleted file mode 100644 index 3a3a83e..0000000 --- a/src/SimBleGattSelfTest.h +++ /dev/null @@ -1,27 +0,0 @@ -#pragma once - -// Test-only seam for the NimBLE shim's self-test. -// -// A3 owns the real SimBleLink implementation. Until it lands, and to keep the -// self-test free of sockets either way, SimBleGattSelfTestStub.cpp implements -// SimBleLink over these two calls: feed() plays the reader thread, emitted() -// captures what would have gone down the socket. -// -// Delete this pair once the shim is exercised through the real transport. - -#include -#include - -#include "SimBleLink.h" - -namespace simble_selftest { - -// Hands one decoded op to the sink SimBleGatt registered, exactly as the -// reader thread would. -void feed(const SimBleEvent &event); - -// Every JSON line the shim emitted, oldest first. -std::vector emitted(); -void clearEmitted(); - -} // namespace simble_selftest diff --git a/src/SimBleGattSelfTest.cpp b/tests/sim_ble_gatt_selftest.cpp similarity index 87% rename from src/SimBleGattSelfTest.cpp rename to tests/sim_ble_gatt_selftest.cpp index 425fdc0..7c10681 100644 --- a/src/SimBleGattSelfTest.cpp +++ b/tests/sim_ble_gatt_selftest.cpp @@ -3,16 +3,17 @@ // contract calls for. // // Build and run (one line, no continuations): -// g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/simble_selftest +// g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/sim_ble_gatt_selftest // src/NimBLEDevice.cpp src/SimBleGatt.cpp src/SimBleProtocol.cpp -// src/SimBleGattSelfTestStub.cpp src/SimBleGattSelfTest.cpp -// /tmp/simble_selftest +// tests/sim_ble_gatt_stub.cpp tests/sim_ble_gatt_selftest.cpp +// /tmp/sim_ble_gatt_selftest // // SimBleProtocol.cpp is linked for portFromEnv() only. The transport itself is -// the stub: no socket is opened. +// the stub in tests/sim_ble_gatt_stub.cpp: no socket is opened. // -// Not part of the simulator build. It links the stub in -// SimBleGattSelfTestStub.cpp instead of the real transport. +// **Never under src/.** See sim_ble_gatt_selftest.h. + +#include #include #include @@ -23,7 +24,7 @@ #include "NimBLEDevice.h" #include "SimBleGatt.h" -#include "SimBleGattSelfTest.h" +#include "sim_ble_gatt_selftest.h" #include "host/ble_gap.h" namespace { @@ -163,6 +164,28 @@ int main() { "advertising up event emitted"); check(advertising->start(), "advertising->start again is a no-op success"); + // --- attach replays the current state to a late client ----------------- + // The transport synthesizes this op when a client connects. `stack up` was + // emitted by init(), before any listener existed, so a client can never have + // received it: the replay is the only way it learns. + simble_selftest::clearEmitted(); + feedAndSettle(op("attach")); + { + const std::vector replay = simble_selftest::emitted(); + const bool order = + replay.size() == 3 && + replay[0].find("\"ev\":\"stack\",\"state\":\"up\"") != std::string::npos && + replay[1].find("\"ev\":\"gatt\"") != std::string::npos && + replay[2].find("\"ev\":\"advertising\",\"up\":true") != + std::string::npos; + check(order, "attach replays stack up, then gatt, then advertising"); + if (!order) { + for (const std::string &line : replay) printf(" got: %s\n", line.c_str()); + } + check(g_counters.onConnect == 0, + "attach fires no firmware callback of its own"); + } + // --- 5: a write with no connection is refused, no callback ------------- simble_selftest::clearEmitted(); SimBleEvent write = op("write"); @@ -307,5 +330,10 @@ int main() { "deinit emits stack down"); printf("\n%d checks, %d failures\n", g_checks, g_failures); - return g_failures == 0 ? 0 : 1; + // Flush, then leave without running global destructors. A test binary must + // report its verdict even when something else in the link is unhappy at + // teardown; a hijacked main() that returned into a foreign destructor is what + // taught that (see the header of this file). + fflush(stdout); + _exit(g_failures == 0 ? 0 : 1); } diff --git a/tests/sim_ble_gatt_selftest.h b/tests/sim_ble_gatt_selftest.h new file mode 100644 index 0000000..37fca7a --- /dev/null +++ b/tests/sim_ble_gatt_selftest.h @@ -0,0 +1,31 @@ +#pragma once + +// Test-only seam for the NimBLE shim's GATT self-test. +// +// sim_ble_gatt_stub.cpp implements SimBleLink over these two calls: feed() +// plays the reader thread, emitted() captures what would have gone down the +// socket. That keeps the GATT model provable without a socket, and keeps the +// assertions deterministic. +// +// **Lives in tests/, and must stay there.** These files define a second main() +// and a second SimBleLink. The library has no srcFilter, so anything under +// src/ lands in the archive next to simulator_main.o and the linker is free to +// satisfy main() from the wrong one. A header comment saying "do not link this" +// is not enforcement; a directory that is not compiled is. + +#include +#include + +#include "SimBleLink.h" + +namespace simble_selftest { + +// Hands one decoded op to the sink SimBleGatt registered, exactly as the +// reader thread would. +void feed(const SimBleEvent &event); + +// Every JSON line the shim emitted, oldest first. +std::vector emitted(); +void clearEmitted(); + +} // namespace simble_selftest diff --git a/src/SimBleGattSelfTestStub.cpp b/tests/sim_ble_gatt_stub.cpp similarity index 80% rename from src/SimBleGattSelfTestStub.cpp rename to tests/sim_ble_gatt_stub.cpp index 08c6d11..1a0239b 100644 --- a/src/SimBleGattSelfTestStub.cpp +++ b/tests/sim_ble_gatt_stub.cpp @@ -1,9 +1,11 @@ -// A SimBleLink that keeps no socket. See SimBleGattSelfTest.h. +// A SimBleLink that keeps no socket. See sim_ble_gatt_selftest.h for why this +// lives in tests/ and must stay there. // -// This file is the self-test's stand-in for A3's transport. It is not part of -// the shim and must not be linked into the simulator. +// It is the GATT self-test's stand-in for the real transport, so the model can +// be driven without a socket. The real transport is exercised separately by +// tests/sim_ble_gatt_attach_selftest.cpp. -#include "SimBleGattSelfTest.h" +#include "sim_ble_gatt_selftest.h" #include From 48e859b2d5eb47ae7a03da049aef97ba52ea3fdf Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 15:47:20 +0200 Subject: [PATCH 07/13] feat: replay the current state to a newly attached client NimBLEDevice::init() emits stack/up, and init() is also what starts the listener, so no client can be attached when that line is written and it is dropped. The event was never racy, it was structurally undeliverable. The gatt table and the first advertising line have the same problem whenever the firmware builds them before a client turns up, which is the normal case. A real central does not have this problem: it scans. The accept path now synthesizes an `attach` op into the sink, the mirror of the synthetic disconnect a lost socket already produces, and the GATT model answers it by emitting what is currently true: stack up, the gatt table if one is built, then the current advertising state. Not a real BLE event, same status as clobber. No change to the frozen SimBleLink.h: the op name is a string. Nothing is replayed about a live connection. A client that was not there for the connect is not the central that made it. The transport gate's two connect-time baselines were snapshotted before the reader had accepted, so they measured the next op against the attach line and failed intermittently. They now wait for the attach op and assert it arrives, which adds a check rather than removing one. 65/65 again, plain, ASan/UBSan and TSan. tests/sim_ble_gatt_attach_selftest.cpp proves the replay over a real socket: it builds a table with nobody connected, connects to itself and reads the three lines. 9 checks. --- src/SimBleGatt.cpp | 32 ++++- src/SimBleGatt.h | 6 + src/SimBleLink.cpp | 20 ++++ tests/sim_ble_gatt_attach_selftest.cpp | 159 +++++++++++++++++++++++++ tests/sim_ble_link_selftest.py | 24 +++- 5 files changed, 239 insertions(+), 2 deletions(-) create mode 100644 tests/sim_ble_gatt_attach_selftest.cpp diff --git a/src/SimBleGatt.cpp b/src/SimBleGatt.cpp index bcd1bf3..d68f48e 100644 --- a/src/SimBleGatt.cpp +++ b/src/SimBleGatt.cpp @@ -76,6 +76,7 @@ bool SimBleGatt::init(const char *deviceName) { m_deviceName = deviceName != nullptr ? deviceName : ""; m_initialized = true; m_advertisingUp = false; + m_tableBuilt = false; clearConnectionLocked(); m_rssi = 0; m_autoConfirm = true; @@ -132,6 +133,7 @@ void SimBleGatt::deinit(bool clearAll) { clearConnectionLocked(); m_serverCallbacks = nullptr; m_initialized = false; + m_tableBuilt = false; if (!clearAll) return; // clearAll deletes the table, same as the real API -- which is why the @@ -239,6 +241,12 @@ SimBleGatt::characteristicValue(NimBLECharacteristic *characteristic) { bool SimBleGatt::startServer() { std::lock_guard lock(m_mutex); if (!m_initialized) return false; + m_tableBuilt = true; + emitGattTableLocked(); + return true; +} + +void SimBleGatt::emitGattTableLocked() const { // One `gatt` line per service. The firmware builds one; a second would get // its own line rather than being folded into the first. for (const NimBLEService *service : m_services) { @@ -259,7 +267,22 @@ bool SimBleGatt::startServer() { line += "]}"; emitLine(line); } - return true; +} + +// Answers the transport's synthetic `attach` op. `stack up` is emitted by +// init(), which is also what starts the listener, so no client can ever be +// connected in time to receive it -- the event is not racy, it is structurally +// undeliverable. Same for a `gatt` table built before the client showed up. A +// real central has no such problem: it scans and sees advertising. +// +// So a newly attached client is told what is true right now, in the order it +// would have heard it: the stack, then the table, then advertising. Nothing is +// replayed about a live connection, because a client that was not there for the +// connect is not the central that made it. +void SimBleGatt::replayStateLocked() const { + emitLine("{\"ev\":\"stack\",\"state\":\"up\"}"); + if (m_tableBuilt) emitGattTableLocked(); + emitAdvertisingLocked(); } // --- a live link ------------------------------------------------------------ @@ -425,6 +448,9 @@ void SimBleGatt::onReaderEvent(const SimBleEvent &event) { out.kind = HostEvent::Kind::Rssi; } else if (event.op == "auto_confirm") { out.kind = HostEvent::Kind::AutoConfirm; + } else if (event.op == "attach") { + // Synthesized by the transport on accept, never sent by a client. + out.kind = HostEvent::Kind::Attach; } else { out.kind = HostEvent::Kind::Unknown; } @@ -649,6 +675,9 @@ void SimBleGatt::dispatch(HostEvent &event) { m_autoConfirmDelayMs = event.a != 0 ? event.a : kDefaultAutoConfirmDelayMs; return; + case HostEvent::Kind::Attach: + replayStateLocked(); + return; case HostEvent::Kind::Unknown: emitErrorLocked("unknown op " + event.op); return; @@ -692,6 +721,7 @@ void SimBleGatt::dispatch(HostEvent &event) { break; case HostEvent::Kind::Rssi: case HostEvent::Kind::AutoConfirm: + case HostEvent::Kind::Attach: case HostEvent::Kind::Unknown: break; } diff --git a/src/SimBleGatt.h b/src/SimBleGatt.h index d27f0f7..769aa50 100644 --- a/src/SimBleGatt.h +++ b/src/SimBleGatt.h @@ -112,6 +112,7 @@ class SimBleGatt { ConnParams, Rssi, AutoConfirm, + Attach, Unknown }; Kind kind = Kind::Unknown; @@ -142,6 +143,8 @@ class SimBleGatt { NimBLEConnInfo connInfoLocked() const; void clearConnectionLocked(); void emitAdvertisingLocked() const; + void emitGattTableLocked() const; + void replayStateLocked() const; void emitErrorLocked(const std::string &message) const; mutable std::mutex m_mutex; @@ -162,6 +165,9 @@ class SimBleGatt { std::vector m_characteristics; NimBLEServerCallbacks *m_serverCallbacks = nullptr; bool m_advertisingUp = false; + // True once the firmware called NimBLEServer::start(). A client that attaches + // later gets the table replayed; before that there is no table to describe. + bool m_tableBuilt = false; // Connection state. Owned by the client: it sets the MTU and the timing. bool m_connected = false; diff --git a/src/SimBleLink.cpp b/src/SimBleLink.cpp index 82049ac..f827fd2 100644 --- a/src/SimBleLink.cpp +++ b/src/SimBleLink.cpp @@ -241,9 +241,29 @@ void acceptClient(LinkState &s) { s.rx.clear(); s.skippingLongLine = false; + void (*sink)(void *, const SimBleEvent &) = nullptr; + void *ctx = nullptr; { std::lock_guard lock(s.mtx); s.clientFd = fd; + sink = s.sink; + ctx = s.sinkCtx; + } + + // Synthesize an `attach` op, the mirror of dropClient's synthetic + // `disconnect`. A client that connects after the firmware built its GATT + // table has no other way to learn the current state: `stack up` was emitted + // before any listener existed, so it went nowhere, and a real central would + // have learned all this by scanning. The GATT model answers this op by + // replaying what is currently true. **Not a real BLE event.** + // + // Delivered outside the lock and after clientFd is set: the model's reply + // goes back through emit(), which takes the same mutex and needs a connected + // client to write to. + if (sink != nullptr) { + SimBleEvent ev; + ev.op = "attach"; + sink(ctx, ev); } } diff --git a/tests/sim_ble_gatt_attach_selftest.cpp b/tests/sim_ble_gatt_attach_selftest.cpp new file mode 100644 index 0000000..59f6548 --- /dev/null +++ b/tests/sim_ble_gatt_attach_selftest.cpp @@ -0,0 +1,159 @@ +// sim_ble_gatt_attach_selftest -- the GATT model against the REAL transport. +// +// The other GATT self-test uses a stub SimBleLink, so it cannot prove the one +// behaviour that only exists in the socket path: a client that connects after +// the firmware built its GATT table is told the current state without asking. +// This binary links src/SimBleLink.cpp, opens a real loopback socket to itself +// and reads what arrives. +// +// Build and run (one line, no continuations): +// g++ -std=c++17 -Wall -Wextra -pthread -Isrc +// -o /tmp/sim_ble_gatt_attach_selftest src/NimBLEDevice.cpp +// src/SimBleGatt.cpp src/SimBleLink.cpp src/SimBleProtocol.cpp +// tests/sim_ble_gatt_attach_selftest.cpp +// /tmp/sim_ble_gatt_attach_selftest +// +// **Never under src/.** It defines main(); see sim_ble_gatt_selftest.h. + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "NimBLEDevice.h" +#include "SimBleLink.h" + +namespace { + +const char *kServiceUuid = "5a1e6d00-73a4-4f1e-9b8f-2c6e1a8f0001"; +const char *kCmdUuid = "5a1e6d00-73a4-4f1e-9b8f-2c6e1a8f0003"; + +int g_failures = 0; +int g_checks = 0; + +void check(bool ok, const char *what) { + ++g_checks; + if (!ok) ++g_failures; + printf("%s %s\n", ok ? "PASS" : "FAIL", what); +} + +int connectTo(uint16_t port) { + const int fd = ::socket(AF_INET, SOCK_STREAM, 0); + if (fd < 0) return -1; + sockaddr_in addr{}; + addr.sin_family = AF_INET; + addr.sin_port = htons(port); + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + if (::connect(fd, reinterpret_cast(&addr), sizeof(addr)) != 0) { + ::close(fd); + return -1; + } + timeval tv{}; + tv.tv_sec = 3; + ::setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); + return fd; +} + +// Reads until `want` newline-terminated lines have arrived or the socket +// timeout fires. +std::vector readLines(int fd, size_t want) { + std::vector lines; + std::string buffer; + char chunk[1024]; + while (lines.size() < want) { + const ssize_t got = ::recv(fd, chunk, sizeof(chunk), 0); + if (got <= 0) break; + buffer.append(chunk, static_cast(got)); + size_t at = 0; + for (;;) { + const size_t nl = buffer.find('\n', at); + if (nl == std::string::npos) break; + lines.push_back(buffer.substr(at, nl - at)); + at = nl + 1; + } + buffer.erase(0, at); + } + return lines; +} + +bool has(const std::string &line, const char *needle) { + return line.find(needle) != std::string::npos; +} + +void checkReplay(const std::vector &lines, const char *label) { + const bool ok = lines.size() == 3 && + has(lines[0], "\"ev\":\"stack\",\"state\":\"up\"") && + has(lines[1], "\"ev\":\"gatt\"") && + has(lines[2], "\"ev\":\"advertising\",\"up\":true"); + check(ok, label); + if (!ok) { + for (const std::string &line : lines) printf(" got: %s\n", line.c_str()); + } +} + +} // namespace + +int main() { + // Find a port the listener actually binds. init() cannot report a bind + // failure, but SimBleLink::running() can. + uint16_t port = 0; + for (uint16_t candidate = 45311; candidate < 45361; ++candidate) { + setenv("CROSSPOINT_SIM_BLE_PORT", std::to_string(candidate).c_str(), 1); + NimBLEDevice::init("sim-attach-selftest"); + if (SimBleLink::get().running()) { + port = candidate; + break; + } + NimBLEDevice::deinit(true); + } + if (port == 0) { + printf("FAIL no free loopback port in 45311..45360\n"); + fflush(stdout); + _exit(1); + } + printf("listener on 127.0.0.1:%u\n", static_cast(port)); + check(NimBLEDevice::isInitialized(), "init with a port set"); + + // Build the table with nobody connected. Every event these emit is dropped, + // which is the whole reason the replay exists. + NimBLEServer *server = NimBLEDevice::createServer(); + NimBLEService *service = server->createService(kServiceUuid); + service->createCharacteristic(kCmdUuid, NIMBLE_PROPERTY::WRITE | + NIMBLE_PROPERTY::INDICATE); + check(server->start(), "server->start with nobody connected"); + check(NimBLEDevice::getAdvertising()->start(), + "advertising->start with nobody connected"); + + const int first = connectTo(port); + check(first >= 0, "a client connects"); + checkReplay(readLines(first, 3), + "a late client receives stack up, gatt and advertising, unasked"); + + // The slot frees when the client leaves, and the next client gets the same + // replay. The dropped socket also synthesizes a disconnect, which the model + // answers with an error because no central was connected -- expected, and it + // must not disturb the next replay. + ::close(first); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + const int second = connectTo(port); + check(second >= 0, "a second client connects after the first left"); + checkReplay(readLines(second, 3), "the replay happens for every client"); + + ::close(second); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + NimBLEDevice::deinit(true); + check(!NimBLEDevice::isInitialized(), "deinit tears the stack down"); + check(!SimBleLink::get().running(), "deinit stops the listener"); + + printf("\n%d checks, %d failures\n", g_checks, g_failures); + fflush(stdout); + _exit(g_failures == 0 ? 0 : 1); +} diff --git a/tests/sim_ble_link_selftest.py b/tests/sim_ble_link_selftest.py index 9f08582..322f271 100644 --- a/tests/sim_ble_link_selftest.py +++ b/tests/sim_ble_link_selftest.py @@ -121,6 +121,25 @@ def connect(): return sock +def attach_baseline(harness, before): + """Waits out the synthetic `attach` op the accept path delivers, and returns + the sink-line count to measure the next op against. + + Not a race in the transport: the op is delivered on accept, which happens + after connect() has already returned. Snapshotting the baseline without + waiting for it makes the next op's expected SINK line land one slot late. + Fails loudly if the op never arrives, so this stays an assertion rather than + a sleep.""" + deadline = time.time() + 2 + while time.time() < deadline: + lines = harness.sink_lines() + if len(lines) > before and parse_sink(lines[before]).get("op") == "attach": + return len(lines) + time.sleep(0.005) + check(False, "accept delivers a synthetic attach op") + return len(harness.sink_lines()) + + class LineReader: def __init__(self, sock): self.sock = sock @@ -277,8 +296,10 @@ def main(): else: print(" skip no routable address on this host") + base = len(harness.sink_lines()) sock = connect() reader = LineReader(sock) + attach_baseline(harness, base) print("\n== every op, explicit and defaulted ==") for label, payload, expected in OPS: @@ -392,9 +413,10 @@ def main(): time.sleep(0.01) check(got is not None and got.get("op") == "disconnect" and got.get("a") == "19", f"a lost socket synthesizes disconnect reason 0x13 -> {got}", str(got)) + base = len(harness.sink_lines()) sock = connect() reader = LineReader(sock) - before = len(harness.sink_lines()) + before = attach_baseline(harness, base) sock.sendall(b'{"op":"mtu","mtu":300}\n') time.sleep(0.3) after = harness.sink_lines() From 5b3879429f819745e166c1ce0db2f02d3bd138eb Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 15:47:30 +0200 Subject: [PATCH 08/13] docs: document the attach replay and the tests/ move Records the state replay as its own section, labelled not-a-real-BLE-event the same way clobber is, so a client author reading a packet trace does not mistake it for something a phone would see. Says what is not replayed and what a client that attaches before init() gets. Rewrites the self-test commands for the new tests/ paths and adds the socket test. States why test files live in tests/ rather than behind a srcFilter. Re-checks every file:line citation in the file. The accept-path insertion moved 21 lines of src/SimBleLink.cpp, which stale-dated eleven citations in the transport section, and the GATT edits moved eighteen of mine. One citation was already off by one before this pass. --- docs/ble-shim.md | 135 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 92 insertions(+), 43 deletions(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index 889b315..36e42e6 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -30,7 +30,7 @@ Write this down first, because it is the part that gets forgotten. - **A busy indication slot.** `indicate()` returns false only for a refusal -- nothing connected, nobody subscribed, wrong properties, empty payload. It never returns false for "the slot is full", because a full slot is clobbered - and the call still succeeds (`src/SimBleGatt.cpp:319`). So the firmware's + and the call still succeeds (`src/SimBleGatt.cpp:342`). So the firmware's park-and-flush path for transfer status, which exists because the command channel can be holding the connection's one slot (`BlePositionServer.cpp:956-958`), is reachable here only through the @@ -94,25 +94,56 @@ dance exists. A timeout test turns it off. - `write` or `subscribe` with no central connected: `error`, no callback. `[verified]` for `write` -- "write with no connection fires no callback" and - "write with no connection emits error" (`src/SimBleGatt.cpp:547`). - `subscribe` takes the same branch (`src/SimBleGatt.cpp:559`) and is + "write with no connection emits error" (`src/SimBleGatt.cpp:586`). + `subscribe` takes the same branch (`src/SimBleGatt.cpp:606`) and is `[contract]`. - `indicate()` with nobody subscribed to that characteristic: returns false. `[verified]` -- "indicate with nobody subscribed returns false" and the same - for a second characteristic (`src/SimBleGatt.cpp:304`). + for a second characteristic (`src/SimBleGatt.cpp:327`). - A subscription belongs to a connection. On `disconnect` the shim clears subscription state itself, because NimBLE fires no unsubscribe callback. `[verified]` -- "disconnect fires no unsubscribe callback, same as NimBLE" and "the subscription is gone after a disconnect" - (`src/SimBleGatt.cpp:726`). + (`src/SimBleGatt.cpp:756`). - Advertising stops on connect and is not restarted by the shim. `[verified]` - -- "advertising goes down on connect" (`src/SimBleGatt.cpp:536`). Only the + -- "advertising goes down on connect" (`src/SimBleGatt.cpp:562`). Only the firmware's own `onDisconnect` brings it back. - Two more the real stack makes, added while building it: a `write` to a characteristic without the WRITE property is an `error` (`[verified]`, "write to an indicate-only characteristic emits error"), and a client op arriving while the stack is down is an `error` with no callback. +### State replay on attach `[verified]` + +**A client receives three events on connect without sending anything:** `stack` +`up`, the `gatt` table if one is built, and the current `advertising` state, in +that order. + +**This is not something a phone would see.** Same status as `clobber`: it exists +because the simulator is not a radio. A real central learns the peripheral's +state by scanning; a TCP client has no scan, and every event it missed is gone. +And it missed all of them: `stack up` is emitted by `NimBLEDevice::init()`, +which is also the call that starts the listener, so at that instant no client +can possibly be attached and the line is dropped +(`src/SimBleLink.cpp:445-446`). The event was never racy. It was structurally +undeliverable. The `gatt` table and the first `advertising` line have the same +problem whenever the firmware builds them before a client turns up, which is +the normal case. + +Mechanism, for a client author who sees it in a packet trace: the accept path +synthesizes an `attach` op into the sink (`src/SimBleLink.cpp:244-267`), the +mirror of the synthetic `disconnect` a lost socket already produces, and the +GATT model answers it by emitting current state +(`src/SimBleGatt.cpp:272-286`). A client never sends `attach` and gets an +`error` if it invents one that the model does not recognise. The replay fires +for **every** client, including the second one to attach after the first left. + +What is **not** replayed: a live connection. A client that was not there for +the `connect` op is not the central that made it, so it is told nothing about +it. A client that attaches before the firmware ever calls +`NimBLEDevice::init()` receives nothing at all -- there is no host thread to +answer, so it must wait for `stack`/`up` to arrive the normal way. + ## Transport specifics `[verified]` The socket, the reader thread and the line framing live in @@ -134,30 +165,30 @@ sends every client op twice, once with explicit fields and once with all fields absent, and the harness prints the decoded `SimBleEvent` for each. All three runs come back clean. ThreadSanitizer dies on this kernel unless ASLR is off, so the driver wraps the binary in `setarch -R` -(`tests/sim_ble_link_selftest.py:75-79`). +(`tests/sim_ble_link_selftest.py:76-80`). ### Loopback only -The listener binds `INADDR_LOOPBACK` (`src/SimBleLink.cpp:336`), never +The listener binds `INADDR_LOOPBACK` (`src/SimBleLink.cpp:356`), never `INADDR_ANY`. This is a hazard decision, not a style one: the process on the other end of this socket runs firmware command handling, so a LAN-reachable port would hand the device model to anything on the network. The gate connects to the host's own routable address and requires a refusal. -Backlog is 4 (`src/SimBleLink.cpp:339`). A second client has to complete its +Backlog is 4 (`src/SimBleLink.cpp:359`). A second client has to complete its connect before it can be told to go away. ### stop() wakes a blocked reader with a self-pipe The reader thread never blocks in `accept()` or `recv()`. It sits in `poll()` over three fds: the listener, the connected client, and the read end of a -self-pipe (`src/SimBleLink.cpp:250-299`). `accept()` and `recv()` run only on +self-pipe (`src/SimBleLink.cpp:270-320`). `accept()` and `recv()` run only on a fd `poll()` already reported readable, and the `recv()` uses `MSG_DONTWAIT` -(`src/SimBleLink.cpp:286`). +(`src/SimBleLink.cpp:306`). `stop()` sets a stop flag, writes one byte into the self-pipe (`src/SimBleLink.cpp:89-95`), shuts a connected client down, then joins -(`src/SimBleLink.cpp:368-378`). Measured: 0 to 2 ms with a client connected. +(`src/SimBleLink.cpp:388-398`). Measured: 0 to 2 ms with a client connected. Why the self-pipe and not the alternatives: @@ -170,38 +201,38 @@ Why the self-pipe and not the alternatives: to `stop()`. The pipe costs two fds and is exact. `stop()` is safe when the link was never started and safe twice in a row -(`src/SimBleLink.cpp:360-366`). `start(0)` returns false and leaves the +(`src/SimBleLink.cpp:380-386`). `start(0)` returns false and leaves the feature off. ### The reader owns the client fd Only the reader thread closes the client socket. `emit()` writes under the mutex and, on a write failure, calls `shutdown()` rather than `close()` -(`src/SimBleLink.cpp:427-431`). That makes the reader's `poll()` return and +(`src/SimBleLink.cpp:447-450`). That makes the reader's `poll()` return and keeps the teardown in one place. The client socket carries `SO_SNDTIMEO` of 5 s (`src/SimBleLink.cpp:47`, -`src/SimBleLink.cpp:236-241`). A client that stops reading cannot hang a +`src/SimBleLink.cpp:236-240`). A client that stops reading cannot hang a firmware thread inside `emit()` forever: the send fails, the link is dropped, the simulator carries on. ### emit() is one line, whole, under one mutex `emit()` frames and writes the whole line while holding the state mutex -(`src/SimBleLink.cpp:410-433`), so two threads cannot interleave halves of two +(`src/SimBleLink.cpp:430-451`), so two threads cannot interleave halves of two lines. Verified: two threads emitting 300 lines each, 600 lines received, every one intact, each thread's lines in its own order, the two threads interleaved at line granularity. An embedded `\n` or `\r` in the caller's JSON would inject a second frame, so -`emit()` replaces both with a space (`src/SimBleLink.cpp:414-417`). Callers do +`emit()` replaces both with a space (`src/SimBleLink.cpp:434-437`). Callers do not have to be careful. ### A lost socket synthesizes a disconnect A client that drops its socket is a link that went away. The reader delivers `op="disconnect"`, `a=0x13` to the sink so the GATT model does not keep -believing a central is connected (`src/SimBleLink.cpp:131-159`). The teardown +believing a central is connected (`src/SimBleLink.cpp:137-158`). The teardown inside `stop()` does **not** synthesize one: `stop()` is not a link event and the model is being destroyed anyway. @@ -304,27 +335,45 @@ linked: seven event shapes are hand rolled (`src/SimBleGatt.cpp:11-59`). Everything in this section was demonstrated by ``` -g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/simble_selftest \ +g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/sim_ble_gatt_selftest \ src/NimBLEDevice.cpp src/SimBleGatt.cpp src/SimBleProtocol.cpp \ - src/SimBleGattSelfTestStub.cpp src/SimBleGattSelfTest.cpp -/tmp/simble_selftest # 40 checks, 0 failures + tests/sim_ble_gatt_stub.cpp tests/sim_ble_gatt_selftest.cpp +/tmp/sim_ble_gatt_selftest # 42 checks, 0 failures ``` -`src/SimBleGattSelfTest.cpp` builds the same table the firmware builds, in the -same order, then drives it. `src/SimBleGattSelfTestStub.cpp` is a `SimBleLink` +`tests/sim_ble_gatt_selftest.cpp` builds the same table the firmware builds, in +the same order, then drives it. `tests/sim_ble_gatt_stub.cpp` is a `SimBleLink` that opens no socket: it hands decoded ops to the sink and captures the emitted -lines, so the GATT half is provable without the transport. `SimBleProtocol.cpp` -is linked for `portFromEnv()` alone. Rebuild with `-fsanitize=thread` and run -under `setarch $(uname -m) -R` for the race check. +lines, so the GATT half is provable without the transport, and the assertions +are deterministic. `SimBleProtocol.cpp` is linked for `portFromEnv()` alone. +Rebuild with `-fsanitize=thread` and run under `setarch $(uname -m) -R` for the +race check. -Both halves also link and run together, real transport and no stub: +A second binary drives the GATT model over the **real** transport, so the one +behaviour a stub cannot show -- the state replay on attach -- is proven on a +socket: ``` -g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/simble_integration \ - src/NimBLEDevice.cpp src/SimBleGatt.cpp src/SimBleLink.cpp \ - src/SimBleProtocol.cpp +g++ -std=c++17 -Wall -Wextra -pthread -Isrc \ + -o /tmp/sim_ble_gatt_attach_selftest src/NimBLEDevice.cpp \ + src/SimBleGatt.cpp src/SimBleLink.cpp src/SimBleProtocol.cpp \ + tests/sim_ble_gatt_attach_selftest.cpp +/tmp/sim_ble_gatt_attach_selftest # 9 checks, 0 failures ``` +It builds the table with nobody connected, then connects to itself and reads +what arrives. It also proves the two halves link together. + +**Both test binaries live in `tests/`, and that is enforcement, not tidiness.** +The library has no `srcFilter`, so anything under `src/` is compiled into the +library archive next to `simulator_main.o` -- and a test file that defines +`main()` can win the link. It did: the simulator built fine, never ran, printed +one line from a global constructor and aborted in a destructor of code it had +never entered. A header comment saying "do not link this" is not enforcement. A +directory that is not compiled cannot be forgotten. Both binaries also end with +`fflush(stdout); _exit()` rather than returning, so a verdict is never lost to +someone else's teardown. + ### The firmware translation unit compiles against it This is what proves the surface complete rather than plausible: @@ -343,7 +392,7 @@ both are real: - **`NimBLEServer::start()`** (`BlePositionServer.cpp:314`). The firmware calls it, not the deprecated `NimBLEService::start()`, and builds the GATT table - with it. It is what emits the `gatt` event (`src/SimBleGatt.cpp:239-262`). + with it. It is what emits the `gatt` event (`src/SimBleGatt.cpp:249-270`). `NimBLEService::start()` exists anyway and returns true. - **`NimBLEServer::setCallbacks()` takes a second argument** (`BlePositionServer.cpp:275`, `deleteCallbacks=false`). The shim ignores it @@ -355,36 +404,36 @@ Nothing else was missing. ### Decisions worth naming - **The indication slot is per connection, not per characteristic** - (`src/SimBleGatt.cpp:319`). That is what the firmware assumes: its transfer + (`src/SimBleGatt.cpp:342`). That is what the firmware assumes: its transfer status channel parks a line precisely because the command channel can be holding the one slot (`BlePositionServer.cpp:956-958`). - **A clobbered burst yields one confirm, not one per call.** The shim's auto-confirm carries the sequence number of the payload it was created for, and a confirm for a payload that has since been clobbered is dropped - silently (`src/SimBleGatt.cpp:599`). Two `indicate()` calls with one confirm + silently (`src/SimBleGatt.cpp:625`). Two `indicate()` calls with one confirm between them is the hardware behaviour: 18 calls, two payloads seen. - **A client `confirm` op confirms whatever is pending**, regardless of which payload the client thought it was confirming. It errors when nothing is pending, or when its `uuid` names a different characteristic than the pending one. - **`advertising` is also emitted with `up:false` when a central connects** - (`src/SimBleGatt.cpp:536`). The event table above lists start/stop; this is + (`src/SimBleGatt.cpp:562`). The event table above lists start/stop; this is the third case, and without it a client's view of advertising would be wrong for the whole connection. - **`start()` while already advertising is a no-op success and emits nothing** - (`src/SimBleGatt.cpp:369`). Real NimBLE behaves that way + (`src/SimBleGatt.cpp:392`). Real NimBLE behaves that way (`NimBLEAdvertising.cpp:194-197`), which is why the firmware's slow-interval switch calls `stop()` first. A shim that emitted a fresh event here would hide that. - **0/0 interval bounds are reported as the fast pair.** The firmware sets 0/0 to mean "let the host pick" (`BlePositionServer.cpp:216-218`), and the host picks `BLE_GAP_ADV_FAST_INTERVAL1`. The `advertising` event reports what - the radio would use, not the sentinel (`src/SimBleGatt.cpp:743`). + the radio would use, not the sentinel (`src/SimBleGatt.cpp:773`). - **`deinit(clearAll)` deletes the table when `clearAll` is true**, same as the real API, which is why the firmware nulls its own characteristic pointers first (`BlePositionServer.cpp:381-385`). With `false` the objects survive. - **`deinit()` called from the host thread is refused with an `error`** - (`src/SimBleGatt.cpp:111`). It would be a self-join. Real NimBLE deinit from + (`src/SimBleGatt.cpp:112`). It would be a self-join. Real NimBLE deinit from the host task is equally broken; the firmware never does it, and a clear line beats an abort. - **The advertising name has no default in the shim.** The firmware passes it @@ -394,7 +443,7 @@ Nothing else was missing. `SimBleGatt::waitIdle()` blocks until the host thread's queue is empty and it is not mid-dispatch, including events not yet due -(`src/SimBleGatt.cpp:492`). It exists so a test can assert that a callback did +(`src/SimBleGatt.cpp:518`). It exists so a test can assert that a callback did **not** fire without racing the host thread. The firmware never calls it, and a client cannot reach it. @@ -402,11 +451,11 @@ client cannot reach it. - `NimBLEDevice::init()` starts **two** threads: a socket reader and a **host thread**. `deinit()` joins both. `[verified]` -- the host thread is started - in `init()` (`src/SimBleGatt.cpp:85`) and joined in `deinit()` - (`src/SimBleGatt.cpp:128`); the reader is `SimBleLink`'s. + in `init()` (`src/SimBleGatt.cpp:86`) and joined in `deinit()` + (`src/SimBleGatt.cpp:129`); the reader is `SimBleLink`'s. - The reader parses lines and pushes events onto the host thread's queue. It never calls firmware code. `[verified]` by reading - `SimBleGatt::onReaderEvent` (`src/SimBleGatt.cpp:395`): it maps an op name to + `SimBleGatt::onReaderEvent` (`src/SimBleGatt.cpp:418`): it maps an op name to a queue entry and returns. Even a state-only op (`rssi`, `auto_confirm`) is queued rather than applied there, so ordering is whatever the client sent. - The host thread dispatches every firmware callback. It is the simulator's @@ -423,7 +472,7 @@ client cannot reach it. across these threads. `[verified]` by reading that file. - One mutex guards the whole GATT model, including the fields inside the `NimBLE*` wrapper objects, and it is **dropped before every firmware - callback** (`src/SimBleGatt.cpp:480`). It has to be: the firmware's + callback** (`src/SimBleGatt.cpp:506`). It has to be: the firmware's `onDisconnect` calls `advertising->start()`, which comes straight back in. `[verified]` -- the self-test runs clean under ThreadSanitizer (`-fsanitize=thread`, under `setarch -R` because TSan needs ASLR off on this @@ -449,7 +498,7 @@ showed. All four are `[verified]`, each by the named self-test check. `indicate()` returned) and "a withheld confirm never fires onStatus" (with `auto_confirm` off, it never fires at all). Even the shim's own auto-confirm goes through the host thread's queue with a delay - (`src/SimBleGatt.cpp:356`), so it cannot short-circuit. + (`src/SimBleGatt.cpp:379`), so it cannot short-circuit. 3. **A second `indicate()` before a confirm clobbers the first.** Real and measured on hardware: back-to-back calls all returned true, the peer saw the first and the last. The shim must reproduce the clobber, not queue politely. From b94b4c0abc77ea05ade49205bdc6b53ccb3c043c Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 15:52:18 +0200 Subject: [PATCH 09/13] docs: drop the FreeRTOS gap claim and re-audit what cannot be answered The bullet claiming the simulator lacked pdMS_TO_TICKS and xSemaphoreCreateBinary, and that the firmware BLE file therefore did not build in-tree, is false on all three counts now. Deleted rather than softened. The firmware side documents its own fix; a second account of it here would drift. The same claim was embedded in the firmware syntax-check recipe as a placeholder -include line. Removed. The recipe now says only what it is for: anything the -include flags miss surfaces as a FreeRTOS name, not a NimBLE one. The stronger evidence replaces it: the whole firmware builds against the shim with no NimBLE symbol missing. Three more stale facts found by auditing content rather than line numbers: - The status line still called the file a seed written before the implementation, contradicting its own next sentence. Both halves are built. - Fault injection was marked all contract. Withholding a confirm and sending a malformed frame are both demonstrated now; the other two need a transfer, so they need the firmware. - The busy-slot bullet listed its refusals as if exhaustive and omitted the stack being down. The heap, radio, peer-GATT-stack and busy-slot bullets are unchanged in substance. All 54 citations still resolve. --- docs/ble-shim.md | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index 36e42e6..d5e1560 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -8,8 +8,7 @@ plays the part of the central (the phone). the real NimBLE library and implements it differently. No NimBLE source is compiled. -Status: **seed**. This file was created before the implementation, so it states -the frozen contract, not measured behaviour. Every claim below is marked +Status: **built**. Both halves exist and run. Every claim below is marked `[contract]` (what the shim must do) or `[verified]` (observed running, with the command that showed it). The transport half is `[verified]`: see "Transport specifics" below. The GATT half is `[verified]` too: see "GATT model @@ -28,19 +27,14 @@ Write this down first, because it is the part that gets forgotten. - **The peer's real GATT stack.** A python client agreeing with the firmware proves the firmware self-consistent, not interoperable with a phone's stack. - **A busy indication slot.** `indicate()` returns false only for a refusal -- - nothing connected, nobody subscribed, wrong properties, empty payload. It - never returns false for "the slot is full", because a full slot is clobbered - and the call still succeeds (`src/SimBleGatt.cpp:342`). So the firmware's + the stack down, nothing connected, nobody subscribed, wrong properties, empty + payload. It never returns false for "the slot is full", because a full slot is + clobbered and the call still succeeds (`src/SimBleGatt.cpp:342`). So the + firmware's park-and-flush path for transfer status, which exists because the command channel can be holding the connection's one slot (`BlePositionServer.cpp:956-958`), is reachable here only through the unsubscribed refusal. No retry-on-false loop is exercised by this shim. -- **Two FreeRTOS symbols the simulator still lacks.** - `BlePositionServer.cpp` uses `pdMS_TO_TICKS` and `xSemaphoreCreateBinary`, - and `src/freertos/` defines neither, so the firmware BLE file does not build - in-tree yet even though every NimBLE symbol it wants now exists. Outside this - shim's own files. `[verified]` by the firmware syntax check below, which had - to supply both. ## Turning it on @@ -376,19 +370,26 @@ someone else's teardown. ### The firmware translation unit compiles against it -This is what proves the surface complete rather than plausible: +The whole firmware builds against this shim with no NimBLE symbol missing. That +is what proves the surface complete rather than plausible. + +The same thing on one translation unit, for a quick check while editing the +headers: ``` g++ -std=c++17 -fsyntax-only -DFREEINK_CAP_BLE_PERIPHERAL=1 -Isrc \ -I/lib/BlePositionServer/include -I/lib/Logging \ -include Arduino.h -include freertos/FreeRTOS.h \ -include freertos/task.h -include freertos/semphr.h \ - -include \ /lib/BlePositionServer/src/BlePositionServer.cpp ``` -Clean. Two NimBLE calls the seed contract did not list turned up doing it, and -both are real: +The `-include` flags stand in for what the firmware's own build has already +pulled in by the time it reaches this file. Anything they miss surfaces as a +FreeRTOS name, not a NimBLE one, so it does not touch what this run is for. + +Two NimBLE calls the seed contract did not list turned up building it, and both +are real: - **`NimBLEServer::start()`** (`BlePositionServer.cpp:314`). The firmware calls it, not the deprecated `NimBLEService::start()`, and builds the GATT table @@ -512,9 +513,14 @@ showed. All four are `[verified]`, each by the named self-test check. ## Fault injection -The point of a shim over hardware. All `[contract]`: +The point of a shim over hardware. - Withhold a confirm (`auto_confirm` false, then never send `confirm`). -- Drop the link mid-transfer (`disconnect` while a transfer is running). + `[verified]` -- "a withheld confirm never fires onStatus". - Send a malformed frame (trailing bytes, bad path, oversized length). + `[verified]` -- twenty malformed inputs in the transport gate, each answered + with `error` and no crash. +- Drop the link mid-transfer (`disconnect` while a transfer is running). + `[contract]`: needs a transfer, so it needs the firmware. - Send a transfer `begin` without subscribing to the status characteristic. + `[contract]`, same reason. From 4cf5eefd2f7928fe31943b3be12cc8ea4cdec164 Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 16:54:04 +0200 Subject: [PATCH 10/13] docs: what four firmware-driven runs showed about the BLE shim The shim's own doc was written from its self-tests. Real firmware has now been driven through it: pin the gatt event's props type on the wire (the one field nobody typed, and it broke every client), record what the socket structurally cannot answer (throughput, MTU negotiation, an over-MTU refusal, radio loss, a hole from a withheld confirm, storage failures), note that auto_confirm survives a client disconnect, add speed as a fifth fidelity hazard in both directions, and promote the four fault-injection cases that needed firmware from contract to verified. Also: a grep over call syntax cannot detect a stale API contract, and six firmware citations had drifted. --- docs/ble-shim.md | 153 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 144 insertions(+), 9 deletions(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index d5e1560..4bc7101 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -33,8 +33,50 @@ Write this down first, because it is the part that gets forgotten. firmware's park-and-flush path for transfer status, which exists because the command channel can be holding the connection's one slot - (`BlePositionServer.cpp:956-958`), is reachable here only through the + (`BlePositionServer.cpp:958-966`), is reachable here only through the unsubscribed refusal. No retry-on-false loop is exercised by this shim. +- **Throughput, of anything.** A `write` op returns when TCP took the bytes. + There is no ATT write response, no connection interval and no radio in + between, so a transfer rate measured here is a measurement of loopback. + Numbers, so nobody quotes one: a 52 KB file pushed through this shim clocked + **3377 KB/s** and, in a second run, **4196 KB/s**; the same laptop tool + measured **2.6 KB/s** over a real radio at MTU 256. Three orders of + magnitude. `[verified]` by running both. +- **MTU negotiation, and the ATT bearer's own size check.** The client declares + the MTU with the `connect` or `mtu` op; nothing negotiates and nothing + refuses an oversized write. A 485-byte frame was accepted on a link the + firmware believed had a 15-byte payload budget, and the firmware processed it + whole. That is deliberate on the client side -- BlueZ turns an over-MTU write + into a long write, so a client cannot honestly reject one locally -- but the + consequence is that firmware which never compares an arriving frame against + its own payload arithmetic looks correct here and is bounded only by the real + bearer. `[verified]` by running. +- **A link that dies without notice.** `disconnect` is a polite op the client + sends. A supervision timeout has no equivalent here, so a run exercises the + device-side disconnect cleanup and never the radio-loss path. `[verified]` -- + a transfer dropped mid-stream reached the firmware's `onDisconnect` hook and + its cleanup ran; the loss path did not. +- **A hole in the delivered bytes from a withheld confirm.** The `indicate` + event goes out when the payload enters the pending slot, and `clobber` only + when a later payload takes that slot (captured at `src/SimBleGatt.cpp:343`, both + lines emitted at `:358-368`). So a + client is handed the clobbered payload and *then* told it was dropped. On + hardware a clobbered indication is genuinely gone -- 18 back-to-back calls, + the peer saw the first and the last. Withholding one confirm therefore cannot + reproduce a missing byte range here; it reproduces the stall and the + `clobber` event only. A gap can still be produced the other way, by making + the firmware itself stop sending: a confirm withheld on the first chunk of a + multi-chunk reply aborts the rest of that reply at the source, and those + bytes are really lost. `[verified]` -- both cases run. +- **Storage failures behind a transfer.** The simulated card is a host + directory. `HalStorage::mkdir` returns true on `EEXIST` + (`src/HalStorage.cpp:326`) and `HalStorage::open` opens a directory *as a + directory* and reports it open (`src/HalStorage.cpp:306-309`), so a firmware + "could not create the directory" branch is unreachable here and a plain file + standing where a directory component belongs surfaces as an open failure + instead. A full card needs a size-limited loop filesystem; a shared host + filesystem cannot produce one. `[verified]` by reading, and by a transfer that + answered ready on a directory. ## Turning it on @@ -72,6 +114,18 @@ firmware uses. `auto_confirm` defaults on so a simple client does not have to know the confirm dance exists. A timeout test turns it off. +**`auto_confirm` is not connection state, and it is not reset when a client +leaves.** It is set at `src/SimBleGatt.cpp:674` and cleared only by +`init()` (`src/SimBleGatt.cpp:82`); `clearConnectionLocked` +(`src/SimBleGatt.cpp:744`) resets the MTU at `:747` and does not touch it. +A reconnecting client therefore inherits the previous client's setting. Real +hardware has no such notion, so this is a test-isolation trap rather than a +fidelity gap -- but a test that reconnects must set `auto_confirm` explicitly +instead of trusting the documented default. It cost one confused pass: a +control run with confirms nominally on hit 3000 ms timeouts, because an earlier +run had turned them off on a simulator process that was still alive. +`[verified]` -- reproduced, then traced to those lines. + ### Simulator to client | ev | fields | when | @@ -84,6 +138,21 @@ dance exists. A timeout test turns it off. | `connparams_request` | `min`, `max`, `latency`, `timeout` | firmware called `updateConnParams` | | `error` | `msg` | a client op the real stack would refuse | +**`props` is an integer, and that is part of the contract.** The `gatt` event +carries each characteristic's NimBLE property bitmask as a JSON number, not a +list and not a string: `{"uuid":"...0002","props":8}` +(`src/SimBleGatt.cpp:264`, `line += std::to_string(characteristic->m_properties)`). +Observed values and their meaning, so a client can decode without guessing: +`1` read, `8` write, `16` notify, `32` indicate, and `56` (`0x38`) the +write|notify|indicate combination the command characteristic carries +(`src/NimBLECharacteristic.h:26-31`). This is written down because it was the +one field whose type nobody pinned: a laptop client built against the same +prose assumed a list, called `list()` on the number, and every tool using it +died inside `connect()` with `TypeError: 'int' object is not iterable` -- before +a single byte of traffic. The two halves had each picked something reasonable +and had never been connected to each other. `[verified]` by reading the socket +raw. + ### Rules the shim enforces, because the real stack does - `write` or `subscribe` with no central connected: `error`, no callback. @@ -391,23 +460,45 @@ FreeRTOS name, not a NimBLE one, so it does not touch what this run is for. Two NimBLE calls the seed contract did not list turned up building it, and both are real: -- **`NimBLEServer::start()`** (`BlePositionServer.cpp:314`). The firmware calls +- **`NimBLEServer::start()`** (`BlePositionServer.cpp:322`). The firmware calls it, not the deprecated `NimBLEService::start()`, and builds the GATT table with it. It is what emits the `gatt` event (`src/SimBleGatt.cpp:249-270`). `NimBLEService::start()` exists anyway and returns true. - **`NimBLEServer::setCallbacks()` takes a second argument** - (`BlePositionServer.cpp:275`, `deleteCallbacks=false`). The shim ignores it + (`BlePositionServer.cpp:283`, `deleteCallbacks=false`). The shim ignores it and never owns the pointer; the firmware registers a static object, so nothing leaks either way (`src/NimBLEDevice.cpp:37`). Nothing else was missing. +**Neither was findable by grep, and that is the lesson.** The API list this +shim was built against was produced by grepping the firmware file for +`NimBLE[A-Za-z]*::[a-zA-Z_]*` and `->[a-zA-Z_]*(`. That output is not silent +about these two calls -- it contains `->start(` and `->setCallbacks(` -- it is +*ambiguous* about them, in exactly the two ways that matter. `->start(` does +not say which class the pointer had, and the only class-qualified `start` the +grep found, `NimBLEService::start`, came from a **comment** saying that call is +deprecated and not used. `->setCallbacks(` does not say how many arguments were +passed. A grep over call syntax cannot answer a question about types or arity, +so it cannot detect a stale API contract; only a compiler can. Freeze the list +if you like, but verify it by building, not by re-running the grep that wrote +it. + +**The whole surface has since been driven by real firmware, not only compiled +against.** Four independent sessions in one day pushed a position feed, a +line-oriented command channel with multi-line replies at MTU 23 and MTU 517, a +52 KB file transfer with twenty-odd refusal cases, and a device-initiated +fetch loop through this shim, and every finding they produced was in the +firmware rather than here. That is the strongest statement available about the +surface being complete: a plausible-looking fake fails on the second +unusual thing a real caller does. + ### Decisions worth naming - **The indication slot is per connection, not per characteristic** (`src/SimBleGatt.cpp:342`). That is what the firmware assumes: its transfer status channel parks a line precisely because the command channel can be - holding the one slot (`BlePositionServer.cpp:956-958`). + holding the one slot (`BlePositionServer.cpp:958-966`). - **A clobbered burst yields one confirm, not one per call.** The shim's auto-confirm carries the sequence number of the payload it was created for, and a confirm for a payload that has since been clobbered is dropped @@ -427,12 +518,12 @@ Nothing else was missing. switch calls `stop()` first. A shim that emitted a fresh event here would hide that. - **0/0 interval bounds are reported as the fast pair.** The firmware sets - 0/0 to mean "let the host pick" (`BlePositionServer.cpp:216-218`), and the + 0/0 to mean "let the host pick" (`BlePositionServer.cpp:225-226`), and the host picks `BLE_GAP_ADV_FAST_INTERVAL1`. The `advertising` event reports what the radio would use, not the sentinel (`src/SimBleGatt.cpp:773`). - **`deinit(clearAll)` deletes the table when `clearAll` is true**, same as the real API, which is why the firmware nulls its own characteristic pointers - first (`BlePositionServer.cpp:381-385`). With `false` the objects survive. + first (`BlePositionServer.cpp:388-393`). With `false` the objects survive. - **`deinit()` called from the host thread is refused with an `error`** (`src/SimBleGatt.cpp:112`). It would be a self-join. Real NimBLE deinit from the host task is equally broken; the firmware never does it, and a clear line @@ -511,16 +602,60 @@ showed. All four are `[verified]`, each by the named self-test check. device runs. 23 is the pessimistic default; 517 is the fast path. `[verified]` -- "MTU defaults to 23 when the client says nothing". +A fifth one is not a "must be right" but a "must be remembered", because it +falsifies results in both directions: + +5. **The shim is orders of magnitude faster than a radio, and that changes + which races are reachable.** Two consequences seen in one session. A race + the firmware's own comment prices as theoretical -- two file arrivals + collapsing into one because the activity loop polls a single-slot handoff -- + fired on the first attempt, because a whole file transfer finished in + milliseconds where a radio needs seconds. That is evidence the race exists + and that its price is higher than the comment says; it is **not** evidence + it can happen on hardware. In the other direction, the shim's own 10 ms + auto-confirm delay is longer than a whole transfer, so two indications that + a radio would separate by seconds land inside one confirm window and produce + a `clobber` a device would never see. A `clobber` in a fast run is a + simulator artefact until a slow run reproduces it. `[verified]` -- both + observed, the second chased with verbose logging and dismissed. + ## Fault injection -The point of a shim over hardware. +The point of a shim over hardware. The last four needed firmware attached and +were `[contract]` until a firmware with a file-transfer receiver was driven +through them. - Withhold a confirm (`auto_confirm` false, then never send `confirm`). `[verified]` -- "a withheld confirm never fires onStatus". +- **Withhold a confirm and leave it withheld, to time a firmware's own retry + budget.** `[verified]` -- a firmware waiting 3000 ms per indication took + 3000.x ms per line, 23 times over 69 s, on two independent clocks (the + client's monotonic clock and the firmware's own millisecond log). This is the + path a synchronous confirm can never execute. +- **Withhold the confirm for one chunk in the middle of a multi-chunk reply**, + rather than for a whole line. `[verified]`, and it is the sharper tool: the + firmware abandoned the rest of that reply, so the peer held an unterminated + prefix and the next reply block was appended straight onto it. One corrupt + line, no error, and the reply's own terminator still arrived. - Send a malformed frame (trailing bytes, bad path, oversized length). `[verified]` -- twenty malformed inputs in the transport gate, each answered with `error` and no crash. - Drop the link mid-transfer (`disconnect` while a transfer is running). - `[contract]`: needs a transfer, so it needs the firmware. + `[verified]` -- the firmware's disconnect hook ran, its partial file was + deleted and its transfer state was cleared, shown by a later transfer being + accepted rather than refused as busy. Note what this is *not*: a polite + `disconnect` op, never a radio dropout. - Send a transfer `begin` without subscribing to the status characteristic. - `[contract]`, same reason. + `[verified]` -- refused, and the refusal was parked and delivered the moment + the client subscribed, so a client can receive a verdict for a frame it has + already given up on. +- **Write a field value the wire format forbids.** `[verified]` -- a client can + put any byte string on a characteristic, including one whose length or field + domain the format does not allow, which is how a coordinate outside the + planet reached a renderer. Nothing between the client and the firmware + validates anything. +- **Corrupt the card underneath a transfer while it streams.** The simulated SD + is a host directory, so a `.part` file can be damaged from outside while + every byte on the wire stays correct. `[verified]` -- it separates "the + firmware checksummed the bytes it received" from "the firmware checksummed + the file it wrote", and here it was the latter. From ed3b445007346ac780f866ada6eb04b2cc726f4a Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 16:55:28 +0200 Subject: [PATCH 11/13] fix: READ was carrying broadcast's property bit NIMBLE_PROPERTY::READ was 0x0001. That is BLE_GATT_CHR_F_BROADCAST. Read is 0x0002 (nimble/host/include/host/ble_gatt.h:133), and standard GATT agrees, so it was not a NimBLE quirk being matched. Harmless today and wrong for upstream. No firmware characteristic sets READ, so nothing observable rode on it and no test could have caught it. But the property bitmask now goes out on the wire as an integer and a client decodes it back into names, so a readable characteristic would have made the shim say "broadcast" where the device meant "read". WRITE, NOTIFY and INDICATE were already right: 0x0008, 0x0010 and 0x0020, at ble_gatt.h:139, :142 and :145. Each constant now carries its source line, and the block says why 0x0001 is not read. WRITE_NO_RSP and the four flags above INDICATE stay undeclared: the firmware does not use them. The doc's props decoding table said `1` read, propagated from the same mistake. Corrected, and it now names the header to check rather than asking the reader to trust the table. --- docs/ble-shim.md | 11 +++++++++-- src/NimBLECharacteristic.h | 15 +++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index 4bc7101..7556e81 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -143,9 +143,16 @@ carries each characteristic's NimBLE property bitmask as a JSON number, not a list and not a string: `{"uuid":"...0002","props":8}` (`src/SimBleGatt.cpp:264`, `line += std::to_string(characteristic->m_properties)`). Observed values and their meaning, so a client can decode without guessing: -`1` read, `8` write, `16` notify, `32` indicate, and `56` (`0x38`) the +`2` read, `8` write, `16` notify, `32` indicate, and `56` (`0x38`) the write|notify|indicate combination the command characteristic carries -(`src/NimBLECharacteristic.h:26-31`). This is written down because it was the +(`src/NimBLECharacteristic.h:32-37`). Those are NimBLE's own +`BLE_GATT_CHR_F_*` values, read off the pinned host header at +`nimble/host/include/host/ble_gatt.h:130-145` -- check them there rather than +trusting this table. **`1` is broadcast, not read**, and it briefly appeared +here as read: the shim had the wrong constant, no firmware characteristic sets +READ so nothing observable rode on it, and no test could have caught it. It +would have surfaced the first time a characteristic became readable and a +client decoded the bitmask back into names. This is written down because it was the one field whose type nobody pinned: a laptop client built against the same prose assumed a list, called `list()` on the number, and every tool using it died inside `connect()` with `TypeError: 'int' object is not iterable` -- before diff --git a/src/NimBLECharacteristic.h b/src/NimBLECharacteristic.h index 43e19cf..c8891f2 100644 --- a/src/NimBLECharacteristic.h +++ b/src/NimBLECharacteristic.h @@ -23,11 +23,18 @@ // expression like `WRITE | NOTIFY | INDICATE` is an integer. Values are // NimBLE's own, so a props number in an emitted `gatt` event means the same // thing here as in a NimBLE header. +// +// Read off the pinned NimBLE host, not from memory -- `BLE_GATT_CHR_F_*` in +// nimble/host/include/host/ble_gatt.h:130-145. Only the four the firmware uses +// are declared. In particular 0x0001 is **broadcast**, not read: read is +// 0x0002 (ble_gatt.h:133), and standard GATT agrees, so this is not a NimBLE +// quirk. A shim carrying broadcast's bit under the name READ would tell a +// client "broadcast" wherever the device meant "read". namespace NIMBLE_PROPERTY { -static constexpr uint32_t READ = 0x0001; -static constexpr uint32_t WRITE = 0x0008; -static constexpr uint32_t NOTIFY = 0x0010; -static constexpr uint32_t INDICATE = 0x0020; +static constexpr uint32_t READ = 0x0002; // ble_gatt.h:133 +static constexpr uint32_t WRITE = 0x0008; // ble_gatt.h:139 +static constexpr uint32_t NOTIFY = 0x0010; // ble_gatt.h:142 +static constexpr uint32_t INDICATE = 0x0020; // ble_gatt.h:145 } // namespace NIMBLE_PROPERTY class NimBLECharacteristic; From 804768838bd89b56fa84366cbe0ffe4ad7cb700e Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 17:17:15 +0200 Subject: [PATCH 12/13] fix: indicate() refused three cases real NimBLE never refuses The shim returned false from indicate() when nothing was connected, when nobody was subscribed, and when the characteristic lacked NOTIFY/INDICATE, under a comment claiming those were refusals the real stack makes. It makes none of them. The firmware calls the two-argument indicate(), so connHandle defaults to BLE_HS_CONN_HANDLE_NONE (NimBLECharacteristic.h:60) and the call lands in sendValue (NimBLECharacteristic.cpp:272-328). That function has no connection check, no CCCD check and no property check: rc starts at 0, only the peer loop can move it, and with no peers the loop body never runs. It returns true. ble_gatts_indicate_custom refuses only on out of memory or, under BLE_GATT_CACHING, an unaware peer (ble_gattc.c:4888-4936), so an unsubscribed peer gets a real indication PDU and the pending slot is genuinely taken. Measured against the real firmware with a connected-but-unsubscribed peer: false: reply indicate failed after 40 attempts ~1008 ms, flag never set true: reply unconfirmed after 3000 ms 3000 ms, flag set Different duration, different branch, different log line, and different persistent state: only the second sets lastConfirmTimedOut_, which is what suppresses FETCH_CANCEL downstream. The simulator was sending developers into code the device never reaches. Nothing connected returns true and leaves the slot alone, emitting nothing: real NimBLE builds and sends nothing there, and filling the slot would let a later connect-then-confirm confirm a payload from before the link existed. A connected but unsubscribed peer takes the slot and gets the indicate event, because the PDU really does go out -- but no auto-confirm is scheduled, since a client with the CCCD off has nothing to confirm. That is what reproduces the firmware's confirm timeout. Two more corrections found in the same read. An empty payload is a different operation in real NimBLE, not a refusal: it falls through to ble_gatts_chr_updated and still returns true. The stack-down and null-pointer guards stay false and are now labelled as shim guards, because real NimBLE dereferences a null server there rather than returning anything. Clobber semantics are untouched: a second indicate() before a confirm still overwrites and still returns true. The two self-test checks that asserted the inversion are inverted rather than deleted, with the consequence added: unsubscribed indicate is true, the event still goes out, and nothing confirms it. The disconnect check that leaned on the old return value now proves the subscription cleared by its consequence -- no confirm on the new link, and a confirm again after re-subscribing. 50 checks. --- docs/ble-shim.md | 100 +++++++++++++++++++++++--------- src/SimBleGatt.cpp | 62 ++++++++++++++++---- tests/sim_ble_gatt_selftest.cpp | 71 ++++++++++++++++++++--- 3 files changed, 187 insertions(+), 46 deletions(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index 7556e81..732c141 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -26,15 +26,16 @@ Write this down first, because it is the part that gets forgotten. whatever coexistence rules the target platform has. - **The peer's real GATT stack.** A python client agreeing with the firmware proves the firmware self-consistent, not interoperable with a phone's stack. -- **A busy indication slot.** `indicate()` returns false only for a refusal -- - the stack down, nothing connected, nobody subscribed, wrong properties, empty - payload. It never returns false for "the slot is full", because a full slot is - clobbered and the call still succeeds (`src/SimBleGatt.cpp:342`). So the - firmware's - park-and-flush path for transfer status, which exists because the command - channel can be holding the connection's one slot - (`BlePositionServer.cpp:958-966`), is reachable here only through the - unsubscribed refusal. No retry-on-false loop is exercised by this shim. +- **A false return from `indicate()`, of any kind.** Real NimBLE's `sendValue` + has no connection check, no CCCD check and no property check + (`NimBLECharacteristic.cpp:272-328`), and `ble_gatts_indicate_custom` refuses + only on out-of-memory or, under `BLE_GATT_CACHING`, an unaware peer + (`ble_gattc.c:4888-4936`). Neither of those is modelled here, so **the shim's + `indicate()` never returns false except on its own two guards** (stack down, + null characteristic), and neither of those is reachable from the firmware. So + the firmware's park-and-flush path for transfer status, and its + retry-on-false loop, are both unreachable in the simulator. Whatever those + paths do, they are untested here. - **Throughput, of anything.** A `write` op returns when TCP took the bytes. There is no ATT write response, no connection interval and no radio in between, so a transfer rate measured here is a measurement of loopback. @@ -58,7 +59,7 @@ Write this down first, because it is the part that gets forgotten. its cleanup ran; the loss path did not. - **A hole in the delivered bytes from a withheld confirm.** The `indicate` event goes out when the payload enters the pending slot, and `clobber` only - when a later payload takes that slot (captured at `src/SimBleGatt.cpp:343`, both + when a later payload takes that slot (captured at `src/SimBleGatt.cpp:382`, both lines emitted at `:358-368`). So a client is handed the clobbered payload and *then* told it was dropped. On hardware a clobbered indication is genuinely gone -- 18 back-to-back calls, @@ -115,9 +116,9 @@ firmware uses. dance exists. A timeout test turns it off. **`auto_confirm` is not connection state, and it is not reset when a client -leaves.** It is set at `src/SimBleGatt.cpp:674` and cleared only by +leaves.** It is set at `src/SimBleGatt.cpp:716` and cleared only by `init()` (`src/SimBleGatt.cpp:82`); `clearConnectionLocked` -(`src/SimBleGatt.cpp:744`) resets the MTU at `:747` and does not touch it. +(`src/SimBleGatt.cpp:786`) resets the MTU at `:789` and does not touch it. A reconnecting client therefore inherits the previous client's setting. Real hardware has no such notion, so this is a test-isolation trap rather than a fidelity gap -- but a test that reconnects must set `auto_confirm` explicitly @@ -164,19 +165,27 @@ raw. - `write` or `subscribe` with no central connected: `error`, no callback. `[verified]` for `write` -- "write with no connection fires no callback" and - "write with no connection emits error" (`src/SimBleGatt.cpp:586`). - `subscribe` takes the same branch (`src/SimBleGatt.cpp:606`) and is + "write with no connection emits error" (`src/SimBleGatt.cpp:628`). + `subscribe` takes the same branch (`src/SimBleGatt.cpp:648`) and is `[contract]`. -- `indicate()` with nobody subscribed to that characteristic: returns false. - `[verified]` -- "indicate with nobody subscribed returns false" and the same - for a second characteristic (`src/SimBleGatt.cpp:327`). +- `indicate()` with nobody subscribed, or with nothing connected at all: + **returns true**, because real NimBLE does. Read `sendValue` + (`NimBLECharacteristic.cpp:272-328`): `rc` starts at 0, the peer loop is the + only thing that can move it, and there is no CCCD or property test anywhere. + With no peers the loop body never runs and it returns true; with an + unsubscribed peer it transmits anyway. The firmware says the same thing in + its own words -- "indicate() succeeds into an empty room" + (`BlePositionServer.cpp:79-81`). `[verified]` -- "indicate with nobody + subscribed returns true", "indicate with nothing connected returns true" + (`src/SimBleGatt.cpp:348`). - A subscription belongs to a connection. On `disconnect` the shim clears subscription state itself, because NimBLE fires no unsubscribe callback. `[verified]` -- "disconnect fires no unsubscribe callback, same as NimBLE" - and "the subscription is gone after a disconnect" - (`src/SimBleGatt.cpp:756`). + and "the subscription is gone after a disconnect: no confirm on the new + link", paired with "a fresh subscribe restores the confirm, so the clear was + real" (`src/SimBleGatt.cpp:798`). - Advertising stops on connect and is not restarted by the shim. `[verified]` - -- "advertising goes down on connect" (`src/SimBleGatt.cpp:562`). Only the + -- "advertising goes down on connect" (`src/SimBleGatt.cpp:604`). Only the firmware's own `onDisconnect` brings it back. - Two more the real stack makes, added while building it: a `write` to a characteristic without the WRITE property is an `error` (`[verified]`, @@ -503,31 +512,31 @@ unusual thing a real caller does. ### Decisions worth naming - **The indication slot is per connection, not per characteristic** - (`src/SimBleGatt.cpp:342`). That is what the firmware assumes: its transfer + (`src/SimBleGatt.cpp:382`). That is what the firmware assumes: its transfer status channel parks a line precisely because the command channel can be holding the one slot (`BlePositionServer.cpp:958-966`). - **A clobbered burst yields one confirm, not one per call.** The shim's auto-confirm carries the sequence number of the payload it was created for, and a confirm for a payload that has since been clobbered is dropped - silently (`src/SimBleGatt.cpp:625`). Two `indicate()` calls with one confirm + silently (`src/SimBleGatt.cpp:667`). Two `indicate()` calls with one confirm between them is the hardware behaviour: 18 calls, two payloads seen. - **A client `confirm` op confirms whatever is pending**, regardless of which payload the client thought it was confirming. It errors when nothing is pending, or when its `uuid` names a different characteristic than the pending one. - **`advertising` is also emitted with `up:false` when a central connects** - (`src/SimBleGatt.cpp:562`). The event table above lists start/stop; this is + (`src/SimBleGatt.cpp:604`). The event table above lists start/stop; this is the third case, and without it a client's view of advertising would be wrong for the whole connection. - **`start()` while already advertising is a no-op success and emits nothing** - (`src/SimBleGatt.cpp:392`). Real NimBLE behaves that way + (`src/SimBleGatt.cpp:434`). Real NimBLE behaves that way (`NimBLEAdvertising.cpp:194-197`), which is why the firmware's slow-interval switch calls `stop()` first. A shim that emitted a fresh event here would hide that. - **0/0 interval bounds are reported as the fast pair.** The firmware sets 0/0 to mean "let the host pick" (`BlePositionServer.cpp:225-226`), and the host picks `BLE_GAP_ADV_FAST_INTERVAL1`. The `advertising` event reports what - the radio would use, not the sentinel (`src/SimBleGatt.cpp:773`). + the radio would use, not the sentinel (`src/SimBleGatt.cpp:815`). - **`deinit(clearAll)` deletes the table when `clearAll` is true**, same as the real API, which is why the firmware nulls its own characteristic pointers first (`BlePositionServer.cpp:388-393`). With `false` the objects survive. @@ -542,7 +551,7 @@ unusual thing a real caller does. `SimBleGatt::waitIdle()` blocks until the host thread's queue is empty and it is not mid-dispatch, including events not yet due -(`src/SimBleGatt.cpp:518`). It exists so a test can assert that a callback did +(`src/SimBleGatt.cpp:560`). It exists so a test can assert that a callback did **not** fire without racing the host thread. The firmware never calls it, and a client cannot reach it. @@ -554,7 +563,7 @@ client cannot reach it. (`src/SimBleGatt.cpp:129`); the reader is `SimBleLink`'s. - The reader parses lines and pushes events onto the host thread's queue. It never calls firmware code. `[verified]` by reading - `SimBleGatt::onReaderEvent` (`src/SimBleGatt.cpp:418`): it maps an op name to + `SimBleGatt::onReaderEvent` (`src/SimBleGatt.cpp:460`): it maps an op name to a queue entry and returns. Even a state-only op (`rssi`, `auto_confirm`) is queued rather than applied there, so ordering is whatever the client sent. - The host thread dispatches every firmware callback. It is the simulator's @@ -571,7 +580,7 @@ client cannot reach it. across these threads. `[verified]` by reading that file. - One mutex guards the whole GATT model, including the fields inside the `NimBLE*` wrapper objects, and it is **dropped before every firmware - callback** (`src/SimBleGatt.cpp:506`). It has to be: the firmware's + callback** (`src/SimBleGatt.cpp:548`). It has to be: the firmware's `onDisconnect` calls `advertising->start()`, which comes straight back in. `[verified]` -- the self-test runs clean under ThreadSanitizer (`-fsanitize=thread`, under `setarch -R` because TSan needs ASLR off on this @@ -597,7 +606,7 @@ showed. All four are `[verified]`, each by the named self-test check. `indicate()` returned) and "a withheld confirm never fires onStatus" (with `auto_confirm` off, it never fires at all). Even the shim's own auto-confirm goes through the host thread's queue with a delay - (`src/SimBleGatt.cpp:379`), so it cannot short-circuit. + (`src/SimBleGatt.cpp:421`), so it cannot short-circuit. 3. **A second `indicate()` before a confirm clobbers the first.** Real and measured on hardware: back-to-back calls all returned true, the peer saw the first and the last. The shim must reproduce the clobber, not queue politely. @@ -609,6 +618,39 @@ showed. All four are `[verified]`, each by the named self-test check. device runs. 23 is the pessimistic default; 517 is the fast path. `[verified]` -- "MTU defaults to 23 when the client says nothing". +### How the first of those got broken: the shim copied a comment, not a library + +Worth reading before adding to this shim, because nothing in the process caught +it. + +The shim's `indicate()` used to return **false** when nobody was subscribed, +when nothing was connected, and when the characteristic lacked NOTIFY/INDICATE, +under a comment reading "refusals the real stack makes". Real NimBLE makes none +of those three checks. The value came from a firmware comment that is itself +wrong -- `BlePositionServer.cpp:294-296` claims "a busy/still-pending +`indicate()` actually returns false for the retry loop to catch" -- rather than +from `sendValue`, which was on disk the whole time. + +The cost, measured against the real firmware with a connected-but-unsubscribed +peer: + +``` +false: [ERR] reply indicate failed after 40 attempts: INFO pos=0 (~1008 ms) +true: [ERR] reply unconfirmed after 3000 ms, 11 bytes dropped (3000 ms) +``` + +Different duration, different branch, different log line, and different +persistent state: only the second path sets `lastConfirmTimedOut_` +(`BlePositionServer.cpp:639`), which is the flag that suppresses `FETCH_CANCEL` +downstream. A developer reproducing the unsubscribed case in the simulator was +exercising code the device never reaches. + +**No test could have caught it.** Two self-test checks asserted the inversion as +correct, so the suite agreed with the bug. A test written from the same belief +as the code confirms the belief, not the behaviour. The only thing that would +have caught it is reading the library being faked -- which is the rule this repo +already has for the pinned SDK, applied to a vendored library instead. + A fifth one is not a "must be right" but a "must be remembered", because it falsifies results in both directions: diff --git a/src/SimBleGatt.cpp b/src/SimBleGatt.cpp index d68f48e..c241b17 100644 --- a/src/SimBleGatt.cpp +++ b/src/SimBleGatt.cpp @@ -317,18 +317,58 @@ bool SimBleGatt::indicate(NimBLECharacteristic *characteristic, uint32_t autoConfirmDelayMs = 0; uint32_t seq = 0; + bool subscribed = false; + { std::lock_guard lock(m_mutex); + // **Shim guards, not NimBLE behaviour.** Real NimBLE reaches + // `NimBLEDevice::getServer()->getPeerDevices()` with no null check + // (NimBLECharacteristic.cpp:296), so with the stack down or a null + // characteristic it dereferences a null pointer rather than returning + // anything. A fake must not crash where the real thing crashes, so these + // two return false. Neither is reachable from the firmware, which checks + // `begun_` and its own pointers first (BlePositionServer.cpp:547). if (!m_initialized || characteristic == nullptr) return false; - if (data == nullptr || len == 0) return false; - // Refusals the real stack makes. Each one is a false return with no event - // and no callback. - if (!m_connected) return false; - if (characteristic->m_subValue == 0) return false; - if ((characteristic->m_properties & - (NIMBLE_PROPERTY::INDICATE | NIMBLE_PROPERTY::NOTIFY)) == 0) { - return false; - } + + // **Real NimBLE refuses nothing here.** The firmware calls the + // two-argument indicate(), so `connHandle` defaults to + // BLE_HS_CONN_HANDLE_NONE (NimBLECharacteristic.h:60) and the call lands + // in sendValue (NimBLECharacteristic.cpp:272-328). That function has no + // connection check, no CCCD check and no property check. It loops over + // `getPeerDevices()`; `rc` starts at 0 and only a real transmit error + // moves it, so **it returns true**. + // + // Nothing connected: the loop body never runs, nothing is built and + // nothing is sent, and `rc` is still 0 at `done:`. So: true, and the slot + // is untouched. Filling it would let a later connect-then-confirm confirm + // a payload from before the link existed. + // + // The firmware states the same fact in its own words + // (BlePositionServer.cpp:79-81): "indicate() succeeds into an empty room". + if (!m_connected) return true; + + // Connected: from here real NimBLE transmits regardless of subscription + // and regardless of properties. ble_gatts_indicate_custom does no CCCD + // lookup -- it calls ble_att_clt_tx_indicate unconditionally and records + // the pending handle (ble_gattc.c:4922-4936). Its only refusals are out of + // memory and, under BLE_GATT_CACHING, an unaware peer. So an unsubscribed + // peer gets a real indication PDU it will drop, and the slot is genuinely + // taken -- which is exactly why the firmware then waits out its 3000 ms + // confirm (BlePositionServer.cpp:629). + // + // Whether anybody subscribed decides one thing only: whether a confirm can + // ever come back. `auto_confirm` is the client saying "I confirm what I + // receive", and a client with the CCCD off receives nothing to confirm. + subscribed = characteristic->m_subValue != 0; + + // Empty payload is a different operation in real NimBLE, not a refusal: + // sendValue falls through to ble_gatts_chr_updated + // (NimBLECharacteristic.cpp:317-319), which pushes the characteristic's + // stored value to whoever subscribed, and still returns true. The shim + // does not model the stored-value push, so it returns true and emits + // nothing. Unreachable from the firmware, which never indicates zero bytes + // (BlePositionServer.cpp:547, :566-570). + if (data == nullptr || len == 0) return true; // One slot per connection, not per characteristic: the firmware's transfer // status channel parks a line precisely because the command channel can be @@ -352,7 +392,9 @@ bool SimBleGatt::indicate(NimBLECharacteristic *characteristic, uuid = m_pendingUuid; payload = m_pendingPayload; seq = m_pendingSeq; - autoConfirm = m_autoConfirm; + // No subscriber, no confirm -- ever. See the reasoning above: this is what + // reproduces the firmware's confirm timeout instead of its retry loop. + autoConfirm = m_autoConfirm && subscribed; autoConfirmDelayMs = m_autoConfirmDelayMs; } diff --git a/tests/sim_ble_gatt_selftest.cpp b/tests/sim_ble_gatt_selftest.cpp index 7c10681..b27c217 100644 --- a/tests/sim_ble_gatt_selftest.cpp +++ b/tests/sim_ble_gatt_selftest.cpp @@ -215,13 +215,41 @@ int main() { check(sawEvent("\"ev\":\"advertising\",\"up\":false"), "advertising goes down on connect"); - // --- indicate with nobody subscribed returns false --------------------- + // --- indicate with nobody subscribed returns TRUE ---------------------- + // Real NimBLE has no CCCD check: sendValue transmits and returns true + // (NimBLECharacteristic.cpp:272-328). A shim returning false here sends the + // firmware down its 40 x 25 ms retry loop instead of its 3000 ms confirm + // wait -- different duration, different log line, different persistent + // state. These two assertions used to claim the opposite; they are inverted + // so a regression to it fails. const uint8_t payloadA[] = {'l', 'i', 'n', 'e', '-', 'A', '\n'}; const uint8_t payloadB[] = {'l', 'i', 'n', 'e', '-', 'B', '\n'}; - check(!cmdChar->indicate(payloadA, sizeof(payloadA)), - "indicate with nobody subscribed returns false"); - check(!statusChar->indicate(payloadA, sizeof(payloadA)), - "indicate on an unsubscribed second characteristic returns false"); + simble_selftest::clearEmitted(); + g_counters.onStatus = 0; + check(cmdChar->indicate(payloadA, sizeof(payloadA)), + "indicate with nobody subscribed returns true"); + check(sawEvent("\"ev\":\"indicate\""), + "an unsubscribed indication still goes out on the wire"); + SimBleGatt::get().waitIdle(); + check(g_counters.onStatus == 0, + "nothing confirms an unsubscribed indication, even with auto_confirm on"); + check(statusChar->indicate(payloadA, sizeof(payloadA)), + "indicate on an unsubscribed second characteristic returns true"); + check(sawEvent("\"ev\":\"clobber\""), + "the second unsubscribed indication clobbered the first, slot and all"); + + // Nothing connected is the other true: the peer loop never runs, so nothing + // is built, nothing is sent, and the slot is left alone. + SimBleEvent dropForNoConn = op("disconnect"); + dropForNoConn.a = 0x13; + feedAndSettle(dropForNoConn); + simble_selftest::clearEmitted(); + check(cmdChar->indicate(payloadA, sizeof(payloadA)), + "indicate with nothing connected returns true"); + check(simble_selftest::emitted().empty(), + "indicate with nothing connected emits nothing at all"); + feedAndSettle(connect); + simble_selftest::clearEmitted(); SimBleEvent subscribe = op("subscribe"); subscribe.uuid = kCmdUuid; @@ -308,6 +336,7 @@ int main() { // --- 5: disconnect clears the subscription, with no callback ----------- simble_selftest::clearEmitted(); g_counters.onSubscribe = 0; + g_counters.onDisconnect = 0; SimBleEvent disconnect = op("disconnect"); disconnect.a = 0x13; feedAndSettle(disconnect); @@ -316,8 +345,36 @@ int main() { "disconnect fires onDisconnect with the reason"); check(g_counters.onSubscribe == 0, "disconnect fires no unsubscribe callback, same as NimBLE"); - check(!cmdChar->indicate(payloadA, sizeof(payloadA)), - "the subscription is gone after a disconnect"); + // The old assertion here was `!indicate(...)`, which only passed because of + // the inversion above. Proving the subscription cleared now takes the + // consequence rather than the return value: reconnect, indicate, and see + // that no confirm comes back until a fresh subscribe. + feedAndSettle(connect); + g_counters.onStatus = 0; + check(cmdChar->indicate(payloadA, sizeof(payloadA)), + "indicate is accepted again on the new link"); + SimBleGatt::get().waitIdle(); + check(g_counters.onStatus == 0, + "the subscription is gone after a disconnect: no confirm on the new link"); + SimBleEvent resub = op("subscribe"); + resub.uuid = kCmdUuid; + resub.a = 2; + feedAndSettle(resub); + // auto_confirm was switched off earlier to test a withheld confirm. Back on, + // so the positive half of this pair can actually confirm. + SimBleEvent autoConfirmOn = op("auto_confirm"); + autoConfirmOn.flag = true; + autoConfirmOn.a = 10; + feedAndSettle(autoConfirmOn); + g_counters.onStatus = 0; + check(cmdChar->indicate(payloadB, sizeof(payloadB)), + "indicate accepted after re-subscribing"); + SimBleGatt::get().waitIdle(); + check(g_counters.onStatus == 1, + "a fresh subscribe restores the confirm, so the clear was real"); + SimBleEvent dropAgain = op("disconnect"); + dropAgain.a = 0x13; + feedAndSettle(dropAgain); check(ble_gap_conn_rssi(g_counters.lastHandle, &rssiOut) != 0, "ble_gap_conn_rssi fails with no connection"); check(posChar != nullptr, "the position characteristic outlives the link"); From 3ff80e0195b6ac9c7e17284277fd29fe4a4c9d65 Mon Sep 17 00:00:00 2001 From: Roman Fordinal Date: Sun, 23 Aug 2026 19:12:02 +0200 Subject: [PATCH 13/13] docs: name the downstream firmware generically, for an upstream reader Nineteen comment and doc references pointed at BlePositionServer.cpp and FREEINK_CAP_BLE_PERIPHERAL -- a file and a build flag that do not exist upstream, which made the branch unreviewable by anyone outside this fork. Each one now states the behaviour instead of the address. Nothing load-bearing was lost: the authority in these comments was always NimBLE's own source (NimBLECharacteristic.cpp, ble_gattc.c, ble_gatt.h), which an upstream reader can check, and the firmware was only ever provenance. The syntax-check recipe keeps its shape with placeholders rather than one fork's paths. Gates unchanged: 50 GATT checks, 9 attach checks, 65 transport checks. --- docs/ble-shim.md | 58 +++++++++++++++------------------ src/NimBLEDevice.h | 5 ++- src/SimBleGatt.cpp | 26 +++++++-------- src/host/ble_gap.h | 6 ++-- tests/sim_ble_gatt_selftest.cpp | 4 +-- 5 files changed, 46 insertions(+), 53 deletions(-) diff --git a/docs/ble-shim.md b/docs/ble-shim.md index 732c141..bf40292 100644 --- a/docs/ble-shim.md +++ b/docs/ble-shim.md @@ -173,9 +173,9 @@ raw. (`NimBLECharacteristic.cpp:272-328`): `rc` starts at 0, the peer loop is the only thing that can move it, and there is no CCCD or property test anywhere. With no peers the loop body never runs and it returns true; with an - unsubscribed peer it transmits anyway. The firmware says the same thing in - its own words -- "indicate() succeeds into an empty room" - (`BlePositionServer.cpp:79-81`). `[verified]` -- "indicate with nobody + unsubscribed peer it transmits anyway. A firmware built against real NimBLE + puts the same thing in one line: "indicate() succeeds into an empty room". + `[verified]` -- "indicate with nobody subscribed returns true", "indicate with nothing connected returns true" (`src/SimBleGatt.cpp:348`). - A subscription belongs to a connection. On `disconnect` the shim clears @@ -462,11 +462,11 @@ The same thing on one translation unit, for a quick check while editing the headers: ``` -g++ -std=c++17 -fsyntax-only -DFREEINK_CAP_BLE_PERIPHERAL=1 -Isrc \ - -I/lib/BlePositionServer/include -I/lib/Logging \ +g++ -std=c++17 -fsyntax-only -Isrc \ + -I/ -I/ \ -include Arduino.h -include freertos/FreeRTOS.h \ -include freertos/task.h -include freertos/semphr.h \ - /lib/BlePositionServer/src/BlePositionServer.cpp + /.cpp ``` The `-include` flags stand in for what the firmware's own build has already @@ -476,14 +476,12 @@ FreeRTOS name, not a NimBLE one, so it does not touch what this run is for. Two NimBLE calls the seed contract did not list turned up building it, and both are real: -- **`NimBLEServer::start()`** (`BlePositionServer.cpp:322`). The firmware calls - it, not the deprecated `NimBLEService::start()`, and builds the GATT table - with it. It is what emits the `gatt` event (`src/SimBleGatt.cpp:249-270`). +- **`NimBLEServer::start()`**. A firmware may call it, rather than the + deprecated `NimBLEService::start()`, and build the GATT table with it. It is what emits the `gatt` event (`src/SimBleGatt.cpp:249-270`). `NimBLEService::start()` exists anyway and returns true. -- **`NimBLEServer::setCallbacks()` takes a second argument** - (`BlePositionServer.cpp:283`, `deleteCallbacks=false`). The shim ignores it - and never owns the pointer; the firmware registers a static object, so - nothing leaks either way (`src/NimBLEDevice.cpp:37`). +- **`NimBLEServer::setCallbacks()` takes a second `deleteCallbacks` argument.** + The shim ignores it and never owns the pointer, so a caller registering a + static object leaks nothing either way (`src/NimBLEDevice.cpp:37`). Nothing else was missing. @@ -512,9 +510,9 @@ unusual thing a real caller does. ### Decisions worth naming - **The indication slot is per connection, not per characteristic** - (`src/SimBleGatt.cpp:382`). That is what the firmware assumes: its transfer - status channel parks a line precisely because the command channel can be - holding the one slot (`BlePositionServer.cpp:958-966`). + (`src/SimBleGatt.cpp:382`). That is what a firmware with two indicating + characteristics has to assume: one channel parks a line precisely because the + other can be holding the single slot. - **A clobbered burst yields one confirm, not one per call.** The shim's auto-confirm carries the sequence number of the payload it was created for, and a confirm for a payload that has since been clobbered is dropped @@ -533,19 +531,17 @@ unusual thing a real caller does. (`NimBLEAdvertising.cpp:194-197`), which is why the firmware's slow-interval switch calls `stop()` first. A shim that emitted a fresh event here would hide that. -- **0/0 interval bounds are reported as the fast pair.** The firmware sets - 0/0 to mean "let the host pick" (`BlePositionServer.cpp:225-226`), and the - host picks `BLE_GAP_ADV_FAST_INTERVAL1`. The `advertising` event reports what +- **0/0 interval bounds are reported as the fast pair.** A caller sets 0/0 to + mean "let the host pick", and the host picks `BLE_GAP_ADV_FAST_INTERVAL1`. The `advertising` event reports what the radio would use, not the sentinel (`src/SimBleGatt.cpp:815`). - **`deinit(clearAll)` deletes the table when `clearAll` is true**, same as the - real API, which is why the firmware nulls its own characteristic pointers - first (`BlePositionServer.cpp:388-393`). With `false` the objects survive. + real API, which is why a caller should null its own characteristic pointers + first. With `false` the objects survive. - **`deinit()` called from the host thread is refused with an `error`** (`src/SimBleGatt.cpp:112`). It would be a self-join. Real NimBLE deinit from - the host task is equally broken; the firmware never does it, and a clear line - beats an abort. -- **The advertising name has no default in the shim.** The firmware passes it - in, and the shim never invents one, so no device name is baked in here. + the host task is equally broken, and a clear line beats an abort. +- **The advertising name has no default in the shim.** The caller passes it in + and the shim never invents one, so no device name is baked in here. ### `waitIdle()` is the one addition that is not NimBLE @@ -626,12 +622,12 @@ it. The shim's `indicate()` used to return **false** when nobody was subscribed, when nothing was connected, and when the characteristic lacked NOTIFY/INDICATE, under a comment reading "refusals the real stack makes". Real NimBLE makes none -of those three checks. The value came from a firmware comment that is itself -wrong -- `BlePositionServer.cpp:294-296` claims "a busy/still-pending +of those three checks. The value came from a comment in the firmware this shim +was developed against, which is itself wrong -- it claims "a busy/still-pending `indicate()` actually returns false for the retry loop to catch" -- rather than from `sendValue`, which was on disk the whole time. -The cost, measured against the real firmware with a connected-but-unsubscribed +The cost, measured against that firmware with a connected-but-unsubscribed peer: ``` @@ -640,9 +636,9 @@ true: [ERR] reply unconfirmed after 3000 ms, 11 bytes dropped (3000 ms) ``` Different duration, different branch, different log line, and different -persistent state: only the second path sets `lastConfirmTimedOut_` -(`BlePositionServer.cpp:639`), which is the flag that suppresses `FETCH_CANCEL` -downstream. A developer reproducing the unsubscribed case in the simulator was +persistent state: only the second path sets the firmware's +confirm-timed-out flag, which suppresses a downstream cancel. A developer +reproducing the unsubscribed case in the simulator was exercising code the device never reaches. **No test could have caught it.** Two self-test checks asserted the inversion as diff --git a/src/NimBLEDevice.h b/src/NimBLEDevice.h index 40dcca5..7a020a1 100644 --- a/src/NimBLEDevice.h +++ b/src/NimBLEDevice.h @@ -80,9 +80,8 @@ class NimBLEServer { bool deleteCallbacks = true); // Builds the GATT table and emits the `gatt` event. False when the stack is - // down. Not in the shim's original contract list -- the firmware calls it - // (BlePositionServer.cpp:314) in place of the deprecated - // NimBLEService::start(). + // down. A downstream firmware calls this in place of the deprecated + // NimBLEService::start(), which is why it exists here. bool start(); // Asks the central for new connection parameters. A request, not a command: diff --git a/src/SimBleGatt.cpp b/src/SimBleGatt.cpp index c241b17..6696f93 100644 --- a/src/SimBleGatt.cpp +++ b/src/SimBleGatt.cpp @@ -326,8 +326,8 @@ bool SimBleGatt::indicate(NimBLECharacteristic *characteristic, // (NimBLECharacteristic.cpp:296), so with the stack down or a null // characteristic it dereferences a null pointer rather than returning // anything. A fake must not crash where the real thing crashes, so these - // two return false. Neither is reachable from the firmware, which checks - // `begun_` and its own pointers first (BlePositionServer.cpp:547). + // two return false. Neither is reachable from a firmware that checks its own + // initialised flag and its own pointers first. if (!m_initialized || characteristic == nullptr) return false; // **Real NimBLE refuses nothing here.** The firmware calls the @@ -343,8 +343,8 @@ bool SimBleGatt::indicate(NimBLECharacteristic *characteristic, // is untouched. Filling it would let a later connect-then-confirm confirm // a payload from before the link existed. // - // The firmware states the same fact in its own words - // (BlePositionServer.cpp:79-81): "indicate() succeeds into an empty room". + // A firmware built against real NimBLE puts the same fact in one line: + // "indicate() succeeds into an empty room". if (!m_connected) return true; // Connected: from here real NimBLE transmits regardless of subscription @@ -353,8 +353,8 @@ bool SimBleGatt::indicate(NimBLECharacteristic *characteristic, // the pending handle (ble_gattc.c:4922-4936). Its only refusals are out of // memory and, under BLE_GATT_CACHING, an unaware peer. So an unsubscribed // peer gets a real indication PDU it will drop, and the slot is genuinely - // taken -- which is exactly why the firmware then waits out its 3000 ms - // confirm (BlePositionServer.cpp:629). + // taken -- which is exactly why a firmware waiting on a confirm waits out its + // whole timeout instead of failing fast. // // Whether anybody subscribed decides one thing only: whether a confirm can // ever come back. `auto_confirm` is the client saying "I confirm what I @@ -366,13 +366,12 @@ bool SimBleGatt::indicate(NimBLECharacteristic *characteristic, // (NimBLECharacteristic.cpp:317-319), which pushes the characteristic's // stored value to whoever subscribed, and still returns true. The shim // does not model the stored-value push, so it returns true and emits - // nothing. Unreachable from the firmware, which never indicates zero bytes - // (BlePositionServer.cpp:547, :566-570). + // nothing. Unreachable from a firmware that never indicates zero bytes. if (data == nullptr || len == 0) return true; - // One slot per connection, not per characteristic: the firmware's transfer - // status channel parks a line precisely because the command channel can be - // holding it (BlePositionServer.cpp:956-958). + // One slot per connection, not per characteristic. That is what the real + // host does, and a firmware with two indicating characteristics feels it: + // one channel has to park a line while the other holds the slot. // // An overwrite is what real hardware does. 18 back-to-back indicate() // calls all returned true and the peer saw the first and the last, so a @@ -791,9 +790,8 @@ void SimBleGatt::clearConnectionLocked() { m_latency = 0; m_timeout = 0; // A subscription belongs to a connection, and NimBLE fires no unsubscribe - // callback when the link drops. The shim clears it here, silently, because - // the firmware relies on there being no callback - // (BlePositionServer.cpp:713-716). + // callback when the link drops. The shim clears it here, silently, because a + // firmware written against real NimBLE relies on there being no callback. for (NimBLECharacteristic *characteristic : m_characteristics) { characteristic->m_subValue = 0; } diff --git a/src/host/ble_gap.h b/src/host/ble_gap.h index 6e9f165..b9b352f 100644 --- a/src/host/ble_gap.h +++ b/src/host/ble_gap.h @@ -2,10 +2,10 @@ // Shim for NimBLE's host/ble_gap.h. // -// The firmware includes this header directly for one function: +// A firmware may include this header directly for one function: // ble_gap_conn_rssi(). No GAP wrapper exists on NimBLEServer or -// NimBLEConnInfo, so the firmware reaches past the C++ API to the C host -// (see the include comment in the firmware's BlePositionServer.cpp). +// NimBLEConnInfo, so reading RSSI means reaching past the C++ API to the C +// host. // // The host error codes and the advertising interval defaults live here too. // Real NimBLE spreads them over host/ble_hs.h and host/ble_gap.h; the shim diff --git a/tests/sim_ble_gatt_selftest.cpp b/tests/sim_ble_gatt_selftest.cpp index b27c217..d75f491 100644 --- a/tests/sim_ble_gatt_selftest.cpp +++ b/tests/sim_ble_gatt_selftest.cpp @@ -1,6 +1,6 @@ // Self-test for the NimBLE shim. No socket, no device, no firmware: it drives -// the shim's API the way BlePositionServer does and asserts the behaviour the -// contract calls for. +// the shim's API the way a firmware BLE server does and asserts the behaviour +// the contract calls for. // // Build and run (one line, no continuations): // g++ -std=c++17 -Wall -Wextra -pthread -Isrc -o /tmp/sim_ble_gatt_selftest