feat(foxglove,ros2): demand-driven per-topic subscriptions (pj.topic_subscription.v1)#190
Open
facontidavide wants to merge 6 commits into
Open
feat(foxglove,ros2): demand-driven per-topic subscriptions (pj.topic_subscription.v1)#190facontidavide wants to merge 6 commits into
facontidavide wants to merge 6 commits into
Conversation
…ons (pj.topic_subscription.v1) Adopt the new SDK per-topic-pause contract so the bridge exposes its full channel catalog cheaply while transmitting data only for topics the host is actually displaying: - Add kCapabilityPerTopicPause and implement pluginExtension() for "pj.topic_subscription.v1": set_active_topics (entered on the host GUI thread) writes a mutex-protected latest-wins slot (DesiredTopicsSlot) — declarative full set, self-coalescing, no command queue. - Advertise ALL supported channels to the host via notifyAvailableTopics() on start and after every server advertise/unadvertise delta, retaining full ChannelInfo (schema included) per channel; the dialog's saved selection becomes an optional advertise filter (empty = advertise everything). - Demand mode is decided once per start by the first notifyAvailableTopics() call: an old host lacking the tail slot returns an error and the plugin falls back to the legacy selection-driven subscribe path unchanged. - onPoll drains the slot and reconcileSubscriptions() — the single reconciliation point, also run after advertise deltas — diffs desired vs live subscriptions via the pure computeSubscriptionDiff(), batching subscribe/unsubscribe frames and binding parsers on the poll thread only. - Dialog OK gate becomes connected-only (selection no longer required). - Unit tests for the latest-wins slot (cross-thread), the subscription diff, and the advertise filter (foxglove_topic_subscription_test). The extern/plotjuggler_core submodule moves to e566d96 (the unreleased feat/topic-subscription-abi contract, additive tail-slot ABI on top of v0.14.0). SDK_VERSION intentionally stays 0.14.0: the recipe version at e566d96 is unchanged and no release tag exists yet, so the usual bump_core_version.py flow applies only once the SDK ships a tagged release. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng vocabulary Found by a live end-to-end run of the demand chain (real plugin .so against the mcap player, driven through pj.topic_subscription.v1): - The fresh-connect (saved-config, dialog-less) start raced ix::WebSocket's async start(): the socket reports Closed until its background thread begins the attempt, and the wait loop treated that first reading as terminal — start() failed against a reachable server. Closed is now terminal only after the attempt was observed Connecting. - foxglove_mcap_player.py advertised MCAP schema-encoding vocabulary verbatim; MCAP writers use "proto" where the ws-protocol says "protobuf", so clients classified the channel unsupported. Normalize on send. E2E sequence verified end to end: advertise reaches the host, zero messages while paused, set_active_topics subscribes exactly the desired topic (server log confirms), data flows, empty set stops the flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ic_subscription.v1) Adopt the same SDK per-topic-pause contract the Foxglove bridge adopted in 8dae26f, so the ROS 2 plugin advertises its full discovered-topic catalog cheaply while streaming CDR bytes only for topics the host is actually displaying: - Add kCapabilityPerTopicPause and implement pluginExtension() for "pj.topic_subscription.v1": set_active_topics (host GUI thread) writes a mutex-protected latest-wins slot (DesiredTopicsSlot — a ROS-local twin of the Foxglove bridge's, duplicated rather than shared since the two plugins don't link each other). - The streamer's own node now discovers the ROS graph too (throttled ~1s get_topic_names_and_types(), independent of the dialog's own discovery, which is torn down on accept/reject): advertises the full filtered set via notify_available_topics(), with each topic's schema synthesized via typesupport introspection (buildRos2Schema) and cached per type name so it is built once regardless of how many topics/advertise refreshes reuse it. Multi-type topics and types whose schema synthesis fails are skipped with a one-time warning each. - Demand mode is decided ONCE per start by the first notify_available_topics attempt: an old host lacking the tail slot falls back to today's selection-driven subscribe path, byte-for-byte (factored into a shared subscribeTopic() helper used by both onStart's legacy loop and the demand reconcile path). - onPoll reconciles: computeRos2SubscriptionDiff() (name-keyed — ROS 2 has no channel-id indirection like Foxglove's — pure and unit-tested) diffs the desired set against live subscriptions over the filtered discovered topics, subscribing/unsubscribing on the poll thread only. New subscriptions from the poll thread use qos_wait=0 (sample once, no blocking) since onPoll must never block; onStart's initial legacy subscribe keeps the original 2s QoS-discovery window. - Dialog OK gate becomes discovery-running-only: topic selection is no longer required. selected_topics_ keeps its saveConfig format but is now dual-role — the legacy subscribe list on old hosts, an OPTIONAL advertise filter in demand mode (empty = advertise everything). - Unit tests (ros2_topic_subscription_test, 19 cases) for the latest-wins slot, the subscription diff, the advertise filter, and the multi-type split — factored into a ROS-free header (ros2_topic_subscription.hpp) so the test target builds without PJ_BUILD_ROS2_DISTRO or a sourced ROS 2 environment. extern/plotjuggler_core stays at e566d96 (already the tip from the Foxglove adoption) — no further SDK change needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… staleness Three fixes from the milestone review (Claude + Codex): - The onStart discovery fallback (saved selection seeding an empty first scan) is now ADVERTISE-ONLY: subscribing off the seed used qos_wait=0 against a graph with no discoverable publisher, permanently locking in a default-QoS subscription that a BEST_EFFORT/TRANSIENT_LOCAL publisher never matches. Real subscriptions wait for the first genuine scan (~1s), which now reconciles unconditionally. - computeRos2SubscriptionDiff keys current subscriptions by (topic, type): a topic re-typed on the graph while subscribed lands in both diff lists, recreating the GenericSubscription under the new type instead of silently decoding the wrong CDR layout. Regression test added. - ensureBinding invalidates its per-topic parser-binding cache when the live subscription's type no longer matches the type the binding was created for (the superseded binding stays idle host-side; there is no unbind ABI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
SDK ABI slice is now a proper PR: PlotJuggler/plotjuggler_sdk#140 (branch |
SDK PR #140 merged and tagged v0.15.0 (doc fixes, hpp-to-cpp refactor, and the 0.14.0->0.15.0 version bump landed after this branch's earlier e566d96 pin). Used scripts/bump_core_version.py to move SDK_VERSION + the extern/plotjuggler_core submodule in lockstep (--check passes). Verified against the new pin: data_stream_foxglove_bridge builds clean, 5/5 tests pass (incl. foxglove_topic_subscription_test); data_stream_ros2's ROS-free target builds clean, 4/4 tests pass (incl. ros2_topic_subscription_test). The full aggregate ./build.sh still hits the known pre-existing protobuf/grpc Conan conflict, unrelated to this pin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
SDK #140 merged and tagged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Both streaming plugins adopt the new SDK per-topic-pause contract (companion host PR: PlotJuggler/PJ4#347): they advertise their full topic catalog to the host while transmitting data only for topics the host is actually displaying. On old hosts (no
notify_available_topicstail slot) both plugins fall back to today's selection-driven subscribe path unchanged.data_stream_foxglove_bridge
ChannelInforetained, schema included) on connect and after every server advertise/unadvertise delta; the dialog's saved selection becomes an optional advertise filter (empty = advertise everything; OK gate is now connected-only).pj.topic_subscription.v1:set_active_topics(host GUI thread) writes a mutex-protected latest-wins slot;onPolldrains it andreconcileSubscriptions()— the single reconciliation point, also run after advertise deltas and the post-reconnect re-advertise — diffs desired vs live subs via a pure, unit-testedcomputeSubscriptionDiff, batching subscribe/unsubscribe frames and binding parsers on the poll thread only. The filter bounds subscriptions too, not just advertising.ix::WebSocket's asyncstart()(initialClosedreading treated as terminal), andfoxglove_mcap_player.pyadvertised MCAP'sprotoschema-encoding vocabulary where the ws-protocol saysprotobuf.data_stream_ros2
get_topic_names_and_types()), advertising the full filtered set with schemas synthesized per type via typesupport introspection (cached; multi-type topics and unresolvable types skipped with one-time warnings). TheonStartselection seed is advertise-only — real subscriptions wait for the first genuine scan so QoS adaptation never locks in an unadaptable default.qos_wait=0(onPoll must not block); the legacy path keeps the original 2 s QoS-discovery window.Verification
e566d96(feat/topic-subscription-abi, pushed; = SDKorigin/main+ 1).SDK_VERSIONintentionally unchanged (no release tag yet; recipe version at e566d96 is still 0.14.0).-Werror-clean against a sourced distro..sowith a mock host vtable): foxglove against the MCAP player (advertise → 0 msgs paused → server-confirmed single subscribe frame → 90 msgs → flow stops on empty set) and ros2 through the production proxy against a live 20 Hz talker (same sequence, 60 msgs).Known-red, pre-existing (not this PR)
toolbox_quaterniondoes not compile against any post-fix(data_load_mcap): don't pin whole decompressed chunks on hot byte fetch #135 SDK (itscreate_data_processorcode targets the pre-fix(data_load_mcap): don't pin whole decompressed chunks on hot byte fetch #135 API from thesdk-te-combinedline this repo's main pins) — migration debt owed by a plugins-main SDK bump, unaffected by this change.protobuf 3.21 vs 6.33Conan conflict (grpc), independent of this PR.🤖 Generated with Claude Code