-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split SteeringOdometry into ForwardKinematics and InverseKinematics #1475
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry that I haven't responded earlier in the linked issue. I added some requirements there, please consider them, and also consider installing pre-commit because there are again issues with it.
I run
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nakul-py I think you need to run the pre-commit run --all-files
command again.
The pre-commit only changes the files that are staged but not the unstaged one. I think when you ran this command. They are unstaged changed.
Right now, if you run it, it should show you the pre-commit changes after running the command.
Please also make sure it builds. I see that the changes in this PR are not compiling
steering_controllers_library.cpp:53:30: error: no matching function for call to ‘steering_kinematics::SteeringKinematics::SteeringKinematics()’
steering_kinematics.hpp:70:32: error: ‘class ForwardKinematics’ has no member named ‘calculate’
steering_kinematics.hpp:63:63: error: no matching function for call to ‘rcpputils::RollingMeanAccumulator<double>::RollingMeanAccumulator()’
steering_kinematics.hpp:63:63: error: no matching function for call to ‘rcpputils::RollingMeanAccumulator<double>::RollingMeanAccumulator()’
forward_kinematics.hpp:26:3: error: ‘Odometry’ does not name a type
Hey @saikishor i am trying to building the project by |
I'm not aware of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several build errors now
/workspaces/ros2_rolling_ws/src/ros2_controllers/steering_controllers_library/include/steering_controllers_library/forward_kinematics.hpp:26:3: error: ‘Odometry’ does not name a type
26 | Odometry calculate(
| ^~~~~~~~
please fix them before you push.
regarding tcb_span, this is part of generate_parameter_library. Have you run rosdep to install the dependencies?
In general: What is your approach now with adding three classes: kinematics, forward-, and inverse kinematics? I'd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be safe to delete completely.
Description
This pull request refactors the
SteeringOdometry
class into separateForwardKinematics
andInverseKinematics
classes to improve code clarity and maintainability.Fixes #1461