Tags · timtrautmann/sendspin-cpp · GitHub
Skip to content

Tags: timtrautmann/sendspin-cpp

Tags

v0.9.5

Toggle v0.9.5's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Add an opt-in plaintext message trace for conformance tooling (Sendsp…

…in#205)

Many sendspin-cpp client cases in the [conformance
matrix](https://sendspin.github.io/conformance/implementations/sendspin-cpp.html)
fail because the adapter cannot see the messages the library exchanges:
received audio hashes, raw chunk frames, and the protocol-baseline
trace. This adds an opt-in plaintext trace, mirroring
sendspin-server-cpp's `ServerConfig::trace`.

## Change

- `SENDSPIN_ENABLE_MESSAGE_TRACE`: a host-only CMake option, off by
default, with no Kconfig entry. With it off, the build is unchanged, ESP
included.
- `SendspinClientConfig::message_trace` receives a
`SendspinTracedMessage` for every message on the protocol task. Each one
carries:
  - connection id, direction and stamp
- the plaintext: inbound after decryption and reassembly, outbound
before encryption (each fragment on its own), plus the cleartext
handshake
- `audio_bytes`: for a player audio chunk, the encoded audio it carries
- The pairing secrets a client sends (`long_term_psk`, `wrapped_psk`,
`wrapped_nonce_B`) read `"<redacted>"`.
- `MessageTracer` (`src/message_trace.h`) is owned per connection. The
trace points are gated inline in `connection.cpp` and in
`NoiseTransport::encrypt_in_place()`.
- The gate is listed in CLAUDE.md, `conventions.md`, `internals.md` and
the house-patterns skill. `integration-guide.md` documents the option.
- CI's test and TSan jobs build with the trace on; the build and no-opus
jobs leave it off.

## Notes

- Once this lands, the conformance adapter
(`adapters/sendspin-cpp/client/main.cpp` in Sendspin/conformance) needs
updating to turn the option on and read the trace.

## Tests

- End-to-end against the fake encrypted server, read only through the
trace. It asserts:
- the exact opening sequence from `client/init` to `server/activate`,
with the transport JSON readable
  - an audio chunk's `audio_bytes`
  - a single connection id, and stamps on the client clock
- A redaction table whose secret rows are built by the real pairing
formatters, with control rows: a message without secrets and a binary
message reach the trace unchanged and uncopied.
- Not covered by any test: inbound reassembly and outbound
fragmentation.

v0.9.4

Toggle v0.9.4's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Add an RMS line-sense option to the source client example (Sendspin#201)

Addresses Sendspin#133.

The spec has the server alone start a source stream (roles/source/v1.md
"Default streaming behavior"); a client reports line sense as a hint.
The library already supports that through `SourceRoleConfig::line_sense`
and `SourceRole::set_signal()`, and detection stays in the application.

- `docs/integration-guide.md`: says detection is the application's job
and to call `set_signal()` on each change.
- `examples/source_client`: `-s DBFS` meters each buffer's RMS level and
reports present after 100 ms at or above the threshold, absent after 2 s
below. Capture runs from startup in that mode; audio reaches
`write_audio()` only while the stream is open. Without `-s` the example
is unchanged.

Testing: built and run on macOS; `-s -70` on the laptop mic reported
absent then present, and an invalid threshold is rejected. No unit tests
(example code).

v0.9.3

Toggle v0.9.3's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Call the network provider on the main loop only (Sendspin#189)

The protocol task polled `SendspinNetworkProvider::is_network_ready()`
(`maybe_start_ws_server()`) while the WebSocket server was down.
ESPHome's implementation reads plain main-loop fields
(`WiFiComponent::connected_`, `EthernetComponent::state_`), so the poll
was a data race, and any other platform would hit the same one. The
persistence provider is already main-loop only.

## Change

- The protocol task's poll was the only call off the main loop. Now
`start()` and every `loop()` call `poll_network_provider()`, which
stores the answer in `std::atomic<bool> network_ready_`. The protocol
task reads that atomic before starting the server.
- When the network comes up, `loop()` wakes the protocol task. The task
no longer has a network timer, and `NETWORK_POLL_INTERVAL_MS` is
removed.
- Latency improves: the server starts within one `loop()` period (about
16 ms in ESPHome) instead of up to 1 s.
- **Release note:** a consumer that never calls `loop()` never gets a
server if the network was down at `start()`.

## Docs

The `SendspinNetworkProvider` doc comment now says it is called on the
main loop only. Also updated: the `start()` and `loop()` docs, the
integration guide (provider section, threading summary), internals (tick
timers, the drain, atomics), conventions (only the main loop calls the
providers) and CLAUDE.md.

## Tests

- `TheNetworkProviderIsCalledOnTheMainLoopOnly`: the provider reports
the network down through `start()` and a run of `loop()` calls, then up.
The server starts, and every call happened on the test thread. It fails
on the old code, and removing the wake hangs it.
- The `NextDeadline` network-poll row becomes a `NO_DEADLINE` control.

698/698 under ASan and TSan.

v0.9.2

Toggle v0.9.2's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Deprecate the unuseful pairing_window_supported config flag (Sendspin…

…#187)

## Summary

`SendspinClientConfig::pairing_window_supported` only declared that the
application implemented the pairing-window gesture callbacks, and every
configuration with it off was a trap:

- A `static_pairing_code` set without the flag was silently not
advertised, although `start()` succeeded. Every static attempt is
gesture-gated (pairing.md "Pairing Window"), so there is no valid static
setup with the flag off.
- A `dynamic_pairing_code` attempt held back by the 20-round limit
(pairing.md "Rounds") is gated either way. The flag only suppressed the
`on_open_pairing_window` prompt, leaving the attempt at
`client/pair-pending` until the server gave up.

## Changes

- Deprecate the field: it stays `[[deprecated]]` and ignored, with
removal planned for v0.10.0. `static_pairing_code` is advertised
whenever it is set and the dynamic code is not offered.
- `SendspinClientConfig` is wrapped in a `-Wdeprecated-declarations`
suppression so its implicit copy and move members do not warn under
`-Werror`; a consumer's own assignment still warns (checked with Clang
and GCC 16).
- `on_open_pairing_window` always fires for a gesture-gated attempt. The
callbacks already default to no-ops, so a device with no gesture UI
behaves as it did with the flag off.
- Integration guide: mark the config row deprecated and drop the
flag-off paragraph; note that the gesture also clears the dynamic round
limit, so a device offering either code method should prompt on
`on_open_pairing_window` and call `confirm_pairing_window()`.
- Examples and tests updated;
`GatedAttemptWithoutWindowSupportSkipsPrompt` is deleted with the
behavior it covered.

## Deprecation

Not breaking, so this can ship as a point release. Code that sets
`pairing_window_supported` still compiles, with a deprecation warning;
delete the assignment before v0.10.0.

One behavior change for flag-off configurations: a `static_pairing_code`
set with the flag left `false` is now advertised (it was silently not
advertised before), and `on_open_pairing_window` fires for a gated
attempt (a no-op unless overridden). The release notes should name both.

## Testing

- Host suite under ASan and under TSan: 694/694 each.
- Host build with `-DENABLE_WERROR=ON` (library, examples, tests):
clean, 694/694.
- Host examples (basic_client, tui_client, source_client) build.
- ESP-IDF not compiled; the change touches no platform code.

v0.9.1

Toggle v0.9.1's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Allocate PSRAM thread stacks with 8-bit capable caps (Sendspin#184)

## Summary

`platform_configure_thread()` set `stack_alloc_caps = MALLOC_CAP_SPIRAM`
alone. `esp_pthread_set_cfg()` rejects caps without `MALLOC_CAP_8BIT`
(`components/pthread/pthread.c`, same check in IDF 5.4, 5.5 and 6.1) and
the return value was ignored, so every `psram_stack` option has been
silently dropped since the first commit.

The rejection drops the whole config, not just the caps: the thread
started with the calling thread's previous pthread config (or the
Kconfig defaults, 3072 bytes), so stack size, priority and name were
wrong too. Affected: SsSync, SsArt, SsVis, SsSrc and SsProto when their
PSRAM option is on. The httpd stack (`httpd_psram_stack`) was not
affected, since `xTaskCreatePinnedToCoreWithCaps` accepts
`MALLOC_CAP_SPIRAM` alone.

## Changes

- Request `MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT` for PSRAM stacks.
- Log an error under `sendspin.thread` if the config is ever rejected.

## Testing

- ESP-IDF 5.5 build (ESP32-S3, SPIRAM) compiles.
- Hardware: with `protocol_task_psram_stack` toggled, internal free heap
now changes as expected.

v0.9.0

Toggle v0.9.0's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Cache host CI compiles with ccache (Sendspin#183)

The host CI jobs (Build, Unit tests, Build and test without Opus,
ThreadSanitizer) now compile through ccache. After Sendspin#182 these
compile-bound jobs set the overall CI time; most of their compile time
is FetchContent dependencies that rarely change.

- Each job installs ccache; the host CMake build picks it up
automatically (Sendspin#181).
- `actions/cache/restore` pulls the job's newest cache;
`actions/cache/save` runs only on pushes to main, so PR runs reuse
main's objects without filling the repository cache quota. Caches are
capped at 300M per job.
- `ccache --show-stats` after each build shows the hit rate in the log.

This PR's own run starts cold since nothing is saved from a PR; the
speedup shows on PRs after the first main push.

v0.8.0

Toggle v0.8.0's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Remove the deprecated positional send_command() overload (Sendspin#125)

`ControllerRole::send_command(SendspinControllerCommand,
std::optional<uint8_t>, std::optional<bool>)` was deprecated in favor of
the `ClientCommandControllerObject` form, which can also carry seek
parameters. The deprecation notice scheduled removal for v0.8.0, which
is the next release, so this removes it.

- Drops the declaration from `include/sendspin/controller_role.h` and
the definition from `src/controller_role.cpp`
- Drops the migration note from `docs/integration-guide.md`

No in-tree callers remain (examples and tests already use the struct
form). 149/149 host tests pass.

`SendspinClient::start_server()` is the only other deprecated symbol;
its notice targets v0.9.0, so it is left in place.

v0.7.2

Toggle v0.7.2's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Silence a GCC dangling-reference warning in controller parsing (Sends…

…pin#106)

Bind the `supported_commands` array to a named local before iterating it
in `process_server_state_controller()`, so GCC stops emitting
`-Wdangling-reference` there.

## Why

The warning fires on every ESP-IDF build of the component (including via
ESPHome), and is present in released v0.7.1 and on current `main`:

```text
src/protocol.cpp:554:73: warning: possibly dangling reference to a temporary [-Wdangling-reference]
  554 |              controller_object["supported_commands"].as<JsonArrayConst>()) {
      |                                                                         ^
src/protocol.cpp:554:52: note: 'MemberProxy<JsonObject, RamString>' temporary created here
```

The range initializer chained `.as<JsonArrayConst>()` off the
`MemberProxy` temporary returned by `operator[]`. Only the final
temporary in such a chain gets lifetime extension in C++20, so the proxy
is destroyed at the end of the full-expression while the loop body is
still running.

**It was benign.** `JsonArrayConst` stores only a `const ArrayData*` and
a `const ResourceManager*`, both pointing into the `JsonDocument`'s
memory pool, and `JsonArrayConstIterator` holds an `ArrayData::iterator`
plus that same resource pointer. Neither the array nor its iterators
retain anything that points into the dead proxy, so the destroyed
temporary was never read. GCC's heuristic cannot see through the proxy
type, though, so this fixes the code rather than suppressing the
diagnostic.

## How

```diff
-        for (JsonVariantConst command_var :
-             controller_object["supported_commands"].as<JsonArrayConst>()) {
+        JsonArrayConst commands_array =
+            controller_object["supported_commands"].as<JsonArrayConst>();
+        for (JsonVariantConst command_var : commands_array) {
```

This is also what the rest of the file already does. The five other
array loops (`active_roles`, artwork `channels`, visualizer `types`, and
the `roles` lists in `stream/end` and `stream/clear`) each name a local
first; the loop at line 264 ranges over a function parameter. This was
the only chained range initializer left, so the change makes the file
consistent rather than introducing a new idiom.

No behavior change.

## Testing

macOS builds with clang, which does not implement `-Wdangling-reference`
and so never reproduced this. Verified with Homebrew `g++-15` driven at
the host build's exact flags and include set:

- Warning reproduces verbatim before the change, and is gone after it.
- Swept every file in `src/` and `src/host/` with
  `-Wall -Wextra -Wdangling-reference -fsyntax-only`: no
  dangling-reference warnings remain anywhere.
- Host clang build rebuilds clean; `ctest` 123/123 pass.
- `clang-format --dry-run -Werror src/protocol.cpp` clean.

v0.7.1

Toggle v0.7.1's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Cut stack use on the JSON receive path (Sendspin#104)

The ESP httpd task runs with the ESP-IDF default 4 KB stack
(`HTTPD_DEFAULT_CONFIG`), and every inbound JSON message is parsed and
dispatched on it, underneath httpd's own call chain. Measured with
`-fstack-usage` on xtensa-esp32s3 g++ 14.2 at `-Os`, a server/state
message carrying metadata cost about 1250 bytes of library frames alone,
before httpd's own usage below it and before any logging on top. That is
a third of the whole stack spent in this library, and it is what the
reported httpd stack overflow (issue Sendspin#102) was eating into.

Almost all of it was avoidable duplication rather than real working set:

- server/state was parsed into one `ServerStateMessage` aggregate (288
bytes), so every section's storage stayed live in the caller's frame for
the whole parse while each section parser built a second copy of the
same fields in its own. A `ServerMetadataStateDelta` is 200 bytes and
was materialized twice at once, then a third time by the by-value
handle_server_state parameter.

- GCC inlined the section parsers into `process_server_state_message`
and did not overlap their locals, so metadata, color, and controller
storage all coexisted in a single 736 byte frame, the largest in the
library.

`server/state` is now parsed one section at a time. Each section is an
out-of-line function that fills a caller-owned struct in place, the
client scopes each section separately so the compiler can reuse the
slots, and a section is only parsed when its role is present. The three
`handle_server_state` overloads take an rvalue reference instead of a
by-value copy. `stream/start` gets the same in-place treatment for its
player, artwork, and visualizer sections.

Measured frames, before -> after:

    process_json_message              512 -> 320
    process_server_state_message      736 -> gone
      process_server_state_metadata          288
      process_server_state_controller        224
      process_server_state_color             192
    process_stream_start_message      496 -> 448

Deepest `server/state` path: 1248 -> 608 bytes (51% less). Deepest
`stream/start` path: 1008 -> 768.

No behavior change: same fields, same validation, same tri-state delta
semantics. tests/test_protocol.cpp moves to the per-section entry
points.

Closes Sendspin#102. It seems like this is a healthy enough margin without
needing to increase the default task stack. The encryption work will
require a small bump to the httpd task stack.

v0.7.0

Toggle v0.7.0's commit message

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Artwork role: back-pressure, timing offset, and lateness reporting (S…

…endspin#97)

Adds three opt-in artwork role features aimed at smooth cross-fades on
ESPHome displays, all backward-compatible (defaults preserve today's
decode-and-display-every-frame behavior) beyond a function signature
breaking change.

### Changes

- **Per-slot `frame_done` acknowledgment (opt-in back-pressure):** Set
`ImageSlotPreference::require_frame_done` and the role delivers at most
one un-acked "delivery" (a frame or a clear) at a time for that slot;
newer payloads are buffered latest-wins until the consumer calls
`ArtworkRole::frame_done(slot)`. A clear supersedes any un-acked frame.
Lets a consumer hold the gate open for the duration of a cross-fade
animation instead of having buffers reused mid-fade. No timeout by
design — the ack is the contract.

- **Per-slot `display_offset_ms`:** Shifts a slot's display deadline
(positive fires early, mirroring `PlayerRoleConfig::fixed_delay_us`) so
a cross-fade can straddle the track boundary.

- **Display lateness reporting:** `on_image_display()` now takes a
`lateness_ms` argument reporting how far past the deadline it fired, so
a consumer can shorten a fade to still end on schedule, or snap
instantly when joining mid-track. `lateness_ms == 0` is reserved for the
no-connection case; a connected on-time display never reports 0.

New `tests/test_artwork_role.cpp` covers the gate state machine
(hold/supersede/clear/restart/reentrant-ack), the deadline/offset
arithmetic, and the lateness mapping.

### Breaking change

`on_image_display(uint8_t)` signature is now `on_image_display(uint8_t,
uint32_t lateness_ms)`.