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] Allow integration of states in goal trajectories #190

Merged

Conversation

destogl
Copy link
Member

@destogl destogl commented May 28, 2021

Second part of #161,

Enabling integration of states in goal trajectories. This means that goal trajectories may have only velocity of acceleration defined. Then posiitions and/or velocities will be integrated from inputs.

@destogl
Copy link
Member Author

destogl commented May 28, 2021

I have here issue where uncrustify and cpplint are fighting...

@destogl destogl force-pushed the jtc-allow-integration-of-goal-trajectories branch from c260f0c to bf8f81a Compare September 8, 2021 21:29
@destogl destogl force-pushed the jtc-allow-integration-of-goal-trajectories branch from bf8f81a to 83de425 Compare January 7, 2022 08:25
@destogl destogl force-pushed the jtc-allow-integration-of-goal-trajectories branch from 83de425 to 52f1617 Compare January 14, 2022 18:41
@destogl destogl marked this pull request as draft March 1, 2022 16:47
@destogl destogl marked this pull request as ready for review March 1, 2022 16:47
@bmagyar
Copy link
Member

bmagyar commented Apr 6, 2022

can you please rebase this?

EXPECT_NEAR(point_before_msg.positions[0], expected_state.positions[0], EPS);
EXPECT_NEAR(point_before_msg.velocities[0], expected_state.velocities[0], EPS);
// is 0 because point_before_msg does not have velocity defined
EXPECT_NEAR(1.0, expected_state.accelerations[0], EPS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you document in a comment why you expect 1.0 for the sake of future test extensions?

@destogl destogl force-pushed the jtc-allow-integration-of-goal-trajectories branch from 2de56e6 to 86e3a9f Compare April 20, 2022 18:50
Copy link
Member Author

@destogl destogl left a comment

Choose a reason for hiding this comment

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

I have added all the changes and commented to the rest.

The relevant tests are passing:

  • "test_trajectory" - set of tests
  • "missing_positions_message_accepted" test in "test_trajectory_controller" set (newly added test)

@bmagyar bmagyar merged commit 0e7c260 into ros-controls:master Apr 20, 2022
@destogl destogl deleted the jtc-allow-integration-of-goal-trajectories branch April 21, 2022 20:03
destogl added a commit to StoglRobotics-forks/ros2_controllers that referenced this pull request Apr 26, 2022
)

* Add allow_integration flag

* Added position and velocity deduction to trajectory.

* Added support for deduction of states from their derivatives.

* Apply suggestions from code review

Co-authored-by: Victor Lopez <3469405+v-lopez@users.noreply.github.com>

* Correct warnings and formating.

* Update test to use more human-readable values.

* Apply reviewers comments.

* Last test fixes.

Co-authored-by: Denis Štogl <destogl@users.noreply.github.com>
Co-authored-by: Victor Lopez <3469405+v-lopez@users.noreply.github.com>
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.

None yet

5 participants