Skip to content

RobotTrajectory as standard container#720

Merged
henningkayser merged 10 commits intomoveit:mainfrom
DLu:trajectory_iterator
Nov 4, 2021
Merged

RobotTrajectory as standard container#720
henningkayser merged 10 commits intomoveit:mainfrom
DLu:trajectory_iterator

Conversation

@DLu
Copy link
Copy Markdown
Contributor

@DLu DLu commented Oct 4, 2021 •
edited
Loading

Description

Ported version of moveit/moveit#1162 for MoveIt2. This currently does not have any tests.

  • Add iterator to RobotTrajectory class.
  • Add .size() method to RobotTrajectory class.
  • Add basic tests for RobotTrajectory class.
  • Add equality operator for RobotState class in order to complete RobotTrajectory tests. Note: This seems outside the scope.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@DLu DLu added the help wanted Extra attention is needed label Oct 4, 2021
@DLu DLu mentioned this pull request Oct 4, 2021
Merged
1 task
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 4, 2021 •
edited
Loading

Codecov Report

Merging #720 (f3d7964) into main (812a805) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head f3d7964 differs from pull request most recent head 9ebeac1. Consider uploading reports for the commit 9ebeac1 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #720      +/-   ##
==========================================
+ Coverage   54.88%   54.92%   +0.05%     
==========================================
  Files         196      196              
  Lines       21359    21380      +21     
==========================================
+ Hits        11720    11741      +21     
  Misses       9639     9639
Impacted Files Coverage Δ
...include/moveit/robot_trajectory/robot_trajectory.h 90.77% <100.00%> (+4.73%) ⬆️
moveit_ros/moveit_servo/src/servo_node.cpp 68.89% <0.00%> (-0.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 834f6da...9ebeac1. Read the comment docs.

@DLu DLu force-pushed the trajectory_iterator branch from e021245 to 8e62747 Compare October 29, 2021 20:40
@DLu DLu marked this pull request as ready for review November 1, 2021 19:56
@DLu DLu removed the help wanted Extra attention is needed label Nov 1, 2021
@DLu DLu requested a review from tylerjw November 2, 2021 13:49
henningkayser
henningkayser reviewed Nov 3, 2021
View reviewed changes
henningkayser
henningkayser approved these changes Nov 3, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

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

Looks good. CI failure seems unrelated to me

@henningkayser henningkayser merged commit a41876a into moveit:main Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henningkayser henningkayser henningkayser approved these changes

@tylerjw tylerjw Awaiting requested review from tylerjw

Assignees

@DLu DLu

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DLu @tylerjw @henningkayser