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

image_rotate: clean up #862

Merged
merged 6 commits into from
Jan 19, 2024
Merged

Conversation

mikeferguson
Copy link
Member

This is the first component/node with a cleanup pass to be fully implemented:

  • Fix ros2 version: image_rotate does not initialize the target and source _vectors in constructor #740 by initializing vectors. Do this by declaring parameters AFTER we define the callback
  • Implement lazy subscribers (I missed this in the earlier PRs)
  • Add image_transport parameter so we can specify that desired transport of our subscriptions
  • Update how we test for connectivity (and update the debug message - has nothing to do with whether we are remapped, it's really about whether we are connected)

"Topic 'image' has not been remapped! Typical command-line usage:\n"
"\t$ ros2 run image_rotate image_rotate image:=<image topic>");
return 1;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This debug message didn't work at all - made a more robust version by checking the overlap of our topic name with the actual topics that exist (as many of our depth_image_proc stuff does)

if (subscriber_count_) { // @todo: Could do this without an interruption at some point.
unsubscribe();
subscribe();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

this is unneeded - none of the parameters that are able to be updated would impact how the subscribers would actually be setup

@mikeferguson mikeferguson requested a review from ahcorde January 19, 2024 03:24
@mikeferguson mikeferguson changed the title image_rotate clean up image_rotate: clean up Jan 19, 2024
@mikeferguson mikeferguson merged commit 21e5f25 into ros-perception:rolling Jan 19, 2024
3 checks passed
@mikeferguson mikeferguson deleted the fix_740 branch January 19, 2024 14:17
Kotochleb pushed a commit to Kotochleb/image_pipeline that referenced this pull request May 27, 2024
This is the first component/node with a cleanup pass to be fully
implemented:

* Fix ros-perception#740 by initializing vectors. Do this by declaring parameters
AFTER we define the callback
 * Implement lazy subscribers (I missed this in the earlier PRs)
* Add image_transport parameter so we can specify that desired transport
of our subscriptions
* Update how we test for connectivity (and update the debug message -
has nothing to do with whether we are remapped, it's really about
whether we are connected)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ros2 version: image_rotate does not initialize the target and source _vectors in constructor
2 participants