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

[WIP] Porting ros2 #83

Closed
wants to merge 194 commits into from
Closed

[WIP] Porting ros2 #83

wants to merge 194 commits into from

Conversation

hsd-dev
Copy link
Contributor

@hsd-dev hsd-dev commented Jun 9, 2022

Originally from #82 started by @wawanbreton

@hsd-dev hsd-dev changed the title Porting ros2 [WIP] Porting ros2 Jun 9, 2022
@hsd-dev hsd-dev mentioned this pull request Jun 9, 2022
8 tasks
@wawanbreton
Copy link

I got permission denied when pushing to this branch, can you give me some rights ?

else if (level == 23)
{
config_->max_num_points_scan = config.max_num_points_scan;
for(const auto &parameter : parameters)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dynamic parameters are indeed much simpler in ROS2 !

@hsd-dev hsd-dev force-pushed the porting-ros2 branch 2 times, most recently from 20e959b to 939eedd Compare June 9, 2022 09:54
@hsd-dev
Copy link
Contributor Author

hsd-dev commented Jun 9, 2022

The current code seems to be valid only for foxy. We might need to have separate branch for galactic and upwards https://github.com/PepperlFuchs/pf_lidar_ros_driver/runs/6810174179?check_suite_focus=true#step:4:685

For now, I will disable galactic and upwards

@wawanbreton
Copy link

The current code seems to be valid only for foxy. We might need to have separate branch for galactic and upwards

What makes you say that ? I'm working with galaxy only

@hsd-dev
Copy link
Contributor Author

hsd-dev commented Jun 9, 2022

@wawanbreton please check the CI output linked above. Didn't you get that error?

Edit: the error exists for foxy as well actually https://github.com/PepperlFuchs/pf_lidar_ros_driver/runs/6810315345?check_suite_focus=true

@wawanbreton
Copy link

No I didn't get the error, pcl-conversion is actually available for galactic

@hsd-dev
Copy link
Contributor Author

hsd-dev commented Jun 9, 2022

right, we have commented out the deps https://github.com/PepperlFuchs/pf_lidar_ros_driver/blob/porting-ros2/pf_driver/package.xml#L13. Since CI starts with a clean docker, these packages have not been installed.

@hsd-dev
Copy link
Contributor Author

hsd-dev commented Jun 9, 2022

@wawanbreton
Copy link

My commit containing the DAE files instead of the STL has been wiped out, was it intentional ?

@hsd-dev
Copy link
Contributor Author

hsd-dev commented Jun 9, 2022

sorry about that. I think I pushed commits around the same time as you. Could you fetch the latest commits, rebase and push that commit again?

@hsd-dev
Copy link
Contributor Author

hsd-dev commented Jun 9, 2022

Also, take a look at the CI output linked above

/root/target_ws/src/pf_lidar_ros_driver/pf_driver/src/pf/pf_interface.cpp:220:25: error: missing template arguments before ‘feed_time’
    220 |   std::chrono::duration feed_time = std::min(duration, std::chrono::milliseconds(std::chrono::seconds(60)));
        |                         ^~~~~~~~~
  /root/target_ws/src/pf_lidar_ros_driver/pf_driver/src/pf/pf_interface.cpp:221:46: error: ‘feed_time’ was not declared in this scope
    221 |   watchdog_timer_ = node_->create_wall_timer(feed_time, std::bind(&PFInterface::feed_watchdog, this));

@ptruka
Copy link
Contributor

ptruka commented Nov 17, 2023

Many thanks to everyone for their support!
Due to the general changes to the code and the resulting differences to the ros1 driver, a repository for ros2 was created:
https://github.com/PepperlFuchs/pf_lidar_ros2_driver

@ptruka ptruka closed this Nov 17, 2023
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.

6 participants