Repository navigation
Conversation
ModelToProto and ModelFromProto convert a model to and from commonpb.KinematicModel, with the configuration level pair underneath. DH parameters expand into links and joints, orientations keep the representation the file used, and the message carries the limits the model enforces rather than the ones its file declared. SimpleModel holds visual geometries and properties for the message, since the SVA schema has no place for them. GetKinematics fills model and keeps replaying the file for the deprecated bytes, writing an SVA file from the model only when there is no file to replay. Readers prefer model and fall back to the bytes. go.mod points go.viam.com/api at the api branch until viamrobotics/api#899 merges and tags. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Visual geometries are held as spatialmath.GeometryConfig and the component properties as a plain struct, so Go callers never touch the KinematicModel message. The converters in model_proto.go are the only place the proto appears. A geometry whose center is the identity no longer comes back with an orientation config after a round trip. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Static and translational frames hold a list of geometries. LinkConfig gains a geometries field for the ones beyond the first, while the first stays in geometry so a reader that predates the field still sees one. An unlabelled geometry is named after its link when it is the only one, as before, and <link>_collision_<i> otherwise, on the model and on the wire. A label written in a file is kept rather than replaced with the link id, so the wire and the Go model agree. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
viambot
added
the
safe to test
This was referenced Oct 14, 2026
Main moved api to v0.1.597 and annotated the deprecated kinematics fields on its own in the navigation refactor, so those annotations are main's now and the api replace comes off. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
viambot
added
safe to test
JohnN193
marked this pull request as ready for review
October 15, 2026 21:13
JohnN193
requested review from
Bohdan Karavan (bkaravan),
Dan Gottlieb (dgottlieb) and
Nicolas Menard (npmenard)
October 16, 2026 15:19
Main now carries the geometry work from #6654 with its review fixes, so the older copy of it on this branch gives way to main's and only the typed model remains here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
viambot
added
safe to test
Member
Author
i'll make sure to do stacked prs going forward! |
Nicolas Menard (npmenard)
approved these changes
Oct 26, 2026
Nicolas Menard (npmenard)
left a comment
Nicolas Menard (npmenard)
left a comment
Member
There was a problem hiding this comment.
LGTM just a couple of nits.
| // property was never set, since neither value is meaningful at zero, and a zero property is left | ||
| // off the wire. | ||
| type KinematicProperties struct { | ||
| // OperatingFreqHz is the rate the component expects to be driven at. |
There was a problem hiding this comment.
nit maybe maximum?
| return &commonpb.GetKinematicsResponse{Format: commonpb.KinematicsFileFormat_KINEMATICS_FILE_FORMAT_UNSPECIFIED} | ||
| } | ||
| pb, err := ModelToProto(model) | ||
| if err != nil { |
There was a problem hiding this comment.
what is particular about this error path if we are call fileKinematicsResponse either way? would a comment or log message give more context?
| if err != nil { | ||
| return nil, err | ||
| } | ||
| pb, err := ModelConfigToProto(sva) |
There was a problem hiding this comment.
this also call svaConfig i assume this is fine
The tool speed property says maximum, KinematicModelToProtobuf builds the file response once and says why a model that cannot be written as a message sends it alone, and ModelToProto expands DH parameters once rather than having ModelConfigToProto do it again. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
viambot
added
safe to test
Contributor
Availability
Quality
Performance
Ordered Sequence 1 plan-by-plan: cappuccina-5fb95a4c (98 plans replayed in recorded order)Time moved (seconds, warm passes, by absolute change)
Joint travel moved (L2, warm passes, by absolute change)
All 98 plans (cold pass and warm passes)Every plan solved every pass in both runs. Time (seconds)
Joint travel (L2)
The above data was generated by running scenes defined in the
|
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.
First rdk piece of RSDK-14233, the Enhanced Arm Kinematics API, for ticket RSDK-14748. Builds on the api change in viamrobotics/api#899, in api v0.1.597, follows #6654, and replaces the referenceframe part of the prototype in #6494.
Changes
ModelToProtoandModelFromProtoconvert a model to and fromcommonpb.KinematicModel, withModelConfigToProtoandModelConfigFromProtounderneath for the configuration alone. Links keep whichever orientation representation the file used. DH parameters expand into the joint and link each one stands for, named the wayParseConfigalready names them.NewModelWithLimitOverridesor aninput_range_overrideshows up on the wire. A revolute joint with no position bound goes out asJOINT_TYPE_CONTINUOUSand comes back as the unbounded revolute joint the URDF reader has always built.original_file_source_path_diagnostic.<link>_collision_<i>for several unlabelled ones. The frames andLinkConfigside of that merged as RSDK-14771: carry several collision geometries per link #6654, and main is merged in here, so this PR holds only the list handling inmodel_proto.go.SimpleModelholds visual geometries per link asspatialmath.GeometryConfigand the component properties as a plainKinematicPropertiesstruct, with accessors. No proto type on the model, the converters inmodel_proto.goare the only place the message appears. Nothing in RDK reads them, they ride the message for viewers and SDKs, and they survive a limit override.KinematicModelToProtobuffillsmodel. The deprecated bytes keep replaying the file the model was read from, and a model that never had a file, one built from a configuration in code or read from the message, gets them written as an SVA file so old clients still have something to parse.KinematicModelFromProtobufprefersmodeland falls back to the bytes.Notes
modelsees no change in what it gets, since the bytes still replay the file. The scope's line that the bytes are derived from the model is read here as precedence for readers. Always deriving them from the model is a one line change if reviewers want it, with the side effect that a URDF backed component would hand old clients SVA JSON instead of the URDF and the mesh map.TestParseJSONFilefails for xarm6 on main at 2176b91 without any of these changes.Testing
go build ./...,go testover referenceframe, spatialmath, the arm, gantry and gripper components and the frame system service, andgolangci-lint run ./....🤖 Generated with Claude Code