ifm / libo3d3xx

Driver and utilities for IFM Efector O3D3xx depth cameras
Apache License 2.0
38 stars 39 forks source link

Documentation Bugs #110

Closed KlausKohlmann closed 4 years ago

KlausKohlmann commented 6 years ago

At the moment a student is working with the lib and ROS and he gaves me the following feedback to the documentation:

• If libo3d3xx is already written in prerequisite then it is not required to install that again and no need to build it again

"Next, you should be sure to install libo3d3xx. This ROS package assumes you have installed libo3d3xx via the supported debian installer. Step-by-step instructions for that process now follows: $ git clone https://github.com/lovepark/libo3d3xx.git $ cd libo3d3xx $ mkdir build $ cd build $ cmake .. $ make"

Instead of these we can check our camera status using $ o3d3xx-ls

• In ROS kinetic all the directories of launch files are under /opt/ros/kinetic/share where as in documentation it creats in ~/ros due to which roslaunch cannot recognize the launch file so there should be respective changes required in documentation and ~/.bashrc.

• In the documentaion its written that we need to make changes in ~/.bash_profile which does not exist so we should update it to new file ~/.bashrc.

tpanzarella commented 6 years ago

Hi Klaus,

I have a few general comments about this, then I will try to address each point in turn:

  1. I think these comments are really directed toward the o3d3xx-ros project and not this one. It would have been more appropriate to file this issue there. No need to do that now though, we know what we are talking about here.
  2. Especially for newer users, I highly encourage everyone who is just starting out with the camera to prefer ifm3d and ifm3d-ros. As of today, the only reason I point people to libo3d3xx is if the plan is to run on-camera algos. This is for two reasons: i) we do not currently have a camera image with ifm3d embedded; ii) there is no libsync integration with ifm3d (yet). When new users want to get started with the newer ifm3d stack, we can point them to step-by-step installation instructions for both Ubuntu 14.04 + ROS Indigo or Ubuntu 16.04 + ROS Kinetic. In both cases, the only assumption is a clean install of the OS. Nothing else. If the user follows the instructions verbatim, they should not run into problems.

OK, the above being said, let me try to address the comments from above:

• If libo3d3xx is already written in prerequisite then it is not required to install that again and no need to build it again

"Next, you should be sure to install libo3d3xx. This ROS package assumes you have installed libo3d3xx via the supported debian installer. Step-by-step instructions for that process now follows: $ git clone https://github.com/lovepark/libo3d3xx.git $ cd libo3d3xx $ mkdir build $ cd build $ cmake .. $ make"

Instead of these we can check our camera status using $ o3d3xx-ls

Sure. I assumed it to be implied that if you already have the libo3d3xx software installed, you would not install it again. However, for those that did not have it installed, for convenience some quick-start shell commands were provided. I have removed that from the README on the head in o3d3xx-ros. The changes can be found here.

• In ROS kinetic all the directories of launch files are under /opt/ros/kinetic/share where as in >documentation it creats in ~/ros due to which roslaunch cannot recognize the launch file so there >should be respective changes required in documentation and ~/.bashrc.

Users are free to install the packages wherever they would like. This is the role of -DCMAKE_INSTALL_PREFIX=.... With respect to this comment in particular, I would not encourage users to install these packages in the ROS distribution directories as is suggested but rather put them where they install their custom built stuff or in their current catkin workspace. The instructions provide one way to do it, but it is not meant to be prescriptive. Indeed, the text surrounding this part of the installation instructions elucidate this a bit more.

It is certainly a goal in the near term to get ifm3d and ifm3d-ros included as part of the core ROS distribution so all of this becomes moot. But, I do not currently have the bandwidth for this at the moment.

• In the documentaion its written that we need to make changes in ~/.bash_profile which does not exist so we should update it to new file ~/.bashrc.

So, some baseline Linux knowledge is assumed by our users. I tend to prefer ~/.bash_profile over ~/.bashrc because for years, I have run my terminals as login shells and so, ~/.bash_profile is the natural place to run login scripts. If a user does not run their terminal as a login shell, ~/.bashrc is fine. It should be noted, that we cannot cover all scenarios here and, as stated earlier, some Linux knowledge will be necessary for our users. For example, the choice of ~/.bash_profile vs. ~/.bashrc assumes bash. We certainly have users that prefer different shells (e.g., csh, tcsh, zsh, ksh) and they would need to adapt these instructions for their particular shell environment as well.

Hopefully, some of the above elucidates some issues. Beyond the change I made above regarding the o3d3xx-ros build instructions. Do you see any other "calls to action" here?

KlausKohlmann commented 6 years ago

Hello Tom,

Thanks for your feedback. The goal was that you get the feedback which our student gave us. So we are not so deep in ROS and the statement from Christian was also that you should know that.

So from my point of view no additionally action is necessary.

Thanks

Klaus

Von: Tom Panzarella [mailto:notifications@github.com] Gesendet: Mittwoch, 25. Oktober 2017 15:23 An: lovepark/libo3d3xx libo3d3xx@noreply.github.com Cc: Kohlmann, Klaus klaus.kohlmann@ifm.com; Author author@noreply.github.com Betreff: Re: [lovepark/libo3d3xx] Documentation Bugs (#110)

Hi Klaus,

I have a few general comments about this, then I will try to address each point in turn:

  1. I think these comments are really directed toward the o3d3xx-roshttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_o3d3xx-2Dros&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=dswDbyrC54YNbhcGUN2Jy1oGHgHYgrT1ZeOeaFWo71Q&e= project and not this one. It would have been more appropriate to file this issue there. No need to do that now though, we know what we are talking about here.
  2. Especially for newer users, I highly encourage everyone who is just starting out with the camera to prefer ifm3dhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_ifm3d&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=8FmMQL8rSsioXMtFPhsGroktkMSRwtDxvPJhbdH4YKE&e= and ifm3d-roshttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_ifm3d-2Dros&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=6inOPofgpPKFTgjCq2WRU9ulXcEkoAf7g_QC5ZA6Huw&e=. As of today, the only reason I point people to libo3d3xx is if the plan is to run on-camera algos. This is for two reasons: i) we do not currently have a camera image with ifm3d embedded; ii) there is no libsync integration with ifm3d (yet). When new users want to get started with the newer ifm3d stack, we can point them to step-by-step installation instructions for both Ubuntu 14.04 + ROS Indigohttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_ifm3d-2Dros_blob_master_doc_indigo.md&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=qhKzAm5vzco0RAgRbBqLzT-0u_k1Q1Gqu02ekHglaC4&e= or Ubuntu 16.04 + ROS Kinetichttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_ifm3d-2Dros_blob_master_doc_kinetic.md&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=LKS3wuR_5Q-MikW8wlKYcLvI4YjbuGmS461vABR3r_0&e=. In both cases, the only assumption is a clean install of the OS. Nothing else. If the user follows the instructions verbatim, they should not run into problems.

OK, the above being said, let me try to address the comments from above:

• If libo3d3xx is already written in prerequisite then it is not required to install that again and no need to build it again

"Next, you should be sure to install libo3d3xx. This ROS package assumes you have installed libo3d3xx via the supported debian installer. Step-by-step instructions for that process now follows: $ git clone https://github.com/lovepark/libo3d3xx.githttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_libo3d3xx.git&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=Mi3aJ665AbkAVhIdu-dgWGHuH0X7CjfOXiHgyGwKq-Y&e= $ cd libo3d3xx $ mkdir build $ cd build $ cmake .. $ make"

Instead of these we can check our camera status using $ o3d3xx-ls

Sure. I assumed it to be implied that if you already have the libo3d3xx software installed, you would not install it again. However, for those that did not have it installed, for convenience some quick-start shell commands were provided. I have removed that from the README on the head in o3d3xx-ros. The changes can be found herehttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_o3d3xx-2Dros-23building-2Dand-2Dinstalling-2Dthe-2Dsoftware&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=Zoh4_Pp51-lUpfKIeOBnfy2UzwUPhNIDhDunLuA94vk&e=.

• In ROS kinetic all the directories of launch files are under /opt/ros/kinetic/share where as in >documentation it creats in ~/ros due to which roslaunch cannot recognize the launch file so there >should be respective changes required in documentation and ~/.bashrc.

Users are free to install the packages wherever they would like. This is the role of -DCMAKE_INSTALL_PREFIX=.... With respect to this comment in particular, I would not encourage users to install these packages in the ROS distribution directories as is suggested but rather put them where they install their custom built stuff or in their current catkin workspace. The instructions provide one way to do it, but it is not meant to be prescriptive. Indeed, the text surrounding this part of the installation instructions elucidate this a bit more.

It is certainly a goal in the near term to get ifm3d and ifm3d-ros included as part of the core ROS distribution so all of this becomes moot. But, I do not currently have the bandwidth for this at the moment.

• In the documentaion its written that we need to make changes in ~/.bash_profile which does not exist so we should update it to new file ~/.bashrc.

So, some baseline Linux knowledge is assumed by our users. I tend to prefer ~/.bash_profile over ~/.bashrc because for years, I have run my terminals as login shells and so, ~/.bash_profile is the natural place to run login scripts. If a user does not run their terminal as a login shell, ~/.bashrc is fine. It should be noted, that we cannot cover all scenarios here and, as stated earlier, some Linux knowledge will be necessary for our users. For example, the choice of ~/.bash_profile vs. ~/.bashrc assumes bash. We certainly have users that prefer different shells (e.g., csh, tcsh, zsh, ksh) and they would need to adapt these instructions for their particular shell environment as well.

Hopefully, some of the above elucidates some issues. Beyond the change I made above regarding the o3d3xx-ros build instructions. Do you see any other "calls to action" here?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lovepark_libo3d3xx_issues_110-23issuecomment-2D339327972&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=aPpgaCKCfKt8fTId0W7jwpoRE9DJphLKAJw65lcqkM0&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AfjKYxgZgqWCHmfOfZDIl5e-5Fd96h8T18ks5svzY2gaJpZM4QFetW&d=DwMFaQ&c=riR7jviByh3sGm7GIiSlHkFN0_aSATB6A8x0nHa2EM0&r=Q-BDszNuWHMY_PJdS59HrPyomrwJSUshtzOhOo3jW2Q&m=KIZAoHY0yqGXGta0lxhdPCGeRFqedR_d2PTqAGHvWvk&s=oe6V1w5c3_Xu6GQoRV8akP-EqNZxsW6x849j3PRIXlc&e=.

theseankelly commented 4 years ago

Closing out stale issues (older than a year). Feel free to reactivate as appropriate.