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.

Sign up for GitHub

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

Jump to bottom

[fetcheus] fix radian in :go-pos-unsafe #1166

Merged
merged 5 commits into from
Oct 21, 2020
Merged

[fetcheus] fix radian in :go-pos-unsafe #1166

merged 5 commits into from
Oct 21, 2020

Conversation

knorth55
Copy link
Member

@knorth55 knorth55 commented Nov 8, 2019

:go-pos-unsafe d is given as degree!

@knorth55 knorth55 requested a review from 708yamaguchi November 8, 2019 14:56
@knorth55 knorth55 mentioned this pull request Nov 8, 2019
Closed
29 tasks
708yamaguchi
708yamaguchi approved these changes Nov 9, 2019
View reviewed changes
Copy link
Member

@708yamaguchi 708yamaguchi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think this is fatal bug and this PR need to be merged.
(shortest-angle) assumes radian input, but degree input is used now.
https://github.com/jsk-ros-pkg/jsk_robot/blob/master/jsk_fetch_robot/fetcheus/fetch-interface.l#L249-L251

By the way, I feel it confusing that variable d is deg and d0 and d2 is rad...

@knorth55 knorth55 mentioned this pull request Nov 9, 2019
Open
12 tasks
Affonso-Gui
Affonso-Gui approved these changes Nov 11, 2019
View reviewed changes
Copy link
Member

@Affonso-Gui Affonso-Gui left a comment

Choose a reason for hiding this comment

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

From :go-pos documentation:

using [m] and [degree] is historical reason from original hrpsys code

@knorth55
Copy link
Member Author

travis passed!

@YutoUchimi YutoUchimi added this to the 1.1.1 milestone Nov 19, 2019
@knorth55
Copy link
Member Author

I want to add test but waiting for pr2eus move base test.
jsk-ros-pkg/jsk_pr2eus#420

@knorth55
Copy link
Member Author

knorth55 commented Apr 4, 2020

I add navigation test for fetch #1212

@knorth55 knorth55 mentioned this pull request Jul 24, 2020
Closed
@knorth55 knorth55 mentioned this pull request Oct 21, 2020
Open
@k-okada k-okada merged commit 5fc38ca into jsk-ros-pkg:master Oct 21, 2020
@knorth55 knorth55 deleted the fix-go-pos-unsafe branch October 21, 2020 11:30
@knorth55 knorth55 mentioned this pull request Sep 11, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@708yamaguchi 708yamaguchi 708yamaguchi approved these changes

@Affonso-Gui Affonso-Gui Affonso-Gui approved these changes

Assignees
No one assigned
Labels
bugfix fetch15 please add test
Projects
None yet
Milestone
1.1.1
Development

Successfully merging this pull request may close these issues.
5 participants
@knorth55 @708yamaguchi @Affonso-Gui @k-okada @YutoUchimi