RSDK-14385: publish configured joint velocity and acceleration limits - #80
Open
JohnN193 wants to merge 2 commits into
Open
RSDK-14385: publish configured joint velocity and acceleration limits#80JohnN193 wants to merge 2 commits into
JohnN193 wants to merge 2 commits into
Conversation
`get_kinematics` opened `kinematics/<model>.json` and returned the bytes verbatim, so callers got position bounds only. The SVA schema now carries optional per-joint `max_velocity` and `max_acceleration`, and the values this arm runs at are already in its config, so we add them on the way out and the motion service can generate trajectories from them. `sva_with_joint_limits` takes an SVA document and the per-joint limits and returns the document with the two fields written. Config is radians and the schema is degrees, so it converts. A zero is a real limit of zero and is written as such, since only an absent field means unbounded. It refuses a few things rather than guessing. The limits are positional, so the vectors have to be exactly as long as the document's `joints` array. A prismatic joint wants mm/s and RDK does not convert what we write, so a radian value would be read as millimetres. A mimic joint takes its limits from its source, and RDK rejects the entire model if one carries limits of its own. None of these are reachable with the three shipped 6-joint revolute models, but all of them would be silent the day a new model lands. `get_kinematics` catches those failures, warns, and serves the unpatched document. RDK builds the machine's whole frame system from this call, and `validate_config_` accepts a configured DOF that disagrees with the model's joint count (a scalar paired with an array of another length), which used to be harmless here because we returned the file untouched. Losing the limits is what every caller got before this existed, so that is what we fall back to rather than failing the machine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
|
JohnN193 - I'm going to hold off on this until we finish viam-modules/universal-robots#227 unless you think there is value in pursuing both in parallel. |
`validate_joint_limit_attribute` rejects a non-positive element before it could reach here, so this cannot fire today. That is exactly the problem: `sva_with_joint_limits` was correct only for as long as that stayed true, and a caller assembling the vectors itself would have published a negative speed as though it were a limit. Zero is still written, since only an absent field means unbounded and a zero limit is a real one saying the joint does not move. Negative is not a limit at all. Also note at the joint-count check that it doubles as the guard against limits we never populated, since a default-constructed Eigen vector is empty rather than a run of zeros. The universal-robots module needed an explicit optional for this because its fixed-size vector defaults to zeros, which would have been published as an arm configured not to move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Ticket 8 of RSDK-14385.
get_kinematicsopenedkinematics/<model>.jsonand returned the bytes verbatim, so callers got position bounds only. The SVA schema now carries optional per-jointmax_velocityandmax_acceleration, and the values this arm actually runs at are already in its config, so we attach them on the way out.What changed
sva_with_joint_limits(sva_json, velocity_rad, accel_rad)inutils.cpp: parse, write the two fields per joint, re-serialize. Config is radians and the schema is degrees, so it converts.get_kinematicspipes the file bytes through it.sva_joint_limits_testtarget, plus the missing entry in thetesttarget's explicit executable list in theMakefile.jsoncpp was already a dependency (
conanfile.py, used bytrajectory_logger), so this adds none.What it refuses, and why
The limits are positional, since the config gives us arrays with no joint names, so the vectors must be exactly as long as the document's
jointsarray. Beyond that it rejects:DegToRadtomax_velocityfor revolute and leaves prismatic unconverted, so a radian-derived value on a rail would be read as mm/s. Aspeed_rad_per_secconfig cannot express mm/s.ErrMimicWithLimitsif a mimic joint carries limits of its own, so publishing one would be worse than publishing no limits.None of these are reachable with the three shipped models (all 6-joint, revolute, no mimic). All of them would be silent the day a new model file lands, which is why they are checks rather than comments.
Failure handling
get_kinematicscatches, warns with the file and reason, and serves the unpatched document. RDK builds the machine's entire frame system from this call, andvalidate_config_accepts a configured DOF that disagrees with the model's joint count (speed_rad_per_sec: 1.0withacceleration_rad_per_sec2: [1,2,3]validates and yields DOF 3). That used to be harmless here because we returned the file untouched. Losing the limits is exactly what every caller got before this existed, so that is the fallback rather than failing the machine's motion.A stricter alternative is to reject the mismatch in
validate_config_, which would fail such a config at startup instead. That would reject configs that work today, so I left it out; happy to add it if preferred.Zero
A zero is written as an explicit
0, since only an absent field means unbounded. This module's validation rejects a zero element, but the schema permits one and dropping it would describe an axis that cannot move as unbounded.Testing
Nine cases: per-joint degree conversion with distinct values, the rest of the document untouched (ids, parents, axes, min/max, links all compared against the original), zero written not dropped, joint-count mismatch, vector-length disagreement, malformed / non-SVA / no-joints, absent
kinematic_param_typetreated as SVA (matchingreferenceframe/model_json.go), and non-revolute / mimic / untyped refusal. One case walkssrc/kinematics/*.jsonso every shipped model is checked.Not covered:
get_kinematicsitself, which needs a configured arm and a controller.7/7 unit tests green,
-Wall -Werrorclean, clang-tidy and clang-format clean.🤖 Generated with Claude Code