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

eloquent version doesn't work #1336

Closed
JaehyunShim opened this issue Aug 19, 2020 · 11 comments
Closed

eloquent version doesn't work #1336

JaehyunShim opened this issue Aug 19, 2020 · 11 comments

Comments

@JaehyunShim
Copy link

Hi,

I tested the eloquent branch and below didn't work.
$ ros2 launch realsense2_camera rs.launch.py

There are two parts that have to be fixed.

  1. install location for launch
  1. in CMakeLists.txt
    install(TARGETS ${PROJECT_NAME}
    DESTINATION lib/${PROJECT_NAME}
    )
  1. realsense2_camera is not an executable (but a library). Make it an executable.
  1. in CMakeListst.txt
    rclcpp_components_register_node(${PROJECT_NAME}
    PLUGIN "realsense2_camera::RealSenseNodeFactory"
    EXECUTABLE realsense2_camera_exec)

install(TARGETS ${PROJECT_NAME}
DESTINATION lib/${PROJECT_NAME}
)
2) in rs.launch.py
launch_ros.actions.Node(
package='realsense2_camera', node_executable='realsense2_camera_exec',
output='screen',
emulate_tty=True,
),

Ryan

@RealSenseSupport
Copy link
Collaborator

Hi @rjshim,

Thank you for your valuable remarks. As you probably have noticed, wrapper for Eloquent is at early stages of development. Therefore, many of the features are still not implemented and it would be more helpful to submit these not implemented parts as PR and not as issues. Would you like to submit PRs for the two points you have mentioned?
Please let us know if you need additional assistance with this matter.

@JaehyunShim
Copy link
Author

@RealSenseCustomerSupport

Yeah, I can make some contribution to this repository but there are several points that I want you to note. If you agree, I will fork this repository and get to work on modification.

  1. realsense2_node package is redundant (and can be removed). There is no need for a new package just to make an executable. I will merge the source code with the main function in the package to the realsense_camera package so the executable can be simply run by the rs.launch.py file.
  2. realsense_camera_msgs can be (or better be) renamed to realsense2_camera_msgs. The current naming causes unnecessary misunderstanding.

On the other hand, if possible, I hope Intel to support the following points as soon as possible.

  1. Install manual for the driver-level library is confusing. Can I get any help with this?
  2. As of August 9, 2020, Xacro works in ROS2. realsense_description package can also be added now!
  3. Multiple packages developed and maintained by different teams within the company (link) cause confusion to users.

Ryan

@RealSenseSupport
Copy link
Collaborator

@rjshim,

Yes, it works the way you've described. After you done with the changes in your repository you're submitting the request. Our developers will review it and will decide whether or not the changes you have made should be merged into official branch.
Regarding your questions about the Install manual, realsense_description package and ros2_intel_realsense repository: since the ROS2 Wrapper is a relatively new project, there are parts that still were not implemented and parts that are still there for historical reasons only. As the project evolves, necessary changes will be made for these parts. As of this moment, there is no specified date of addressing the matters you have mentioned.

Thank you!

@doronhi
Copy link
Contributor

doronhi commented Aug 24, 2020

@rjshim , thank you so very much for your helpful remarks.
As was mentioned, this branch is under heavy construction right now. As a matter of fact, the next commit may change it altogether.
For that reason, as opposed to contributions to the development branch, I do not encourage PRs right now. I Do encourage helpful remarks of the sort that you just made and I'll take them into consideration.
This branch is published for that reason - to have the useful remarks during the development process but this is definitely not a working version yet.
You can also follow the helpful notes I got so far here
Thanks again and please feel free to comment wherever you find more convenient.

@JaehyunShim
Copy link
Author

JaehyunShim commented Aug 25, 2020

@RealSenseCustomerSupport @doronhi

Thank you for the response. Then, I will wait for the next update as @doronhi said.

As an active user of Realsense products, I strongly agree with the points that are discussed in the link you attached.
I hope to see the next update asap (we bought a bunch of D435i models recently expecting to replace imu sensors with them).
I also recommend you directly get to work on Foxy as Eloquent will be EOL soon (yet, there are no big changes you need to make to switch from Eloquent to Foxy).

Thanks!
Ryan

@doronhi
Copy link
Contributor

doronhi commented Sep 13, 2020

@RealSenseCustomerSupport

Yeah, I can make some contribution to this repository but there are several points that I want you to note. If you agree, I will fork this repository and get to work on modification.

  1. realsense2_node package is redundant (and can be removed). There is no need for a new package just to make an executable. I will merge the source code with the main function in the package to the realsense_camera package so the executable can be simply run by the rs.launch.py file.
  2. realsense_camera_msgs can be (or better be) renamed to realsense2_camera_msgs. The current naming causes unnecessary misunderstanding.

On the other hand, if possible, I hope Intel to support the following points as soon as possible.

  1. Install manual for the driver-level library is confusing. Can I get any help with this?
  2. As of August 9, 2020, Xacro works in ROS2. realsense_description package can also be added now!
  3. Multiple packages developed and maintained by different teams within the company (link) cause confusion to users.

Ryan

@rjshim , Contrary to my original plan, the version you referred to in your first comment will most probably be the basis for the first eloquent (or foxy) release. I think that regardless to what kind of version it will be, your contribution will be most beneficial either way. Therefor, contrary to my previous remark, I am very much interested in viewing and considering your suggestion and I think a PR is the most convenient way to achieve that.

@RealSenseSupport
Copy link
Collaborator

@rjshim,

As @doronhi has mentioned, can you please submit PRs for the features you have mentioned in this GH issue and close it?

Thank you!

@JaehyunShim
Copy link
Author

JaehyunShim commented Sep 23, 2020

@RealSenseSupport @doronhi

These are the steps that I suggest you follow and work on this repository together.

  1. Make a new branch for devel codes (codes for the software you are currently developing)
    The first problem is that this repository uses one branch (which is 'eloquent') for released codes and devel codes. A new norm adopted by many ROS oriented open-source projects (ref1, ref2) is that you use the 'master' branch for devel codes and make 'version-devel' branches for released codes. However, this repository also contains ROS1 codes so I suggest you make 'ros2' branch instead of 'master' and upload newly updated codes there (ref). Also you can also change the branch name 'development' to 'master' too. You will only need to merge the updated codes in the 'ros2' branch to each 'version-devel' branch when you are done with testing it and ready to binary-release it (ref). By following this norm, users wouldn't have to waste their time on debugging the codes in the 'ros2'branch' which is still under development.

  2. As you said I removed realsense2_node and merged it to realsense2_camera. Also, I changed realsense_camera_msgs to realsense2_camera_msgs. Once you make 'ros2' branch, I will fork it and send a pull request with the modified codes.

Regards,
Ryan

@RealSenseSupport
Copy link
Collaborator

@JaehyunShim Your PR now merged into eloquent branch. Thanks very much for your sharing! And can we close this accordingly?

@JaehyunShim
Copy link
Author

@RealSenseSupport

Thank you :)

@liushuya7
Copy link

"No Image" in Rviz2
Screenshot from 2020-11-24 23-57-52
I built this package on ROS2 foxy Ubuntu 20.04. However, there is no image showing from the /color/image_raw topic. I tried it on Ubuntu 18.04 with ROS2 Eloquent, it did stream the image topic but lagging heavily. Has anyone successfully run this package on ROS2 foxy Ubuntu 20.04?

It is solved. The problem is due to "Reliability Policy". Refer to: ROS2 Image subscriber

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

No branches or pull requests

4 participants