Typed protobuf wire for the MQTT messages - #26
Open
gonzalocasas wants to merge 5 commits into
Open
gonzalocasas wants to merge 5 commits into
gonzalocasas wants to merge 5 commits into
Conversation
COMPAS XR owns its MQTT message contract, so it owns the .proto for it. compas_xr.proto defines the coordination Header, an explicit JointTrajectory, and the six trajectory request/approval messages. The trajectory replaces the untyped "dict of joint names and joint values" that travelled as JSON. Modelling it explicitly means both the Python and C# sides agree on the shape without either depending on compas_fab. The robot base frame travels as compas_pb.data.FrameData rather than a raw dict. Wires compas_pb's invoke tasks in, following the same layout antikythera uses: generate-proto-classes for local development, create-proto-bundle and create-class-assets for release artifacts. Generated *_pb2 files are gitignored; the .proto is the source of truth. Requires compas_pb >=1.2, which is where create_proto_bundle landed, and which is also the version the C# runtime targets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BREAKING CHANGE: COMPAS XR messages now travel as protobuf rather than COMPAS JSON. A publisher on the new wire cannot be read by a subscriber still on JSON, so every participant in a project has to move together. Adds conversions.py, which maps each message class to its generated protobuf class. compas_pb finds it through the compas_pb.plugins entry point and imports it for its side effects, so nothing has to import it explicitly. compas_eve has no global codec hook, so the Grasshopper components pass ProtobufMessageCodec() explicitly, which keeps the wire visible at the call site. Verified over a live broker and against the C# runtime: all six message types round-trip in both directions with byte-identical encodings, including the absent-optionals case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generated *_pb2 modules are not committed, so every job that imports compas_xr.mqtt has to build them first. Enable run_prebuild on the build and publish actions, which runs the new pre-build task. Adds an assets job that publishes the .proto bundle and the generated bindings for each supported language as release assets, so the C# runtime can pin a released schema version instead of scraping this repository at some ref. This mirrors what antikythera does for its own schemas. Also refreshes uv.lock, which still described the pre-restructure dependency set: it listed neither compas-model nor compas-pb and resolved compas-timber to 0.7.0, a version without compas_timber.model. A `uv sync` against it produced an environment that could not import the package under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jckenny59
reviewed
Sep 9, 2026
jckenny59
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for the major effort here in the proto conversions! 😄
After the small fixes from review comments, this is basically LGTM from my side. 👍
On the “second opinion” point about trajectory shape: I’m not sure I have a strong enough thought to give you a great second opinon. Maybe we should either (a) validate quickly with a couple of real planner outputs, or (b) discuss in the next meeting before we lock this in.
But I am good with whatever, I usually default to you in these scenarios so I am for sure open! 😄
Two review findings. trajectory_id was written by all six serializers but read back by none. Every message class derives it from element_id in its constructor, so a value that arrived on the wire was dropped and silently replaced. It round-tripped only because both sides happen to derive the same string. Deserializers now carry the wire value across, falling back to the derived one when the sender left the field empty. joint_names and joint_values are parallel arrays on the wire, and nothing checked that they were the same length. A mismatch reached the reader as a misaligned trajectory rather than an error. trajectory_to_pb now raises ValueError naming the offending waypoint index and both lengths. Serialization is unchanged, so the wire bytes are identical for any trajectory that was already valid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI failed with "cannot import name 'compas_xr_pb2'". Two causes, both mine. compas-actions.build was pinned at @v4, which has no run_prebuild input. The workflow passed it anyway and the action logged "Unexpected input(s) 'run_prebuild'" as a warning, not an error, so the prebuild silently never ran and the bindings were never generated. I had checked that input against the action's main branch rather than the pinned tag. Bumped to @v5, which supports it; v5 keeps every input v4 accepted. compas-actions.publish@v3 already supports it, so release.yml's publish step was fine. Generating them would still not have been enough: setuptools discovers packages with packages.find, which requires __init__.py, so compas_xr.proto was never included in the distribution at all. Antikythera has no __init__.py there either, but it builds with hatchling, which packages by directory — that detail does not carry across build backends. Added the marker; verified by building a wheel, installing it into a clean environment and round-tripping a message. Also adds the CHANGELOG entries the changelog check requires. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jckenny59
approved these changes
Sep 10, 2026
jckenny59
left a comment
Collaborator
There was a problem hiding this comment.
LGTM 👍 Thanks @gonzalocasas enjoy vacation. 😄
This branch has not been deployed
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
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.
COMPAS XR owns its MQTT message contract, so it owns the
.protofor it. Until now the messages travelled as COMPAS JSON with no schema, and the C# side had no counterpart at all — the two stacks could exchange geometry but not messages.The schema
compas_xr.protodefines the coordinationHeader, an explicitJointTrajectory, and the six trajectory request/approval messages.The trajectory replaces the untyped "dict of joint names and joint values". Modelling it explicitly means both languages agree on the shape without either depending on
compas_fab:The robot base frame travels as
compas_pb.data.FrameDatarather than a raw dict.Worth a second opinion: the trajectory shape was derived from the docstring, since
result.trajectoryarrives from whichever planner a project plugs in.trajectory_to_pbaccepts three shapes — acompas_fab-style object with.points(matched structurally, not imported), a list of mappings, and a column-major{joint: [values]}dict — and deserialization always returns the list-of-points form. If real planner output carries velocities or accelerations, the schema should grow before this ships.Registration
Conversions register through the
compas_pb.pluginsentry point, the way antikythera does.compas_pbdiscovers and imports the module for its side effects, so nothing imports it explicitly. Verified in a process that never mentions the module.compas_evehas no global codec hook, so the Grasshopper components passProtobufMessageCodec()explicitly, which keeps the wire visible at the call site rather than hidden behind a type.Distribution
Follows the compas_pb architecture guidance: the release workflow publishes the
.protobundle and per-language bindings as release assets, so downstream runtimes pin a released version instead of scraping this repository at some ref. Generated*_pb2files are gitignored;run_prebuildgenerates them in CI before anything importscompas_xr.mqtt.Also refreshes
uv.lock, which still described the pre-restructure dependency set — it listed neithercompas-modelnorcompas-pband resolvedcompas-timberto0.7.0, a version withoutcompas_timber.model. Auv syncagainst it produced an environment that could not import the package under test.Verification
Over a live broker, and against the C# runtime in compas-net#44: all six message types round-trip in both directions with byte-identical encodings, including the absent-optionals case. 25/25 tests pass, lint clean.
🤖 Generated with Claude Code