feat(signalk): subscribe WebSocket with sendMeta=all by default

[dirkwa]

Summary

The signalk-server only pushes metadata deltas (units, zones, displayName, displayUnits) over the WebSocket when the client opts in via ?sendMeta=all on the stream URL. Without it, SensESP-based firmware that wants per-path metadata has to do a separate REST GET /signalk/v1/api/<dot.path>/meta per path — extra round trips, fragile on flaky links, and no automatic refresh on metadata changes.

Subscribe with sendMeta=all by default so any SensESP-based firmware gets metadata in-stream alongside values.

How

• SKWSClient::connect_ws appends &sendMeta=all to the stream URL when send_meta_enabled_ is true (default).
• New getter/setter is_send_meta_enabled() / set_send_meta_enabled() for explicit opt-out.
• Persisted alongside the existing WS settings (to_json/from_json) and surfaced in the admin UI ConfigSchema as a checkbox.

Backwards compatibility

Existing SKValueListener consumers are unaffected. on_receive_updates already iterates only update["values"], so meta-only deltas (which carry an update["meta"] array but no update["values"]) are silently ignored by current consumers. Firmware that wants metadata can now parse it directly from the incoming JsonDocument without an additional HTTP round trip.

Cost

Negligible: one meta-delta per subscribed path at subscribe time, plus on metadata changes (typically never during a session).

Why default-on

Most modern SK clients (KIP, freeboard-sk, instrumentpanel) already do this implicitly via ?sendMeta=all. Without it, every SensESP-based UI/HMI that wants units or zone-based coloring has to reinvent a REST polling path. Default-on is the least-surprise choice.

Motivation

Surfaced while building a runtime-loadable HMI player on ESP32-P4 that wanted to color widgets per SignalK alarm-state zones. Realised the zones never showed up in the stream and the firmware had to do extra HTTP /meta polling. Fix at the SensESP layer benefits every consumer in the same situation.

[dirkwa]

Pushed an additional commit (2da59a9) that adds the consumer-side hook: SKWSClient::on_meta(callback).

Without this, the sendMeta=all subscription causes metadata to arrive at the client but on_receive_updates silently drops it (it only iterates updates[].values[]). The new on_meta callback fans out updates[].meta[] entries — one per path, one per metadata change — so consumers can react in-stream without an HTTP /meta poll.

Verified end-to-end: SensESP-based firmware bound to a path with SK-configured zones now sees them ~50ms after subscribe, no HTTP round trip needed. See https://github.com/dirkwa/sensesp-p4-cockpit/blob/jlp-json-layout-player/src/jlp/zone_registry.cpp for a real consumer.

[mairas]

The sendMeta=all subscription default is well-justified — agreed on that part, and the backwards-compat reasoning (existing SKValueListener consumers ignore meta-only deltas) is correct.

The on_meta callback, though, doesn't fit how the rest of SensESP handles inbound SK data, and as written it has a concurrency defect. I'd like to see it reworked into a ValueProducer before merge.

The callback fires on the WS task, under the lock, against a freed document

meta_callback_(...) is invoked inside on_receive_updates, which runs on the WS task and holds received_updates_semaphore (taken at signalk_ws_client.cpp:345, released at :366). Two problems:

1. Lock held across user code. The meta fan-out doesn't touch received_updates_ at all, yet it runs an arbitrary user callback inside that critical section, extending it and blocking the main task's process_received_updates from draining the value queue. Re-entrancy into the same non-recursive semaphore would deadlock.
2. Dangling reference. const JsonObject& meta points into the message document, which is freed once on_receive_updates returns. The doc comment tells consumers to marshal UI work onto the event loop via onDelay(0, ...) — but anything captured that way reads freed memory on the main task.

The convention: a path-routed ValueProducer, dispatched on the main task

The value side already solves all of this. on_receive_updates only copies deltas into received_updates_; process_received_updates (main task) path-matches registered SKListeners and calls parse_value, which emits through ValueProducer to observers — off a copied document, on the main task, with multiple consumers and pipeline composition for free (SKValueListener<T> : public SKListener, public ValueProducer<T>).

Meta should ride the same machinery: a meta listener that subscribes to a path and is a ValueProducer, fed by copying meta deltas into the queue in on_receive_updates and dispatching them in process_received_updates. That dissolves both defects above as a side effect (copied value, main-task emission, no lock over user code), and gives the same composability as values — multiple consumers, path routing, transform chaining — instead of a single global callback.

For the emitted type, there's already an SKMetadata class (signalk_metadata.h) used on the output side; reusing it for the inbound side would keep send/receive symmetric, though the exact shape is your call.

Happy to look at a revision. The subscription change could also land on its own if you'd prefer to split the producer work into a follow-up.

[dirkwa]

Thanks for the detailed review — the concurrency and lifetime concerns are spot-on, and I've reworked it accordingly. Pushed a follow-up commit (the branch is also rebased onto 3.4.0).

The callback is gone, replaced by a path-routed producer that mirrors the value side:

• New SKMetadataListener : public SKListener, public ValueProducer<SKMetaView>. Because it's an SKListener, subscribe_listeners() subscribes its path automatically and process_received_updates routes to it on the main task — same machinery as SKValueListener, no separate registry needed.
• No user code under the lock on the WS task. on_receive_updates now only copies meta entries into owned documents and queues them; the fan-out happens on the main task.
• No dangling reference. Meta rides the same queue as values; on the main task the owned doc is moved into a shared_ptr<const JsonDocument> (zero extra deep copy) that backs the emitted value, so it safely outlives any onDelay(0) consumer.
• Queue entries are tagged out of band (ReceivedUpdate{bool is_meta; JsonDocument doc}) rather than by JSON shape — a value can legitimately be an object too — and routed via a non-RTTI wants_meta() on SKListener (RTTI is commonly off on ESP32).

On the emitted type: I went with a dedicated inbound SKMetaView rather than reusing SKMetadata. SKMetadata is an output-only serializer (add_entry, no zones, no parsing); making it serve both directions produces a hybrid where add_entry ignores half the fields. SKMetaView is its inbound counterpart — typed units/displayName/description/displayUnits/zones for ergonomics, plus the full received object retained (owned, refcounted) so nothing is lost. Happy to revisit the naming/shape.

One thing I want to flag explicitly: for meta I kept emit()/notify() running under the locks on the main task, symmetric with the existing value path (process_received_updates), rather than deferring the emit outside the lock. Your "no user code under the received_updates lock" note is fully satisfied for the WS task (the defective path is removed), but if you meant the stricter literal reading — no notify under the lock on either task — say so and I'll collect (listener, doc) pairs and emit after releasing, for meta only. I left it symmetric to avoid a meta-vs-value asymmetry you didn't ask for.

Per-kind queue budgets (SENSESP_MAX_RECEIVED_{VALUE,META}_UPDATES, default 20 each) keep a metadata burst from evicting pending values. Output-side zone symmetry (SKMetadata::add_entry emitting zones) I've left as a potential follow-up. Compiles clean on pioarduino_esp32.

[mairas]

The rework addresses both defects from the last review — verified against the branch:

• WS task no longer runs user code under the lock. on_receive_updates only copies values/meta into owned documents and enqueues; dispatch and emit happen on the main task.
• No dangling reference. The path is copied to an owned String before the move, and the owned doc is moved into a shared_ptr<const JsonDocument> that backs SKMetaView::raw, so it outlives any deferred consumer (the typed members are independent copies too).

The producer shape is what I was after, and the SKMetaView-vs-SKMetadata split is the right call — SKMetadata is output-only.

On your open question about emit under the lock: keep it symmetric, which is what you already have. parse_meta→emit runs on the main task under the same two locks the value path holds for parse_value→emit. The defect was specifically WS-task user code under the lock, and that's gone. Main-task emit-under-lock is pre-existing for every value listener; making meta defer-outside-lock would introduce a value/meta asymmetry (including possible reordering of a value and a meta on the same path) for no gain. If it's ever worth addressing it should be a uniform value+put+meta change, not meta-only. So no change needed here.

I pushed one commit (76143e0) with two small follow-ups so this doesn't need another round-trip:

1. Made parse_meta a virtual on SKListener (default no-op, mirroring parse_value) and dispatch through the base pointer. This drops the static_cast<SKMetadataListener*> and the WS client's include of signalk_metadata_listener.h — the client no longer depends on the concrete subclass, and the "anything overriding wants_meta() must be an SKMetadataListener" contract goes away.
2. Skip copying meta deltas onto the receive queue unless a registered listener consumes them. With sendMeta=all on by default the server pushes meta to every client; without a consumer those entries were copied into owned documents only to be dropped. The check reads the listener list under SKListener's semaphore before taking received_updates_semaphore_, preserving the global lock order.

Compiles clean on pioarduino_esp32 (built a template project with an SKMetadataListener consumer to exercise the new header, the override, and the dispatch).

One thing worth confirming before this is fully wrapped: the earlier ~50ms end-to-end check was on the callback version, and the WS-task → queue → main-task → emit path is new. A quick run against a live SK server confirming an SKMetaView actually reaches a consumer would close the loop. Design-wise I'm happy with it — merging on green CI.