Skip to content
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

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

bmagyar
Copy link
Member

@bmagyar bmagyar commented Jun 17, 2024

Even though the desired and actual fields are deprecated, we should still fill them in.

Copy link

codecov bot commented Jun 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.63%. Comparing base (deeb9d0) to head (b184ce7).

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     
Flag Coverage Δ
unittests 86.63% <100.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ory_controller/src/joint_trajectory_controller.cpp 83.83% <100.00%> (+0.06%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes perfect sense

@bmagyar
Copy link
Member Author

bmagyar commented Jun 17, 2024

FYI @MarqRazz

Copy link
Contributor

@MarqRazz MarqRazz left a 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;
Copy link
Contributor

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?

@christophfroehlich
Copy link
Contributor

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.

maybe I don't get what you try to achieve, but the reference field is the interpolated output?

Copy link
Contributor

@christophfroehlich christophfroehlich left a 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

@MarqRazz
Copy link
Contributor

MarqRazz commented Jun 19, 2024

maybe I don't get what you try to achieve, but the reference field is the interpolated output?

I am looking for the interpolated output to the commanded hardware. According to the message reference is the input and output is for the hardware. If I wanted to see the interpolated values for velocity or acceleration I can't when my arm is only commanded with position.

What are actual and desired for? All they contain is time_from_start and they are always zero.

I'm all for removing them too!

@bmagyar bmagyar merged commit b9cc260 into humble Jun 19, 2024
11 of 12 checks passed
@bmagyar bmagyar deleted the jtc-report-on-deprecated-feedback branch June 19, 2024 17:08
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.

5 participants