Skip to content

The built-in WebSocket transport is optional - #42

Merged
genmon merged 3 commits into
mainfrom
genmon/no-ws-when-not-default
Sep 4, 2026
Merged

genmon merged 3 commits into
mainfrom
genmon/no-ws-when-not-default

Conversation

@genmon

@genmon genmon commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Two fixes a consumer needs before it can stop holding a WebSocket it does not use.

The built-in WebSocket transport is optional

Courier auto-registers "ws" only when Config::host is set and
defaultTransport is unset or "ws", and it logs the skip otherwise. Sandbox's
constructor did not mirror that: it cached
courier().transport<WebSocketTransport>("ws") unconditionally. So a Sandbox
pointed at another default lane — defaultTransport = "mqtt", for a device
consolidating everything onto one MQTT connection — tripped
Client::transport<T>'s assert, and under NDEBUG bound a null reference that
crashed at the first ws() call instead.

_ws now stays null when Courier did not register one, hasWs() reports it, and
both internal ws() callers are gated on it: the WS-path default in
onCourierTransportsWillConnect, and the mic-stream fallback in
updateMicStream. ws() keeps its assert, so a caller that genuinely needs WS
still fails loudly rather than late.

The message filter runs before the deprecation notice

The legacy un-channelled path logged [deprecated] un-channelled ... and then
offered the frame to onMessageFilter. But the filter is the documented
interposition point, so a host that consumes a frame there never took delivery of
a deprecated message.

It matters for a platform wrapper whose default transport routes its own traffic.
Courier's client hook fires alongside a per-transport hook rather than instead
of it, so a wrapper that routes (say) MQTT by topic sees every inbound frame a
second time on the client lane and consumes it in the filter. Logging each of
those as deprecated was both noise and a lie — and at MQTT message rates, a line
per message.

Verification

Consumed by inanimate-tech/hawthorn-firmware on its voice-over-MQTT branch and
run on hardware (M5Stack StopWatch, ESP32-S3): boots, registers, connects MQTT as
its only persistent transport, and streams a voice session with no WebSocket
present. 481 native tests pass in that repo, including its Sandbox and
message-dispatch suites.

🤖 Generated with Claude Code

genmon and others added 3 commits September 4, 2026 11:20
Courier auto-registers "ws" only when Config::host is set and
defaultTransport is unset or "ws". Sandbox's constructor asked for it
unconditionally, so a Sandbox whose default lane is another transport —
defaultTransport = "mqtt", for a device consolidating everything onto one
MQTT connection — tripped Client::transport<T>'s assert, and under NDEBUG
bound a null reference that crashed at the first ws() call.

_ws now stays null when Courier did not register one, hasWs() reports it,
and both internal ws() callers are gated on it: the WS-path default in
onCourierTransportsWillConnect and the mic-stream fallback in
updateMicStream. ws() keeps its assert, so a caller that needs WS still
fails loudly rather than late.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The legacy un-channelled path logged "[deprecated] un-channelled ..." and
then offered the frame to onMessageFilter. But the filter is the documented
interposition point, so a host that consumes a frame there never took
delivery of a deprecated message.

It matters for a platform wrapper whose default transport routes its own
traffic. Courier's client hook fires ALONGSIDE a per-transport hook rather
than instead of it, so a wrapper that routes, say, MQTT by topic sees every
inbound frame a second time on the client lane and consumes it in the
filter. Logging each of those as deprecated was both noise and a lie.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Condense the changelog entries to one line each and drop the -dev suffix
from library.json / idf_component.yml.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@genmon
genmon merged commit cf29120 into main Sep 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant