feat(cmake): make colcon build fast again #146
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I tested this PR with the following launch configurations, with PCAPs:
I currently don"t have any |
|
I ran the command without a pcap and there were no issues, does that error only happen when you start replaying the pcap? |
|
@knzo25 Yes, for Robosense, the InfoDecoder only gets instantiated once the first pointcloud packet has been received.. |
|
@knzo25 It still seems to occur after the fix:
demangles to |
|
Aah, the monitor. I only tested the driver itself by forcing a the initialization in the constructor. I will fix it in the next commit |
|
Fixed in a0ef63b |
|
@knzo25 Now there's no unknown symbol error but a segfault: [Unknown/Just-In-Time compiled code] (Unknown Source:0)
libnebula_hw_interfaces_robosense.so!std::this_thread::sleep_for<long, std::ratio<1l, 1l> >() (/usr/include/c++/11/bits/this_thread_sleep.h:82)
libnebula_hw_interfaces_robosense.so!nebula::drivers::RobosenseHwInterface::InfoInterfaceStart(nebula::drivers::RobosenseHwInterface * const this) (/home/maxschmeller/nebula-knzo/src/nebula_hw_interfaces/src/nebula_robosense_hw_interfaces/robosense_hw_interface.cpp:144)
librobosense_hw_ros_wrapper.so!nebula::ros::RobosenseHwInterfaceRosWrapper::StreamStart(nebula::ros::RobosenseHwInterfaceRosWrapper * const this) (/home/maxschmeller/nebula-knzo/src/nebula_ros/src/robosense/robosense_hw_interface_ros_wrapper.cpp:46)
librobosense_hw_ros_wrapper.so!nebula::ros::RobosenseHwInterfaceRosWrapper::RobosenseHwInterfaceRosWrapper(nebula::ros::RobosenseHwInterfaceRosWrapper * const this, options) (/home/maxschmeller/nebula-knzo/src/nebula_ros/src/robosense/robosense_hw_interface_ros_wrapper.cpp:38)
librobosense_hw_ros_wrapper.so!__gnu_cxx::new_allocator<nebula::ros::RobosenseHwInterfaceRosWrapper>::construct<nebula::ros::RobosenseHwInterfaceRosWrapper, rclcpp::NodeOptions const&>(nebula::ros::RobosenseHwInterfaceRosWrapper * __p) (/usr/include/c++/11/ext/new_allocator.h:160)
librobosense_hw_ros_wrapper.so!std::allocator_traits<std::allocator<nebula::ros::RobosenseHwInterfaceRosWrapper> >::construct<nebula::ros::RobosenseHwInterfaceRosWrapper, rclcpp::NodeOptions const&>(nebula::ros::RobosenseHwInterfaceRosWrapper * __p) (/usr/include/c++/11/bits/alloc_traits.h:516)
librobosense_hw_ros_wrapper.so!std::_Sp_counted_ptr_inplace<nebula::ros::RobosenseHwInterfaceRosWrapper, std::allocator<nebula::ros::RobosenseHwInterfaceRosWrapper>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<rclcpp::NodeOptions const&>(std::_Sp_counted_ptr_inplace<nebula::ros::RobosenseHwInterfaceRosWrapper, std::allocator<nebula::ros::RobosenseHwInterfaceRosWrapper>, (__gnu_cxx::_Lock_policy)2> * const this) (/usr/include/c++/11/bits/shared_ptr_base.h:519)
librobosense_hw_ros_wrapper.so!std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<nebula::ros::RobosenseHwInterfaceRosWrapper, std::allocator<nebula::ros::RobosenseHwInterfaceRosWrapper>, rclcpp::NodeOptions const&>(nebula::ros::RobosenseHwInterfaceRosWrapper *& __p, std::__shared_count<(__gnu_cxx::_Lock_policy)2> * const this) (/usr/include/c++/11/bits/shared_ptr_base.h:650)
librobosense_hw_ros_wrapper.so!std::__shared_ptr<nebula::ros::RobosenseHwInterfaceRosWrapper, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<nebula::ros::RobosenseHwInterfaceRosWrapper>, rclcpp::NodeOptions const&>(std::__shared_ptr<nebula::ros::RobosenseHwInterfaceRosWrapper, (__gnu_cxx::_Lock_policy)2> * const this) (/usr/include/c++/11/bits/shared_ptr_base.h:1342)
librobosense_hw_ros_wrapper.so!std::shared_ptr<nebula::ros::RobosenseHwInterfaceRosWrapper>::shared_ptr<std::allocator<nebula::ros::RobosenseHwInterfaceRosWrapper>, rclcpp::NodeOptions const&>(std::shared_ptr<nebula::ros::RobosenseHwInterfaceRosWrapper> * const this) (/usr/include/c++/11/bits/shared_ptr.h:409)
librobosense_hw_ros_wrapper.so!std::allocate_shared<nebula::ros::RobosenseHwInterfaceRosWrapper, std::allocator<nebula::ros::RobosenseHwInterfaceRosWrapper>, rclcpp::NodeOptions const&>() (/usr/include/c++/11/bits/shared_ptr.h:863)
librobosense_hw_ros_wrapper.so!std::make_shared<nebula::ros::RobosenseHwInterfaceRosWrapper, rclcpp::NodeOptions const&>() (/usr/include/c++/11/bits/shared_ptr.h:879)
librobosense_hw_ros_wrapper.so!rclcpp_components::NodeFactoryTemplate<nebula::ros::RobosenseHwInterfaceRosWrapper>::create_node_instance(rclcpp_components::NodeFactoryTemplate<nebula::ros::RobosenseHwInterfaceRosWrapper> * const this, const rclcpp::NodeOptions & options) (/opt/ros/humble/include/rclcpp_components/rclcpp_components/node_factory_template.hpp:45)
libcomponent_manager.so!rclcpp_components::ComponentManager::on_load_node(std::shared_ptr<rmw_request_id_s>, std::shared_ptr<composition_interfaces::srv::LoadNode_Request_<std::allocator<void> > >, std::shared_ptr<composition_interfaces::srv::LoadNode_Response_<std::allocator<void> > >) (Unknown Source:0)
libcomponent_manager.so!std::_Function_handler<void (std::shared_ptr<rmw_request_id_s>, std::shared_ptr<composition_interfaces::srv::LoadNode_Request_<std::allocator<void> > >, std::shared_ptr<composition_interfaces::srv::LoadNode_Response_<std::allocator<void> > >), std::_Bind<void (rclcpp_components::ComponentManager::*(rclcpp_components::ComponentManager*, std::_Placeholder<1>, std::_Placeholder<2>, std::_Placeholder<3>))(std::shared_ptr<rmw_request_id_s>, std::shared_ptr<composition_interfaces::srv::LoadNode_Request_<std::allocator<void> > >, std::shared_ptr<composition_interfaces::srv::LoadNode_Response_<std::allocator<void> > >)> >::_M_invoke(std::_Any_data const&, std::shared_ptr<rmw_request_id_s>&&, std::shared_ptr<composition_interfaces::srv::LoadNode_Request_<std::allocator<void> > >&&, std::shared_ptr<composition_interfaces::srv::LoadNode_Response_<std::allocator<void> > >&&) (Unknown Source:0)
libcomponent_manager.so![Unknown/Just-In-Time compiled code] (Unknown Source:0)
librclcpp.so!rclcpp::Executor::execute_service(std::shared_ptr<rclcpp::ServiceBase>) (Unknown Source:0)
librclcpp.so!rclcpp::Executor::execute_any_executable(rclcpp::AnyExecutable&) (Unknown Source:0)
librclcpp.so!rclcpp::executors::SingleThreadedExecutor::spin() (Unknown Source:0) |
|
If you need a PCAP for testing, you can take the ones here. Also, the PCAP replay script: Terminal 1 cd nebula_ws
source install/setup.bash
ros2 launch nebula_ros nebula_launch.py sensor_model:=Helios sensor_ip:=127.0.0.1 host_ip:=127.0.0.1 data_port:=2010 gnss_port:=3010Terminal 2 cd pcap_replay/pcap_replay
./replay.py -l path/to/my.pcap |
|
Since has been closed in favor of the already merged PR on |
I will be doing it in a few hours. Btw, try not to close/merge other people PRs without consultation. For the robosense one, since Mehmet wrote the code, I wanted to confirm he had no concerns about that line of code |
The PR closed itself after changing the branch to develop and rebasing (as there were no changes in the branch after rebase), I agree this was not optimal |
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
…nd cleaned unused parts Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
knzo25
force-pushed
the
feat/cmake_optimization
branch
from
a0ef63b
to
dca4fb8
Compare
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
|
@mojomex This PR: Tested with |
knzo25
changed the title
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
|
@mojomex This PR This PR + faster ament Develop Develop + faster ament |
mojomex
approved these changes
Jun 7, 2024
There was a problem hiding this comment.
LGTM!
🟢 Evaluation
🟢 Build Testing
Compiled every package in Nebula with --packages-up-to to see if all dependencies of the respective package are met. Here is the test script used:
pkgs=(
continental_msgs
continental_srvs
nebula_common
nebula_msgs
pandar_msgs
robosense_msgs
nebula_decoders
nebula_hw_interfaces
nebula_tests
nebula_ros
nebula_examples
nebula_sensor_driver
)
for pkg in "${pkgs[@]}"
do
clear
echo "====== TEST BUILDING UP TO $pkg ======"
rm -rf install/ log/ build/
colcon build --symlink-install --packages-up-to "$pkg" || exit
doneResult: All dependencies (at least at build time) were met.
🟢 Running with RosBag/PCAP
For each vendor, I launched Nebula with either PCAP (Hesai, Robosense) or RosBag (Velodyne). There was no data or real sensor available for Continental, so just launched it without test data.
Result: There were no undefined symbols or segfaults.
🟢 Code Review
The duplicate parameter_descriptors.cpp in nebula_examples is not particularly nice but we ideally don't want to link against nebula_ros either since it is not used in any unit tests. Maybe, as a separate PR, we can make parameter descriptors header-only.
mojomex
added a commit
to mojomex/nebula
that referenced
this pull request
Jun 7, 2024
mojomex
added a commit
that referenced
this pull request
Jun 26, 2024
…162) * chore: fix pre-commit versions and clang-format hook failure * chore(docs): fix documentation pre-commit errors * chore: fix pre-commit and compiler warnings in all code files modified between main..develop * chore(yamllint): ignore yaml document start in clang-format config as it is required * chore: add copyright and license headers * chore(pre-commit): allow multiple documents in YAML files to make .clang-format file check pass * chore: make pre-commit pass for parameter schema code * chore: add copyright and license to all source files * chore: implement pre-commit suggestions in all CPP/HPP files * chore: whitespace changes in non-cpp files to satisfy pre-commit * chore: flake8 changes to satisfy pre-commit * fix: allow implicit conversion to status types again * chore: clean up imports * chore: add override/inline where necessary * chore(nebula_ros): remove obsolete wrapper base types * chore: move nolint comments to be robust to formatting linebreaks * chore(velodyne): fix indentation in velodyne calibration files to satisfy yamllint and ignore them in prettier to prevent conflicting styles * chore(hesai): fix decoder test config yaml quotations * chore: whitespace changes * chore: remove empty, un-parseable local.cspell.json * chore(prettier): ignore yaml and json files as they are handled by yamllint/clang-tidy respectively * chore: make pre-commit pass on new files after #146 * chore: update cspell to pass spell-check * chore: rename contributing.md docs page to make failing link check pass
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Type
Work in progress
Related Links
Description
Review Procedure
Remarks
Pre-Review Checklist for the PR Author
PR Author should check the checkboxes below when creating the PR.
Checklist for the PR Reviewer
Reviewers should check the checkboxes below before approval.
Post-Review Checklist for the PR Author
PR Author should check the checkboxes below before merging.
CI Checks