Skip to content

Fixed period computation in test#693

Merged
bmagyar merged 1 commit intoros-controls:jtc-featuresfrom
b-robotized-forks:fix-delta-time
Jul 7, 2023
Merged

Fixed period computation in test#693
bmagyar merged 1 commit intoros-controls:jtc-featuresfrom
b-robotized-forks:fix-delta-time

Conversation

@gwalck
Copy link
Copy Markdown
Contributor

@gwalck gwalck commented Jul 6, 2023

As discussed and confirmed in #686 there seem to be a wrong computation of the period (dt) passed to traj_controller->update in the UpdateController test helper function.

Fixing it does not make tests fail, but is, first necessary to avoid keeping wrong computations, and second for future up-coming improvement that relies on dt to update the state interface as well.

Copy link
Copy Markdown
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

well-spotted!

@bmagyar
Copy link
Copy Markdown
Member

bmagyar commented Jul 6, 2023

Please run pre-commit run --all in the repo and fix the highlighted warnings for the CI to be happy too :)

@gwalck
Copy link
Copy Markdown
Contributor Author

gwalck commented Jul 6, 2023

Please run pre-commit run --all in the repo and fix the highlighted warnings for the CI to be happy too :)

Sorry, I know this, did it several times, and for the PR (which is only a single change compared to many I did), I did a cherry-pick and apparently this introduced a line. I forgot to re-run. I think I had a reason to not "install" the pre-commit but I forgot which one.

@bmagyar
Copy link
Copy Markdown
Member

bmagyar commented Jul 6, 2023

I find precommit on every commit to be slowing my work down and be generally annoying so I also never actually run pre-commit install :)

Copy link
Copy Markdown
Member

@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!

@bmagyar bmagyar added the backport-humble Triggers PR backport to ROS 2 humble. label Jul 7, 2023
@bmagyar bmagyar merged commit de7083e into ros-controls:jtc-features Jul 7, 2023
@gwalck gwalck deleted the fix-delta-time branch July 12, 2023 12:20
@gwalck gwalck mentioned this pull request Jul 13, 2023
@destogl
Copy link
Copy Markdown
Member

destogl commented Jul 17, 2023

@Mergifyio backport humble

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jul 17, 2023

backport humble

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Jul 17, 2023
christophfroehlich pushed a commit to christophfroehlich/ros2_controllers that referenced this pull request Jul 17, 2023
christophfroehlich pushed a commit to christophfroehlich/ros2_controllers that referenced this pull request Jul 17, 2023
bmagyar pushed a commit that referenced this pull request Jul 17, 2023
mergify bot pushed a commit that referenced this pull request Jul 17, 2023
mergify bot pushed a commit that referenced this pull request Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-humble Triggers PR backport to ROS 2 humble.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants