Skip to content

Conversation

GustavoDCC
Copy link
Contributor

I want to receive the messages from /pose_estimation node. Then, I will try to crop an specific part of the image (for example: head or chest) to call the service to extract the color.

@GustavoDCC GustavoDCC requested a review from MatthijsBurgh May 2, 2024 15:45
Copy link
Member

@MatthijsBurgh MatthijsBurgh left a comment

Choose a reason for hiding this comment

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

This is a first good step. Though the current implementation does create a service client on each image. This is very inefficient. Please change it to a class, i.e. https://github.com/tue-robotics/hero_bringup/blob/aadf90641a2517e71c790ceb2e7c00d14a865a80/scripts/gazebo_pose_setter

@MatthijsBurgh
Copy link
Member

And make sure to add the new script to be executable and add it to the list of scripts to be installed, (keep the list alphabetical) As right now catkin_lint is complaining...

@GustavoDCC GustavoDCC requested a review from MatthijsBurgh May 3, 2024 08:22
Comment on lines 10 to 14
class ExtractColorClient(object):
def __init__(self, image_topic, color_service):
self.image_sub = rospy.Subscriber(image_topic, Image, self.color_callback)
self.color_proxy = rospy.ServiceProxy(color_service, Recognize)
self.color_proxy.wait_for_service(timeout=20)
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
class ExtractColorClient(object):
def __init__(self, image_topic, color_service):
self.image_sub = rospy.Subscriber(image_topic, Image, self.color_callback)
self.color_proxy = rospy.ServiceProxy(color_service, Recognize)
self.color_proxy.wait_for_service(timeout=20)
class ExtractColorClient:
def __init__(self, image_topic, color_service):
self.color_proxy = rospy.ServiceProxy(color_service, Recognize)
self.color_proxy.wait_for_service(timeout=20)
self.image_sub = rospy.Subscriber(image_topic, Image, self.color_callback)

As the subscriber will start immediately, but the service client will not be ready yet. So that could lead to an error.

Comment on lines 24 to 25
parser.add_argument('--topic', default='/image', type=str, help='Topic')
args = parser.parse_args()
Copy link
Member

Choose a reason for hiding this comment

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

Maybe also add an argument for the service

Copy link
Member

Choose a reason for hiding this comment

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

Maybe good to let rospy filter the ros args

Suggested change
parser.add_argument('--topic', default='/image', type=str, help='Topic')
args = parser.parse_args()
parser.add_argument('--topic', default='/image', type=str, help='Topic')
args = parser.parse_args(rospy.myargv())

@GustavoDCC GustavoDCC requested a review from MatthijsBurgh May 4, 2024 07:36
@MatthijsBurgh MatthijsBurgh changed the title Create a node listener and call service (color_extractor) Create a node listener and call service May 5, 2024
@MatthijsBurgh MatthijsBurgh merged commit d91bc5a into master May 5, 2024
@MatthijsBurgh MatthijsBurgh deleted the extract_color_from_topic branch May 5, 2024 08:18
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