RSDK-13546- compute jacobian from model table#17
Conversation
nfranczak
changed the title
acmorrow
left a comment
acmorrow
left a comment
There was a problem hiding this comment.
I left some general comments, but this needs to be reworked to avoid a dependency on the C++ SDK inside the totg library. We can't do that per our goal of using trajex by way of cgo inside the RDK.
| xtensor | ||
| xtl | ||
| PRIVATE | ||
| viam-cpp-sdk::viamsdk |
There was a problem hiding this comment.
Unfortunately, creating this dependency edge is pretty much not permissible for viam-trajex-totg. It would mean dragging the C++ SDK and all of its transitive dependencies in when we link trajex into the RDK. Worse, for a static build, it would require producing a link line which explicitly referenced all of those libraries.
| @@ -0,0 +1,450 @@ | |||
| // Tests for the simplified jacobian module (Option B: plain-return API, | |||
There was a problem hiding this comment.
Try to teach your claude that it shouldn't reference ephemeral state of local information in code it produces. It is hard: it really likes to do this, but Option B doesn't mean anything.
| // Joint-type encodings for column 9 of the model-table tensor. | ||
| // These match viam::sdk::JointType: revolute=0, continuous=1, | ||
| // prismatic=2, fixed=3. | ||
| constexpr double kRev = 0.0; |
There was a problem hiding this comment.
Style in our C++ is k_rev not kRev.
|
|
||
| } // namespace | ||
|
|
||
| // ============================================================================ |
There was a problem hiding this comment.
You don't need these block comments. You can just comment on the BOOST_AUTO_TEST_SUITE itself, since that naturally forms a scope:
// The tests in this suite validate that `J * q_dot` produces the expected Cartesian velocity
BOOST_AUTO_TEST_SUITE(jacobian_velocity_tests)
|
|
||
| namespace viam::trajex::jacobian { | ||
|
|
||
| // Compute the geometric Jacobian for a URDF-style model table at joint |
There was a problem hiding this comment.
If this is in a header, it should be doxygen comment style, with ///. The parameters and return values should be tagged with the appropriate doxygen annotations. Have a look at the path.hpp or trajectory.hpp headers for examples. Probably good to tell claude that it should always follow the local commenting style and to study the codebase under development for the local patterns.
2 participants
No description provided.