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

Fetch notifies trashbox status in go-to-kitchen demo. #1458

Merged
merged 11 commits into from
Jul 16, 2022

Conversation

708yamaguchi
Copy link
Member

@708yamaguchi 708yamaguchi commented Apr 20, 2022

Please merge after #1449

I cherry-picked

With this PR,

  • fetch goes in front of the trashbox in the go-to-kitchen demo.
  • fetch notifies the status by object recognition and email.

@k-okada
Copy link
Member

k-okada commented Apr 21, 2022

please paste screenshot of notification mail @708yamaguchi

@k-okada
Copy link
Member

k-okada commented Apr 21, 2022

please see comments #1463 (comment)

@708yamaguchi
Copy link
Member Author

This is today's go-to-kitchen mail.

trashcan_mail

The trashcan status is reported like below.
In addition, the trashcan picture is attached. (but not embedded...)

Following trashcan occupancy is reported.
 - At 2022-04-21T09:02:43, Trashcan is full. Trashcan occupancy is 1.24093 in trash can.

@708yamaguchi
Copy link
Member Author

I rebased origin/master to resolve conflicts.

@k-okada
Copy link
Member

k-okada commented Jul 14, 2022

@708yamaguchi please resolve conflicts

@708yamaguchi 708yamaguchi force-pushed the use-relative-coords branch from 8a9b8ee to 0bd3601 Compare July 14, 2022 10:46
@708yamaguchi
Copy link
Member Author

I rebase the current origin/master to resolve conflicts.

@@ -140,8 +140,7 @@
(if is-charging :charging :discharging)))


(defun go-to-spot (name &optional (relative-coords (make-coords))
&key (undock-rotate nil) (clear-costmap t))
(defun go-to-spot (name &key (relative-pos nil) (relative-rot nil) (undock-rotate nil) (clear-costmap t))
Copy link
Member

@k-okada k-okada Jul 15, 2022

Choose a reason for hiding this comment

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

Are we ok to change API?
Specially why you changed coordinates to pos/rot?
since go-to-spot will be put in pr2eus level in future, please add documentation string

Copy link
Member Author

@708yamaguchi 708yamaguchi Jul 15, 2022

Choose a reason for hiding this comment

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

Thank you very much for your review.
As you pointed out, documentation was lacking, so I added an explanation. 268ea1c (#1458)

After talking with @tkmtnt7000, we came to the conclusion that it is easier to understand if relative-pos and relative-rot are specified in the different coords (specifically, world (map) and local (spot) coords respectively).
knorth55#140 (comment)

For example, assume using relative-coords in each spot's coords.
In this case, when specifying the pos of the relative-coords, we need to care about the orientation of each spot.
This is very complicated because each spot has a different orientation.

@k-okada k-okada merged commit 3980376 into jsk-ros-pkg:master Jul 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants