Skip to content

feat(pj_scene_protocol): SDK module for marker / scene wire format#81

Merged
facontidavide merged 3 commits intomainfrom
feat/scene-protocol
May 5, 2026
Merged

feat(pj_scene_protocol): SDK module for marker / scene wire format#81
facontidavide merged 3 commits intomainfrom
feat/scene-protocol

Conversation

@facontidavide
Copy link
Copy Markdown
Contributor

Summary

Introduces pj_scene_protocol as a 4th sibling SDK module alongside pj_base, pj_datastore, pj_plugins. It carries the canonical schema and Foxglove ImageAnnotations Protobuf codec — writer and reader together — so a plugin author that produces or consumes markers links one target with pj_base-only deps.

Source content was moved from PJ4 (pj_media/pj_marker_protocol/ + pj_media/core/scene_decoder*). Folding the reader in eliminates the "matching reader still lives in pj_media_core for now" asymmetry the old CMake apologised for.

Renamed during the move: pj_marker_protocolpj_scene_protocol. The scope was always wider than 2D markers (image_annotation.h documents the 3D ScenePrimitive / Grid expansion path); the rename matches Foxglove's own "scene" terminology and is cheap before any external code depends on the directory / target / include name. Type identifiers (ImageAnnotation, SceneFrame, kSchemaImageAnnotations, etc.) are unchanged.

What's in the module

pj_scene_protocol/
├── CMakeLists.txt                    # mirrors pj_base
├── include/pj_scene_protocol/
│   ├── image_annotation.h            # value types: Point2, ColorRGBA, ImageAnnotation, SceneFrame, …
│   ├── image_annotation_codec.h      # serializeImageAnnotation() + kSchemaImageAnnotations
│   └── scene_decoder.h               # ISceneDecoder + makeSceneDecoder()
├── src/
│   ├── image_annotation_codec.cpp    # hand-rolled writer (no libprotobuf)
│   ├── scene_decoder.cpp             # factory dispatcher
│   └── scene_decoder_protobuf.cpp    # hand-rolled reader, full Points/Circle/Text decode
├── tests/
│   ├── image_annotation_codec_test.cpp   # 9 cases incl. golden-byte + round-trip
│   └── scene_decoder_test.cpp            # decoder coverage + error paths
└── docs/
    ├── ARCHITECTURE.md               # wire format spec, type catalog, encoding rules
    └── USER_GUIDE.md                 # producer + consumer recipes, pitfalls

Build / integration

  • plotjuggler_core/CMakeLists.txtadd_subdirectory(pj_scene_protocol).
  • run_clang_tidy.sh — new module added to the source-dirs list.
  • CLAUDE.md — module added to Modules list, dependency graph, Key Documentation table, and the Read all documentation glossary entry.

In-flight cleanup landed during the move

  • scene_decoder_protobuf.cpp top-of-file comment fixed — the stale "MVP: decodes PointsAnnotation fully... CircleAnnotation/TextAnnotation bodies skipped" claim was contradicted by the implementation (decodes all three; round-trip tests cover them).
  • All pj_marker_protocol/... and pj_media_core/scene_decoder.h includes rebased onto pj_scene_protocol/....

Commit layout

  1. style: clang-format v22 pass across pj_base, pj_datastore, pj_plugins — pure formatting, surfaced because pre-commit re-ran clang-format v22 across the tree while formatting the new module. Isolated from the feature commit so review stays focused.
  2. feat(pj_scene_protocol): SDK module for marker / scene wire format — the substantive change.

Phase 2 (separate, follow-up PR)

PJ4 deletes its local pj_media/pj_marker_protocol/ and the reader files in pj_media/core/, bumps the submodule pointer to this PR's merge commit, and rebases its includes onto pj_scene_protocol/.... Sequenced after this PR merges.

Test plan

  • ./build.sh clean to 100%
  • ./test.sh — submodule-local 44/44 pass (42 baseline + 2 new: image_annotation_codec_test 9 cases, scene_decoder_test)
  • ./run_clang_tidy.sh — local environment is missing clangd-22; needs to be run by reviewer or in CI before merge
  • Reviewer eye-pass on docs/ARCHITECTURE.md and docs/USER_GUIDE.md (newly written, no prior version to diff against)

🤖 Generated with Claude Code

Davide Faconti and others added 3 commits May 5, 2026 11:11
The pre-commit hook pins clang-format to v22.1.0 (.pre-commit-config.yaml).
Running it across the tree consolidates a handful of multi-line wraps into
single lines that fit the 120-col limit under the v22 ruleset. Pure
formatting; no behaviour or signature changes.

Surfaced as a side effect of formatting the new pj_scene_protocol module
in the same pre-commit pass; landed as a separate commit so the feature
commit stays focused.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduce pj_scene_protocol as a 4th sibling SDK module (pj_base-only deps),
alongside pj_base, pj_datastore, pj_plugins. Carries the canonical schema
plus the Foxglove ImageAnnotations Protobuf codec — *both writer and reader*
in the same module.

Source content moved from PJ4 (pj_media/pj_marker_protocol/ and
pj_media/core/scene_decoder*). Folding the reader in eliminates the
"matching reader still lives in pj_media_core for now" asymmetry the old
CMake had to apologise for. A plugin author that produces or consumes
markers now links one target.

Renamed during the move: pj_marker_protocol -> pj_scene_protocol. The
scope was always wider than 2D markers (image_annotation.h documents the
3D ScenePrimitive / Grid expansion path); the rename matches Foxglove's
own "scene" terminology and is cheap before any external code depends on
the directory / target / include name. Type identifiers
(ImageAnnotation, SceneFrame, kSchemaImageAnnotations, etc.) are
unchanged.

Module contents:
- include/pj_scene_protocol/{image_annotation,image_annotation_codec,scene_decoder}.h
- src/{image_annotation_codec,scene_decoder,scene_decoder_protobuf}.cpp
- tests/{image_annotation_codec,scene_decoder}_test.cpp
- docs/ARCHITECTURE.md (wire format spec, type catalog, encoding rules)
- docs/USER_GUIDE.md (producer + consumer recipes, common pitfalls)

In-flight cleanup:
- scene_decoder_protobuf.cpp top-of-file comment fixed (the stale "MVP:
  decodes PointsAnnotation fully... CircleAnnotation/TextAnnotation
  bodies skipped" claim was contradicted by the implementation, which
  has decoded all three since the round-trip tests landed).
- All `pj_marker_protocol/...` and `pj_media_core/scene_decoder.h`
  includes rebased onto `pj_scene_protocol/...`.

Build wiring:
- plotjuggler_core/CMakeLists.txt: add_subdirectory(pj_scene_protocol).
- run_clang_tidy.sh: add the new module to the source-dirs list.
- CLAUDE.md: module added to Modules list, dependency graph, Key
  Documentation table, and the "Read all documentation" glossary entry.

Phase 2 (PJ4 side) follows in a separate PR once this lands: PJ4 deletes
its local pj_media/pj_marker_protocol and the reader files in
pj_media/core, bumps the submodule pointer, and rebases its includes
onto pj_scene_protocol.

Build: ./build.sh clean to 100%. Tests: 44/44 pass (42 baseline + 2 new
module tests — image_annotation_codec_test, scene_decoder_test).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@facontidavide facontidavide merged commit 6c3f16c into main May 5, 2026
2 checks passed
@facontidavide facontidavide deleted the feat/scene-protocol branch May 5, 2026 09:26
facontidavide added a commit that referenced this pull request May 5, 2026
…ia (#82)

The schema-spec headers used to point at `pj_media/docs/datatypes_2D.md §8`
for the canonical type catalog, dating from when this module lived inside
PJ4's pj_media tree. After the relocation in #81, that PJ4 doc was
rewritten to point AT pj_scene_protocol/docs/ARCHITECTURE.md — making the
header cross-reference circular.

Both header comments now point at the sibling docs/ARCHITECTURE.md.
Drops the residual `pj_media`-specific phrasing from scene_decoder.h's
class comment.

Co-authored-by: Davide Faconti <dfaconti@aurynrobotics.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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