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

Ros2 crystal #23

Open
wants to merge 33 commits into
base: ros2-crystal
Choose a base branch
from

Conversation

vandanamandlik
Copy link

@vandanamandlik vandanamandlik commented Jan 7, 2019

  • ros2-crystal branch is created from ros2-devel.

  • Then I did following two changes for crystal

    Replaced manual conversion of nanoseconds to seconds with function seconds() in message_filter.hpp

      rclcpp::Time class of Bouncy was not having seconds() function but now its available in crystal so used it here.
    
      vandanamandlik@5dbb984
    

    Removed manually clone and build steps of message_filters and launch package from readme because now it available in crystal, Changed raw pointer to shared_pointer.

      vandanamandlik@4995443
    
      message filter package was not there in bouncy, So in order to build laser assembler
      I was cloning and buiding it from intel message filter repository.
      Now its available in crystal release so that step is skipped in crystal.
    
      launch package was not fully ported in bouncy so there I was using master branch of launch packge repository. But now its available in crystal so this step is also skipped in crystal.
    
      Changed raw pointer to shared pointer in base_assembler.hpp
      subscribe method of intel's message_filters package was accepting raw node pointer only,
      virtual void subscribe(rclcpp::Node * node, const std::string& topic, const rmw_qos_profile_t qos = rmw_qos_profile_default) = 0;
      In crystal subscribe() method can accept shared_pointer so changed raw pointer to shared_pointer in crystal.
    

Could you please review it and give your feedback ?

vandanamandlik and others added 30 commits December 17, 2018 17:44
These files were not there in sensor_msgs package of ros2 so just copied it from ROS1 and ported to ROS2 here.
laser assembler uses convertPointCloudToPointCloud2() function from point_cloud_conversion.h file.
Renamed base_assembler_srv.h to base_assembler_srv.hpp.
Also ported both files to ROS2 as per ROS2 migration guide.
and base_assembler_srv.h file to base_assembler_srv.hpp in previous commit so removed it here.
because message_filter.h file from tf2_ros package of bouncy release was not ported to ROS2.
so picked it from link https://github.com/ros2/geometry2/blob/ros2/tf2_ros/include/tf2_ros/message_filter.h and fixed some erros related to time conversion.
… part of the communication.

Request structure which is the type of the request part of the service
Response structure which is the type of the request part of the service
- src/laser_scan_assembler_srv.cpp
- src/merge_clouds.cpp
- src/point_cloud2_assembler.cpp
- src/point_cloud_assembler.cpp
- src/point_cloud_assembler_srv.cpp
… to ROS2 and also tested it.

non_zero_cloud_test passed successfully.
- Removed commented and unused code.
- Fixed logger issue for bouncy.
- Modified message_filter.hpp to work on bouncy.
…pp' because it is changed in filters package.

- changed string type variable to std::string type.
  - Changed raw pointer to shared pointer in base_assembler.hpp file.
    - Modified README.md file for crystal (Latest changes of message_filters and launch package are available in crystal release,
      We can use it directly. So removed that steps from README.md file)
…le in rclcpp::Time class of crystal release.
  - Added one known issue and future work.
@jonbinney
Copy link
Contributor

@vandanamandlik I think most of the comments on your other PR will transfer over to this one. Once we get that PR merged, I'll do a quick pass over this one.

Updated CMakeLists.txt file from filters package has solved issue.
Setting this flag is not more required.
… in laser_assembler package itself.

Previously there was separate package to generate laser_assembler.srv
…ssembler.cpp files to ros2.

NOTE: These are just ported but not tested as there is no test case to test all these files.
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

Successfully merging this pull request may close these issues.

2 participants