Repository navigation
A weather station in Python and in C++, and the two FFI doors under them - #12
Merged
Merged
Conversation
…and enhance error handling in StationError
The pendant of weather-station-py, run for the language where the boundary is a C ABI: no interpreter mediates and the compiler stops helping at the extern "C". Built to find out what an FFI layer needs from the station crates before they reach a registry. Two artifacts rather than one, because Rust cannot export C++ — no class, no std::string, no std::function has an ABI stable even between two builds of the same compiler. src/lib.rs exports fourteen ws_* symbols; the C++ RAII and exception layer is header-only and compiled by the consumer's own toolchain. That is the same conclusion weather-station-py reached from the other end. The Python door's crate fixes carried, exactly as its README predicted the C ABI layer would inherit rather than re-solve them: shutdown(&self) is what lets ws_station_close take a const pointer, and is_closed() reading an atomic is what survives a log sink reentering the getters from aimdb's runtime thread mid-shutdown. What C adds: nothing may unwind across the boundary (catch_unwind at every entry point, and a noexcept trampoline in the header for the other direction), every argument is hostile (NULL and non-UTF-8 paths are refused, not mangled), and ownership has to be prose because free is the one entry point that is not thread-safe. What it found: after fork(), the child is told the station is open, its publish returns success, and the reading is dropped. Measured against a live broker. No fix here — pthread_atfork is process-global, and registering it on the application's behalf is the trespass this crate's predecessor already established a rule against. Findings in README.md; the twelve requirements they imply for aimdb core in REQUIREMENTS.md, four of them release-gating. Run it with `make spike-cpp` (needs mosquitto and a C++17 compiler). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRVG2B9A8hfX8JMiXsW6nD
Fixes both, in patches/ against the sibling aimdb checkout, and records what tracing them turned up. Neither needed a new idea. The unbounded spin in AimDbHandle::new becomes a channel and one blocking_recv, which is the shape new_from_builder has used since it was written — a runtime that fails to start is now AttachFailed instead of a caller that never returns. The 10 ms poll in detach_internal becomes recv_timeout on a channel the helper thread sends through. The poll was the floor under every shutdown in every language binding: close() drops from 10 ms to 0-1 ms across four spike rounds. aimdb-sync no longer busy-waits anywhere. Two corrections and one escalation: CR-2's claim that the spin could hang ws_station_open_profile was wrong. There are two attach() entry points and weather-station takes the other one; the spin was never on this door's path. The second constructor is the original design-007 entry point, superseded once AimDbBuilder::build() went async and had to run inside the runtime thread. It has no in-tree callers left, which is why nothing exercised the spin. Worth keeping now that it is fixed, but it needs a test. CR-6 is not a long-standing API shape to reconsider. aimdb-sync's own changelog files it under Changed (breaking) for the unreleased 0.6.0: SyncConsumer's methods took &self and it was Clone + Sync in 0.5.0. The compiler confirms the loss of Sync and root-causes it to Box<dyn BufferReader<T> + Send> in aimdb-core's Reader<T>. That makes it the one finding so far that is release-gating rather than patchable later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRVG2B9A8hfX8JMiXsW6nD
Extends the busy-wait patch. The channel that replaced the spin now carries
Result<T, String> rather than a bare handle, so the three outcomes have
three arms: the value, the thread's own reason for failing, and a dropped
sender — the last being the one that used to be a hang.
The Err arm is an addition rather than a consequence of removing the spin,
and it is what an FFI consumer needs. A startup failure used to arrive as
"runtime thread failed to send handle" while the real io::Error went only
to a log sink the caller may never have installed. new_from_builder gets
the same treatment on both of its channels, which also recovers the reason
a database build failed.
Measured by forcing a build failure and catching at the C++ boundary:
before: Failed to attach database: Runtime thread failed to build database
after: Failed to attach database: Failed to build database: missing
parameter 'broker.url'
aimdb-sync is fmt clean, weather-station --features sync passes 16/2/4, and
make spike-cpp is all-green with close() at 0-1 ms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MRVG2B9A8hfX8JMiXsW6nD
Two passes of the review said different things about SyncConsumer and both were wrong the same way. REQUIREMENTS.md filed it as an API shape to reconsider; the last commit escalated it to the one release-gating finding. probes/cr6-consumer-shape says neither. &mut self is the honest signature: a reader is a cursor, poll_recv advances it, so reading mutates. The question was never whether it could be &self but what a consumer represents — and the answer makes the FFI shape fall out. A consumer is a subscription. AimDbHandle::consumer takes &self, the handle is Sync, and every call is a fresh subscribe_boxed with its own cursor: four threads created one each concurrently and all four saw every value, at 891ns per consumer. So the consumer half of a C ABI needs no core change. One ws_consumer* per thread, single-owner, the same ownership rule the header already states for ws_station_free. Two more things measured because they were the ones actually worth worrying about: sixteen threads parked in get() at once are all served, because Waiter::block_on drives on the calling thread rather than consuming a runtime worker; and Arc<Mutex<SyncConsumer>> already compiles for the genuinely shared case, since Mutex<T> is Sync whenever T is Send. Moving that Mutex inside SyncConsumer — what CR-6 asked for — would impose split semantics on the majority who want fan-out, and would break try_get's never-blocks contract. What is left is prose: say what a consumer is, and warn that replacing a 0.5.0 clone() with consumer() silently changes split into fan-out. That is the one way this change can break a working program with no compile error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRVG2B9A8hfX8JMiXsW6nD
DbErrorKind, eight kinds, each naming a different thing the caller does: Retry, Lagged, Closed, Transport, Data, Configuration, Usage, Internal. Eight rather than StationErrorKind's three because a database has more than three answers; the test is not how few but whether each kind is a distinct action. Lagged is separate from Retry because the caller lost data; Closed is separate because retrying cannot help. SyncError::kind() returns the same enum rather than one of its own, so an FFI layer has one switch for the whole stack. Db(e) delegates, timeouts are Retry, RuntimeShutdown is Closed. Both enums are now non_exhaustive, which follows a house style that already exists: RpcError, CodecError and AuthError in session/mod.rs are all non_exhaustive with small action-shaped variant sets. DbError and SyncError were the outliers. Both halves of the property verified rather than asserted. Adding a variant inside aimdb-core is E0004 at kind(), so someone must decide which action it belongs to in the file that owns it. A downstream match naming all 21 variants without a wildcard is also E0004, so reclassification cannot happen silently at a boundary. The blast radius is smaller than the variant count suggests: non_exhaustive blocks matching, not construction, and all 24 real match arms in the workspace are partial. aimdb-core, aimdb-sync, aimdb-tokio-adapter, aimdb-mqtt-connector, aimdb-data-contracts and the mesh workspace all build unchanged, and the spike stays green. The embassy and wasm adapters are not reachable from this workspace and were read, not built. Also exposed: AttachFailed carries a String, so it classifies as Internal even when the cause was a configuration mistake — which after the CR-2 patch is often exactly what its message says. Fixing it means carrying DbError through the startup channel instead of String. Left out because it changes a variant's shape, which is a decision rather than a cleanup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRVG2B9A8hfX8JMiXsW6nD
The only finding the spike measured as broken, and the only one whose fix had to be a process-global, which is why it belonged in the core rather than in the FFI layer. There were two failures, not one. The original round ended the child with _exit(0), so it never ran a destructor. Running one finds that joining a JoinHandle for a thread that does not exist in this process panics inside std with "threads should not terminate unexpectedly" — a Rust backtrace on stderr from inside ~Station. Present before this review's changes too. Detection is measured rather than assumed. try_set is 121ns and std::process::id() is 321ns, so a pid check per publish would cost more than twice the work it guards. Detection is a pthread_atfork child handler and the hot-path check is a relaxed atomic load; re-measured after the change, try_set is 107ns and the guard does not show up. A generation counter rather than a flag, because a bool would poison the child permanently — including for a database the child itself attaches afterwards, which a supervisor that forks per job would hit for no reason. The guard sits before the Weak upgrade deliberately: a forked child's upgrade succeeds, since the Arc was copied with the address space, and that is exactly why the buffer accepts a value nobody will read. Producers, consumers and the factories refuse a stale generation with a new SyncError::ForkedChild; detach and Drop release the join handle instead of joining; StationHandle::is_closed reports closed in a child. The spike's three notes are now four checks. CR-5 paid for itself here: ForkedChild is a new variant, which before this review would have been breaking. SyncError is non_exhaustive as of that work, so it is additive. fork::generation and fork::forked_since are public, which was not the plan. is_closed must not take the mutex shutdown holds, so it cannot ask the AimDbHandle whether the process forked — it records the generation itself. Any FFI layer on aimdb-sync has the same problem for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRVG2B9A8hfX8JMiXsW6nD
rumqttc's own default is use-rustls. Both aimdb-mqtt-connector and weather-station set default-features = false and then explicitly opted into use-native-tls, so the system OpenSSL was chosen over the pure-Rust default. The embedded half already went the other way — design 044's Embassy client is embedded-tls with rustpki — so the host path was the outlier. Three states now, not two: tokio-native-tls, tokio-rustls, or neither. Neither is a genuine choice, because a deployment speaking mqtt:// on a trusted network should link no TLS stack at all; mqtts:// then fails at connect time with a message naming the missing feature. That third state was the real work: rumqttc gates TlsConfiguration and Transport::Tls on having a backend, so the whole mqtts branch has to be cfg'd, not just its argument. The choice lives in weather-station and is passed down, because the pre-flight probe and the connector's data path must not disagree — two backends in one process is two TLS stacks, and a pre-flight that succeeded where the data path failed would defeat the point of probing. weather-station's default is now tokio-runtime + native-tls, so every existing station builds as before; weather-station-cpp, the crate that actually ships as a shared library, takes rustls. Measured on the cdylib: libssl.so.3 and libcrypto.so.3 are gone, and the static link line drops -lssl -lcrypto. It costs size — 3.9 MB to 8.1 MB release, 2.9 MB to 6.6 MB stripped — because rustls links the stack it no longer borrows. That belongs in the decision, not in a footnote. Found on the way: TlsConfiguration::default() expects and unwraps while loading platform certs. Two panics on the connect path, in a crate reached through an FFI boundary where a panic is undefined behaviour. Both backends now build the configuration explicitly. CR-4's rule has to extend to what the library calls, not just to what it writes. make test and make clippy carry all the combinations now — this repository's convention is that the feature matrix lives in the Makefile, and a TLS backend is exactly the kind of choice that rots if CI only ever sees one side of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRVG2B9A8hfX8JMiXsW6nD
These were cut to carry core changes that this repository cannot push. They are being raised as PRs against aimdb instead, so the copies here would only drift. review.md and REQUIREMENTS.md stay: they hold the measurements and the CR-n numbering the PRs are derived from. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… (design 050 §10.2) The C ABI door reached `tracing_subscriber::registry().…try_init()` from inside a cdylib loaded into somebody else's process — deciding where that process's diagnostics go, which is the one thing an FFI layer must not do. It did so because a `tracing::Layer` has nowhere to put the caller's `void *user_data`, and that single missing place is where both of this door's defects came from. `ws_init_logging` now builds a `CSink` — callback, `user_data` and filter in one value — and hands it to `log::set_boxed_logger`. The context travels with the callback, and the first-wins decision is made once, in Rust, for every binding. The header's `detail::SinkHolder` and `detail::sink_holder()` are gone with it. Both defects were that static: - it was assigned on every call, *before* asking whether the install would be accepted — last-wins over a first-wins C layer, so a second caller replaced the first caller's sink and returned false to say it had not; - it was assigned from the calling thread while the trampoline read it from aimdb's runtime thread. `init_logging` now allocates the `(sink, user_data)` pair the C ABI cannot carry in one `void *`, hands it over as `user_data`, and releases it if the install is refused. The trampoline stays — it is what catches a C++ exception that would otherwise unwind into Rust, which is a separate concern from the static and still load-bearing — but it holds nothing. Measured by running the current spike against the previous header: 86,890 events went to the sink that had just been told it was not installed, and the first caller's sink stopped receiving entirely. Against this one, zero. Two new rounds in spike.cpp assert both halves, and the second is written so it cannot pass vacuously. `weather-station` gains a `log` feature mirroring its `tracing` one — required, not cosmetic: the `log_*` macros expand in that crate, so the arm is selected by its features, and without it every call site there also warned under `unexpected_cfgs`. `init_tracing` and `tracing-subscriber` move out of `tokio-runtime` into a new `init-tracing` feature, which `default` carries, so the station binaries and templates are unchanged while a library build can have the Tokio station without a subscriber. `cargo tree` for this door now contains neither `tracing-subscriber` nor `tracing`. Filtering is coarser, deliberately: `EnvFilter` came free with `tracing-subscriber` and went with it. `filter` now takes `level` and `target=level`, comma-separated, longest prefix first. Named as a regression in README.md rather than left to be discovered. Verified: `make clippy`, `make test`, `make fmt-check` clean; the spike's log rounds all pass and still report `aimdb_core::builder` / `aimdb_core::session::pump` targets. The five broker-path failures it reports are pre-existing — identical on this branch before this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
… (design 050 §10.3)
Removing `init_tracing` from this module was meant to end one trespass: an
extension module is a library inside somebody else's application, and
process-wide logging is that application's decision. The replacement left half
of it standing. Reaching a `tracing::Layer` at all meant calling
`tracing_subscriber::registry().…try_init()`, so `init_logging` installed the
interpreter's global subscriber — the same trespass, one layer further down and
harder to see.
`PyLogger` is a `log::Log` instead: an ordinary destination this module owns.
Nothing global is installed on the application's behalf beyond the one logger
`log` exists to hold, and `log::set_boxed_logger` is what makes the `True`/
`False` honest — the first-wins decision is now made once, in `log`, for this
door and the C one alike rather than separately in each.
Everything Python-visible is unchanged: events still arrive on loggers named
after the emitting Rust module with `::` translated to `.`, so
`getLogger("aimdb_core").setLevel(...)` still silences aimdb without silencing
the station. The `MessageVisitor` goes with `tracing` — a `log::Record` carries
its message as `args()` and needs no field visitor.
`weather-station` gains a `log` feature mirroring its `tracing` one — required,
not cosmetic: the `log_*` macros expand in that crate, so the arm is selected by
its features. `init_tracing` and `tracing-subscriber` move out of
`tokio-runtime` into a new `init-tracing` feature that `default` carries, so the
station binaries and templates are unchanged while this crate can take the Tokio
station with `default-features = false` and get neither. `cargo tree -p
weather-station-py` now contains neither `tracing-subscriber` nor `tracing`.
Filtering is coarser, deliberately: `EnvFilter` came free with
`tracing-subscriber` and went with it. `filter` now takes `level` and
`target=level`, comma-separated, longest prefix first, in either spelling —
directive prefixes are normalised to `::` at construction, so a Python caller
can write the dotted logger names this bridge hands out and still match, without
paying for a translation per event on the path the gate exists to keep cheap.
It costs less here than at the C door: `filter` is only the floor that keeps
events from acquiring the GIL, and Python's own per-logger levels do the
fine-grained work above it as they always did. Named as a regression in
README.md rather than left to be discovered.
Verified: `make clippy`, `make test`, `make fmt-check` clean, and `make spike`
reports all checks passed against a live broker — including "the module does not
install a global tracing subscriber", which is now true by construction rather
than by convention, and "one subsystem can be silenced without silencing the
others".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
This branch is nine commits behind `weather-station-cpp`, and one of them — `fafe259` (#10) — tightened the doc comments throughout `weather-station-py/src/lib.rs`. The previous commit was written against the older text, so merging it would have reverted that work in every block it touched, and left the blocks it did not touch looking like a revert too. Re-applied the design-050 change on top of #10's version of the file instead. No behaviour change from the previous commit: `PyLogger` and `init_logging` are the same, and the module-doc note about the bridge is rewritten in #10's register rather than the older one. The diff against #10 is now confined to the imports, the module-doc tail and the bridge itself — `PyStation` is untouched. #10's change to this crate was comment-only, so nothing here depended on it. `make spike` still reports all checks passed; `make clippy`, `make test` and `make fmt-check` clean. Note for whoever merges: this branch is still missing the other eight commits on `weather-station-cpp`, including the C door itself. Which way those branches should converge is a topology decision, not one this commit makes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
Brings the design-050 migration of the pyo3 door onto the branch that carries both. `weather-station-cpp` was nine commits ahead, so its copy of the Python door was still installing the interpreter's global `tracing` subscriber while its `weather-station` had already moved to the `log` facade. Three conflicts, all in files both sides edited: - `weather-station-py/src/lib.rs` — cpp's side was #10's doc tightening, which the py branch had already been rebuilt on top of, so the migrated version is a strict superset. Took it whole; the result is byte-identical to the py branch's file. - `weather-station/Cargo.toml`, `default` — kept `native-tls`. The py branch predates CR-11 and had no TLS backend to name; dropping it here would have left the default build without one. - the same file's `log` feature comment — one word apart; kept "destination", which is the design's vocabulary. One thing neither branch could have got right alone: `weather-station-py` takes `weather-station` with `default-features = false`, and on the py branch that was harmless because a TLS backend was not yet selectable — `rumqttc` named `use-native-tls` directly. Merged, it means no backend is selected at all and `mqtts://` would have failed at run time with nothing failing at build time. Named `rustls` explicitly, for the reason CR-11 gives and which describes this crate exactly: a build that must not hand its host process a second OpenSSL — "an FFI layer shipped as a shared library, above all". The C door made the same call for the same reason. `cargo tree -p weather-station-py` links no OpenSSL (`openssl-probe` is `rustls-native-certs`' cert-store locator, not a link to OpenSSL) and still contains neither `tracing` nor `tracing-subscriber`. That is the one judgement call in this merge rather than a mechanical resolution; `native-tls` is a one-word change if the wheel should keep it. Verified on the merged tree: `make clippy`, `make test`, `make fmt-check` clean; `make spike` and `make spike-cpp` both report all checks passed against a live broker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
`make clippy` fails on rustc 1.98 with `clippy::unnecessary_sort_by`: `sort_by(|a, b| b.0.len().cmp(&a.0.len()))` is a reversed comparison that `sort_by_key` with `Reverse` expresses directly. Same descending order, same stable sort — the longest-prefix-first invariant both filters rely on is unchanged. Both doors carried the identical line; CI only reported the Python one because `make clippy` stops at the first failing package, so the C door's copy was never reached. Fixed in both rather than one at a time. Why local checks missed it: this sandbox was on rustc 1.94.1, where the lint does not fire on a reversed comparison, while CI runs 1.98.0. Toolchain updated to 1.98.0 here, the failure reproduced against the old code, and the whole suite re-run on it — `make clippy`, `make test`, `make fmt-check`, `make lockfile` clean, and `make spike` / `make spike-cpp` both report all checks passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
The status table in `review.md` §8 predates the last three rounds, so this document claimed work as outstanding that has since landed. Each section now opens with its own status line, and a summary table says where all twelve stand. Closed since the last pass: CR-3 (a timed-out shutdown now documents what it leaves behind), CR-6 (resolved as documentation, per `review.md` §4), CR-4's core half, CR-9 (`make check-no-globals`, in CI, with a positive control) and CR-10 (`AimDbHandle::shutdown(&self)` / `is_closed()`, with the four properties pinned by a test in `aimdb-sync`). Still open: CR-7, the delivery signal, which is a decision nobody has made; and CR-8, half done — blocking behaviour and cursor semantics are documented per method, but "may this be called *from* the runtime thread?" is not, and nothing pins it, which is the property both doors' log callbacks depend on. The lockfile refresh is the sibling checkout's `tracing` dependency moving behind `aimdb_core::__private` (design 050 §10.4). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3ehWPzAjjgQFHsKuhKpm
Both are the pendant of `weather-station-openmeteo` — same data source, same profile, no hardware — and both are what the blocking door was built for: they own their poll loop and call `publish_*` when they have a reading, where the Rust template hands aimdb two async sources and lets the record graph drive them. What the mesh defines stays below the FFI boundary: profile format, slot identity, broker handshake, record keys and topics. What is left in each file is the half a station of your own replaces — the fetch. The C++ side carries two deliberately small parsers beside it (a line scanner for the profile's `[app]` coordinates, a scan for two numbers in a known response) because a station reading a sensor deletes both; the mesh tables in that same file are parsed properly, below the ABI. Same rules as the Rust template, so the three stay comparable: coordinates from the profile, then `WEATHER_LAT`/`WEATHER_LON`, then Vienna, and half a pair is an error rather than a silent mix of two sources. `OPEN_METEO_URL` points either station at a self-hosted instance or a fake. Shutdown is a flag, in both: SIGINT and SIGTERM set something the loop waits on and do nothing else. `close` is safe from another thread, which is not the same as safe from a signal handler. `make station-py` and `make station-cpp` run them; libcurl is the C++ station's own dependency, not the library's. Verified end to end against a local mosquitto: both join, publish the versioned contract shape on their slot's topics, and exit 0 on SIGINT. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3ehWPzAjjgQFHsKuhKpm
Both were a paper trail for a round that is finished: of the twelve items they tracked, ten are closed and merged into aimdb, and the two that are not — a delivery signal, and a per-method runtime-thread contract — are obsolete as filed. Keeping them would mean maintaining a second, staler account of crates that now document themselves. What they said that still matters is said where it applies instead: the fork handler is process-global, so only the crate owning the runtime thread may register it; a consumer is a subscription with its own cursor, so one per thread is the shape a binding wants; rustls is here so a shared library does not hand its host a second OpenSSL. The four CR-n citations in the two manifests and the Python README are rewritten to say the thing rather than cite a number. `probes/` stays — it is an experiment rather than a requirements document, it still builds, and its README now states its finding instead of pointing into a deleted file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3ehWPzAjjgQFHsKuhKpm
Both doors have a station on top of them now, which exercises the same paths against a real broker and is a thing someone runs rather than a harness that reports findings. The spikes were the experiment that came before that, and their questions are answered. Gone with them: the `spike-probe` feature and `ws_debug_panic`, which existed so the harness could measure the panic guard rather than assert it — a shipped library must not export a way to panic on purpose. The cdylib still exports exactly its fourteen `ws_*` symbols. Both READMEs are rewritten from a findings narrative into what a caller has to know. Every operational caveat survives — nothing may unwind and `panic = "abort"` compiles the guard out, the GIL must be outermost, a non-UTF-8 path is refused, `close` is not a flush, the sink cannot be uninstalled, a forked child is refused — while the measurement numbers and the round-by-round account do not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Z3ehWPzAjjgQFHsKuhKpm
…blish
Both doors called `is_closed()` before each publish and turned a `true` into
`WS_ERR_CLOSED` / a `StationError`. That check could not do what it looked like
it did. `SyncProducer::set` already performs it — a `Weak` upgrade that yields
`SyncError::RuntimeShutdown`, classified `DbErrorKind::Closed` — so the door's
check was a second one, on a different object, racing the first. Its own doc
said as much ("best-effort … a close racing this check merely means the call
fails one layer down"). `SyncProducer::check` exists precisely so a facade need
not write one: "this is the check `set` performs, not a second one beside it,
so the answer cannot drift from what a publish would find."
What the pre-check actually bought was the status code, and only because
`StationError::kind()` was throwing the classification away:
Self::Db(_) => StationErrorKind::Runtime,
`WS_ERR_CLOSED` had no other producer, so removing the check without fixing
this would have collapsed a closed station into `WS_ERR_RUNTIME`.
So: `StationErrorKind` gains `Closed`, and `kind()` delegates to the inner
`DbError`/`SyncError` rather than flattening every one of them to `Runtime`.
The doors then drop `ensure_open` entirely. The status code and the C++
exception type are unchanged — `WS_ERR_CLOSED` still maps to `ClosedError`, and
Python still raises `StationError` — but they now come from the check the
publish itself performs, so they are right under every interleaving rather than
most of them, and the publish path loses a redundant atomic load.
Two changes a caller can observe, both improvements:
- The message. It was always "this station is closed"; it is now aimdb's own —
"Runtime thread has shut down", or "created before a fork(); this process has
no runtime thread for it", which says which of the two happened.
- A forked child's publish reaches `ClosedError` through the classification
rather than through `is_closed()` happening to agree with it.
`StationErrorKind` is now `#[non_exhaustive]` — it grew once, so it can grow
again — and both doors match it with a wildcard arm that falls back to the
runtime/base case.
The spikes that would have exercised this are gone, so the guard is a unit test
on the classification: a publish after shutdown, and a forked child, are
`Closed`; other aimdb failures stay `Runtime`. Deleting `ensure_open` is only
sound while that holds, and the test says so.
Verified: `make clippy`, `make test`, `make test-embedded`, `make wasm-check`,
`make fmt-check`, `make lockfile` clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
`aimdb-knx-connector` no longer depends on `tracing`: aimdb#238 (design 050 §10.4/§10.5) moved the four direct-`tracing::` connectors onto the `log_*` facade, which reaches both destinations through `aimdb_core::__private`, so the dependency stopped being declared. This workspace records that in its lock, because it reaches the aimdb crates by path. Not a change to this branch's code — it drifted underneath. The lockfile check ran green here before aimdb main moved, and CI checks out aimdb's default branch with no ref, so the two only agree once this is regenerated. Re-verified against aimdb main at 06d207a, which also carries #239's `AimDbHandle` shutdown contract: `make clippy`, `make test`, `make test-embedded`, `make wasm-check`, `make fmt-check`, `make lockfile` all clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
`weather-station-cpp/probes/` goes the same way as the spikes: standalone experiments that settled a design question before the doors existed, kept only as long as the question was open. `cr6-consumer-shape` asked what shape the consumer half of an FFI binding can take, given `SyncConsumer` is `Send + !Sync` with `&mut self` receivers. Its answer — a consumer is a subscription with its own cursor, so one per thread is the shape, and `Arc<Mutex<_>>` is what splits a stream — is not lost with it. `aimdb-sync` states all three on `SyncConsumer` itself, which is where someone hits the question, and both door READMEs point there. Nothing referenced the directory: its own cargo workspace, no Makefile target, no workspace membership, no links. `make clippy`, `make test`, `make fmt-check` and `make lockfile` unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
…heck Two follow-ups from the PR discussion. **A regression test for what the spikes used to prove.** `spike.cpp` carried two rounds asserting the defects design 050 deleted stayed deleted; removing the spikes took them with it, leaving the fix correct but unguarded. This puts the Rust half back as a unit test — the crate is `cdylib`/`staticlib`, so an integration test cannot link it, but the `ws_*` functions are callable from inside the crate. It installs a sink, emits, has a *second* install refused, emits again, and asserts the refused sink was never called, the first kept receiving, `user_data` arrived unchanged, and the emitting module reached C as the target. One test, not four: `set_boxed_logger` is once per process and every test here shares it. Verified to bite — with `user_data` replaced by `null_mut()` in `CSink::log` it fails on that assertion rather than passing quietly. What it does not cover, and should be said plainly: the C++ header's own half. `init_logging` allocating the `(sink, user_data)` pair, handing it over, and freeing it when the install is refused is C++ logic, and proving it needs a C++ test. This guards the Rust side of the contract the header depends on. `make test` runs it; the crate had no test line before. **`detail::check` → `detail::throw_if_failed`.** The old name read as a liveness check next to `ensure_open` and `SyncProducer::check`, which is what prompted the question on the PR. It is neither: the C ABI reports by return code because it cannot throw, and this is the only place that code is read. Six call sites, no behaviour change. Verified: `make clippy`, `make test`, `make fmt-check`, `make lockfile` clean, and `station.cpp` links against the renamed header (libcurl installed here to build it rather than assume). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
Both READMEs ended their consumer paragraph with "not yet answered". The shape is answered now, and writing it down while the reasoning is fresh is cheaper than reconstructing it at implementation time. The decision, in both: a factory rather than a handle. One consumer per (key, thread), each with its own cursor, kept on the thread that asked for it. Deliberately no shared consumer, even though `aimdb-sync` offers `Arc<Mutex<SyncConsumer<T>>>` for splitting a stream — exporting that means handing a mutex-guarded handle whose whole purpose is being shared between threads to a language with no borrow flag, which is the aliasing problem these paragraphs already warn about, arriving by invitation. It costs little because every consumer sees every value. An application that wants each value to reach exactly one worker does not need an API for it: one thread pulls the whole stream into a queue it already has. Fan-out is free; partitioning is application architecture. Each door records why its own version bites differently — the C header must carry three prose contracts no signature can (one per thread, never share the pointer, never call a blocking read inside an async runtime), and a Python consumer cannot be a `frozen` pyclass, so a shared one would fail with "Already borrowed" exactly where `close` used to. Also corrects a framing that reads the wrong way round. "Drop to `aimdb-core` without `aimdb-sync`" is a real escape hatch, but a Rust-only one: someone who installs the wheel or links the library cannot take it without writing Rust, at which point they are not a user of the door. For them it is the ceiling, and both READMEs now say so rather than implying an exit that isn't there. Documentation only — no code, no manifests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
Source across both doors goes 2062 → 1939 lines, READMEs 336 → 306. No code changed; every cut is a comment or a README paragraph. The largest saving came from one observation: `weather-station-cpp/src/lib.rs` was documenting the C API to a Rust reader who is not its audience. A C caller reads `weather_station.h`, which carries the same contracts in the place they are actually consulted. Those doc comments are now one line plus `# Safety`, which is the part rustc and clippy want and the part a Rust maintainer needs. The Python door was trimmed more carefully, because pyo3 turns `///` into Python `__doc__`: the module-level essays are Rust-internal and went, while `Station` and `init_logging` keep the text `help()` shows — the example, the logger-naming rule, the filter grammar. The `frozen` rationale moved from `///` to `//`, since it explains an implementation choice to a maintainer and was leaking into the Python docstring. The C header kept nearly everything: it is the consumer's contract and 59% prose by design. Only the maintainer-facing notes went — why the file is hand-written rather than cbindgen-generated (which the README already says), and a cross-reference to the Python door's GIL rule that a C caller has no use for. Elsewhere, history went where the code is now plainly right: how the header used to keep a racing `SinkHolder`, what the OpenSSL link line used to contain, what the spike measured. Git and the READMEs hold that; a reader of the current code does not need it inline. Several of the longest passages were ones added earlier in this branch — the consumer boundary and the first-wins test's preamble — and they took the deepest cuts. Verified: `make clippy`, `make test`, `make fmt-check`, `make lockfile` clean; `station.cpp` links against the trimmed headers; the Python module imports and `init_logging.__doc__` still reads correctly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
Extends the previous pass to the crate the doors sit on: 3527 → 3471 lines across doors, headers, example stations and `weather-station/src`. The crate docs took most of it (157 → 117). It has no README, so those docs are its documentation — compressed rather than gutted, keeping the three-doors summary, the runtime split, and the embassy/`links = "embassy-time"` constraint, which is the one hard-won fact in the file. Elsewhere the cuts were smaller than the previous pass, because what remains is a different kind of comment: doctested examples on public API, and rationale that answers "why is this a macro" or "why match on the kind rather than the variant". Trimming those removes information rather than repetition, so they were tightened, not dropped. Verified: `make clippy`, `make test`, `make fmt-check` clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
306 → 182 lines. The structure was the problem more than the length: "What it found", "The one thing that was broken, and is now fixed", "Accepted, not fixed", "The rest holds", "Notes for whoever ships". That is a spike report, and it was the right shape while the spikes existed. They are gone, and these are doors someone uses. Both now answer the questions a reader has, in the order they have them: what this is, how to run it, how to use it, the rules, the limits, what is left before shipping. The C door's rules became a table — eight one-line entries where there were eight paragraphs — and the Python door gained the worked example it never had, which is what a reader actually copies. No facts dropped, only the narration around them. Everything checkable was re-verified rather than carried over on trust: fourteen exported `ws_*` symbols (`nm -D`), the `OPEN_METEO_URL` / `WEATHER_LAT` / `WEATHER_LON` names, and the `ldd` output behind the rustls claim. What went was the discursive voice — "that is a finding rather than a shortcut", "worth stating as a rule rather than a coincidence" — and the rationale behind decisions a reader does not need to re-litigate to use the door. The reasoning survives in git and in the design docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QgaHbjCPgkNS6Y34jxqVYH
…ency for C++ station
…andling across FFI boundaries
…ity on data handling and profile coordinates
…n and observation fetching
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #11, which is this branch's base — everything in it is included here.
Two new station templates and the FFI layers they stand on.
weather-station-openmeteohas a pendant in each language now: same data source, same profile, no hardware.What is here
The doors.
weather-station-pybindsStationHandlethrough pyo3;weather-station-cppexposes it as a C ABI (fourteenws_*symbols) with a header-only C++ layer over it. The split is not a preference: Rust cannot export C++, so the boundary carries the mechanism and the language-shaped API is written in that language. Both install alogdestination rather than the host process's subscriber — a library inside somebody else's application does not get to decide where that application's diagnostics go.The stations.
weather-station-py/python/station.pyandweather-station-cpp/cpp/station.cpp. They own their poll loop and callpublish_*when they have a reading, where the Rust template hands aimdb two async sources and lets the record graph drive them — which is what the blocking door exists for. What the mesh defines stays below the boundary: profile format, slot identity, broker handshake, record keys and topics. What is left in each file is the half a station of your own replaces, the fetch.They follow the Rust template's rules so the three stay comparable: coordinates from the profile's
[app], thenWEATHER_LAT/WEATHER_LON, then Vienna, and half a pair is an error rather than a silent mix of two sources. Shutdown is a flag in both — SIGINT and SIGTERM set something the loop waits on and do nothing else, sinceclosebeing safe from another thread is not the same as safe from a signal handler.Changes to
weather-stationso a foreign caller can use it at all:shutdown(&self)that is idempotent and safe during a publish,is_closed()reading an atomic rather than the mutex shutdown holds, andStationError::kind()classifying failures by what a caller can do about them.Running them
make station-cppneeds libcurl, which is the station's dependency rather than the library's.OPEN_METEO_URLpoints either at a self-hosted Open-Meteo or at a fake.Verification
Both stations run end to end against a local mosquitto: they join, publish the versioned contract shape on their slot's topics (
{"schema_version":2,"celsius":…}onstation/17/temperature), and exit 0 on SIGINT.make checkpasses.Notes for review
aimdb-syncdocuments onSyncConsumer.close()is not a flush: a reading published immediately before a close may not reach the broker. Stations publish on a cadence, so what is lost is a reading nobody would have read — but a station that publishes once and exits would want a delivery signal, and there isn't one yet.🤖 Generated with Claude Code
https://claude.ai/code/session_012Z3ehWPzAjjgQFHsKuhKpm
Generated by Claude Code