-
Notifications
You must be signed in to change notification settings - Fork 69
Update camera sensor only when needed #37
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
Conversation
Signed-off-by: Ian Chen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but maybe could use a debug statement.
I did two quick tests:
Could you double-check and let me know how are you testing it? |
Signed-off-by: Ian Chen <[email protected]>
I'm testing with the I did the same with |
I just tried it again and it seems to work as expected. This is the output when the plugin is enabled:
And this is the output when I close the plugin:
|
Great, it's working as it should. The problem could just be happening on my machine. @mjcarroll are you able to help do a quick test to confirm? If you see the msg |
This works for me. |
This is the exact behavior I observed building citadel packages from source locally. LGTM. |
Updated camera sensor to render and generate data only if there are subscribers or if it needs to save data to disk. This avoid unnecessary computation cost when the camera sensor is idle.
While testing I noticed that
this->dataPtr->pub.HasConnections()
reports correct value when I subscribe usingign topic -e -t
. However, if I test with theImageDisplay
plugin in ign-gazebo, theHasConnections
will always return true even after the plugin is closed. I verified that the ImageDisplay's destructor is called so maybe we're doing something wrong? cc @cagueroSigned-off-by: Ian Chen [email protected]