Skip to content

Conversation

@thedevmystic
Copy link
Contributor

@thedevmystic thedevmystic commented Dec 4, 2025

Hello, respected maintainers, and reviewers!
Again, this is Surya!

I've completed interpolation_methods.hpp, now I'm refactoring tolerances.hpp.

This PR addresses,

  1. Seperation of Concerns, the interface is seperated from the implementation.
  2. All non-inline functions are moved to tolerances.cpp for reduction in compile time.
  3. Addition of extensive comments and descriptions.
  4. Addition of useful utility functions.
  5. Addition of explicit tolerances.

NOTE:
About the changes, in tolerances yaml, should I keep it, or not?

NOTE:
About pre-commit failures, I'll address them after the PR is ready for merger.


And I'm getting an error, and I'm unsure how to deal with it, if you can help, I'll appreciate it very much.

Error Description:

/home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/build/joint_trajectory_controller/include/joint_trajectory_controller/joint_trajectory_controller_parameters.hpp:242:26: error: ‘struct joint_trajectory_controller::StackParams::Constraints’ has no member named ‘__map_joints’; did you mean ‘MapJoints’?
    242 |       output.constraints.__map_joints.trajectory.position = params.constraints.__map_joints.trajectory.position;

I don't know, why I'm having this. I didn't changed the field name in yaml, so it should still be accessible via joints_map, but I'm getting errors. And when I change __map_joints to MapJoints in yaml, I get,

/root/target_ws/src/ros2_controllers/joint_trajectory_controller/src/tolerances.cpp:83:50: error: ‘const struct joint_trajectory_controller::Params::Constraints’ has no member named ‘joints_map’; did you mean ‘Mapjoints’?
     83 |     auto const & joint_constraints = constraints.joints_map.at(joint);
        |                                                  ^~~~~~
        |                                                  Mapjoints

But previous function was able to compile with joints_map, I tried many things, but I got error everytime.

Tags:
@christophfroehlich, @saikishor

@christophfroehlich
Copy link
Member

NOTE: About the changes, in tolerances yaml, should I keep it, or not?

  • How do velocity constraints play together with stopped_velocity_tolerance? This is not clear from the parameter descriptions.
  • We need a graceful transition if we change parameters. Think about how old configurations could still work even if we add new options; deprecate the old parameters.

Please consider #1180 and #1677 for reference (as the PR got stale, it is ok to take this).

NOTE: About pre-commit failures, I'll address them after the PR is ready for merger.

why not installing pre-commit to automatically let the job be done? (pre-commit install)

Tags: @christophfroehlich, @saikishor

No need for tagging maintainers in the initial PR, we see it and will come back to you when we get time.

@christophfroehlich christophfroehlich linked an issue Dec 7, 2025 that may be closed by this pull request
@thedevmystic
Copy link
Contributor Author

Ok, I'll address all you questions one by one.

How do velocity constraints play together with stopped_velocity_tolerance? This is not clear from the parameter descriptions.

I'll do it!!

We need a graceful transition if we change parameters. Think about how old configurations could still work even if we add new options; deprecate the old parameters.

Of course, backporting is essential.

why not installing pre-commit to automatically let the job be done? (pre-commit install)

On my system, pre-commit just don't run, if you have seen my previous PR.

No need for tagging maintainers in the initial PR, we see it and will come back to you when we get time.

Sorry, about that. But in other projects where I work, I was encouraged to tag respective people in a draft PR. But if it's not the case here. Ok!

And a essential thing.
Can you please guide me, on the error, I'm getting? It has frustrated me, and I just can't figure it out.

Thanks, @christophfroehlich!

Regards,
Surya!

@thedevmystic
Copy link
Contributor Author

thedevmystic commented Dec 9, 2025

I've added both parameters for the __map_joints. Now both trajectory and goal are either struct(new) or double(old). And I'll modify the implementation in source files, when I get free time, because my exams are near :)

And about, the

How do velocity constraints play together with stopped_velocity_tolerance? This is not clear from the parameter descriptions.

Can you give a demo description, how you want it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[JTC] Add more options for tolerance configuration

2 participants