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

feat(GPSR): object properties (count/weight/size) #151

Merged
merged 7 commits into from
Jul 6, 2024

Conversation

Sveali41
Copy link
Collaborator

@Sveali41 Sveali41 commented Apr 3, 2024

add the object detection script to the gpsr task

@jws-1 jws-1 changed the title object_recognition feat(GPSR): object properties (count/weight/size) Apr 23, 2024
Copy link
Member

@jws-1 jws-1 left a comment

Choose a reason for hiding this comment

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

Great job and first contribution! I also saw a working demo of this.

Sorry that this has taken me so long to review, I kept forgetting about it 😆.. I've left some comments, mostly relating to moving your work into lasr_skills and removing comments/print statements etc, but feel free to disagree! But after that, we can get this merged straight in!

Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this file!

Copy link
Member

Choose a reason for hiding this comment

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

I think this would be better off in skills/src/lasr_skills/, and would mean we can delete the files that I've marked :)

Copy link
Member

Choose a reason for hiding this comment

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

To be removed

Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove this now too, since I saw everything working on the robot!

Copy link
Member

Choose a reason for hiding this comment

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

To be removed

Comment on lines 105 to 106
# userdata.detections_3d = [(detection, point),(detection, point),(detection, point)]
# the detection includes the size of the bounding box and the center point of the box
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# userdata.detections_3d = [(detection, point),(detection, point),(detection, point)]
# the detection includes the size of the bounding box and the center point of the box

Comment on lines 115 to 116
# got the area dic of the detected objects
# rank the size of each object
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# got the area dic of the detected objects
# rank the size of each object

def execute(self, userdata):
# got the area dic of the detected objects
# rank the size of each object
print("we're doing size")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
print("we're doing size")

area_dict = self.property_size_calculation(detections_types, userdata.detections_3d)
sorted_size = sorted(area_dict.items(), key=lambda item: item[1], reverse = True)
userdata.sorted_size = sorted_size
print(sorted_size)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
print(sorted_size)

smach.StateMachine.__init__(
self,
outcomes=["succeeded", "failed"],
output_keys=["detections_3d"],
Copy link
Member

Choose a reason for hiding this comment

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

I think we should also have the various object properties in the output_keys, so that we can utilise them later on

Copy link
Member

@jws-1 jws-1 left a comment

Choose a reason for hiding this comment

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

Good work :)

@jws-1 jws-1 merged commit d03cd8f into LASR-at-Home:main Jul 6, 2024
1 check passed
jws-1 added a commit that referenced this pull request Jul 7, 2024
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