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

Remove dependency on jsk_perception for separated build #1820

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

wkentaro
Copy link
Member

@wkentaro wkentaro commented Aug 5, 2016

Some node in jsk_perception is used in the sample launch files in jsk_pcl_ros,
and this is why jsk_perception is added as run_depend.
However, dependency on jsk_perception by jsk_pcl_ros always requires build of
jsk_perception even when the user want to use jsk_pcl_ros only.
I think this is problem, because jsk_perception is also a large package,
and recognition packages for 2D and 3D should be separated.

@k-okada
Copy link
Member

k-okada commented Aug 5, 2016

humm, by this change, ros-indigo-jsk-perception will not donwloaded if you apt-get install ros-indigo-jsk-pcl-ros, and you failed to run sample program.

@wkentaro
Copy link
Member Author

wkentaro commented Aug 5, 2016

humm, by this change, ros-indigo-jsk-perception will not donwloaded if you apt-get install ros-indigo-jsk-pcl-ros, and you failed to run sample program.

Yeah, I think that is problem.
The dependency was added in recent commit by me, so the effect seems small,
but maybe I need to wait for the next major release.
As my general idea, the sample programs which depend on both jsk_pcl_ros and jsk_perception should be located in another package like jsk_recognition_samples, or jsk_recognition_tutorials. Do you have any idea? @k-okada

@k-okada
Copy link
Member

k-okada commented Aug 6, 2016

humm, I thought adding jsk_perception to test_depends, but it seems that
dependency is removed from debs

https://github.com/ros-perception/opencv_apps-release/blob/release/indigo/opencv_apps/package.xml

$ aptitude show ros-indigo-opencv-apps
Package: ros-indigo-opencv-apps
New: yes
State: installed
Automatically installed: no
Version: 1.11.13-0trusty-20160713-022500-0700
Priority: extra
Section: misc
Maintainer: Kei Okada <[email protected]>
Architecture: amd64
Uncompressed Size: 3,825 k
Depends: libboost-system1.54.0, libc6 (>= 2.14), libconsole-bridge0.2,
libgcc1 (>= 1:4.1.1), libopencv-core2.4,
         libopencv-highgui2.4, libopencv-imgproc2.4,
libopencv-objdetect2.4, libopencv-video2.4, libstdc++6 (>=
         4.2.1), ros-indigo-cv-bridge, ros-indigo-dynamic-reconfigure,
ros-indigo-image-transport,
         ros-indigo-message-runtime, ros-indigo-nodelet, ros-indigo-roscpp,
ros-indigo-std-msgs,
         ros-indigo-std-srvs
Conflicts: ros-indigo-opencv-apps
Description: The opencv_apps package, most of code is taken from
https://github.com/Itseez/opencv/tree/master/samples/cpp

◉ Kei Okada

On Fri, Aug 5, 2016 at 5:22 PM, Kentaro Wada [email protected]
wrote:

humm, by this change, ros-indigo-jsk-perception will not donwloaded if you
apt-get install ros-indigo-jsk-pcl-ros, and you failed to run sample
program.

Yeah, I think that is problem.
The dependency was added in recent commit by me
#1426, so the effect
seems small,
but maybe I need to wait for the next major release.
As my general idea, the sample programs which depend on both jsk_pcl_ros
and jsk_perception should be located in another package like
jsk_recognition_samples, or jsk_recognition_tutorials. Do you have any
idea? @k-okada https://github.com/k-okada


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1820 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAeG3CfroICg6OdBC-dqPofv_-7VH1HYks5qcvK4gaJpZM4JdUv7
.

@wkentaro
Copy link
Member Author

$ aptitude show ros-indigo-opencv-apps

Why you are referring to opencv_apps?

@k-okada
Copy link
Member

k-okada commented Aug 11, 2016

just to confirm that
https://github.com/ros-perception/opencv_apps/blob/indigo/package.xml has
test_depends packages

  <test_depend>rostest</test_depend>
  <test_depend>rosbag</test_depend>
  <test_depend>rosservice</test_depend>
  <test_depend>rostopic</test_depend>
  <test_depend>image_proc</test_depend>
  <test_depend>image_view</test_depend>
  <test_depend>topic_tools</test_depend>

but that is not included in deb depends

$ aptitude show ros-indigo-opencv-apps
Package: ros-indigo-opencv-apps
New: yes
State: installed
Automatically installed: no
Version: 1.11.13-0trusty-20160713-022500-0700
Priority: extra
Section: misc
Maintainer: Kei Okada <[email protected]>
Architecture: amd64
Uncompressed Size: 3,825 k
Depends: libboost-system1.54.0, libc6 (>= 2.14), libconsole-bridge0.2,
libgcc1
         (>= 1:4.1.1), libopencv-core2.4, libopencv-highgui2.4,
         libopencv-imgproc2.4, libopencv-objdetect2.4, libopencv-video2.4,
         libstdc++6 (>= 4.2.1), ros-indigo-cv-bridge,
         ros-indigo-dynamic-reconfigure, ros-indigo-image-transport,
         ros-indigo-message-runtime, ros-indigo-nodelet, ros-indigo-roscpp,
         ros-indigo-std-msgs, ros-indigo-std-srvs
Conflicts: ros-indigo-opencv-apps
Description: The opencv_apps package, most
``

--
◉ Kei Okada


On Wed, Aug 10, 2016 at 1:56 PM, Kentaro Wada <[email protected]>
wrote:

> $ aptitude show ros-indigo-opencv-apps
>
> Why you are referring to opencv_apps?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/jsk-ros-pkg/jsk_recognition/pull/1820#issuecomment-238765897>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAeG3I086BwCMQnI0RunsSvm1AG696bbks5qeVoQgaJpZM4JdUv7>
> .
>

@wkentaro
Copy link
Member Author

It is described as below here:
http://www.ros.org/reps/rep-0140.html#test-depend-multiple

Generated Debian packages are built without the unit tests or their dependencies.

@wkentaro
Copy link
Member Author

humm, I thought adding jsk_perception to test_depends

If the jsk_perception is added as test_depends, the build of jsk_perception will be skipped when building jsk_pcl_ros?

@k-okada
Copy link
Member

k-okada commented Aug 12, 2016

i thought adding jsk_perception to test_depends is ok for you

  1. download jsk_perception source tree with wstools
  2. do not compile jsk_perception when compile jsk_pcl_ros

but it does not bring jsk_preception when you apt-get jsk_pcl_ros
btw which node in jsk_perception depends on test code of jsk_pcl_ros?

◉ Kei Okada

On Thu, Aug 11, 2016 at 4:43 PM, Kentaro Wada [email protected]
wrote:

humm, I thought adding jsk_perception to test_depends

If the jsk_perception is added as test_depends, the build of
jsk_perception will be skipped when building jsk_pcl_ros?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1820 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAeG3D7TGKeOWEs7nf5wnJ6LsRtZydEGks5qetK1gaJpZM4JdUv7
.

@wkentaro
Copy link
Member Author

wkentaro commented Aug 30, 2016

There are no tests in jsk_pcl_ros that depend on jsk_perception but launch files.
https://github.com/jsk-ros-pkg/jsk_recognition/blob/master/jsk_pcl_ros/launch/door_handle_detection.launch#L72

@k-okada k-okada mentioned this pull request Aug 30, 2016
9 tasks
@k-okada
Copy link
Member

k-okada commented Sep 6, 2016

if you still need this, I'll merge, I suspect we'll revert this in future, but that's ok

  1. For me waiting 25 min and 35 min is not so different, or is this critical if we move to Travis?
    screenshot from 2016-09-06 14 15 56
  2. i think 3D perception has there categories, 2D Depth processing, 2.5D Point Cloud processing, 3D Point Cloud processing, and 2D Depth processing can build on top of 2D Image Processing, which we build within jsk_perception packages, so I think jsk_pcl_ros and jsk_perception is not separatable.

@wkentaro
Copy link
Member Author

wkentaro commented Sep 6, 2016

  1. For me waiting 25 min and 35 min is not so different, or is this critical if we move to Travis?

Actually, it fails because of timeout even if we separate these packages, so moving to Travis seems impossible at this time.
#1810

  1. i think 3D perception has there categories, 2D Depth processing, 2.5D Point Cloud processing, 3D Point Cloud processing, and 2D Depth processing can build on top of 2D Image Processing, which we build within jsk_perception packages, so I think jsk_pcl_ros and jsk_perception is not separatable.

I see, I agree that separating completely 2D/3D processing seems impossible, but I think that does not indicate jsk_pcl_ros and jsk_perception are not separable. I just prefer dependency graph like below, because both jsk_pcl_ros and jsk_perception are very large package.

In the figure, I use purple that depends on both 2D/3D packages, so I agree that it is too difficult to separate `jsk_recognition_utils` to 2D and 3D completely, but I think separating `jsk_pcl_ros` and `jsk_perception` is not so difficult.

@k-okada k-okada merged commit 00b136b into jsk-ros-pkg:master Sep 6, 2016
@k-okada
Copy link
Member

k-okada commented Sep 6, 2016

ok, merged this PR, but please do not hesitate to write code depend on jsk_perception, as we discussed these days, 2D image processing has been long history on research and if we can write 3D perception code based on 2D depth image, we can make use of algorithm developed in 2D image processing, i.e. deep learning image segmentation, so at this moment , what we should do is how to find the 2D image processing algorithm that used in Depth or 2.5D image processing or find completely new 3D algorithm

@wkentaro
Copy link
Member Author

wkentaro commented Sep 6, 2016

ok, merged this PR, but please do not hesitate to write code depend on jsk_perception, as we discussed these days, 2D image processing has been long history on research and if we can write 3D perception code based on 2D depth image, we can make use of algorithm developed in 2D image processing, i.e. deep learning image segmentation, so at this moment , what we should do is how to find the 2D image processing algorithm that used in Depth or 2.5D image processing or find completely new 3D algorithm
Delete branch

I see. Thank you for the advice.

But is that ok to merge this before releasing 0.3.22?
It seems better to release this at 0.4.0: #1854

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants