Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Add NodeletLazy abstract class for lazy transport#45

Merged
mikaelarguedas merged 6 commits intoros:indigo-develfrom
wkentaro:nodelet-lazy
Sep 19, 2016
Merged

Add NodeletLazy abstract class for lazy transport#45
mikaelarguedas merged 6 commits intoros:indigo-develfrom
wkentaro:nodelet-lazy

Conversation

@wkentaro
Copy link
Copy Markdown
Contributor

@wkentaro wkentaro commented Aug 10, 2016

This class is to crete nodelet for 'lazy' transport.
Class for Python node is already merged in kinetic-devel branch of topic_tools. ros/ros_comm#713

Lazy transport is an essential feature for perception nodelets that handle image and point cloud, because
the processing should run only when it is necessary to reduce the CPU load.

FYI, the similar nodelet abstract class is used in ros-perception/opencv_apps package. https://github.com/ros-perception/opencv_apps/blob/indigo/include/opencv_apps/nodelet.h

@wkentaro
Copy link
Copy Markdown
Contributor Author

Maintainers, could you please review this?
Possibly, the first step to view how it works is below:

cd test_nodelet_topic_tools
catkin build --this
rostest test/test_nodelet_lazy.launch --text

@wkentaro
Copy link
Copy Markdown
Contributor Author

Any comments, please/

@mikaelarguedas
Copy link
Copy Markdown
Member

sorry for the late reply, will review this shortly.
Thanks!

@wkentaro
Copy link
Copy Markdown
Contributor Author

wkentaro commented Sep 9, 2016

Friendly ping.

@wkentaro
Copy link
Copy Markdown
Contributor Author

ping

@mikaelarguedas
Copy link
Copy Markdown
Member

finally got back decent internet, will review this today, sorry for the very long wait

// timer to warn when no connection in a few seconds
ever_subscribed_ = false;
timer_ever_subscribed_ = nh_->createWallTimer(
ros::WallDuration(5),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to expose this timer as a ros param ?

if (!lazy_)
{
boost::mutex::scoped_lock lock(connection_mutex_);
ever_subscribed_ = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

set flag after subscribe succeeded

{
if (!ever_subscribed_)
{
ever_subscribed_ = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same as above


/** @brief
* Advertise a topic and watch the publisher. Publishers which are
* created by this method.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rephrase this as a single sentence? the meaning of "Publishers which are created by this method" is unclear to me. Did you mean "update the list of Publishers created by this method" ?

@mikaelarguedas
Copy link
Copy Markdown
Member

@wkentaro Code looks good to me overall, a few nicktip comments to address/clarify but all of them are cosmetics.

@wkentaro
Copy link
Copy Markdown
Contributor Author

wkentaro commented Sep 19, 2016

Thank you for the reviews. I fixed the code.

@mikaelarguedas
Copy link
Copy Markdown
Member

great! thanks for iterating on this

@mikaelarguedas mikaelarguedas merged commit e5cb198 into ros:indigo-devel Sep 19, 2016
@wkentaro
Copy link
Copy Markdown
Contributor Author

Thanks!

@wkentaro wkentaro deleted the nodelet-lazy branch September 19, 2016 21:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants