-
Notifications
You must be signed in to change notification settings - Fork 327
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
[JTC] Report on deprecated feedback #1172
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## humble #1172 +/- ##
==========================================
- Coverage 86.66% 86.63% -0.04%
==========================================
Files 86 86
Lines 7485 7490 +5
Branches 616 618 +2
==========================================
+ Hits 6487 6489 +2
Misses 767 767
- Partials 231 234 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Makes perfect sense
FYI @MarqRazz |
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 will help with the issue I was running into but once the deprecated message is removed the new one is still missing functionality.
If I send this a trajectory you can see/plot position, velocity, acceleration
in the reference
value published in the controller_state
. If I wanted to plot the output
you can only see the values calculated for the claimed interface (i.e. position
in my case).
The old functionality allowed me to see/plot position, velocity, acceleration
of the input & output
so that I can visualize what the interpolation is doing to my trajectory.
state_publisher_legacy_->msg_.desired.positions = desired_state.positions; | ||
state_publisher_legacy_->msg_.desired.velocities = desired_state.velocities; | ||
state_publisher_legacy_->msg_.desired.accelerations = desired_state.accelerations; | ||
state_publisher_legacy_->msg_.actual.positions = current_state.positions; |
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.
Why only position
for actual
?
maybe I don't get what you try to achieve, but the reference field is the interpolated output? |
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.
I vote for deleting the fields in the control msg instead. If there's something missing in the current design, let's discuss the roadmap proposal, implemented with ros-controls/control_msgs#86
I haven't realized this is for humble
I am looking for the interpolated output to the commanded hardware. According to the message What are I'm all for removing them too! |
Even though the
desired
andactual
fields are deprecated, we should still fill them in.