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

Check yaw rates instead of positions when yoyoing (retargeted #124 to main) #133

Merged
merged 6 commits into from
Jan 6, 2022

Conversation

chapulina
Copy link
Contributor

I cherry-picked the commits from #124 onto main so it can be merged faster and we don't need to wait for #89 and #96.

The only difference in the test here from #124 is a higher tolerance for the yaw rate (2e-2 instead if 2e-3). I left a TODO there so we reduce the tolerance after those PRs get in.

arjo129 and others added 6 commits January 5, 2022 14:11
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
This  PR depends on #89, #96.

Recommended review order is #89 > #96 > this PR. If we want I can cherry pick this branch as the changes are independent of the underlying code. The merge order should be this PR > #96 > #89.

There is a slight change in the center of rotation of the yoyo mission with #96. This leads to test expectations failing. Rather than use position to check whether the vehicle is yoyoing, I think we should use @tfoote's suggestion and use yaw rate which makes the test independent of the center of rotation and hence more robust while at the same time ensuring that the vehicle actually moves in a circle.

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina merged commit 303549a into main Jan 6, 2022
@chapulina chapulina deleted the chapulina/arjo/yoyo_test branch January 6, 2022 01:52
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.

2 participants