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

Add NodeletLazy abstract class for lazy transport #45

Merged
merged 6 commits into from Sep 19, 2016

Conversation

Projects
None yet
2 participants
@wkentaro
Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2016

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 wkentaro force-pushed the wkentaro:nodelet-lazy branch from fded192 to 36c4eca Aug 10, 2016

@wkentaro wkentaro force-pushed the wkentaro:nodelet-lazy branch from 36c4eca to 3178c41 Aug 10, 2016

@wkentaro wkentaro force-pushed the wkentaro:nodelet-lazy branch from 3178c41 to 2172c43 Aug 10, 2016

@wkentaro wkentaro closed this Aug 11, 2016

@wkentaro wkentaro reopened this Aug 11, 2016

@wkentaro

This comment has been minimized.

Copy link
Contributor Author

commented Aug 23, 2016

Any comments, please/

@mikaelarguedas

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2016

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

@wkentaro

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2016

Friendly ping.

@wkentaro

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2016

ping

@mikaelarguedas

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2016

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),

This comment has been minimized.

Copy link
@mikaelarguedas

mikaelarguedas Sep 19, 2016

Contributor

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

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

This comment has been minimized.

Copy link
@mikaelarguedas

mikaelarguedas Sep 19, 2016

Contributor

set flag after subscribe succeeded

{
if (!ever_subscribed_)
{
ever_subscribed_ = true;

This comment has been minimized.

Copy link
@mikaelarguedas

mikaelarguedas Sep 19, 2016

Contributor

same as above


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

This comment has been minimized.

Copy link
@mikaelarguedas

mikaelarguedas Sep 19, 2016

Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2016

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

@wkentaro wkentaro force-pushed the wkentaro:nodelet-lazy branch from e82dc40 to 33e29aa Sep 19, 2016

@wkentaro wkentaro force-pushed the wkentaro:nodelet-lazy branch from 33e29aa to 50c0a10 Sep 19, 2016

@wkentaro

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2016

Thank you for the reviews. I fixed the code.

@mikaelarguedas

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2016

great! thanks for iterating on this

@mikaelarguedas mikaelarguedas merged commit e5cb198 into ros:indigo-devel Sep 19, 2016

3 checks passed

Ipr__nodelet_core__ubuntu_trusty_amd64 Build finished.
Details
Jpr__nodelet_core__ubuntu_trusty_amd64 Build finished.
Details
Kpr__nodelet_core__ubuntu_xenial_amd64 Build finished.
Details
@wkentaro

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2016

Thanks!

@wkentaro wkentaro deleted the wkentaro:nodelet-lazy branch Sep 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.