Skip to content

[JTC] Remove deprecated open_loop_control code #1598

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

Merged

Conversation

Thieso
Copy link
Contributor

@Thieso Thieso commented Mar 19, 2025

Follow up on #1525.

Is this sufficient or should I add a note somewhere (release notes or something)?.

Copy link

codecov bot commented Mar 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.02%. Comparing base (945a360) to head (0c19b15).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1598      +/-   ##
==========================================
+ Coverage   84.81%   85.02%   +0.20%     
==========================================
  Files         127      127              
  Lines       12114    12085      -29     
  Branches     1036     1029       -7     
==========================================
  Hits        10275    10275              
+ Misses       1501     1480      -21     
+ Partials      338      330       -8     
Flag Coverage Δ
unittests 85.02% <100.00%> (+0.20%) ⬆️

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

Files with missing lines Coverage Δ
...ory_controller/src/joint_trajectory_controller.cpp 85.99% <100.00%> (+0.35%) ⬆️
...ntroller/test/test_trajectory_controller_utils.hpp 84.66% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Thanks for the follow-up.

You could just add this PR to the release notes in the same bullet point of #1525, but the text is already good as it is.

@christophfroehlich
Copy link
Contributor

the downstream ur_controllerspackage fails now.

   Starting >>> ur_controllers
  --- stderr: ur_controllers
  /home/runner/work/ros2_controllers/ros2_controllers/.work/downstream_ws/src/UniversalRobots/Universal_Robots_ROS2_Driver/ur_controllers/src/scaled_joint_trajectory_controller.cpp: In member function ‘virtual controller_interface::return_type ur_controllers::ScaledJointTrajectoryController::update(const rclcpp::Time&, const rclcpp::Duration&)’:
  /home/runner/work/ros2_controllers/ros2_controllers/.work/downstream_ws/src/UniversalRobots/Universal_Robots_ROS2_Driver/ur_controllers/src/scaled_joint_trajectory_controller.cpp:171:19: error: ‘struct joint_trajectory_controller::Params’ has no member named ‘open_loop_control’
    171 |       if (params_.open_loop_control) {

@Thieso are you working with UR robots? If not, I can open a PR there once we merge this here.

@Thieso
Copy link
Contributor Author

Thieso commented Mar 19, 2025

@Thieso are you working with UR robots? If not, I can open a PR there once we merge this here.

No I am not. Only kinova right now.

@christophfroehlich christophfroehlich added the hold Holding off merging this PR until some condition label May 2, 2025
@christophfroehlich christophfroehlich moved this from Jazzy to Kilted in Roadmap / Features May 2, 2025
@christophfroehlich christophfroehlich removed the hold Holding off merging this PR until some condition label May 16, 2025
@christophfroehlich christophfroehlich merged commit fa06e90 into ros-controls:master May 17, 2025
22 of 31 checks passed
@github-project-automation github-project-automation bot moved this from Kilted to Done in Roadmap / Features May 17, 2025
nitin2606 pushed a commit to nitin2606/ros2_controllers that referenced this pull request May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants