Skip to content

refactor(rtps)!: consolidate to a single rtps component + thorough docs (mermaid) - #712

Open
finger563 wants to merge 4 commits into
mainfrom
feat/rtps-consolidation-docs
Open

refactor(rtps)!: consolidate to a single rtps component + thorough docs (mermaid)#712
finger563 wants to merge 4 commits into
mainfrom
feat/rtps-consolidation-docs

Conversation

@finger563

Copy link
Copy Markdown
Contributor

What & why

REFACTOR_PLAN.md's stated end state is a single component named rtps using the embeddedRTPS engine. This PR executes that consolidation now that the RMI/AMI work has merged (#711):

  • Deletes the original from-scratch components/rtps (discovery + best-effort only, never ROS 2-interop; its README/RELIABLE_RTPS_PLAN.md overstated its status).
  • Renames the interop-proven rtps_embeddedrtps, taking over the published espp/rtps component-registry slot.
  • Adds thorough documentation with mermaid diagrams.

Rename + removal

  • git rm old components/rtps; git mv rtps_embedded → rtps; example rtps_embedded_example.cpp → rtps_example.cpp; pc tests rtps_embedded_* → rtps_* (golden .inc wire bytes unchanged — only comments).
  • Rewired every path/name reference: build.yml (dedup the example matrix entry), rtps_interop.yml (path triggers + harness cd), lib/espp.cmake (engine source paths), the interop harness (run.sh/run_interop.sh/Dockerfile), doc/Doxyfile (old rtps.hpp → the 5 facade headers), Kconfig, idf_component.yml. upload_components.yml already listed components/rtps, so the registry slot transfers cleanly.
  • The two historical design docs (REFACTOR_PLAN.md, RMI_AMI_DESIGN.md) intentionally keep their rtps_embedded transition narrative.

Documentation (mermaid)

  • Rewrote components/rtps/README.md (layered-architecture + messaging-topology diagrams) and doc/en/protocols/rtps.rst (architecture, discovery, reliable pub/sub HEARTBEAT/ACKNACK, RMI related_sample_identity correlation, AMI goal state machine) and wired the facade headers into the API reference.
  • All 7 mermaid diagrams validated with mmdc. Note: sphinx uses mermaid_output_format='raw' (client-side render), so neither CI nor the sphinx build catches mermaid syntax errors — they must be validated locally.

Robustness (surfaced by the consolidation gate)

The native-cancel demo/test used an unbounded goal + fixed-sleep cancel. Under CI timing, a cancel that raced the goal-accept round-trip was dropped, the n=100000 goal ran for ~50 min, and — because stop() now joins execute threads (from #711) — stop() blocked on it and wedged the gate. Fixed: bounded n + cancel triggered off the first feedback (deterministic: the goal is accepted and executing, so cancel_goal() has its handle).

⚠️ Breaking change

The from-scratch rtps component (espp::RtpsParticipant from rtps.hpp, the SPDP/SEDP discovery scaffold) is removed. espp/rtps now provides the embeddedRTPS-backed facade (pub/sub + services + actions). Consumers of the old API migrate to the new facade headers (rtps_participant.hpp, rtps_pubsub.hpp, rtps_service.hpp, rtps_action.hpp).

Gate

  • Golden byte-for-byte wire format unchanged.
  • Interop 27/27 from the new path (components/rtps/interop), incl. ROS 2 (Jazzy/FastDDS) both directions + python wheel demo.
  • Host lib builds; native-cancel loopback + python native_cancel pass; all mermaid diagrams render.

🤖 Generated with Claude Code

…docs

REFACTOR_PLAN.md's end state: a single component named `rtps` using the
embeddedRTPS engine. This deletes the original from-scratch `components/rtps`
(discovery + best-effort only, never ROS 2-interop, docs overstated its status)
and renames the interop-proven `rtps_embedded` -> `rtps`, taking over the
published `espp/rtps` component-registry slot.

Rename + removal:
- git rm old components/rtps; git mv rtps_embedded -> rtps; example
  rtps_embedded_example.cpp -> rtps_example.cpp and pc tests rtps_embedded_* ->
  rtps_* (golden .inc wire bytes unchanged - only comments).
- Rewired every path/name reference: build.yml (dedup example entry),
  rtps_interop.yml (paths + harness cd), lib/espp.cmake (engine source paths),
  interop harness (run.sh/run_interop.sh/Dockerfile), Doxyfile (old rtps.hpp ->
  the 5 facade headers), Kconfig, idf_component.yml. upload_components.yml already
  listed `components/rtps`. The two historical design docs keep their
  rtps_embedded narrative intentionally.

Documentation (mermaid):
- Rewrote components/rtps/README.md (architecture + messaging-topology diagrams)
  and doc/en/protocols/rtps.rst (architecture, discovery, reliable pub/sub,
  RMI correlation, AMI state diagrams) + wired the facade headers into the API
  reference. All 7 diagrams validated with mmdc. sphinx uses raw (client-side)
  mermaid, so neither CI nor sphinx catches syntax errors - validate locally.

Robustness (surfaced by the consolidation gate):
- The native-cancel demo/test used an unbounded goal + fixed-sleep cancel; under
  CI timing a lost cancel let the goal run for minutes and stop()'s execute-thread
  join blocked on it. Now bounded n + cancel triggered off the first feedback
  (deterministic - the goal is accepted and executing, so cancel_goal() has its
  handle).

BREAKING CHANGE: the from-scratch `rtps` component (espp::RtpsParticipant from
rtps.hpp, SPDP/SEDP discovery scaffold) is removed. `espp/rtps` now provides the
embeddedRTPS-backed facade (pub/sub + services + actions). Consumers of the old
API must migrate to the new facade headers (rtps_participant.hpp etc.).

Gate: golden byte-for-byte unchanged, interop 27/27 from components/rtps/interop,
host lib builds, native cancel loopback + python demo pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 15, 2026 00:48
@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

✅Static analysis result - no issues found! ✅

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the consolidation to a single rtps component by replacing the legacy from-scratch implementation with the embeddedRTPS-backed facade, updating host tests/interop harness paths, and refreshing docs (including mermaid diagrams).

Changes:

  • Consolidate/rename the embeddedRTPS-backed component surface into components/rtps (engine + facade + typed pub/sub, services, actions) and remove rtps_embedded.
  • Update CI/workflows, host-side golden/interop tests, and build system wiring to point at the new rtps paths.
  • Make cancellation demos/tests deterministic under CI load by bounding goals and canceling based on first feedback.

Reviewed changes

Copilot reviewed 45 out of 121 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
python/rtps_rpc_demo.py Makes native action cancel test deterministic (bounded goal, cancel on first feedback).
pc/tests/rtps_typed_pubsub.cpp Updates plan/doc path reference for typed pub/sub host test.
pc/tests/rtps_service_naming.cpp Updates design doc path reference for service naming unit test.
pc/tests/rtps_pubsub.cpp Updates component path references in pub/sub loopback test header comments.
pc/tests/rtps_native_action_loopback.cpp Bounds cancel test goal duration to avoid CI hangs.
pc/tests/rtps_interop_sub.cpp Renames usage text + plan/doc references to rtps.
pc/tests/rtps_interop_pub.cpp Renames usage text + plan/doc references to rtps.
pc/tests/rtps_golden.inc Renames golden include header comments and generator command.
pc/tests/rtps_golden.cpp Renames golden test driver and include to rtps_*.
pc/tests/rtps_facade_frag.cpp Updates plan/doc path reference for fragmentation test.
lib/python_bindings/rtps_bindings.cpp Updates component path references in bindings header comments.
lib/espp.cmake Updates host build include/source paths from rtps_embedded to rtps.
doc/en/protocols/rtps_rmi_ami.rst Updates docs to refer to rtps component paths.
doc/Doxyfile Switches Doxygen inputs from legacy rtps.hpp to the facade headers.
components/rtps/src/utils/Diagnostics.cpp Adds/maintains engine diagnostics counter definitions under new component.
components/rtps/src/entities/Writer.cpp Adds writer implementation under consolidated rtps engine sources.
components/rtps/src/entities/StatelessReader.cpp Adds stateless reader implementation under consolidated rtps engine sources.
components/rtps/Kconfig Renames menu and updates wording to match consolidated rtps component.
components/rtps/interop/run.sh Updates harness entrypoint paths to components/rtps/interop.
components/rtps/interop/ros2_fibonacci_server.py Adds ROS 2 action server helper for interop matrix.
components/rtps/interop/ros2_big_publisher.py Adds ROS 2 big-payload publisher helper for fragmentation interop leg.
components/rtps/interop/ros2_add_two_ints_server.py Adds ROS 2 service server helper for interop matrix.
components/rtps/interop/README.md Updates instructions/paths and references to renamed golden test.
components/rtps/interop/Dockerfile Adds ROS 2 Jazzy-based image for interop matrix + wheel build/testing.
components/rtps/include/rtps/utils/udpUtils.hpp Adds embeddedRTPS UDP port/address helpers under rtps.
components/rtps/include/rtps/utils/sysFunctions.hpp Adds embeddedRTPS time helpers under rtps.
components/rtps/include/rtps/utils/printutils.hpp Adds embeddedRTPS debug printing helpers under rtps.
components/rtps/include/rtps/utils/Log.hpp Adds embeddedRTPS verbosity macro header under rtps.
components/rtps/include/rtps/utils/hash.hpp Adds embeddedRTPS string hashing helper under rtps.
components/rtps/include/rtps/utils/Diagnostics.hpp Declares diagnostics counters surfaced by the engine.
components/rtps/include/rtps/utils/constants.hpp Adds constants header placeholder under rtps.
components/rtps/include/rtps/utils/CdrBuffer.hpp Updates golden-test reference in discovery CDR buffer docs.
components/rtps/include/rtps/storages/ThreadSafeCircularBuffer.tpp Adds circular buffer implementation (with optional dynamic growth).
components/rtps/include/rtps/storages/ThreadSafeCircularBuffer.hpp Adds circular buffer interface (with optional dynamic growth).
components/rtps/include/rtps/storages/StorageArray.hpp Introduces static-vs-dynamic storage policy wrapper for engine pools.
components/rtps/include/rtps/storages/SimpleHistoryCache.hpp Adds history cache with optional dynamic growth path.
components/rtps/include/rtps/storages/PayloadBuffer.hpp Adds payload buffer wrapper used by cache changes.
components/rtps/include/rtps/storages/MemoryPool.hpp Adds memory pool with optional dynamic growth path.
components/rtps/include/rtps/storages/CacheChange.hpp Adds cache-change type including inline-QoS correlation support.
components/rtps/include/rtps/rtps.hpp Adds top-level engine header entrypoint.
components/rtps/include/rtps/rpc/service_naming.hpp Adds ROS 2 service name/type mangling helpers.
components/rtps/include/rtps/rpc/sample_identity.hpp Adds related_sample_identity constants + serialization helpers.
components/rtps/include/rtps/rpc/action_naming.hpp Adds ROS 2 action name/type mangling helpers.
components/rtps/include/rtps/messages/MessageReceiver.hpp Adds message receiver interface for engine message parsing/dispatch.
components/rtps/include/rtps/entities/WriterProxy.hpp Adds writer-proxy type for reader state tracking.
components/rtps/include/rtps/entities/Writer.hpp Adds writer base class + related_sample_identity publish helper.
components/rtps/include/rtps/entities/StatelessWriter.hpp Adds stateless writer interface and fragmentation hook.
components/rtps/include/rtps/entities/StatelessReader.hpp Adds stateless reader interface.
components/rtps/include/rtps/entities/StatefulWriter.hpp Adds stateful writer interface and scheduler-driven heartbeat design.
components/rtps/include/rtps/entities/StatefulReader.hpp Adds stateful reader interface.
components/rtps/include/rtps/entities/ReaderProxy.hpp Adds reader-proxy type for writer state tracking.
components/rtps/include/rtps/entities/Reader.hpp Adds reader base class + inline-QoS correlation surface + reassembly slot.
components/rtps/include/rtps/entities/Participant.hpp Adds engine participant interface and endpoint management.
components/rtps/include/rtps/entities/Domain.hpp Adds engine domain interface + protocol scheduler task design.
components/rtps/include/rtps/discovery/TopicData.hpp Adds SEDP topic data parsing/serialization and matching helpers.
components/rtps/include/rtps/discovery/SPDPAgent.hpp Adds SPDP agent interface and scheduler-driven announce design.
components/rtps/include/rtps/discovery/SEDPAgent.hpp Adds SEDP agent interface and endpoint match/delete logic.
components/rtps/include/rtps/discovery/ParticipantProxyData.hpp Adds SPDP proxy data parsing and liveliness tracking helpers.
components/rtps/include/rtps/discovery/BuiltInEndpoints.hpp Adds builtin endpoints bundle used by discovery agents.
components/rtps/include/rtps/config.hpp Centralizes limits-profile selection and host default for dynamic storage.
components/rtps/include/rtps/config_host_large.hpp Adds large host limits profile header.
components/rtps/include/rtps/config_esp32.hpp Adds ESP32 embedded limits profile header.
components/rtps/include/rtps/config_desktop.hpp Adds default host limits profile header.
components/rtps/include/rtps/communication/PacketInfo.hpp Adds packet container used by transport/dispatch.
components/rtps/include/rtps/communication/EsppTransport.hpp Adds espp socket-reactor/thread-pool based transport adapter.
components/rtps/include/rtps/common/Locator.hpp Adds RTPS locator types + wire-format read helpers.
components/rtps/include/rtps_service.hpp Adds typed service client/server wrappers over byte-level participant API.
components/rtps/include/rtps_pubsub.hpp Adds typed pub/sub wrappers + small ROS 2 topic helper.
components/rtps/include/rtps_participant.hpp Updates facade docs/references to consolidated rtps engine path.
components/rtps/include/rtps_message.hpp Adds shared typed-layer concepts + serialize/deserialize helpers.
components/rtps/include/rtps_action.hpp Adds typed action client/server wrappers over byte-level participant API.
components/rtps/idf_component.yml Updates component metadata for consolidated rtps.
components/rtps/example/sdkconfig.defaults Updates example defaults (performance + Ethernet + partitions).
components/rtps/example/README.md Reorients example docs around Ethernet + typed APIs + ROS 2 interop.
components/rtps/example/pc/CMakeLists.txt Renames host example project target to match rtps.
components/rtps/example/main/Kconfig.projbuild Simplifies example Kconfig for publish period + optional self-test participant.
components/rtps/example/main/CMakeLists.txt Updates example dependencies from wifi to ethernet-kit + typed deps.
components/rtps/example/CMakeLists.txt Updates example component list for new dependencies and removes wifi.
components/rtps/CMakeLists.txt Registers consolidated engine sources and Kconfig-driven build defines.
components/rtps_embedded/README.md Removes old component README after consolidation.
components/rtps_embedded/idf_component.yml Removes old component manifest after consolidation.
components/rtps_embedded/example/sdkconfig.defaults Removes old example defaults after consolidation.
components/rtps_embedded/example/README.md Removes old example README after consolidation.
components/rtps_embedded/example/partitions.csv Removes old example partition table after consolidation.
components/rtps_embedded/example/main/Kconfig.projbuild Removes old example Kconfig after consolidation.
components/rtps_embedded/example/main/CMakeLists.txt Removes old example main component CMake after consolidation.
components/rtps_embedded/example/CMakeLists.txt Removes old example project CMake after consolidation.
components/rtps_embedded/CMakeLists.txt Removes old component build file after consolidation.
.github/workflows/rtps_interop.yml Updates interop workflow triggers and working directory to rtps.
.github/workflows/build.yml Removes rtps_embedded/example from the build matrix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread components/rtps/idf_component.yml
Comment thread components/rtps/example/README.md Outdated
Address PR #712 review:
- idf_component.yml: restore a documentation: link (dropped in the rename),
  pointing at the new docs page, matching socket/rtsp.
- example/README.md: the Kconfig menu is now titled 'RTPS' (not 'RTPS (rtps)').

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@finger563

Copy link
Copy Markdown
Contributor Author

Addressed both Copilot comments in d4d55c6:

  • idf_component.yml: restored a documentation: link (.../protocols/rtps.html), matching socket/rtsp — it was dropped when the manifest was renamed from rtps_embedded.
  • example/README.md: the menuconfig section is now titled RTPS (the Kconfig menu title), so the reference no longer says RTPS (rtps).

@finger563
finger563 requested a balanced review from Copilot August 15, 2026 01:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 45 out of 121 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

doc/en/protocols/rtps.rst:1

  • This table states RTPS_LIMITS_PROFILE defaults to embedded, but elsewhere in the repo the host build tooling indicates a host default (e.g., lib/espp.cmake sets RTPS_LIMITS_PROFILE to host when not defined). To avoid confusing users, please document the default as platform-dependent (ESP-IDF: embedded; host/non-ESP: host), or align the actual default behavior to match the docs.
    python/rtps_rpc_demo.py:1
  • The return value of nexecuting.wait(5.0) is ignored. If no feedback arrives (or the callback is not actually tied to feedback), the code will still call cancel_goal() without having established that the goal is executing/accepted, which can reintroduce the race/flakiness this change is trying to remove. Consider checking the boolean result of wait() and marking the demo/test as failed (or taking an alternate deterministic step) when feedback doesn’t arrive in time, rather than proceeding to cancel unconditionally.

Comment thread .github/workflows/rtps_interop.yml
Comment thread .github/workflows/rtps_interop.yml
finger563 and others added 2 commits August 14, 2026 22:26
The consolidation removed the old from-scratch rtps.hpp (discovery scaffold);
the esp32-p4-function-ev-board example was its only remaining consumer and broke
with 'rtps.hpp: No such file or directory'. Migrate to the new facade:
- rtps.hpp -> rtps_participant.hpp
- Config: node_name/participant_id/advertised_address/announce_period/
  on_participant_discovered/on_endpoint_discovered -> interface_address +
  on_publisher_matched (sets rtps_has_peers) + log_level
- add_writer .topic_name -> .topic
- gate publish on rtps_has_peers (matched) instead of discovered_participants()
- the new facade doesn't expose remote peer addresses, so drop the peer-ping
  branch of the connectivity self-test (keep the gateway ping)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Declare 'published' inside the can_publish block where it is used, resolving the
static-analysis finding introduced when the peer-state assignment was removed
during the RTPS facade migration.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@finger563
finger563 requested a balanced review from Copilot August 15, 2026 04:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 46 out of 122 changed files in this pull request and generated no new comments.

Suppressed comments (3)

python/rtps_rpc_demo.py:1

  • nexecuting.wait(5.0) ignores its return value, so on timeout the code still calls cancel_goal() even though the goal may not be accepted/executing (reintroducing the race this change is trying to eliminate). Consider branching on the wait result: only call cancel_goal() once nexecuting is set, otherwise record a deterministic failure (or retry/wait longer) and proceed to shutdown cleanly.
    doc/en/protocols/rtps.rst:1
  • The sequence ````ACKNACK\ s is likely to render incorrectly in RST; it reads like an accidental escape. Prefer writing “``ACKNACK``s” (no escape) or rephrasing (e.g., “the reader sends ``ACKNACK`` messages for missing samples”).
    doc/en/protocols/rtps.rst:1
  • This table entry is ambiguous/misleading given elsewhere the host build defaults to host (e.g., lib/espp.cmake sets RTPS_LIMITS_PROFILE to "host" when not defined). Consider clarifying the default by platform (e.g., “embedded default on ESP-IDF; host default for non-ESP builds”) to prevent users from assuming the wrong profile is active.

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.

2 participants