chore: minor controller changes#40
Merged
danielsanjosepro merged 12 commits intolearnsyslab:mainfrom Feb 28, 2026
Merged
Conversation
danielsanjosepro
requested changes
Feb 27, 2026
Collaborator
danielsanjosepro
left a comment
danielsanjosepro
left a comment
There was a problem hiding this comment.
Thank you a lot for contributing :) All in all, really good! Thanks for catching the bug #41 this needs a fix for sure. I left a few comments that we need to discuss before merging. Feel free to add yourself to the community contributors list and add a link to your project.
danielsanjosepro
requested changes
Feb 27, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes small structural and performance-oriented improvements to the cartesian_controller, adjusts controller parameters, and updates the build/export configuration for the crisp_controllers library.
Changes:
- Refactors joint state read/filter logic into
updateCurrentState()and preallocates Jacobian pseudo-inverse/identity matrices outside the control loop. - Introduces a new
jacobian_regularizationparameter and changes how regularization is configured/used for pseudo-inverses. - Updates CMake exports to use target-based ament exports; lowers Python requirement in
pyproject.tomlto 3.10.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cartesian_controller.yaml | Adds jacobian_regularization, tweaks defaults, and removes nullspace.regularization. |
| src/cartesian_controller.cpp | Moves state update into helper, preallocates matrices, and adjusts pseudo-inverse regularization usage. |
| include/crisp_controllers/cartesian_controller.hpp | Declares updateCurrentState(), num_joints_, and preallocated matrices. |
| CMakeLists.txt | Switches to target-based export approach and expands exported dependencies. |
| pyproject.toml | Lowers requires-python from >=3.11 to >=3.10. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
yizhongzhang1989
pushed a commit
to yizhongzhang1989/crisp_controllers
that referenced
this pull request
Mar 17, 2026
…ble parameter for inverse mass matrix computation (learnsyslab#40) * Bump down python requirement * Fixed library expoert issues * Minor controller changes * Minor controller changes * Format CMakeLists * Revert parameters header inclusion * chore: add regularization to operational space pseudoinverse * fix: targets exported before dependencies (incorrecttly) * chore: revert back to nullspace regularization parameter * fix: dq assignment and num_joint usage * chore: Update docstrings for updateCurrentState --------- Co-authored-by: Daniel San José Pro <42489409+danielsanjosepro@users.noreply.github.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.
Hi all,
I'm working on a similar controller that extends the cartesian_controller and thought I might kick back some small changes/improvements upstream.
Changes
This is my first contribution here, so please let me know if I missed any styling guidelines or if you'd like the commits structured differently. Looking forward to hearing your feedback!