Skip to content

Test/grasp heights #1013

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
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Test/grasp heights #1013

wants to merge 10 commits into from

Conversation

@jlunenburg
Copy link
Contributor

@jlunenburg jlunenburg commented Mar 7, 2020

Contains test and some 'fixes' in the grasp point determination to enable this.

@jlunenburg jlunenburg requested a review from PetervDooren March 7, 2020 15:34
PetervDooren
PetervDooren requested changes Mar 7, 2020
View reviewed changes
@jlunenburg
Copy link
Contributor Author

jlunenburg commented Mar 10, 2020

I haven't tested my latest updates (don't want to install all stuff on my personal or work laptop). If someone wants to test it can be merged provided @PetervDooren approves.

@LarsJanssenTUe
Copy link
Contributor

LarsJanssenTUe commented Mar 10, 2020

@jlunenburg this is what I get when running the python grasping_height_test.py while hero-start and hero-free-mode are running, but given that you did not change this I guess there is something sketchy going on?

Traceback (most recent call last): File "grasping_height_test.py", line 17, in <module> from robot_skills.hero_parts.hero_arm import HeroArm ImportError: No module named hero_parts.hero_arm

I also tried this in the hero console and this thing indeed does not seem to exist

@jlunenburg jlunenburg requested a review from PetervDooren March 31, 2020 17:53
@LarsJanssenTUe LarsJanssenTUe mentioned this pull request Mar 31, 2020
Open
PetervDooren
PetervDooren previously approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@PetervDooren PetervDooren left a comment

Choose a reason for hiding this comment

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

I fully approve these changes. However I would like to have this tested on the real robot once we have access again. If the test provides the right information we can merge this.

@PetervDooren PetervDooren dismissed their stale review February 11, 2021 20:16

Outdated check after merge with master

@PetervDooren PetervDooren added help wanted Extra attention is needed maintenance Making existing code better in terms of style and setup requires robot This can only be worked on with access to the lab labels Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PetervDooren PetervDooren PetervDooren left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

help wanted Extra attention is needed maintenance Making existing code better in terms of style and setup requires robot This can only be worked on with access to the lab

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jlunenburg @LarsJanssenTUe @PetervDooren