diff --git a/docs/conventions.md b/docs/conventions.md index 9f5c2ca5..de0a435e 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -185,9 +185,10 @@ checklists in `.claude/skills/` apply these standards to a diff. is bounded where it is parsed: read in place, refused past its cap, or cut to its field's size. Peer text a log line prints that no field bounds (a type, a reason, an unrecognized value) is cut to `PEER_TEXT_LOG_BYTES` - (`peer_text_for_log()`). Bounded text in a public type built for views - (`TrackMetadata`, `GroupState`) is a `std::string_view` into storage the - library keeps, valid for as long as its declaration says; a public type + (`peer_text_for_log()`). Bounded text in a public type or callback built + for views (`TrackMetadata`, `GroupState`, `ServerInformation`, the pairing + callbacks) is a `std::string_view` into storage the library keeps, valid + for as long as its declaration says; a public type still holding `std::string` or `std::vector` is built from the inline value on the thread that hands it to the consumer. `docs/internals.md` "Memory" lists the `operator new` allocations that remain after `start()`, and diff --git a/docs/integration-guide.md b/docs/integration-guide.md index a347d818..6f41701d 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -615,7 +615,7 @@ recency writes only the order. write the next boot rebuilds by itself (the recency order in `RECORD_ORDER`) reports at debug. For a record slot specifically: a rejected write of a just-paired record leaves the pairing working for this boot only - (`on_pairing_succeeded` still fires; the record is gone after a reboot), and a rejected write + (`on_paired` still fires; the record is gone after a reboot), and a rejected write of a removal means the store still holds the old record and will hand it back at the next boot, silently making the revoked PSK valid again (the revoked record is always dropped from RAM regardless of the return value). A supersede and an eviction both write the new record over the @@ -624,7 +624,7 @@ recency writes only the order. already discarded it, which then falls back to unpaired (Sentinel) access. - `commit()` makes every accepted write durable. The library calls it once after a batch of accepted writes that holds pairing material (`KEYPAIR`, `PAIRING_PSK`, or a record slot) and, - for a pairing, does so before `on_pairing_succeeded` fires. Writes to `RECORD_ORDER`, + for a pairing, does so before `on_paired` fires. Writes to `RECORD_ORDER`, `LAST_PLAYED` and `OUTPUT_DELAY` never trigger it, so a provider that queues its writes may batch those on its own schedule. A provider that writes through in `save_blob()` keeps the default, which returns `true`. A `false` return is logged like a rejected write and not @@ -714,30 +714,33 @@ struct MyClientListener : SendspinClientListener { // Called when a server begins a pairing exchange, once per attempt and whatever // the method (Pairing PSK, dynamic pairing code, or static pairing code). - // server_id is the base64url public key of the server initiating pairing. - void on_pairing_started(const std::string& server_id) override { - printf("Pairing started with server %s\n", server_id.c_str()); + // server_id is the base64url public key of the server initiating pairing. The pairing + // callbacks' text views are valid for the duration of the callback. + void on_pairing_attempt(std::string_view server_id) override { + printf("Pairing started with server %.*s\n", static_cast(server_id.size()), + server_id.data()); } // Called when pairing completes successfully and the long-term record is stored. // Subsequent connections from this server are paired sessions (is_paired()). - void on_pairing_succeeded(const std::string& server_id) override { - printf("Pairing succeeded with server %s\n", server_id.c_str()); + void on_paired(std::string_view server_id) override { + printf("Pairing succeeded with server %.*s\n", static_cast(server_id.size()), + server_id.data()); } // Called when a pairing exchange is aborted. Most server-sent abort reasons leave the // connection open so the server can retry or resume normal operation; CONCURRENT_ATTEMPT // and protocol errors (reported as UNKNOWN) close it. - void on_pairing_failed(const std::string& server_id, SendspinPairAbortReason reason) override { - printf("Pairing failed with server %s\n", server_id.c_str()); + void on_pairing_aborted(std::string_view server_id, SendspinPairAbortReason reason) override { + printf("Pairing failed with server %.*s\n", static_cast(server_id.size()), + server_id.data()); } // Dynamic pairing code: emit/withdraw the code the device derived. Only invoked when // SendspinClientConfig::pairing_code_out_channels and pairing_code_formats are both // non-empty. `format` says what `code` is: six decimal digits, or a pairing token to // render as a QR code. - void on_display_pairing_code(const std::string& code, - SendspinPairingCodeFormat format) override { + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat format) override { show_pairing_code_on_display(code, format); } void on_clear_pairing_code() override { @@ -862,10 +865,10 @@ Pairing is server-initiated. The server includes `pairing` in the `activities` o `server/activate` message; the library handles the exchange automatically. The application observes pairing via `SendspinClientListener` callbacks: -1. `on_pairing_started(server_id)` -- the exchange has begun. -2. `on_pairing_succeeded(server_id)` -- the record is stored; the server will +1. `on_pairing_attempt(server_id)` -- the exchange has begun. +2. `on_paired(server_id)` -- the record is stored; the server will re-handshake immediately on the new long-term PSK. -3. `on_pairing_failed(server_id, reason)` -- the exchange failed. See +3. `on_pairing_aborted(server_id, reason)` -- the exchange failed. See `SendspinPairAbortReason` below for the possible reasons. Whether the connection survives depends on the reason. Most reasons the server sends in a @@ -875,7 +878,7 @@ observes pairing via `SendspinClientListener` callbacks: `client/goodbye`. A persistence provider that rejects the durable write does not fail the pairing: - `on_pairing_succeeded` fires, the record is authoritative for the rest of this boot, and a + `on_paired` fires, the record is authoritative for the rest of this boot, and a warning says it will not survive a reboot. Surface that failure from the provider itself if the application needs to act on it. The record is dropped outright, with neither callback firing, only when the store is at capacity with nothing evictable, which the connection budget @@ -885,8 +888,8 @@ observes pairing via `SendspinClientListener` callbacks: if the application needs to track that. A server can also end an attempt with a `server/activate` in place of the next pairing message -(`pairing.md` "Entering and leaving pairing"). Neither `on_pairing_succeeded` nor -`on_pairing_failed` fires then, but `on_clear_pairing_code` / `on_close_pairing_window` still +(`pairing.md` "Entering and leaving pairing"). Neither `on_paired` nor +`on_pairing_aborted` fires then, but `on_clear_pairing_code` / `on_close_pairing_window` still withdraw any prompt the attempt showed; a pairing window the operator opened stays open. #### Pairing PSK @@ -902,7 +905,10 @@ To pair, the server must learn that PSK out of band. Surface it as a **pairing t paste (or scan) into the server: ```cpp -auto token = client.pairing_token(); // e.g. "SP:0AAAQ..." (107 chars), nullopt before start() +// std::array, e.g. "SP:0AAAQ..." with no +// terminating NUL; nullopt before start() +auto token = client.client_pairing_token(); +if (token) printf("%.*s\n", static_cast(token->size()), token->data()); ``` The token is stable for the lifetime of the PSK, so it can be printed at startup, shown in a UI, @@ -923,11 +929,11 @@ the library derives its `psk_id`. An all-zero key or the published Sentinel PSK ship a key every peer knows. `SendspinPsk` wipes its bytes when destroyed, and so does every copy the client makes; a source buffer such as `factory_psk` above is the application's to wipe. -`client.format_pairing_token(factory_psk)` builds the token for any 32-byte PSK rather than the +`client.pairing_token_for(factory_psk)` builds the token for any 32-byte PSK rather than the one in use, which is what a provisioning tool needs to print the token for a key it provisions. It returns `nullopt` before `start()`, since the token also carries the client's identity. -After pairing completes, `on_pairing_succeeded` fires and the long-term record is stored +After pairing completes, `on_paired` fires and the long-term record is stored by the library in the first free record slot. Subsequent boots load that same slot; no further provisioning is needed. @@ -935,7 +941,7 @@ provisioning is needed. The library also supports the two pairing-code methods, gated by `SendspinClientConfig::pairing_code_out_channels` / `pairing_code_formats` / -`static_pairing_code`, and driven by the `SendspinClientListener::on_display_pairing_code` / +`static_pairing_code`, and driven by the `SendspinClientListener::on_pairing_code` / `on_clear_pairing_code` / `on_open_pairing_window` / `on_close_pairing_window` callbacks documented in Step 3 above. A method is advertised only when the platform can carry it: a client that names no out-channel and no format never offers `dynamic_pairing_code`, and the server is @@ -946,7 +952,7 @@ code, so a device that means to offer `static_pairing_code` leaves `pairing_code empty. A **dynamic pairing code** is derived per attempt from the Noise handshake hash and both sides' -nonces, and emitted through `on_display_pairing_code` as either six decimal digits or a pairing +nonces, and emitted through `on_pairing_code` as either six decimal digits or a pairing token to render as a QR code, whichever format the server selected from those advertised. If the operator types a code the device did not emit, the client asks for another round (`client/pair-retry`) and keeps the same code on screen. After 20 rounds without a successful @@ -1169,6 +1175,10 @@ bool connected = client.is_connected(); // Active connection with complete bool synced = client.is_time_synced(); // Time filter has received at least one measurement GroupState group = client.group_state(); // Group id, name, playback state (all optional); // its views hold until control returns to loop(), or stop() +auto server = client.server_information(); // Active server's id and name as of the last loop(), + // nullopt with none; views held like group_state()'s; + // can trail is_connected() by one loop(), so read it + // each loop rather than on its rising edge // Player state uint8_t vol = player.get_volume(); @@ -1189,7 +1199,7 @@ int64_t client_ts = client.get_client_time(server_timestamp); ## Deprecated String API -The view forms hand the metadata and group text over as `std::string_view` into storage the library keeps; the deprecated forms copy it into `std::string`. The `std::string` forms are deprecated in v0.10.0 and removed in v0.11.0; naming one of them draws a `-Wdeprecated-declarations` warning. +The view forms hand the metadata, group, pairing and server text over as `std::string_view` into storage the library keeps, and the pairing token as a `std::array`; the deprecated forms copy it into `std::string`. The `std::string` forms are deprecated in v0.10.0 and removed in v0.11.0. Using a deprecated type or calling a deprecated getter draws a `-Wdeprecated-declarations` warning, but overriding a deprecated callback does not: the metadata and group callbacks warn only because their parameter types are deprecated, and the four pairing callbacks (`on_pairing_started()`, `on_pairing_succeeded()`, `on_pairing_failed()`, `on_display_pairing_code()`) take none, so search for those names. Mark every override `override`, so that a callback removed in v0.11.0 fails to compile rather than silently going uncalled. | Deprecated | Replacement | |---|---| @@ -1198,8 +1208,16 @@ The view forms hand the metadata and group text over as `std::string_view` into | `GroupUpdateObject` | `GroupState` | | `SendspinClientListener::on_group_update()` | `SendspinClientListener::on_group_state()` | | `SendspinClient::get_group_state()` | `SendspinClient::group_state()`, which returns by value | +| `SendspinClientListener::on_pairing_started()` | `SendspinClientListener::on_pairing_attempt()` | +| `SendspinClientListener::on_pairing_succeeded()` | `SendspinClientListener::on_paired()` | +| `SendspinClientListener::on_pairing_failed()` | `SendspinClientListener::on_pairing_aborted()` | +| `SendspinClientListener::on_display_pairing_code()` | `SendspinClientListener::on_pairing_code()` | +| `SendspinClient::pairing_token()` | `SendspinClient::client_pairing_token()`, a `std::array` | +| `SendspinClient::format_pairing_token()` | `SendspinClient::pairing_token_for()`, likewise | +| `ServerInformationObject` | `ServerInformation` | +| `SendspinClient::get_server_information()` | `SendspinClient::server_information()`, main loop only | -The fields keep their names and meaning; only the text type changes. A view handed to a callback is valid for the duration of the callback, and the views `group_state()` returns until control next returns to `loop()` (from inside a callback: until the callback returns) or until `stop()`: copy what is needed beyond that. A view is not guaranteed to be NUL-terminated: print it with `%.*s` (its size, then its data) or copy it into a `std::string` before passing it to an API that expects a C string. Until the removal the library calls the new callback and then the deprecated one with the same content, so a listener overrides one of the two; one that overrides both gets both, the new one first. A `stop()` from the new callback ends the delivery there. +The fields and arguments keep their meaning; only the text type changes. A view handed to a callback is valid for the duration of the callback, and the views `group_state()` and `server_information()` return until control next returns to `loop()` (from inside a callback: until the callback returns) or until `stop()`: copy what is needed beyond that. A view, like a pairing token's array, is not guaranteed to be NUL-terminated: print it with `%.*s` (its size, then its data) or copy it into a `std::string` before passing it to an API that expects a C string. `server_information()` reads a copy `loop()` refreshes as the server changes, so it is main loop only and can read the previous server, or `nullopt`, for one `loop()` after `is_connected()` turns true: read it on each `loop()` rather than only when `is_connected()` rises; `get_server_information()` reads from any thread, and code off the main loop has no replacement and keeps calling it until the removal. Until the removal the library calls the new callback and then the deprecated one with the same content, so a listener overrides one of the two; one that overrides both gets both, the new one first. A `stop()` from the new callback ends the delivery there. ## Thread Safety Summary @@ -1236,7 +1254,7 @@ non-blocking. Callable from any thread: `connect_to()`, `disconnect()`, `leave()`, `confirm_pairing_window()`, `cancel_pairing_window()`, `set_unpaired_access_enabled()` and the getters `is_started()`, `is_connected()`, `is_time_synced()`, `get_client_time()`, -`get_server_information()`, `is_paired()` and `is_unpaired_access_enabled()`. The +`get_server_information()` (deprecated), `is_paired()` and `is_unpaired_access_enabled()`. The requests take effect on the protocol task's next tick; the getters read what that task last published. Controller commands are queued, so a burst of them can fill the queue (see [Sending Commands](#sending-commands)); `connect_to()`, `disconnect()`, `leave()`, @@ -1557,7 +1575,7 @@ The capture buffer and an encoder's chunk scratch are allocated by the first `st | `USER_CANCELLED` | The pairing was cancelled by the user: by the server, or locally by `cancel_pairing_window()` while an attempt was waiting for the gesture | | `UNKNOWN` | Unrecognized abort reason from the server, or a client-local protocol error (which also closes the connection) | -Delivered via `SendspinClientListener::on_pairing_failed`. An activation naming a pairing +Delivered via `SendspinClientListener::on_pairing_aborted`. An activation naming a pairing method or format the client does not offer is answered with a `pair/abort` carrying `method_not_supported` and no listener callback at all; the connection stays open. @@ -1580,7 +1598,7 @@ Listed in `SendspinClientConfig::pairing_code_out_channels` and advertised as th Listed in `SendspinClientConfig::pairing_code_formats` and advertised as the `dynamic_pairing_code` descriptor's `formats`; the server picks one of them and the chosen -format arrives as the `format` argument of `on_display_pairing_code`. +format arrives as the `format` argument of `on_pairing_code`. ### SendspinCodecFormat diff --git a/docs/internals.md b/docs/internals.md index 195a0ca4..66ecd31b 100644 --- a/docs/internals.md +++ b/docs/internals.md @@ -50,7 +50,7 @@ The primitives other than the Inbox and the inbound and outbound rings live in ` | Artwork `SlotGate`s (`DrainTask::slot_gates`, under `slot_mutex`) | Artwork decode thread (arms a gate, parks a complete image); main loop (presents, acks, drops a park at a stream end) | Each other | | `ShadowSlot` (sync-task playback progress) | The consumer's audio thread (`notify_audio_played()`) | Sync task | | `Inbox` (`src/inbox.h`, see [Inbox](#inbox)) | Protocol task, sync task, artwork decode thread | Main loop | -| `ConnectionManager` published state (each admitted slot's clock, the primary's server information) | Protocol task (`refresh_published_state()`) | Any thread (`clock()`, `server_information()`) | +| `ConnectionManager` published state (each admitted slot's clock, the primary's server information) | Protocol task (`refresh_published_state()`) | Any thread (`clock()`, `server_information()`); the main loop keeps its own copy of the server information, from the Inbox's `server_slot` | | `RecordStore` | Protocol task (resolves and changes records) | Main loop (writes them to the provider) | Plain atomics carry the rest: the `connected_` and `paired_` flags on `ConnectionManager`; the admission flag `accepting_` on `ProtocolTask`, written on the main loop under the queue lock; the unpaired-access setting, the lifecycle state, the network provider's last answer (`network_ready_`, written by the main loop's poll and read by the protocol task before it starts the WebSocket server) and the high-performance grant count (`high_performance_granted_`, see [One Main Loop Drain](#one-main-loop-drain)) on `SendspinClient`; and each role's teardown generation (`cleanup_generation`), written by the protocol task and read by the main loop and the role's thread. The role- and connection-local atomics (a connection's last-arrival stamp, upgrade flag and time-frame write time; artwork's `stream_active` and slot epochs, the visualizer's `spectrum_bin_count`, `tracks_downbeats` and `boundary_sequence`, the player's output delay, the controller's supported commands, the source's stream gate and codec, the source task's overflow generation) each state their writer and reader at their declaration. @@ -81,6 +81,7 @@ Every role state slot is a `GenerationSlot`, except the metadata and color ro | `SendspinClient::EventState::persist_slot` | `INBOX_TOPIC_PERSIST` | Protocol task (`server/pair-finalize`, an unpair, a playback activate, a last-played change); main loop (`start()`) | | `SendspinClient::EventState::pairing_slot` | `INBOX_TOPIC_PAIRING` | Protocol task (pairing notes); main loop (`stop()`'s dismissals) | | `SendspinClient::EventState::high_performance_slot` | `INBOX_TOPIC_HIGH_PERFORMANCE` | Protocol task (time-burst acquires and releases, counted separately) | +| `SendspinClient::EventState::server_slot` | `INBOX_TOPIC_SERVER` | Protocol task (the primary connection's server information, or none, each time the primary changes) | | `ControllerRole::Impl::EventState::slot` (generation-stamped) | `INBOX_TOPIC_CONTROLLER` | Protocol task | | `MetadataRole::Impl::EventState::slot` (scheduled, generation-stamped) | `INBOX_TOPIC_METADATA` | Protocol task | | `ColorRole::Impl::EventState::slot` (scheduled, generation-stamped) | `INBOX_TOPIC_COLOR` | Protocol task | @@ -143,9 +144,10 @@ The tick returns the milliseconds until the earliest of its timers: a nursery en While the client is stopped, `SendspinClient::loop()` only frees the outbound attempts `stop()` left parked (`ConnectionManager::reap_finished()`). While started, each call polls the network provider (`poll_network_provider()`, which wakes the protocol task when the network comes up) and then runs `drain_inbox()`, which `stop()` also calls once so teardown callbacks are delivered synchronously: ```api -1. Each role catches up to its current teardown generation (catch_up_teardown()): one - acquire load per role -2. High-performance requests: every acquire, the grant for them, then every release +1. The server information server_slot carries, into the copy server_information() views; then + each role catches up to its current teardown generation (catch_up_teardown()): one acquire + load per role +2. The high-performance requests: every acquire, the grant for them, then every release 3. The time-sync report: on_time_sync_updated() 4. The inbox event ring; each event's role first catches up on the teardown the event is stamped with (catch_up_teardown()) @@ -160,7 +162,7 @@ While the client is stopped, `SendspinClient::loop()` only frees the outbound at on_group_update ``` -Step 1 is what still runs a teardown's main-loop half, and delivers its clear, when the full event ring dropped its stamped `*_CLEARED`. It can only run a half earlier, never apply anything: a payload is applied only when its stamp is the current generation, and the events and role drains after it catch up again at the stamp they act on. The steps gate on two `poll()` snapshots: one before the ring drain (steps 2 to 4) and one after it (steps 6 to 8), which catches bits set while the ring was draining. Provider writes run after the notes are taken and before they are dispatched, so `on_pairing_succeeded` finds the record committed: the protocol task requests the write before it queues the note. Pairing notes follow the event ring, so a role's clear can be delivered ahead of a note queued before it: the two never describe the same listener state, and the one ordering between them that matters, the provider write ahead of `on_pairing_succeeded`, is step 5. Roles whose pending work waits on a deadline rather than on a new message (the metadata and color roles' pending state, artwork's held displays) add a carry-over term to `needs_drain()`, since no inbox bit tracks that wait; the metadata and color roles test their main loop's copy of the pending deadline (`ScheduledStateSlot::pending_at()`), so no tick takes the Inbox mutex while a state waits; the player's STREAM_END held for the sync task waits instead for `sync_idle_slot`, which the sync task writes each time it returns to idle from a stream. +Step 1 is what still runs a teardown's main-loop half, and delivers its clear, when the full event ring dropped its stamped `*_CLEARED`. It can only run a half earlier, never apply anything: a payload is applied only when its stamp is the current generation, and the events and role drains after it catch up again at the stamp they act on. The steps gate on two `poll()` snapshots: one before the ring drain (steps 2 to 4) and one after it (steps 6 to 8), which catches bits set while the ring was draining. Provider writes run after the notes are taken and before they are dispatched, so `on_paired` finds the record committed: the protocol task requests the write before it queues the note. Pairing notes follow the event ring, so a role's clear can be delivered ahead of a note queued before it: the two never describe the same listener state, and the one ordering between them that matters, the provider write ahead of `on_paired`, is step 5. Roles whose pending work waits on a deadline rather than on a new message (the metadata and color roles' pending state, artwork's held displays) add a carry-over term to `needs_drain()`, since no inbox bit tracks that wait; the metadata and color roles test their main loop's copy of the pending deadline (`ScheduledStateSlot::pending_at()`), so no tick takes the Inbox mutex while a state waits; the player's STREAM_END held for the sync task waits instead for `sync_idle_slot`, which the sync task writes each time it returns to idle from a stream. The high-performance hold is a ref-counted request, surfaced to the consumer through `on_request_high_performance()` / `on_release_high_performance()`, for the platform to keep networking responsive (e.g. disable WiFi power saving) during time sync and playback. A time burst requests it through `high_performance_slot` when it comes due, and sends its first time frame only once the main loop has granted that request: step 2 calls the listener for every acquire it took, then adds them to `SendspinClient::high_performance_granted_` and wakes the protocol task, which compares the burst's ticket against that count (`high_performance_granted()`). Acquires and releases are counted separately, so an acquire and a release queued inside one stalled tick (a connection dropped before its grant) still reach the listener as a request followed by its release. Releases wait for nothing. The player holds the same ref count from its drained STREAM_START to its STREAM_END on the main loop, and the source from its drained STREAMING_STARTED to its STREAMING_STOPPED (or the teardown that owes it). @@ -232,7 +234,7 @@ Requests a listener callback makes (`connect_to()`, `disconnect()`, ...) are que - `drain_generation` (on the client's `EventState`) is bumped by `stop()` before its teardown. The outer drain snapshots it once and stops as soon as it changes: the event ring between events, the pairing-note dispatch between notes, and the role drains and the group update before each starts. - Each role's `cleanup_generation` is bumped by its `cleanup()` and stamped onto every event and slot payload it queues afterwards; the ring drain drops stream events whose stamp no longer matches (`event_is_current()`). Inside a role drain the same counter catches a callback that re-entered teardown: the player checks it after each server-command callback and around each `on_stream_start()` and `on_stream_end()`, metadata and color by ending their drain once the teardown's catch-up has forgotten the pending deadline (`ScheduledStateSlot::pending_at()`), and artwork after each display or clear, and each abandons the rest of its work when it moved on. -A state handed to the listener in two forms, the view and the deprecated `std::string` copy, is not handed over in the second once the first re-entered teardown: the group update checks the drain generation between `on_group_state()` and `on_group_update()`, and the metadata role checks between `on_track_metadata()` and `on_metadata()` whether its `TeardownTracker` completed a teardown, which on the main loop only `stop()` can do inside a callback. +A state handed to the listener in two forms, the view and the deprecated `std::string` copy, is not handed over in the second once the first re-entered teardown: the group update checks the drain generation between `on_group_state()` and `on_group_update()`, the pairing-note dispatch between each text-carrying callback and its deprecated form (`on_pairing_attempt()` and `on_pairing_started()`, ...), and the metadata role checks between `on_track_metadata()` and `on_metadata()` whether its `TeardownTracker` completed a teardown, which on the main loop only `stop()` can do inside a callback. ## Message Flow @@ -367,7 +369,7 @@ Client -> Server: noise/handshake msg2 Server -> Client: server/activate (normal operational flow) ``` -The new long-term record must resolve for the re-handshake that immediately follows, so the `server/pair-finalize` handler commits it to `RecordStore` in RAM on the protocol task. The persistence provider is main-loop-only, so the durable write is staged through `persist_slot` and performed by the next drain's `flush_pending_persistence()`, or by the client destructor if it comes first, before `on_pairing_succeeded` fires. Every other change to persisted state (an unpair, a playback handoff, the last-played server) takes the same route: the RAM half runs on the protocol task where the change is decided, and the provider write is left to `flush_pending_persistence()` on the main loop, so a slow flash commit never stalls connection work. The pairing-code methods (CPace) run their state machines in `ConnectionManager` on the protocol task too. +The new long-term record must resolve for the re-handshake that immediately follows, so the `server/pair-finalize` handler commits it to `RecordStore` in RAM on the protocol task. The persistence provider is main-loop-only, so the durable write is staged through `persist_slot` and performed by the next drain's `flush_pending_persistence()`, or by the client destructor if it comes first, before `on_paired` fires. Every other change to persisted state (an unpair, a playback handoff, the last-played server) takes the same route: the RAM half runs on the protocol task where the change is decided, and the provider write is left to `flush_pending_persistence()` on the main loop, so a slow flash commit never stalls connection work. The pairing-code methods (CPace) run their state machines in `ConnectionManager` on the protocol task too. ## Connection Lifecycle @@ -461,7 +463,7 @@ On host, IXWebSocket callbacks hold a `weak_ptr` to the connection the manager a The library allocates from heaps that fail differently. `operator new`, behind every `std::string`, `std::vector` and `std::make_shared` and any `std::function` whose capture outgrows its small buffer, draws on ESPHome only on internal RAM and aborts when it fails. The `platform_malloc` family (`src/platform/memory.h`, behind `PlatformBuffer` and the JSON arena's overflow) places each block by its `MemoryLocation`, and noise-c allocates its handshake, cipher and hash states with `calloc`; both report a failure, which the caller turns into a refused or closed connection or a dropped message. Normalizing a `group_id` that is not 43 base64url characters (`group_id_from_wire()`, while `GROUP_ID_STRICT` is off) has noise-c `calloc` a SHA-256 state on the protocol task for every `group/update` from a server that sends such ids; when that allocation fails, the connection is closed, as for a non-conforming id under `GROUP_ID_STRICT`, rather than applying the rest of the update under the previous group's id. The exceptions are hashing a PSK into its `psk_id`, which aborts when noise-c cannot allocate the hash state, but only while `start()` loads the record store (`psk_id_for()` on a `std::array`), and deriving `SENTINEL_PSK` the same way at static initialization; a pairing that cannot hash its new PSK fails instead. Construction, `add_*()` and `start()` use any of them. After `start()`, the per-connection and per-message buffers come from the `platform_malloc` family: the connection object of an ESP accept and of every `connect_to()`, with its `shared_ptr` control block (`make_connection_in_block()`: `std::allocate_shared()` into a block reserved first, so a failure refuses the session or the connect), the JSON arena's overflow, the handshake's prologue and msg1 buffers, the fallback buffer, the Noise transport's reassembly and send buffers, and on ESP a send that does not fit the `FixedBlockPool`. The sync task allocates on first use from the same family: its decode buffer's storage at the first codec header (the `TransferBuffer` itself sits by value in the task's `SyncContext`), where a failure drops the stream, and the FLAC decoder, constructed in a `PlatformBuffer` at the first FLAC codec header and reset for each later FLAC stream, where a failure fails the header and the stream's chunks are discarded unplayed. micro-flac allocates the rest of its state with `FLAC_MALLOC`, which fails the same way; its own `operator new` allocations, an Ogg demuxer and stored metadata blocks, never run on the `fLaC` and last-STREAMINFO `codec_header` the parser admits. The visualizer's spectrum bins are reserved by `start()` for the configured `n_disp_bins` when the configuration requests the spectrum type, and a stream serving more bins than that drops its spectrum frames rather than growing them on the drain thread. `connect_to()` takes its URL by value and moves it through the request slot into the transport, so the library copies none of it. The host transport, whose IXWebSocket API copies each frame into a `std::string` and which makes a host accept's connection object with `std::make_shared`, and the host-only message trace sit outside this model: ESP replaces the first with esp_http_server and esp_websocket_client and builds without the second. -Bounded values stay inline: the ids and names a connection holds (`IdText`, `ServerName`), activity, player and controller command, visualizer type and source codec sets (`EnumSet`), role sets, `stream/start`'s artwork channels (`InlineVector`) and its player's FLAC codec header, decoded as it is parsed (`FlacCodecHeader`), the protocol task's slot tables and the ESP server's session slots and pending upgrades (`InlineVector`, `std::array`), each connection's `NoiseSession`, which owns the noise-c handshake and cipher states, and `NoiseHandshake`, a pairing attempt's CPace state, its PRS and the code it emits (`InlineText`), and the pairing records (`PairingRecord`, its ids `IdText`): the one a pairing holds pending on its connection and those the record store holds, reserved for `max_pairing_records` when `start()` builds the store. The store's mutators mark the slots a flush owes in a bitset, and the flush encodes one write at a time into a buffer on the main loop's stack. CPace hashes each length-value piece of its transcript as it encodes it rather than building the transcript first. The pairing notes for the main loop trade two buffers reserved at construction between the slot and the drain (`InboxSlot::exchange()`). Role sets take two shapes: ownership, admission and `client/state` intersect and complement them, so `ActiveRoles` and the admitted slots hold `uint16_t` role masks (`role_mask_bit()`, `ALL_ROLES_MASK`), while the roles a `stream/start`, `stream/end` or `stream/clear` covers are only filled and looked up, so they are an `EnumSet`. `client/state`'s artwork channels and visualizer types view the role's configuration, fixed from `add_*()`, which may be called only while the client is stopped (`client.h` "Role registration"), and `client/hello`'s application roles view the client's configuration as `start()` normalized it. A getter that returns `std::string`, such as the server's id and name, builds it from the inline value on the caller's thread. The main loop lists the controller's `supported_commands` and a visualizer stream's `types` from their sets into public vectors each role reserves for every member when it is built. `group/update`'s `group_id` and `group_name` are parsed into `GroupUpdatePayload` (`IdText`, `ServerName`), and the main loop merges each payload into the client's group state, a `GroupUpdatePayload` it keeps, which `group_state()` views, as `on_group_state()` views the payload; it also copies both into the deprecated public strings of `get_group_state()` and of the delta `on_group_update()` is handed, each reserved for its bound when the client is built and swapped aside rather than freed when its field is cleared (`ReservedText`, `src/reserved_text.h`). +Bounded values stay inline: the ids and names a connection holds (`IdText`, `ServerName`), activity, player and controller command, visualizer type and source codec sets (`EnumSet`), role sets, `stream/start`'s artwork channels (`InlineVector`) and its player's FLAC codec header, decoded as it is parsed (`FlacCodecHeader`), the protocol task's slot tables and the ESP server's session slots and pending upgrades (`InlineVector`, `std::array`), each connection's `NoiseSession`, which owns the noise-c handshake and cipher states, and `NoiseHandshake`, a pairing attempt's CPace state, its PRS and the code it emits (`InlineText`), and the pairing records (`PairingRecord`, its ids `IdText`): the one a pairing holds pending on its connection and those the record store holds, reserved for `max_pairing_records` when `start()` builds the store. The store's mutators mark the slots a flush owes in a bitset, and the flush encodes one write at a time into a buffer on the main loop's stack. CPace hashes each length-value piece of its transcript as it encodes it rather than building the transcript first. The pairing notes for the main loop trade two buffers reserved at construction between the slot and the drain (`InboxSlot::exchange()`). Role sets take two shapes: ownership, admission and `client/state` intersect and complement them, so `ActiveRoles` and the admitted slots hold `uint16_t` role masks (`role_mask_bit()`, `ALL_ROLES_MASK`), while the roles a `stream/start`, `stream/end` or `stream/clear` covers are only filled and looked up, so they are an `EnumSet`. `client/state`'s artwork channels and visualizer types view the role's configuration, fixed from `add_*()`, which may be called only while the client is stopped (`client.h` "Role registration"), and `client/hello`'s application roles view the client's configuration as `start()` normalized it. A deprecated getter that returns `std::string` (the server's id and name, a pairing token) builds it from the inline value on the caller's thread. The main loop keeps its own `PublishedServer` copy of the server information, taken from `server_slot` when the primary changes, which `server_information()` views. The pairing callbacks' view forms view the note being dispatched; the deprecated forms are handed one string reserved at construction for `PairingNote::text`'s bound and assigned in place. The main loop lists the controller's `supported_commands` and a visualizer stream's `types` from their sets into public vectors each role reserves for every member when it is built. `group/update`'s `group_id` and `group_name` are parsed into `GroupUpdatePayload` (`IdText`, `ServerName`), and the main loop merges each payload into the client's group state, a `GroupUpdatePayload` it keeps, which `group_state()` views, as `on_group_state()` views the payload; it also copies both into the deprecated public strings of `get_group_state()` and of the delta `on_group_update()` is handed, each reserved for its bound when the client is built and swapped aside rather than freed when its field is cleared (`ReservedText`, `src/reserved_text.h`). A metadata state's text, only the fields `MetadataRoleConfig` keeps, each cut to its cap as it is parsed (`MAX_METADATA_TEXT_BYTES`, `MAX_METADATA_URL_BYTES`), is copied into a text block as it arrives, which `on_track_metadata()` views, and out of it into the deprecated public strings `on_metadata()` is handed. `start()` allocates `METADATA_TEXT_BLOCKS` blocks from the `platform_malloc` family (`MetadataRoleConfig::buffer_location`; a failure fails the start, logged) and keeps them for the role's life. Each place a state can be names its block by index (`MetadataStateBuffer`): the slot's oldest and newest places (`ScheduledStateSlot`) and the main loop's current state, the one last delivered. A delivery swaps the delivered place with the current state under the Inbox mutex, so each block always has one place. `METADATA_TEXT_BLOCKS` is the fewest that keep every state the role must: the delivered state the main loop's callback is viewing, the pending one, and room for an arrival meanwhile, since the protocol task does not wait for the main loop. The protocol task copies the kept text from the parsed document into its place's block inside `ParsedJsonMessage::extract()`, the only window the document is alive, under the Inbox mutex (`MetadataRole::Impl::fill_place()`); the main loop copies the current state's text into the deprecated public strings, each reserved for its field's cap when the role is built (`ReservedText`), with no lock held. @@ -469,7 +471,6 @@ A metadata state's text, only the fields `MetadataRoleConfig` keeps, each cut to | What | When | |------|------| -| The listener's `std::string` arguments: a pairing's `server_id`, and a `qr_code` pairing code's token | Each pairing callback that takes one, on the main loop | | The pairing notes for the main loop beyond the room both of their buffers keep | A batch of more than `PAIRING_NOTE_CAPACITY` notes between two drains, on the protocol task; the grown room is kept | | Another drain buffer for the pairing notes, with room for `PAIRING_NOTE_CAPACITY` | A pairing listener callback that calls `stop()` while a prompt shows: the drain `stop()` runs finds the buffer lent to the dispatch that called it, and reserves one before it takes the dismissals, on the main loop | | The player's stream events held for the sync task's idle | Events beyond the list's kept capacity, on the main loop | diff --git a/examples/basic_client/main.cpp b/examples/basic_client/main.cpp index 2d7f5231..6bd90ed5 100644 --- a/examples/basic_client/main.cpp +++ b/examples/basic_client/main.cpp @@ -436,35 +436,39 @@ int main(int argc, char* argv[]) { } } - void on_pairing_started(const std::string& server_id) override { - fprintf(stderr, ">>> Pairing started with server: %s\n", server_id.c_str()); + void on_pairing_attempt(std::string_view server_id) override { + fprintf(stderr, ">>> Pairing started with server: %.*s\n", + static_cast(server_id.size()), server_id.data()); } - void on_pairing_succeeded(const std::string& server_id) override { - fprintf(stderr, ">>> Pairing succeeded with server: %s\n", server_id.c_str()); + void on_paired(std::string_view server_id) override { + fprintf(stderr, ">>> Pairing succeeded with server: %.*s\n", + static_cast(server_id.size()), server_id.data()); fprintf(stderr, " Long-term record stored. Future connections from this server\n"); fprintf(stderr, " will be paired sessions.\n"); } - void on_pairing_failed(const std::string& server_id, - SendspinPairAbortReason reason) override { - fprintf(stderr, ">>> Pairing failed with server: %s (reason %d)\n", server_id.c_str(), - static_cast(reason)); + void on_pairing_aborted(std::string_view server_id, + SendspinPairAbortReason reason) override { + fprintf(stderr, ">>> Pairing failed with server: %.*s (reason %d)\n", + static_cast(server_id.size()), server_id.data(), static_cast(reason)); } - void on_display_pairing_code(const std::string& code, - SendspinPairingCodeFormat format) override { + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat format) override { if (format == SendspinPairingCodeFormat::QR_CODE) { // This example never offers qr_code (a terminal cannot render one), so a server // can only select digits. - fprintf(stderr, "\n>>> Pairing token: %s\n", code.c_str()); + fprintf(stderr, "\n>>> Pairing token: %.*s\n", static_cast(code.size()), + code.data()); fprintf(stderr, " Scan or paste this into the server to finish pairing.\n\n"); return; } // pairing.md "Pairing Code Presentation" asks for a 3-3 grouping; the separator is // presentation only and never part of the code the operator types. - fprintf(stderr, "\n>>> Pairing code: %s-%s\n", code.substr(0, 3).c_str(), - code.substr(3).c_str()); + const std::string_view first = code.substr(0, 3); + const std::string_view second = code.substr(3); + fprintf(stderr, "\n>>> Pairing code: %.*s-%.*s\n", static_cast(first.size()), + first.data(), static_cast(second.size()), second.data()); fprintf(stderr, " Enter this code on the server to finish pairing.\n\n"); } @@ -528,9 +532,9 @@ int main(int argc, char* argv[]) { // The pairing token carries the client_id and the Pairing PSK together. Paste it into a // server that asks for one to pair via the pairing_psk method; pairing with a code instead // needs nothing from here (the code is printed when the server starts that flow). - auto token = client.pairing_token(); + const auto token = client.client_pairing_token(); if (token.has_value()) { - fprintf(stderr, "Pairing token: %s\n", token->c_str()); + fprintf(stderr, "Pairing token: %.*s\n", static_cast(token->size()), token->data()); } #ifdef SENDSPIN_HAS_MDNS diff --git a/examples/source_client/main.cpp b/examples/source_client/main.cpp index 4af3cfa8..1b621604 100644 --- a/examples/source_client/main.cpp +++ b/examples/source_client/main.cpp @@ -65,6 +65,7 @@ #include #include #include +#include #include #include @@ -545,26 +546,29 @@ int main(int argc, char* argv[]) { fprintf(stderr, ">>> High-performance networking released\n"); } - void on_pairing_started(const std::string& server_id) override { - fprintf(stderr, ">>> Pairing started with server: %s\n", server_id.c_str()); + void on_pairing_attempt(std::string_view server_id) override { + fprintf(stderr, ">>> Pairing started with server: %.*s\n", + static_cast(server_id.size()), server_id.data()); } - void on_pairing_succeeded(const std::string& server_id) override { - fprintf(stderr, ">>> Pairing succeeded with server: %s\n", server_id.c_str()); + void on_paired(std::string_view server_id) override { + fprintf(stderr, ">>> Pairing succeeded with server: %.*s\n", + static_cast(server_id.size()), server_id.data()); } - void on_pairing_failed(const std::string& server_id, - SendspinPairAbortReason reason) override { - fprintf(stderr, ">>> Pairing failed with server: %s (reason %d)\n", server_id.c_str(), - static_cast(reason)); + void on_pairing_aborted(std::string_view server_id, + SendspinPairAbortReason reason) override { + fprintf(stderr, ">>> Pairing failed with server: %.*s (reason %d)\n", + static_cast(server_id.size()), server_id.data(), static_cast(reason)); } - void on_display_pairing_code(const std::string& code, - SendspinPairingCodeFormat /*format*/) override { + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat /*format*/) override { // Only digits are offered. pairing.md "Pairing Code Presentation" asks for a 3-3 // grouping; the separator is presentation only and never part of the code. - fprintf(stderr, "\n>>> Pairing code: %s-%s\n", code.substr(0, 3).c_str(), - code.substr(3).c_str()); + const std::string_view first = code.substr(0, 3); + const std::string_view second = code.substr(3); + fprintf(stderr, "\n>>> Pairing code: %.*s-%.*s\n", static_cast(first.size()), + first.data(), static_cast(second.size()), second.data()); fprintf(stderr, " Enter this code on the server to finish pairing.\n\n"); } @@ -602,9 +606,9 @@ int main(int argc, char* argv[]) { // The pairing token carries the client_id and the Pairing PSK together. Paste it into a // server that asks for one to pair via the pairing_psk method; pairing with a code instead // needs nothing from here (the code is printed when the server starts that flow). - auto token = client.pairing_token(); + const auto token = client.client_pairing_token(); if (token.has_value()) { - fprintf(stderr, "Pairing token: %s\n", token->c_str()); + fprintf(stderr, "Pairing token: %.*s\n", static_cast(token->size()), token->data()); } #ifdef SENDSPIN_HAS_MDNS diff --git a/examples/tui_client/main.cpp b/examples/tui_client/main.cpp index 5fca5ea1..878201ad 100644 --- a/examples/tui_client/main.cpp +++ b/examples/tui_client/main.cpp @@ -81,6 +81,7 @@ #include #include #include +#include #include #include #include @@ -768,31 +769,31 @@ int main(int argc, char* argv[]) { } } - void on_pairing_started(const std::string& /*server_id*/) override { + void on_pairing_attempt(std::string_view /*server_id*/) override { std::lock_guard lock(state.mutex); state.pairing_status = "Pairing..."; } - void on_pairing_succeeded(const std::string& /*server_id*/) override { + void on_paired(std::string_view /*server_id*/) override { std::lock_guard lock(state.mutex); state.pairing_status = "Paired"; } - void on_pairing_failed(const std::string& /*server_id*/, - SendspinPairAbortReason /*reason*/) override { + void on_pairing_aborted(std::string_view /*server_id*/, + SendspinPairAbortReason /*reason*/) override { std::lock_guard lock(state.mutex); state.pairing_status = "Pairing failed"; } - void on_display_pairing_code(const std::string& code, - SendspinPairingCodeFormat format) override { + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat format) override { std::lock_guard lock(state.mutex); // Only digits are offered above, so a code that arrives is six digits; pairing.md // "Pairing Code Presentation" asks for the 3-3 grouping shown here, which is // presentation only and never part of what the operator types. - state.pairing_status = format == SendspinPairingCodeFormat::DIGITS - ? "Code " + code.substr(0, 3) + "-" + code.substr(3) - : "Token " + code; + state.pairing_status = + format == SendspinPairingCodeFormat::DIGITS + ? "Code " + std::string(code.substr(0, 3)) + "-" + std::string(code.substr(3)) + : "Token " + std::string(code); } void on_clear_pairing_code() override { @@ -1004,9 +1005,9 @@ int main(int argc, char* argv[]) { { std::lock_guard lock(state.mutex); state.client_id = client.client_id(); - auto token = client.pairing_token(); + const auto token = client.client_pairing_token(); if (token.has_value()) { - state.pairing_token = std::move(*token); + state.pairing_token.assign(token->data(), token->size()); } } diff --git a/include/sendspin/client.h b/include/sendspin/client.h index 259e70bc..c1e840eb 100644 --- a/include/sendspin/client.h +++ b/include/sendspin/client.h @@ -97,19 +97,34 @@ class SendspinClientListener { // Encryption / pairing callbacks // ======================================== + // Each callback below that carries text is followed by its deprecated std::string form with + // the same content: override one of the two. A listener overriding both gets both, the view + // form first. The text views the pairing notification being delivered and is valid for the + // duration of the callback. + /// @brief Called when a server begins a pairing exchange /// /// server_id is the base64url public key of the server entering pairing. Fires once per - /// attempt whatever the method; the exchange ends at on_pairing_succeeded or - /// on_pairing_failed, or at neither when the server cancels it with a server/activate - /// (pairing.md "Entering and leaving pairing"). - virtual void on_pairing_started(const std::string& /*server_id*/) {} + /// attempt whatever the method; the exchange ends at on_paired or on_pairing_aborted, or at + /// neither when the server cancels it with a server/activate (pairing.md "Entering and leaving + /// pairing"). + virtual void on_pairing_attempt(std::string_view /*server_id*/) {} + + /// @brief Called after on_pairing_attempt() with the same server_id + /// @deprecated Override on_pairing_attempt(); removed in v0.11.0. + [[deprecated("override on_pairing_attempt(); removed in v0.11.0")]] virtual void + on_pairing_started(const std::string& /*server_id*/) {} /// @brief Called when a pairing exchange completes and a long-term record is stored /// /// server_id is the base64url public key of the newly paired server. The server then /// re-handshakes on the new long-term PSK, after which is_paired() reads true. - virtual void on_pairing_succeeded(const std::string& /*server_id*/) {} + virtual void on_paired(std::string_view /*server_id*/) {} + + /// @brief Called after on_paired() with the same server_id + /// @deprecated Override on_paired(); removed in v0.11.0. + [[deprecated("override on_paired(); removed in v0.11.0")]] virtual void on_pairing_succeeded( + const std::string& /*server_id*/) {} /// @brief Called when a pairing exchange is aborted (by the server or by the protocol) /// @@ -117,8 +132,13 @@ class SendspinClientListener { /// pairing or resume normal operation on it (pairing.md "pair/abort"). It is closed for /// CONCURRENT_ATTEMPT, and closed without any further message for the UNKNOWN reported on a /// pairing protocol error (a malformed or out-of-sequence pairing message). - virtual void on_pairing_failed(const std::string& /*server_id*/, - SendspinPairAbortReason /*reason*/) {} + virtual void on_pairing_aborted(std::string_view /*server_id*/, + SendspinPairAbortReason /*reason*/) {} + + /// @brief Called after on_pairing_aborted() with the same server_id and reason + /// @deprecated Override on_pairing_aborted(); removed in v0.11.0. + [[deprecated("override on_pairing_aborted(); removed in v0.11.0")]] virtual void + on_pairing_failed(const std::string& /*server_id*/, SendspinPairAbortReason /*reason*/) {} /// @brief Called when a dynamic pairing code should be emitted to the operator. /// @@ -132,13 +152,16 @@ class SendspinClientListener { /// and is always followed by on_clear_pairing_code. /// Only called when SendspinClientConfig::pairing_code_out_channels and /// ::pairing_code_formats are both non-empty. - virtual void on_display_pairing_code(const std::string& /*code*/, - SendspinPairingCodeFormat /*format*/) {} + virtual void on_pairing_code(std::string_view /*code*/, SendspinPairingCodeFormat /*format*/) {} + + /// @brief Called after on_pairing_code() with the same code and format + /// @deprecated Override on_pairing_code(); removed in v0.11.0. + [[deprecated("override on_pairing_code(); removed in v0.11.0")]] virtual void + on_display_pairing_code(const std::string& /*code*/, SendspinPairingCodeFormat /*format*/) {} /// @brief Called to withdraw the emitted dynamic pairing code. /// - /// Fires after every pairing attempt that triggered on_display_pairing_code, whatever its - /// outcome. + /// Fires after every pairing attempt that triggered on_pairing_code, whatever its outcome. virtual void on_clear_pairing_code() {} /// @brief Called when the operator must perform the device pairing-window gesture to allow a @@ -265,6 +288,7 @@ struct ClientHelloMessage; struct ClientStateMessage; struct GroupUpdatePayload; struct Identity; +struct PublishedServer; /** * @brief Main orchestration class for the sendspin-cpp library @@ -555,34 +579,65 @@ class SendspinClient { return this->client_id_; } - /// @brief Builds the pairing token (pairing.md "Pairing Token") for a Sendspin Pairing - /// PSK: the single "SP:"-prefixed, base32 string that carries this client's static public key - /// alongside `pairing_psk`, for an operator to transfer into a server via copy/paste or QR - /// code to begin the Pairing PSK flow. Clients offering `pairing_psk` SHOULD surface this - /// token rather than the bare PSK. - /// Main loop only. - /// @param pairing_psk The 32-byte Sendspin Pairing PSK. - /// @return The 107-character token string, or nullopt if no identity has been initialized - /// yet (before start() is called). - [[nodiscard]] std::optional format_pairing_token( - const std::array& pairing_psk) const; + /// @brief Length in characters of a pairing token for a Sendspin Pairing PSK (pairing.md + /// "Pairing Token"): "SP:", the version and the base32 body + static constexpr size_t PAIRING_TOKEN_LENGTH = 107; - /// @brief Builds the pairing token for the client's own Sendspin Pairing PSK. + /// @brief Builds the pairing token (pairing.md "Pairing Token") for the client's own Sendspin + /// Pairing PSK: the single "SP:"-prefixed, base32 string that carries this client's static + /// public key alongside the Pairing PSK, for an operator to transfer into a server via + /// copy/paste or QR code to begin the Pairing PSK flow. Clients offering `pairing_psk` SHOULD + /// surface this token rather than the bare PSK. /// The Pairing PSK is SendspinClientConfig::pairing_psk when set, otherwise the stored one, /// generated and persisted on first boot when none is stored, so this token is stable for - /// the lifetime of that key: display it (or its QR code) for the operator to transfer into a - /// server that is setting this client up. - /// Main loop only. - /// @return The 107-character token string, or nullopt before start(). - [[nodiscard]] std::optional pairing_token() const; + /// the lifetime of that key. Main loop only. + /// @return The token's PAIRING_TOKEN_LENGTH characters, with no terminating NUL, or nullopt + /// before start(). + [[nodiscard]] std::optional> client_pairing_token() + const; + + /// @brief Builds the pairing token, as client_pairing_token() does, for any 32-byte Sendspin + /// Pairing PSK, such as one a provisioning tool writes into the device. Main loop only. + /// @return The token's PAIRING_TOKEN_LENGTH characters, with no terminating NUL, or nullopt + /// before start(), since the token also carries the client's identity. + [[nodiscard]] std::optional> pairing_token_for( + const std::array& pairing_psk) const; + + /// @brief Returns pairing_token_for(`pairing_psk`) copied into a std::string. Main loop only. + /// @deprecated Use pairing_token_for(); removed in v0.11.0. + [[deprecated("use pairing_token_for(); removed in v0.11.0"), + nodiscard]] std::optional + format_pairing_token(const std::array& pairing_psk) const; + + /// @brief Returns client_pairing_token() copied into a std::string. Main loop only. + /// @deprecated Use client_pairing_token(); removed in v0.11.0. + [[deprecated("use client_pairing_token(); removed in v0.11.0"), + nodiscard]] std::optional + pairing_token() const; /// @brief Returns true if there is an active connection whose handshake completed and whose /// latest server/activate has arrived. Any thread; reflects the protocol task's last tick. + /// server_information() can lag it by one loop() (see there). bool is_connected() const; - /// @brief Returns the server information from the active connection's hello handshake, or - /// nullopt when no connection is active. Any thread. - std::optional get_server_information() const; + /// @brief Returns the server information from the active connection's hello handshake, as + /// the protocol task had published it when loop() last ran, or nullopt when no connection was + /// active then. Main loop only. The views are valid until control next returns to loop() + /// (from inside a callback: until the callback returns) or until stop(). loop() takes the + /// copy as it starts, so it can read nullopt, or the previous server, for one loop() after + /// is_connected() turns true: read it on each loop() rather than only on that rising edge. + std::optional server_information() const; + + /// @brief Returns the server information the protocol task last published, each field copied + /// into a std::string, or nullopt when no connection is active. Any thread. + /// @deprecated Use server_information() on the main loop; removed in v0.11.0. Code off the + /// main loop has no replacement and keeps calling this until then. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + [[deprecated( + "use server_information(); removed in v0.11.0")]] std::optional + get_server_information() const; +#pragma GCC diagnostic pop /// @brief Returns true if the active connection is a paired session: one keyed by the /// long-term PSK of a pairing record, not the Pairing or Sentinel PSK. Any thread; reflects @@ -992,19 +1047,24 @@ class SendspinClient { /// of its role removals and of anything else it sends. Protocol task only. void on_activation_applied(SendspinConnection* conn); + /// @brief Queues the primary connection's server information, nullptr for none, for the copy + /// the main loop's drain_inbox() keeps and server_information() views. Called by + /// ConnectionManager::refresh_published_state() as the primary connection changes. + void note_server_information(const PublishedServer* server); + // Pairing notifications. Each is called on the protocol task and queues the listener callback // for delivery from the main loop's drain_inbox(). - /// @brief Queue an on_pairing_started notification + /// @brief Queue an on_pairing_attempt notification void note_pairing_started(std::string_view server_id); - /// @brief Queue an on_pairing_succeeded notification + /// @brief Queue an on_paired notification void note_pairing_succeeded(std::string_view server_id); - /// @brief Queue an on_pairing_failed notification + /// @brief Queue an on_pairing_aborted notification void note_pairing_failed(std::string_view server_id, SendspinPairAbortReason reason); - /// @brief Queue an on_display_pairing_code notification + /// @brief Queue an on_pairing_code notification void note_display_pairing_code(std::string_view code, SendspinPairingCodeFormat format); /// @brief Queue an on_clear_pairing_code notification diff --git a/include/sendspin/config.h b/include/sendspin/config.h index 91a90394..202d73f9 100644 --- a/include/sendspin/config.h +++ b/include/sendspin/config.h @@ -190,7 +190,7 @@ struct SendspinClientConfig { /// @brief Channels through which this device can emit a dynamic pairing code to the operator. /// Advertised as `out_channels` on the dynamic_pairing_code descriptor in client/hello /// (pairing.md "client/hello pair-method descriptor"). Set this alongside - /// `pairing_code_formats` when the application implements the on_display_pairing_code / + /// `pairing_code_formats` when the application implements the on_pairing_code / /// on_clear_pairing_code callbacks on its SendspinClientListener. Empty leaves /// dynamic_pairing_code unadvertised. std::vector pairing_code_out_channels{}; @@ -208,7 +208,7 @@ struct SendspinClientConfig { /// derives its psk_id. An all-zero key or the published Sentinel PSK is rejected: start() /// logs an error and returns false. When unset, the library loads the one stored under /// `persistence_keys::PAIRING_PSK`, or generates one on first boot and stores it there. - /// SendspinClient::pairing_token() carries whichever is in use. + /// SendspinClient::client_pairing_token() carries whichever is in use. std::optional pairing_psk{}; /// @brief The static pairing code the device shipped with: exactly 8 decimal digits, drawn diff --git a/include/sendspin/types.h b/include/sendspin/types.h index abfab0e1..278a5b0c 100644 --- a/include/sendspin/types.h +++ b/include/sendspin/types.h @@ -40,8 +40,19 @@ enum class SendspinGoodbyeReason : uint8_t { UNPAIRED, // Server unpaired this device via server/unpair }; -/// @brief Server identity fields received in server/hello messages -struct ServerInformationObject { +/// @brief Server identity fields received in server/hello messages, returned by +/// SendspinClient::server_information(), whose views' lifetime it documents +struct ServerInformation { + std::string_view server_id{}; + /// At most 64 bytes in UTF-8, cut at a character boundary + std::string_view name{}; +}; + +/// @brief ServerInformation with each field copied into a std::string +/// @deprecated Use ServerInformation, returned by SendspinClient::server_information(); removed +/// in v0.11.0. +struct [[deprecated("use ServerInformation and server_information(); removed in " + "v0.11.0")]] ServerInformationObject { std::string server_id{}; /// At most 64 bytes in UTF-8, cut at a character boundary std::string name{}; @@ -94,7 +105,7 @@ enum class MemoryLocation : uint8_t { /// @brief Reason a pairing exchange was aborted. /// /// Mirrors the wire values in the Sendspin pairing protocol. -/// Carried by SendspinClientListener::on_pairing_failed(). +/// Carried by SendspinClientListener::on_pairing_aborted(). enum class SendspinPairAbortReason : uint8_t { ATTEMPT_TIMEOUT, // Server did not complete the exchange in time. CONCURRENT_ATTEMPT, // Another pairing attempt is already in progress. diff --git a/src/client.cpp b/src/client.cpp index 066ba007..cbb61b75 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -60,7 +60,9 @@ #endif #include +#include #include +#include #include static const char* const TAG = "sendspin.client"; @@ -93,7 +95,7 @@ enum class PairingPrompt : uint8_t { struct PairingNote { PairingNoteType type{}; /// server_id for PAIRING_STARTED/SUCCEEDED/FAILED; the emitted code for - /// DISPLAY_PAIRING_CODE; empty otherwise. The drain builds the listener's std::string. + /// DISPLAY_PAIRING_CODE; empty otherwise. The listener's view form views it. IdText text{}; SendspinPairAbortReason reason{}; ///< Valid only for PAIRING_FAILED. /// Emission format of `text`; valid only for DISPLAY_PAIRING_CODE. @@ -116,6 +118,21 @@ std::vector reserved_pairing_notes() { return notes; } +/// @brief Whether `type` carries text, and so has a deprecated std::string callback the drain +/// calls after the view form +constexpr bool carries_text(PairingNoteType type) { + return type == PairingNoteType::PAIRING_STARTED || type == PairingNoteType::PAIRING_SUCCEEDED || + type == PairingNoteType::PAIRING_FAILED || type == PairingNoteType::DISPLAY_PAIRING_CODE; +} + +/// @brief An empty string with room for `capacity` characters, so assigning up to that many never +/// allocates +std::string reserved_string(size_t capacity) { + std::string text; + text.reserve(capacity); + return text; +} + /// @brief Builds a note carrying `text`, a server_id or an emitted pairing code PairingNote make_text_note(PairingNoteType type, std::string_view text) { PairingNote note{.type = type}; @@ -202,6 +219,17 @@ struct HighPerformanceRequests { return true; } +static_assert(SendspinClient::PAIRING_TOKEN_LENGTH == PAIRING_PSK_TOKEN_LENGTH, + "the public pairing token length is the version-0 token's"); + +/// @brief The characters of `token`, which holds exactly PAIRING_TOKEN_LENGTH of them +std::array token_chars(const PairingPskToken& token) { + std::array chars{}; + const std::string_view text = token.view(); + std::copy(text.begin(), text.end(), chars.begin()); + return chars; +} + /// @brief The public view of `group`, its text viewing the payload's inline storage GroupState group_state_of(const GroupUpdatePayload& group) { return {.playback_state = group.playback_state, @@ -248,6 +276,17 @@ struct SendspinClient::EventState { reserved_pairing_notes()}; /// The drain's buffer for the pairing notes, empty between drains. Main loop only. std::vector drained_pairing_notes{reserved_pairing_notes()}; + /// The std::string the deprecated pairing callbacks are handed, assigned in place from each + /// note's text and reserved for the longest (PairingNote::text), so no delivery allocates. + /// Main loop only. A drain nested in a callback (its stop()) delivers only the dismissals, + /// which carry no text, so it never reassigns the string a callback is reading. + std::string deprecated_pairing_text{reserved_string(decltype(PairingNote::text)::capacity())}; + /// The primary connection's server information, written by the protocol task each time the + /// primary changes (ConnectionManager::refresh_published_state()), latest-wins. + InboxSlot> server_slot{inbox, INBOX_TOPIC_SERVER}; + /// The server information the drain last took from server_slot, which server_information() + /// views. Main loop only. + std::optional server_information; /// Bumped by stop() before its teardown so a drain frame already on the stack (a listener /// callback that called stop()) abandons what it has not delivered yet: the events it copied /// out, the rest of the pairing notes, the role drains and the group update. Main loop only. @@ -287,6 +326,35 @@ struct SendspinClient::EventState { #pragma GCC diagnostic pop } + /// @brief Assigns `note`'s text into deprecated_pairing_text and hands it to the note's + /// deprecated callback. Main loop only. + void deliver_deprecated_pairing_note(SendspinClientListener& listener, + const PairingNote& note) { + this->deprecated_pairing_text.assign(note.text.view()); + const std::string& text = this->deprecated_pairing_text; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + switch (note.type) { + case PairingNoteType::PAIRING_STARTED: + listener.on_pairing_started(text); + break; + case PairingNoteType::PAIRING_SUCCEEDED: + listener.on_pairing_succeeded(text); + break; + case PairingNoteType::PAIRING_FAILED: + listener.on_pairing_failed(text, note.reason); + break; + case PairingNoteType::DISPLAY_PAIRING_CODE: + listener.on_display_pairing_code(text, note.format); + break; + case PairingNoteType::CLEAR_PAIRING_CODE: + case PairingNoteType::OPEN_PAIRING_WINDOW: + case PairingNoteType::CLOSE_PAIRING_WINDOW: + break; + } +#pragma GCC diagnostic pop + } + /// @brief Clears the client's group state, keeping its text storage. Main loop only. void clear_group_state() { this->group_state = {}; @@ -610,11 +678,14 @@ void SendspinClient::stop() { // leaves behind is the state a restart begins from. The drain generation bump abandons // the events a drain frame further up the stack (a listener that called stop()) copied // out before this teardown. The group slot is reset too, so the drain below cannot - // repopulate the group state from a delta that arrived before the stop. + // repopulate the group state from a delta that arrived before the stop, and the server + // slot with the copy server_information() views. ++this->event_state_->drain_generation; this->cleanup_connection_state(ALL_ROLES_MASK); this->task_state_->client_state.reset(); this->event_state_->clear_group_state(); + this->event_state_->server_slot.reset(); + this->event_state_->server_information.reset(); // A pairing attempt cut short by the stop leaves its code or pairing-window prompt showing. // Queue the dismissals now, after cleanup_connection_state() dropped the pending notes, so the @@ -938,6 +1009,15 @@ void SendspinClient::drain_inbox() { #endif }; + // --- Server information --- + // First, ahead of the teardown catch-up's clear callbacks, so every callback this drain makes + // reads the server as of the drain's start. A later tick can change it while the drain runs, + // so a group or metadata update delivered further down may come from a newer server than + // server_information() reads until the next loop(). + if ((es.inbox.poll() & INBOX_TOPIC_SERVER) != 0) { + (void)es.server_slot.take(es.server_information); + } + // Each role catches up on the teardowns the protocol task ran since the last drain: one // acquire load per role. Event dispatch and the role drains catch up on their own, at the // stamp they act on; this pass covers a teardown whose stamped *_CLEARED event the full event @@ -1091,7 +1171,7 @@ void SendspinClient::drain_inbox() { // --- Pairing notifications, and the deferred provider writes --- // After the event ring, so a role's clear can precede a pairing note queued before it. The // two never describe the same listener state (role state against the pairing UI). - // The one ordering that matters is a record's provider write ahead of on_pairing_succeeded: + // The one ordering that matters is a record's provider write ahead of on_paired: // the protocol task requests the write before it queues the PAIRING_SUCCEEDED note, so the // flush (which polls for itself) runs after the notes are taken and before they are // dispatched, and every note taken finds its write performed. @@ -1126,20 +1206,20 @@ void SendspinClient::drain_inbox() { } last_note = note.type; } + // The view form views the note in `notes`, which a drain nested in the callback + // cannot reach. switch (note.type) { case PairingNoteType::PAIRING_STARTED: - this->listener_->on_pairing_started(std::string(note.text.view())); + this->listener_->on_pairing_attempt(note.text.view()); break; case PairingNoteType::PAIRING_SUCCEEDED: - this->listener_->on_pairing_succeeded(std::string(note.text.view())); + this->listener_->on_paired(note.text.view()); break; case PairingNoteType::PAIRING_FAILED: - this->listener_->on_pairing_failed(std::string(note.text.view()), - note.reason); + this->listener_->on_pairing_aborted(note.text.view(), note.reason); break; case PairingNoteType::DISPLAY_PAIRING_CODE: - this->listener_->on_display_pairing_code(std::string(note.text.view()), - note.format); + this->listener_->on_pairing_code(note.text.view(), note.format); break; case PairingNoteType::CLEAR_PAIRING_CODE: this->listener_->on_clear_pairing_code(); @@ -1151,6 +1231,11 @@ void SendspinClient::drain_inbox() { this->listener_->on_close_pairing_window(); break; } + // A stop() from the view form ended the delivery, so the deprecated form of the + // note must not follow. + if (carries_text(note.type) && es.drain_generation == drain_generation) { + es.deliver_deprecated_pairing_note(*this->listener_, note); + } // A re-entrant stop() bumped the generation: abandon the rest of the batch. if (es.drain_generation != drain_generation) { break; @@ -1315,16 +1400,16 @@ SourceRole& SendspinClient::add_source(SourceRoleConfig config) { // Queries // ============================================================================ -std::optional SendspinClient::format_pairing_token( - const std::array& pairing_psk) const { +std::optional> +SendspinClient::pairing_token_for(const std::array& pairing_psk) const { if (this->identity_ == nullptr) { return std::nullopt; } - return std::string( - sendspin::format_pairing_token(this->identity_->public_bytes, pairing_psk).view()); + return token_chars(sendspin::format_pairing_token(this->identity_->public_bytes, pairing_psk)); } -std::optional SendspinClient::pairing_token() const { +std::optional> +SendspinClient::client_pairing_token() const { // Main-loop-only, like the other record-store config reads: the Pairing PSK is set when the // store is built, inside start(). if (this->record_store_ == nullptr) { @@ -1334,7 +1419,24 @@ std::optional SendspinClient::pairing_token() const { if (!pairing_psk.has_value()) { return std::nullopt; } - return this->format_pairing_token(pairing_psk->psk); + return this->pairing_token_for(pairing_psk->psk); +} + +std::optional SendspinClient::format_pairing_token( + const std::array& pairing_psk) const { + const auto token = this->pairing_token_for(pairing_psk); + if (!token.has_value()) { + return std::nullopt; + } + return std::string(token->data(), token->size()); +} + +std::optional SendspinClient::pairing_token() const { + const auto token = this->client_pairing_token(); + if (!token.has_value()) { + return std::nullopt; + } + return std::string(token->data(), token->size()); } bool SendspinClient::is_connected() const { @@ -1357,9 +1459,26 @@ int64_t SendspinClient::get_client_time(int64_t server_time) const { return clock.has_value() ? clock->compute_client_time(server_time) : 0; } +std::optional SendspinClient::server_information() const { + const std::optional& server = this->event_state_->server_information; + if (!server.has_value()) { + return std::nullopt; + } + return ServerInformation{.server_id = server->server_id.view(), .name = server->name.view()}; +} + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" std::optional SendspinClient::get_server_information() const { - return this->connection_manager_->server_information(); + // See ConnectionManager::server_information(). + const std::optional server = this->connection_manager_->server_information(); + if (!server.has_value()) { + return std::nullopt; + } + return ServerInformationObject{.server_id = std::string(server->server_id.view()), + .name = std::string(server->name.view())}; } +#pragma GCC diagnostic pop GroupState SendspinClient::group_state() const { return group_state_of(this->event_state_->group_state); @@ -1495,7 +1614,7 @@ void SendspinClient::cleanup_connection_state(uint16_t teardown_roles) { // whose prompt was already delivered (see retain_delivered_dismissals()): a teardown // queues the dismissals for the connection it drops itself, but one queued by an earlier // drop that no drain has delivered yet must survive this one. Callers that need another - // notification to survive teardown (e.g. handle_pair_abort's on_pairing_failed) must + // notification to survive teardown (e.g. handle_pair_abort's on_pairing_aborted) must // call the corresponding note_*() after cleanup_connection_state() returns; see the // ConnectionManager pairing handlers. // An edit, not a merge: it touches only notes already pending, and one that leaves none @@ -1901,6 +2020,16 @@ void SendspinClient::on_activation_applied([[maybe_unused]] SendspinConnection* #endif } +void SendspinClient::note_server_information(const PublishedServer* server) { + this->event_state_->server_slot.update([server](std::optional& current) { + if (server != nullptr) { + current = *server; + } else { + current.reset(); + } + }); +} + void SendspinClient::note_pairing_started(std::string_view server_id) { this->event_state_->push_pairing_note( make_text_note(PairingNoteType::PAIRING_STARTED, server_id)); diff --git a/src/client_dispatch.cpp b/src/client_dispatch.cpp index e71ec502..cb387c05 100644 --- a/src/client_dispatch.cpp +++ b/src/client_dispatch.cpp @@ -766,7 +766,7 @@ void SendspinClient::process_json_message(SendspinConnection& connection, const SS_LOGI(TAG, "server/pair-finalize: no pending pairing record to store"); } if (stored_record) { - // Before on_pairing_succeeded, so the drain that fires it flushes the write + // Before on_paired, so the drain that fires it flushes the write // first. Not fired for the capacity-rejection case. this->request_persist(); this->connection_manager_->on_pairing_succeeded(conn); diff --git a/src/connection.h b/src/connection.h index 7f77c4ed..bd6ab074 100644 --- a/src/connection.h +++ b/src/connection.h @@ -484,7 +484,7 @@ class SendspinConnection : public std::enable_shared_from_this ConnectionManager::clock() const { return this->published_.clocks[this->published_.primary_slot]; } -std::optional ConnectionManager::server_information() const { - std::optional server; - { - std::lock_guard lock(this->published_mutex_); - server = this->published_.server; - } - if (!server.has_value()) { - return std::nullopt; - } - return ServerInformationObject{.server_id = std::string(server->server_id.view()), - .name = std::string(server->name.view())}; +std::optional ConnectionManager::server_information() const { + std::lock_guard lock(this->published_mutex_); + return this->published_.server; } // ============================================================================ @@ -1044,6 +1036,12 @@ void ConnectionManager::refresh_published_state() { this->published_.clocks = clocks; this->published_.primary_slot = primary_slot; } + // After published_mutex_ is released, since both locks are leaves, and ahead of the connected + // flag below, so a loop() after a caller reads that flag finds its server. + if (primary_changed) { + this->client_->note_server_information( + this->published_.server.has_value() ? &*this->published_.server : nullptr); + } // Only ever lowered here: a loss is reported as soon as the slots stop naming the connection, // while connected is raised by publish_connected() at the end of the tick. diff --git a/src/connection_manager.h b/src/connection_manager.h index c6915ad0..44bc782a 100644 --- a/src/connection_manager.h +++ b/src/connection_manager.h @@ -196,6 +196,13 @@ struct ServerPairingMessage { std::array server_kc{}; ///< Server CPace confirmation tag }; +/// @brief The primary connection's server identity, held inline so the publish and the main +/// loop's copy (SendspinClient::server_information()) copy it without a heap allocation +struct PublishedServer { + IdText server_id{}; + ServerName name{}; +}; + /// @brief Pairing-UI display flags snapshotted from a connection's PairingSession. /// /// conn->pairing_session().code_emitted / .window_shown are the sole record of whether a pairing @@ -267,7 +274,7 @@ class ConnectionManager { /// which is keyed on no remaining connection owning one. /// - clock() and the server information follow primary() (the player's owner, else the first /// admitted), so a role owned by another connection converts with the wrong clock. - /// - group/update, leave(), get_server_information() and is_paired() are singular: they follow + /// - group/update, leave(), the server information and is_paired() are singular: they follow /// primary(). /// - The artwork binary gate (SendspinClient::process_binary_message()) passes any admitted /// connection while none owns the role. @@ -391,9 +398,8 @@ class ConnectionManager { /// @brief Returns the primary admitted connection's server information, or nullopt when none /// is admitted. Any thread: copies the inline id and name out of a slot written at admission - /// and cleared on the drop, behind the leaf published_mutex_, and builds the strings on the - /// caller's thread. - std::optional server_information() const; + /// and cleared on the drop, behind the leaf published_mutex_. + std::optional server_information() const; // ======================================== // Transport threads @@ -484,8 +490,7 @@ class ConnectionManager { void on_pairing_message(SendspinConnection* conn, const ServerPairingMessage& message); /// @brief Records a completed pairing: the server/pair-finalize handler stored the long-term - /// record for `conn`. Closes the pairing window the attempt ran under and queues - /// on_pairing_succeeded. + /// record for `conn`. Closes the pairing window the attempt ran under and queues on_paired. void on_pairing_succeeded(SendspinConnection* conn); /// @brief Drops a lost connection (the transport closed with every message it sent processed, @@ -824,7 +829,7 @@ class ConnectionManager { /// /// Captures the code-emission / pairing-window flags before clear_pairing_state() resets /// them, optionally sends a wire pair/abort, clears the pairing state, optionally drops the - /// connection, then queues on_pairing_failed and dismisses any pairing UI left showing (via + /// connection, then queues on_pairing_aborted and dismisses any pairing UI left showing (via /// SendspinClient::note_pairing_ui_dismissals()). When `drop_action` is not KEEP_OPEN, /// drop_connection() -> the admitted-slot cleanup runs before the note_* calls, matching the /// ordering every call site needs (the cleanup wipes the queued pairing notes). `conn` must be @@ -834,7 +839,7 @@ class ConnectionManager { /// (best-effort). Leave nullopt when the abort was received from the peer, or when /// pairing.md "Protocol Errors" forbids sending one. /// @param drop_action Disposition for the connection; see PairingDropAction. - /// @param public_reason Reason delivered to the application via on_pairing_failed. + /// @param public_reason Reason delivered to the application via on_pairing_aborted. /// @param goodbye_reason Read only when drop_action is CLOSE_WITH_GOODBYE, so callers for /// the other actions omit it. void abort_pairing_attempt( @@ -1009,13 +1014,6 @@ class ConnectionManager { /// @brief Each admitted slot's clock (admitted_[slot]), nullopt for a free slot using ClockSlots = std::array, MAX_ADMITTED>; - /// @brief The primary connection's server identity, held inline so the publish copies it - /// without a heap allocation - struct PublishedServer { - IdText server_id{}; - ServerName name{}; - }; - /// @brief What other threads read of the admitted connections struct PublishedState { ClockSlots clocks{}; diff --git a/src/inbox.h b/src/inbox.h index 77551955..2e7024a9 100644 --- a/src/inbox.h +++ b/src/inbox.h @@ -53,6 +53,7 @@ static constexpr uint32_t INBOX_TOPIC_TIME = 1U << 10; // Time-s static constexpr uint32_t INBOX_TOPIC_PAIRING = 1U << 11; // Pairing notes slot static constexpr uint32_t INBOX_TOPIC_HIGH_PERFORMANCE = 1U << 12; // High-performance requests static constexpr uint32_t INBOX_TOPIC_PLAYER_SYNC_IDLE = 1U << 13; // Sync task left a stream +static constexpr uint32_t INBOX_TOPIC_SERVER = 1U << 14; // Server information slot // ============================================================================ // Event ring types diff --git a/src/inline_text.h b/src/inline_text.h index c7e32312..037fcfb9 100644 --- a/src/inline_text.h +++ b/src/inline_text.h @@ -146,6 +146,10 @@ class InlineText { return this->size_ == 0; } + static constexpr size_t capacity() { + return N; + } + bool operator==(std::string_view other) const { return this->view() == other; } diff --git a/src/record_store.cpp b/src/record_store.cpp index 5c7de5fe..562a1805 100644 --- a/src/record_store.cpp +++ b/src/record_store.cpp @@ -180,7 +180,7 @@ void RecordStore::provision_pairing_psk_if_needed() { // pairing_psk is the one pairing method every client must implement (messaging.md // "client/hello"), so a client with no Pairing PSK would advertise a method it cannot // complete. The operator transfers the generated key to a server as a pairing token - // (SendspinClient::pairing_token()). It is stable across reboots once persisted; if + // (SendspinClient::client_pairing_token()). It is stable across reboots once persisted; if // persistence fails it is RAM-only for this boot, so a token printed then will not survive a // reboot. if (!this->pairing_psk_.has_value()) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7df13796..15de695a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -161,13 +161,22 @@ endif() # note_display_pairing_code() and note_clear_pairing_code()), since no public call shows a prompt, # and from inside its listener runs the final tick stop() would join (close_admission(), # protocol_tick()), so the shutdown pass takes the prompt flags it set on its stand-in's -# pairing_session() (window_shown and code_emitted). The stream-events table drives the player's -# handle_stream_start() and handle_stream_end() as the task would, and stages one interleaving by -# waiting on the sync task's item list and the player's sync-idle topic bit, and its clear row calls -# handle_stream_clear() the same way. The next-stream header table plays the sync task itself on -# the test thread, because no public call holds the task between its ACTIVE load and its next -# IDLE wait: it calls SyncTask's wait_for_codec_header() and load_next_chunk() and returns items -# through its inbound_, +# pairing_session() (window_shown and code_emitted). The pairing forms table queues every +# text-carrying note the same way (note_pairing_started(), note_pairing_succeeded(), +# note_pairing_failed() and note_display_pairing_code()) with the protocol task stopped +# (protocol_task_->stop()), since no public call produces each abort reason and code format on +# demand. The server information table hands the main loop two servers between drains as +# ConnectionManager::refresh_published_state() does (note_server_information()), since no public +# call changes the primary twice inside one main-loop tick. A second test also queues the metadata +# role's teardown (its cleanup()) beside the server going to none, so its clear callback reads +# server_information() as the drain delivers it, since no public call stages both in one tick. +# The stream-events table drives the +# player's handle_stream_start() and handle_stream_end() as the task would, and stages one +# interleaving by waiting on the sync task's item list and the player's sync-idle topic bit, and its +# clear row calls handle_stream_clear() the same way. The next-stream header table plays the sync +# task itself on the test thread, because no public call holds the task between its ACTIVE load +# and its next IDLE wait: it calls SyncTask's wait_for_codec_header() and load_next_chunk() and +# returns items through its inbound_, # resets the flags with event_flags_.clear_all() as start() would, sets and reads SyncContext's # active_ordinal and next_header, and reads PlayerRole::Impl::stream_ordinal. The metadata slot # table plays the protocol task's half of the metadata role (handle_server_state(), and for a diff --git a/tests/README.md b/tests/README.md index b046d492..e50a93f8 100644 --- a/tests/README.md +++ b/tests/README.md @@ -120,8 +120,8 @@ Each `test_*.cpp` file covers one unit of cross-platform logic: the command queue's refusals and the lifecycle requests it never refuses, the protocol task's next deadline, the teardown reorder guarantee for every role with main-loop state, stream starts acknowledged by number, the high-performance grant, the time filter slot, the - inbound ring's size, and the metadata role's scheduled-state slot, whose three text blocks - carry every state to the listener in order. + inbound ring's size, the pairing callbacks in each listener form, and the metadata role's + scheduled-state slot, whose three text blocks carry every state to the listener in order. - `test_client_teardown.cpp`: destroying a running client joins every threaded role. - `test_role_deactivation.cpp`: a later `server/activate` that removes a role: output stopped, buffers and state dropped, the roles it keeps left alone, and a removed role added back. diff --git a/tests/lifecycle_test_fixtures.h b/tests/lifecycle_test_fixtures.h index 03dc99ee..98557921 100644 --- a/tests/lifecycle_test_fixtures.h +++ b/tests/lifecycle_test_fixtures.h @@ -56,6 +56,7 @@ extern "C" { } #include +#include #include #include @@ -150,6 +151,28 @@ inline void pump_for(SendspinClient& client, int duration_ms) { } } +/// The server_id server_information() reports, nullopt for none, after one loop(): the protocol +/// task hands the main loop a changed server before it moves the connected flag a test waited +/// on, so that loop() takes it. Expects get_server_information() to carry the same id and name, +/// so it is for a test whose connections have settled. +inline std::optional refreshed_server_id(SendspinClient& client) { + client.loop(); + const std::optional info = client.server_information(); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + const std::optional deprecated = client.get_server_information(); +#pragma GCC diagnostic pop + EXPECT_EQ(info.has_value(), deprecated.has_value()); + if (!info.has_value()) { + return std::nullopt; + } + if (deprecated.has_value()) { + EXPECT_EQ(info->server_id, deprecated->server_id); + EXPECT_EQ(info->name, deprecated->name); + } + return std::string(info->server_id); +} + /// Waits for pred() without pumping the client: for observations on a fake peer after the client /// has been stopped or destroyed, when loop() is not the thing that would satisfy the predicate. /// Unbounded, for the same reason as pump_until. diff --git a/tests/test_allocations.cpp b/tests/test_allocations.cpp index d92791a3..5d5ab574 100644 --- a/tests/test_allocations.cpp +++ b/tests/test_allocations.cpp @@ -586,7 +586,8 @@ constexpr size_t UNTAKEN_STREAM_START_ALLOCATIONS = 0; // One paired connection with a player, a controller, metadata, two artwork channels, a visualizer // and two application-specific roles, one with a support object, so client/hello lists the // application roles and the player, artwork and visualizer objects of client/state are built: from -// the fake server's connect through admission and the first client/state, then one client/state +// the fake server's connect through admission, the first client/state and the main loop's copy of +// the server's information, then one client/state // publish, then one full burst of client/time and server/time exchanges, then the first // controller server/state, then the first group/update messages, then the first metadata // server/state messages, then an artwork and visualizer stream/start and a stream/end and a @@ -650,6 +651,8 @@ TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { [&] { return client.is_connected() && server->client_state_count() > 0; }); expect_library_allocations(close_window(), "accept to admission and the first client/state", ADMISSION_ALLOCATIONS); + EXPECT_TRUE(client.server_information().has_value()) + << "the window ended before the main loop took the admitted server's information"; expect_role_objects(*server, 0); expect_application_roles(*server); @@ -903,14 +906,35 @@ class TestOwnedPersistence : public InMemoryPersistenceProvider { } }; -/// Counts on_pairing_succeeded() on the main loop (the test thread). +/// Counts on_paired() on the main loop (the test thread), and overrides both forms of every +/// pairing callback that carries text, so a window counts the views and the deprecated strings. class PairingCountingListener : public SendspinClientListener { public: - void on_pairing_succeeded(const std::string& /*server_id*/) override { + void on_pairing_attempt(std::string_view /*server_id*/) override {} + void on_paired(std::string_view /*server_id*/) override { ++this->succeeded; } + void on_pairing_aborted(std::string_view /*server_id*/, + SendspinPairAbortReason /*reason*/) override {} + void on_pairing_code(std::string_view /*code*/, + SendspinPairingCodeFormat /*format*/) override {} +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + void on_pairing_started(const std::string& /*server_id*/) override {} + void on_pairing_succeeded(const std::string& /*server_id*/) override { + ++this->deprecated_succeeded; + } + void on_pairing_failed(const std::string& /*server_id*/, + SendspinPairAbortReason /*reason*/) override {} + void on_display_pairing_code(const std::string& code, + SendspinPairingCodeFormat /*format*/) override { + this->deprecated_code_size = code.size(); + } +#pragma GCC diagnostic pop int succeeded{0}; + int deprecated_succeeded{0}; + size_t deprecated_code_size{0}; }; constexpr uint16_t OUTBOUND_TEST_PORT = 19305; @@ -1030,14 +1054,12 @@ constexpr uint16_t PAIRING_TEST_PORT = 19302; constexpr size_t PLAYBACK_HANDOFF_ALLOCATIONS = 0; constexpr size_t UNPAIR_ALLOCATIONS = 0; -// The std::string server_id on_pairing_started() and on_pairing_succeeded() each take on the main -// loop: a public listener signature's cost. -constexpr size_t PAIRING_ALLOCATIONS = 2; +constexpr size_t PAIRING_ALLOCATIONS = 0; // Two paired servers, the incoming one's record the least recently used: from the incoming -// server's connect, through the playback activate that displaces the incumbent and the main -// loop's flush of the record order it moved, then a server/unpair of the incoming server's record -// through the flush that empties its slot. +// server's connect, through the playback activate that displaces the incumbent, the main loop's +// copy of the incoming server's information and its flush of the record order it moved, then a +// server/unpair of the incoming server's record through the flush that empties its slot. TEST(Allocations, RecordFlushesAfterAPlaybackHandoffAndAnUnpair) { PairedPeer incoming = make_paired_peer(); PairedPeer incumbent = make_paired_peer(); @@ -1063,6 +1085,7 @@ TEST(Allocations, RecordFlushesAfterAPlaybackHandoffAndAnUnpair) { }); const int order_writes = persistence.writes(persistence_keys::RECORD_ORDER); + const std::string incoming_id = incoming.server_identity.peer_id(); std::unique_ptr incoming_server; open_window(); { @@ -1075,6 +1098,9 @@ TEST(Allocations, RecordFlushesAfterAPlaybackHandoffAndAnUnpair) { }); expect_library_allocations(close_window(), "playback handoff and its record flush", PLAYBACK_HANDOFF_ALLOCATIONS); + const std::optional server = client.server_information(); + EXPECT_EQ(server.has_value() ? server->server_id : "", incoming_id) + << "the window ended before the main loop took the incoming server's information"; EXPECT_EQ(stored_record_order(persistence), (std::vector{1, 0})) << "the handoff must make the incoming server's record the most recent"; @@ -1098,7 +1124,7 @@ TEST(Allocations, RecordFlushesAfterAPlaybackHandoffAndAnUnpair) { // A pairing_psk pairing (pairing.md "Pairing PSK Flow") on a connection made on the configured // Pairing PSK, from the pairing activate through the main loop's flush of the new record and the -// on_pairing_succeeded() that follows it. +// on_paired() that follows it, in both forms. TEST(Allocations, PairingThroughItsRecordFlush) { std::array pairing_psk{}; platform_random_bytes(pairing_psk.data(), pairing_psk.size()); @@ -1139,6 +1165,7 @@ TEST(Allocations, PairingThroughItsRecordFlush) { pump_while_not(client, [&] { return listener.succeeded > 0; }); expect_library_allocations(close_window(), "pairing through its record flush", PAIRING_ALLOCATIONS); + EXPECT_EQ(listener.deprecated_succeeded, 1) << "the window missed the deprecated form"; EXPECT_EQ(persisted_psk_ids(persistence), std::vector{server.learned_psk_id().value_or("")}) << "the flush must store the record the pairing minted"; @@ -1151,8 +1178,7 @@ TEST(Allocations, PairingThroughItsRecordFlush) { /// Main loop (the test thread) only. class CodePairingListener : public PairingCountingListener { public: - void on_display_pairing_code(const std::string& code, - SendspinPairingCodeFormat /*format*/) override { + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat /*format*/) override { TestOwned test_owned; this->code = code; } @@ -1225,11 +1251,10 @@ std::vector pake_sid(const std::array& handshake_hash, constexpr uint16_t CODE_PAIRING_DIGITS_TEST_PORT = 19303; constexpr uint16_t CODE_PAIRING_QR_TEST_PORT = 19304; -// Only the listener's std::string arguments on the main loop: the server_id on_pairing_started() -// and on_pairing_succeeded() each take, and for a qr_code the 43-character token -// on_display_pairing_code() takes, past both standard libraries' inline capacity. Six digits fit it. -constexpr size_t CODE_PAIRING_DIGITS_ALLOCATIONS = 2; -constexpr size_t CODE_PAIRING_QR_ALLOCATIONS = 3; +// The listener overrides both forms, so the deprecated forms' strings are counted: the server_id +// is past both standard libraries' inline capacity, and so is a qr_code's 43-character token. +constexpr size_t CODE_PAIRING_DIGITS_ALLOCATIONS = 0; +constexpr size_t CODE_PAIRING_QR_ALLOCATIONS = 0; struct CodePairingRow { const char* format; @@ -1239,7 +1264,7 @@ struct CodePairingRow { /// Runs one Dynamic Pairing Code Flow round (pairing.md "Dynamic Pairing Code Flow") in `format` /// against a server the test plays with the library's CPace, from the pairing activate through -/// the main loop's flush of the new record and the on_pairing_succeeded() that follows it. +/// the main loop's flush of the new record and the on_paired() that follows it. void run_code_pairing(const CodePairingRow& row) { TestOwnedPersistence persistence; CodePairingListener listener; // Outlives the client @@ -1335,6 +1360,9 @@ void run_code_pairing(const CodePairingRow& row) { } pump_while_not(client, [&] { return listener.succeeded > 0 || server.closed(); }); expect_library_allocations(close_window(), row.format, row.expected); + EXPECT_EQ(listener.deprecated_succeeded, 1) << "the window missed the deprecated form"; + EXPECT_EQ(listener.deprecated_code_size, listener.code.size()) + << "the window missed the deprecated form of the code"; // The server can open the PSK the client wrapped (pairing.md "Wrapping"), and it is the one // the flush stored. diff --git a/tests/test_client_lifecycle.cpp b/tests/test_client_lifecycle.cpp index e44f041e..cf9d6b99 100644 --- a/tests/test_client_lifecycle.cpp +++ b/tests/test_client_lifecycle.cpp @@ -72,6 +72,7 @@ #include #include #include +#include #include using namespace sendspin; // NOLINT(google-build-using-namespace): test-local convenience @@ -258,9 +259,7 @@ TEST(ClientLifecycle, RestartYieldsALiveClient) { auto server = connect_paired_server(bundle.peer, RESTART_TEST_PORT); pump_until(client, [&] { return client.is_connected(); }); - auto info = client.get_server_information(); - ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->server_id, bundle.peer.server_identity.peer_id()); + EXPECT_EQ(refreshed_server_id(client), bundle.peer.server_identity.peer_id()); // Some group state for stop() to reset. ASSERT_TRUE(server->send_app_json(group_update_playing_json())); @@ -270,12 +269,13 @@ TEST(ClientLifecycle, RestartYieldsALiveClient) { EXPECT_FALSE(client.is_started()); EXPECT_FALSE(client.is_connected()); - EXPECT_FALSE(client.get_server_information().has_value()); + EXPECT_FALSE(client.server_information().has_value()); EXPECT_FALSE(client.group_state().playback_state.has_value()); EXPECT_FALSE(client.group_state().group_id.has_value()); EXPECT_FALSE(client.group_state().group_name.has_value()); #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" + EXPECT_FALSE(client.get_server_information().has_value()); const GroupUpdateObject& deprecated_group = client.get_group_state(); EXPECT_FALSE(deprecated_group.playback_state.has_value()); EXPECT_FALSE(deprecated_group.group_id.has_value()); @@ -3087,8 +3087,7 @@ TEST(MetadataSlot, ThreeBlocksCarryEveryStateToTheListenerInOrder) { /// Counts the pairing prompts and their dismissals. Main loop only. class PairingPromptLog : public SendspinClientListener { public: - void on_display_pairing_code(const std::string& /*code*/, - SendspinPairingCodeFormat /*format*/) override { + void on_pairing_code(std::string_view /*code*/, SendspinPairingCodeFormat /*format*/) override { ++this->displays; } void on_clear_pairing_code() override { @@ -3178,7 +3177,7 @@ TEST(ClientLifecycle, AFullTeardownKeepsTheDismissalsOfShownPrompts) { } /// Calls stop(), and then start() when `restart` is set, from inside the first -/// on_display_pairing_code(). The test thread plays the protocol task, so the listener runs the +/// on_pairing_code(). The test thread plays the protocol task, so the listener runs the /// final tick stop() would join first: admission closes, then the shutdown pass takes the /// admitted connection's prompt flags. class StopFromPairingPromptListener : public PairingPromptLog { @@ -3186,9 +3185,8 @@ class StopFromPairingPromptListener : public PairingPromptLog { StopFromPairingPromptListener(SendspinClient*& client, bool reenter, bool restart) : client_(client), reenter_(reenter), restart_(restart) {} - void on_display_pairing_code(const std::string& code, - SendspinPairingCodeFormat format) override { - PairingPromptLog::on_display_pairing_code(code, format); + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat format) override { + PairingPromptLog::on_pairing_code(code, format); if (this->reenter_ && !this->reentered_) { this->reentered_ = true; this->client_->connection_manager_->close_admission(); @@ -3279,6 +3277,231 @@ TEST(ClientLifecycle, StopFromInsideAPairingCallbackDismissesThePromptOnce) { } } +/// Logs each pairing callback with text, in the form "
". Overrides +/// neither form itself: ViewPairingForms and WithDeprecatedPairingForms add them. Main loop only. +class PairingFormsLog : public SendspinClientListener { +public: + std::vector calls; + /// When set, the first view-form callback calls its stop(). + SendspinClient* stop_from_view_form{nullptr}; + +protected: + void log_view(const std::string& what, std::string_view text) { + this->calls.push_back("view " + what + " " + std::string(text)); + if (SendspinClient* client = std::exchange(this->stop_from_view_form, nullptr)) { + client->stop(); + } + } + void log_deprecated(const std::string& what, const std::string& text) { + this->calls.push_back("deprecated " + what + " " + text); + } +}; + +/// Overrides the view form of each pairing callback with text. +class ViewPairingForms : public PairingFormsLog { +public: + void on_pairing_attempt(std::string_view server_id) override { + this->log_view("attempt", server_id); + } + void on_paired(std::string_view server_id) override { + this->log_view("paired", server_id); + } + void on_pairing_aborted(std::string_view server_id, SendspinPairAbortReason reason) override { + this->log_view("aborted " + std::to_string(static_cast(reason)), server_id); + } + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat format) override { + this->log_view("code " + std::to_string(static_cast(format)), code); + } +}; + +/// Adds the deprecated form of each pairing callback with text to `Base`. +template +class WithDeprecatedPairingForms : public Base { +public: +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + void on_pairing_started(const std::string& server_id) override { + this->log_deprecated("attempt", server_id); + } + void on_pairing_succeeded(const std::string& server_id) override { + this->log_deprecated("paired", server_id); + } + void on_pairing_failed(const std::string& server_id, SendspinPairAbortReason reason) override { + this->log_deprecated("aborted " + std::to_string(static_cast(reason)), server_id); + } + void on_display_pairing_code(const std::string& code, + SendspinPairingCodeFormat format) override { + this->log_deprecated("code " + std::to_string(static_cast(format)), code); + } +#pragma GCC diagnostic pop +}; + +using DeprecatedPairingForms = WithDeprecatedPairingForms; +using BothPairingForms = WithDeprecatedPairingForms; + +// Each pairing callback with text, every abort reason and a code in each format, reaches a +// listener in the forms it overrides: only the view form, only the deprecated std::string form, +// or both, the view form first with the same content. A stop() from the first view-form callback +// ends the delivery there, its deprecated form included. The test thread plays the protocol task, +// queueing the notes as its handlers do. +TEST(ClientLifecycle, PairingCallbacksReachBothForms) { + const std::string server_id = "c2VuZHNwaW4tcGFpcmluZy1jYWxsYmFjay1mb3Jtcw0"; + const std::string token = "SP:14DQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + ASSERT_EQ(server_id.size(), 43U); + ASSERT_EQ(token.size(), 43U); + const SendspinPairAbortReason reasons[] = { + SendspinPairAbortReason::ATTEMPT_TIMEOUT, + SendspinPairAbortReason::CONCURRENT_ATTEMPT, + SendspinPairAbortReason::METHOD_NOT_SUPPORTED, + SendspinPairAbortReason::PAIRING_CODE_MISMATCH, + SendspinPairAbortReason::USER_CANCELLED, + SendspinPairAbortReason::UNKNOWN, + }; + + // Each callback's description and content, in the order the notes are queued. + std::vector delivered = {"attempt " + server_id, "paired " + server_id}; + for (SendspinPairAbortReason reason : reasons) { + delivered.push_back("aborted " + std::to_string(static_cast(reason)) + " " + + server_id); + } + const auto code_call = [](SendspinPairingCodeFormat format, const std::string& code) { + return "code " + std::to_string(static_cast(format)) + " " + code; + }; + delivered.push_back(code_call(SendspinPairingCodeFormat::QR_CODE, token)); + delivered.push_back(code_call(SendspinPairingCodeFormat::DIGITS, "042735")); + const auto in_form = [&delivered](std::initializer_list forms) { + std::vector calls; + for (const std::string& call : delivered) { + for (const char* form : forms) { + calls.push_back(std::string(form) + " " + call); + } + } + return calls; + }; + + ViewPairingForms view_only; + DeprecatedPairingForms deprecated_only; + BothPairingForms both; + BothPairingForms stopping; + struct Row { + const char* name; + PairingFormsLog& log; + bool stop; + std::vector expected; + }; + const Row rows[] = { + {"only the view forms", view_only, false, in_form({"view"})}, + {"only the deprecated forms", deprecated_only, false, in_form({"deprecated"})}, + {"both, the view form first", both, false, in_form({"view", "deprecated"})}, + {"stop() from the first view form", stopping, true, {"view " + delivered.front()}}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.name); + TestNetworkProvider network; + SendspinClient client(make_config(0)); + client.set_network_provider(&network); + client.set_listener(&row.log); + ASSERT_TRUE(client.start()); + client.protocol_task_->stop(); + if (row.stop) { + row.log.stop_from_view_form = &client; + } + + client.note_pairing_started(server_id); + client.note_pairing_succeeded(server_id); + for (SendspinPairAbortReason reason : reasons) { + client.note_pairing_failed(server_id, reason); + } + client.note_display_pairing_code(token, SendspinPairingCodeFormat::QR_CODE); + client.note_display_pairing_code("042735", SendspinPairingCodeFormat::DIGITS); + client.loop(); + + EXPECT_EQ(row.log.calls, row.expected); + client.stop(); + } +} + +// server_information() reports the latest server the protocol task handed the main loop since the +// last loop(), "none" included: a primary that changes twice between two drains leaves only its +// second value. The test thread plays the protocol task's refresh_published_state(), since no +// public call changes the primary twice inside one main-loop tick. +TEST(ClientLifecycle, ServerInformationTakesTheLatestPrimary) { + PublishedServer first; + ASSERT_TRUE(first.server_id.assign("Zmlyc3Qtc2VydmVyLWluZm9ybWF0aW9uLXRlc3QtaWQ")); + ASSERT_TRUE(first.name.assign("First")); + PublishedServer second; + ASSERT_TRUE(second.server_id.assign("c2Vjb25kLXNlcnZlci1pbmZvcm1hdGlvbi10ZXN0LWk")); + ASSERT_TRUE(second.name.assign("Second")); + struct Row { + const char* name; + const PublishedServer* earlier; + const PublishedServer* later; + }; + const Row rows[] = { + {"Control: one server, then another", &first, &second}, + {"a server, then none", &first, nullptr}, + {"none, then a server", nullptr, &second}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.name); + TestNetworkProvider network; + SendspinClient client(make_config(0)); + client.set_network_provider(&network); + ASSERT_TRUE(client.start()); + client.protocol_task_->stop(); + + client.note_server_information(row.earlier); + client.note_server_information(row.later); + client.loop(); + + const std::optional info = client.server_information(); + ASSERT_EQ(info.has_value(), row.later != nullptr); + if (row.later != nullptr) { + EXPECT_EQ(info->server_id, row.later->server_id.view()); + EXPECT_EQ(info->name, row.later->name.view()); + } + client.stop(); + EXPECT_FALSE(client.server_information().has_value()); + } +} + +// The clear a teardown delivers reads the server the same tick left: a connection's loss publishes +// "none" beside the role's teardown, and the drain takes the server before its catch-up fires the +// clear. The test thread plays the protocol task's two halves of that tick. +TEST(ClientLifecycle, AClearReadsTheServerItsTeardownLeft) { + struct ClearLog : MetadataRoleListener { + void on_metadata_clear() override { + this->server_seen = this->client->server_information().has_value(); + ++this->clears; + } + SendspinClient* client{nullptr}; + bool server_seen{true}; + int clears{0}; + }; + PublishedServer server; + ASSERT_TRUE(server.server_id.assign("Y2xlYXItcmVhZHMtdGhlLXNlcnZlci10ZXN0LWlkLTQ")); + ASSERT_TRUE(server.name.assign("Lost")); + TestNetworkProvider network; + SendspinClient client(make_config(0)); + client.set_network_provider(&network); + ClearLog log; + log.client = &client; + client.add_metadata().set_listener(&log); + ASSERT_TRUE(client.start()); + client.protocol_task_->stop(); + + client.note_server_information(&server); + client.loop(); + ASSERT_TRUE(client.server_information().has_value()) << "Control: the server is current"; + + client.note_server_information(nullptr); + client.metadata_->impl_->cleanup(); + client.loop(); + ASSERT_EQ(log.clears, 1); + EXPECT_FALSE(log.server_seen) << "the clear read the server its connection's loss replaced"; + client.stop(); +} + // ============================================================================ // The high-performance grant // ============================================================================ diff --git a/tests/test_connection_lifecycle.cpp b/tests/test_connection_lifecycle.cpp index 75135040..11824def 100644 --- a/tests/test_connection_lifecycle.cpp +++ b/tests/test_connection_lifecycle.cpp @@ -294,9 +294,9 @@ TEST(ConnectionLifecycle, JunkProbeDoesNotBlockRealServer) { std::string(NOISE_SUITE_CHACHAPOLY), server_identity, bundle.peer.record.psk_id, bundle.peer.psk); pump_until(client, [&] { return client.is_connected(); }); - auto info = client.get_server_information(); + EXPECT_EQ(refreshed_server_id(client), server_identity.peer_id()); + const std::optional info = client.server_information(); ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->server_id, server_identity.peer_id()); EXPECT_EQ(info->name, FAKE_SERVER_HELLO_NAME); // The probes never complete a WebSocket handshake, so the transport layer closes them without @@ -347,9 +347,7 @@ TEST(ConnectionLifecycle, SlowOutboundSurvivesUpgradeTier) { // tier must ride that out and still establish. Completion is the whole verdict, so the wait // carries no bound of its own: a cut outbound clock never establishes and hangs here. pump_until(client, [&] { return client.is_connected(); }); - auto info = client.get_server_information(); - ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->server_id, server_identity.peer_id()); + EXPECT_EQ(refreshed_server_id(client), server_identity.peer_id()); client.disconnect(SendspinGoodbyeReason::SHUTDOWN); pump_for(client, 100); @@ -395,9 +393,7 @@ TEST(ConnectionLifecycle, InFlightOutboundDoesNotBlockInboundAdmission) { std::string(NOISE_SUITE_CHACHAPOLY), server_identity, bundle.peer.record.psk_id, bundle.peer.psk); pump_until(client, [&] { return client.is_connected(); }); - auto info = client.get_server_information(); - ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->server_id, server_identity.peer_id()); + EXPECT_EQ(refreshed_server_id(client), server_identity.peer_id()); EXPECT_FALSE(real_server.closed()) << "the real server must be admitted, not rejected while an outbound attempt is in flight"; @@ -433,7 +429,7 @@ TEST(ConnectionLifecycle, TwoServerRaceResolvedByPreference) { identity_a, peer_a.record.psk_id, peer_a.psk, rank_zero_peer_options()); pump_until(client, [&] { - auto info = client.get_server_information(); + auto info = client.server_information(); return info.has_value() && info->server_id == identity_a.peer_id(); }); @@ -444,9 +440,10 @@ TEST(ConnectionLifecycle, TwoServerRaceResolvedByPreference) { identity_b, peer_b.record.psk_id, peer_b.psk, rank_zero_peer_options()); pump_until(client, [&] { - auto info = client.get_server_information(); + auto info = client.server_information(); return info.has_value() && info->server_id == identity_b.peer_id(); }); + EXPECT_EQ(refreshed_server_id(client), identity_b.peer_id()); // The displaced incumbent is released with a goodbye, not left dangling. pump_until(client, [&] { return server_a.closed(); }); @@ -671,7 +668,7 @@ TEST(ConnectionLifecycle, SilentEstablishedPeerIsDropped) { pump_until(client, [&] { return silent.closed(); }); EXPECT_EQ(silent.goodbye_reason().value_or(""), "restart") << "a peer dropped for liveness must be told to restart"; - EXPECT_FALSE(client.get_server_information().has_value()) + EXPECT_FALSE(refreshed_server_id(client).has_value()) << "a dropped peer must not be left as the current connection"; } @@ -779,9 +776,13 @@ TEST(ConnectionLifecycle, TwoPeersAtOnceSettleOnThePreferredOne) { EXPECT_TRUE(reason == "another_server" || reason == "concurrent_attempt") << "the other server must be told why it was released, not '" << reason << "'"; // Server information names the winner as soon as it is installed; the connected flag rises - // only at the end of that tick, so both are waited on. + // only at the end of that tick, so both are waited on. Nothing pumps loop() here, so the + // getter that reads the protocol task's published state is the one that shows it. wait_until([&] { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" auto info = client.get_server_information(); +#pragma GCC diagnostic pop return client.is_connected() && info.has_value() && info->server_id == peer_b.server_identity.peer_id(); }); diff --git a/tests/test_encrypted_lifecycle.cpp b/tests/test_encrypted_lifecycle.cpp index 413a587f..9ccfc993 100644 --- a/tests/test_encrypted_lifecycle.cpp +++ b/tests/test_encrypted_lifecycle.cpp @@ -234,24 +234,24 @@ class PairingCapturePersistenceProvider : public SendspinPersistenceProvider { // (see EventState's dispatch block), so no locking is needed here. class RecordingClientListener : public SendspinClientListener { public: - void on_pairing_started(const std::string& server_id) override { - this->pairing_started_server_id_ = server_id; + void on_pairing_attempt(std::string_view server_id) override { + this->pairing_started_server_id_ = std::string(server_id); this->pairing_started_seq_ = this->next_seq_++; } - void on_pairing_succeeded(const std::string& server_id) override { - this->pairing_succeeded_server_id_ = server_id; + void on_paired(std::string_view server_id) override { + this->pairing_succeeded_server_id_ = std::string(server_id); this->pairing_succeeded_seq_ = this->next_seq_++; - if (this->on_pairing_succeeded_hook) { - this->on_pairing_succeeded_hook(); + if (this->on_paired_hook) { + this->on_paired_hook(); } } - /// Runs inside on_pairing_succeeded(), for a test that reads state as of the callback. - std::function on_pairing_succeeded_hook; + /// Runs inside on_paired(), for a test that reads state as of the callback. + std::function on_paired_hook; - void on_pairing_failed(const std::string& server_id, SendspinPairAbortReason reason) override { - this->pairing_failed_server_id_ = server_id; + void on_pairing_aborted(std::string_view server_id, SendspinPairAbortReason reason) override { + this->pairing_failed_server_id_ = std::string(server_id); this->pairing_failed_reason_ = reason; } @@ -337,9 +337,7 @@ TEST(EncryptedLifecycle, InBandRehandshakeResumesOperational) { // Initial handshake + hello + activate must bring the connection operational. pump_until(client, [&] { return client.is_connected(); }); - auto info = client.get_server_information(); - ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->server_id, server_identity.peer_id()); + EXPECT_EQ(refreshed_server_id(client), server_identity.peer_id()); EXPECT_EQ(server.client_hello_count(), 1); // Trigger the in-band re-handshake on the ADMITTED connection (same PSK: this test isolates @@ -360,9 +358,7 @@ TEST(EncryptedLifecycle, InBandRehandshakeResumesOperational) { EXPECT_EQ(server.client_hello_count(), 1) << "client/hello must not be re-sent"; EXPECT_EQ(server.activate_count(), 2) << "the server's first message under the new keys"; // server_id is unchanged (same server, new session keys). - auto info2 = client.get_server_information(); - ASSERT_TRUE(info2.has_value()); - EXPECT_EQ(info2->server_id, server_identity.peer_id()); + EXPECT_EQ(refreshed_server_id(client), server_identity.peer_id()); EXPECT_FALSE(server.closed()); client.disconnect(SendspinGoodbyeReason::SHUTDOWN); @@ -581,7 +577,7 @@ TEST(EncryptedLifecycle, PairingPskFlowPersistsAndPairsTheSession) { RecordingClientListener listener; std::optional committed_at_success; - listener.on_pairing_succeeded_hook = [&] { + listener.on_paired_hook = [&] { committed_at_success = persistence.committed_record(); }; SendspinClient client(config); @@ -620,10 +616,10 @@ TEST(EncryptedLifecycle, PairingPskFlowPersistsAndPairsTheSession) { EXPECT_EQ(pair_init->pairing_index, 1U); EXPECT_FALSE(pair_init->has_commit_b); - // handle_enter_pairing must fire on_pairing_started for the Pairing-PSK flow too, so the + // handle_enter_pairing must fire on_pairing_attempt for the Pairing-PSK flow too, so the // started/succeeded/failed callback trio stays method-agnostic. ASSERT_TRUE(listener.pairing_started_server_id().has_value()) - << "on_pairing_started was never fired for the pairing-token (Pairing-PSK) flow"; + << "on_pairing_attempt was never fired for the pairing-token (Pairing-PSK) flow"; EXPECT_EQ(listener.pairing_started_server_id().value(), server_identity.peer_id()); // The server's ack commits the record to RAM synchronously on the protocol task (see @@ -640,7 +636,7 @@ TEST(EncryptedLifecycle, PairingPskFlowPersistsAndPairsTheSession) { // An application that acts on the callback (a reboot, say) must find the record durable. ASSERT_TRUE(committed_at_success.has_value()) - << "on_pairing_succeeded fired before the record was committed to the provider"; + << "on_paired fired before the record was committed to the provider"; EXPECT_EQ(committed_at_success->psk_id, server.learned_psk_id().value()); // The application must see the exchange begin before it sees it end. The ordering is @@ -650,7 +646,7 @@ TEST(EncryptedLifecycle, PairingPskFlowPersistsAndPairsTheSession) { ASSERT_TRUE(listener.pairing_started_seq().has_value()); ASSERT_TRUE(listener.pairing_succeeded_seq().has_value()); EXPECT_LT(listener.pairing_started_seq().value(), listener.pairing_succeeded_seq().value()) - << "on_pairing_succeeded was delivered before on_pairing_started"; + << "on_paired was delivered before on_pairing_attempt"; // Rekey onto the newly paired PSK, exactly like the real server does immediately after // acking pair-finalize (see connection.h's note_pairing_finalize_ack()/provisional-timeout @@ -666,9 +662,7 @@ TEST(EncryptedLifecycle, PairingPskFlowPersistsAndPairsTheSession) { // it, so only is_paired() tells the new session apart, and a session that never becomes paired // hangs here for the suite watchdog to name. pump_until(client, [&] { return client.is_paired() && client.is_connected(); }); - auto info = client.get_server_information(); - ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->server_id, server_identity.peer_id()); + EXPECT_EQ(refreshed_server_id(client), server_identity.peer_id()); EXPECT_FALSE(server.closed()); client.disconnect(SendspinGoodbyeReason::SHUTDOWN); @@ -779,7 +773,7 @@ TEST(EncryptedLifecycle, ReactivatePairingOnAlreadyAdmittedConnectionSendsPairFi // this shape happened to be sent): the started/succeeded pairing callbacks must fire, exactly // like a fresh pairing-PSK flow. ASSERT_TRUE(listener.pairing_started_server_id().has_value()) - << "on_pairing_started was never fired for the re-pairing activate"; + << "on_pairing_attempt was never fired for the re-pairing activate"; EXPECT_EQ(listener.pairing_started_server_id().value(), server_identity.peer_id()); pump_until(client, [&] { return listener.pairing_succeeded_server_id().has_value(); }); @@ -4102,8 +4096,12 @@ TEST(EncryptedLifecycle, ASlowRecordWriteDoesNotStallTheProtocolTask) { peer.record.psk_id, peer.psk); persistence.wait_until_entered(); - // The provider stays parked until release() below. + // The provider stays parked until release() below. Read off the main loop, which only the + // any-thread getter supports. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" const auto info = client.get_server_information(); +#pragma GCC diagnostic pop ASSERT_TRUE(info.has_value()); EXPECT_EQ(info->server_id, peer.server_identity.peer_id()); client.disconnect(SendspinGoodbyeReason::USER_REQUEST); diff --git a/tests/test_pairing_state_machine.cpp b/tests/test_pairing_state_machine.cpp index 088c80bb..2547bf73 100644 --- a/tests/test_pairing_state_machine.cpp +++ b/tests/test_pairing_state_machine.cpp @@ -69,6 +69,7 @@ #include #include #include +#include #include #include #include @@ -165,18 +166,18 @@ struct PairingEvent { class RecordingListener : public SendspinClientListener { public: - void on_pairing_started(const std::string& server_id) override { - this->events_.push_back({PairingEventKind::STARTED, server_id, {}, {}, {}}); + void on_pairing_attempt(std::string_view server_id) override { + this->events_.push_back({PairingEventKind::STARTED, std::string(server_id), {}, {}, {}}); } - void on_pairing_succeeded(const std::string& server_id) override { - this->events_.push_back({PairingEventKind::SUCCEEDED, server_id, {}, {}, {}}); + void on_paired(std::string_view server_id) override { + this->events_.push_back({PairingEventKind::SUCCEEDED, std::string(server_id), {}, {}, {}}); } - void on_pairing_failed(const std::string& server_id, SendspinPairAbortReason reason) override { - this->events_.push_back({PairingEventKind::FAILED, server_id, reason, {}, {}}); + void on_pairing_aborted(std::string_view server_id, SendspinPairAbortReason reason) override { + this->events_.push_back({PairingEventKind::FAILED, std::string(server_id), reason, {}, {}}); } - void on_display_pairing_code(const std::string& code, - SendspinPairingCodeFormat format) override { - this->events_.push_back({PairingEventKind::DISPLAY_CODE, {}, {}, code, format}); + void on_pairing_code(std::string_view code, SendspinPairingCodeFormat format) override { + this->events_.push_back( + {PairingEventKind::DISPLAY_CODE, {}, {}, std::string(code), format}); } void on_clear_pairing_code() override { this->events_.push_back({PairingEventKind::CLEAR_CODE, {}, {}, {}, {}}); diff --git a/tests/test_record_store.cpp b/tests/test_record_store.cpp index fbb47c68..7561b6c5 100644 --- a/tests/test_record_store.cpp +++ b/tests/test_record_store.cpp @@ -31,6 +31,7 @@ #include "crypto/constants.h" #include "crypto/keys.h" +#include "crypto/pairing_token.h" #include "fake_persistence.h" #include "log_capture.h" #include "file_persistence_provider.h" @@ -1887,8 +1888,24 @@ TEST(SendspinClientStart, RejectsMalformedApplicationRoles) { } } +namespace { + +/// The characters of a token a getter returned, nullopt for none. +std::optional token_text( + const std::optional>& token) { + if (!token.has_value()) { + return std::nullopt; + } + return std::string(token->data(), token->size()); +} + +} // namespace + // start() hands SendspinClientConfig::pairing_psk to the store, so the pairing token the operator -// is shown carries the configured key rather than the one the provider holds. +// is shown carries the configured key rather than the one the provider holds. Each getter returns +// the token the library's builder makes from the client's public key and the PSK the getter +// names, and the deprecated std::string getters the same characters; before start() every one is +// nullopt. TEST(SendspinClientIdentity, PairingTokenCarriesTheConfiguredPairingPsk) { const SendspinPairingPsk stored = make_pairing_psk(); const std::array configured = make_random_psk(); @@ -1905,11 +1922,27 @@ TEST(SendspinClientIdentity, PairingTokenCarriesTheConfiguredPairingPsk) { } SendspinClient client(std::move(config)); client.set_persistence_provider(&provider); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + EXPECT_FALSE(client.client_pairing_token().has_value()); + EXPECT_FALSE(client.pairing_token_for(configured).has_value()); + EXPECT_FALSE(client.pairing_token().has_value()); + EXPECT_FALSE(client.format_pairing_token(configured).has_value()); ASSERT_TRUE(client.start()); - const auto expected = client.format_pairing_token(configure ? configured : stored.psk); - ASSERT_TRUE(expected.has_value()); - EXPECT_EQ(client.pairing_token(), expected); + const std::optional> client_key = + public_key_from_peer_id(client.client_id()); + ASSERT_TRUE(client_key.has_value()); + const std::string stored_token(format_pairing_token(*client_key, stored.psk).view()); + const std::string configured_token(format_pairing_token(*client_key, configured).view()); + const std::string& in_use = configure ? configured_token : stored_token; + EXPECT_EQ(token_text(client.client_pairing_token()), in_use); + EXPECT_EQ(client.pairing_token(), in_use); + EXPECT_EQ(token_text(client.pairing_token_for(stored.psk)), stored_token); + EXPECT_EQ(client.format_pairing_token(stored.psk), stored_token); + EXPECT_EQ(token_text(client.pairing_token_for(configured)), configured_token); + EXPECT_EQ(client.format_pairing_token(configured), configured_token); +#pragma GCC diagnostic pop client.stop(); } }