Development #5
Development #5
Conversation
- Refactored test_scenario_perception_pipeline.cpp - Added event handler because of interaction problem between panda_arm_spawner and test_scenario_perception_pipeline node
- Added upstream workspace envrionment to CI - Changed ROS version in CI - Fixed CMakeList.txt and package.xml based on the feedbacks of CI - Fixed upstream repository urls - Changed test cases for more convenient benchmarking
- Added middleware implementations to exec_depend ( #1 (comment) ) - Refactored package directory tree structure ( #1 (comment) ) - Added namespace ( #1 (comment) )
- Added documentation for each methods and variables except global variables ( #1 (comment) ) - Put constants into anoymous namespace ( #1 (comment) ) - Modified condition checking in senfTargetPose ( #1 (comment) ) - Fixed middleware packages in package.xml - Renamed the test case config
- Removed ScenarioPerceptionPipelineTestCaseGenerator class and added its methods to ScenarioPerceptionPipelineFixture - Renamed ScenarioPerceptionPipelineFixture/selectTestCase method as ScenarioPerceptionPipelineFixture/selectTestCase - Added documentation for ScenarioPerceptionPipelineFixture/setUp and ScenarioPerceptionPipelineFixture/tearDown methods
Creating Repository Structure
* Created documentations * Revised documentation
|
Hello @sjahr and @henningkayser , could you review this PR? I need your help for this PR. I have added a documentation. I have already answered and applied necessary modifications for @sjahr 's review in #1 |
There was a problem hiding this comment.
Well-structured code and good amount of in-code documentation, thanks!
Is this PR supposed to measure the elapsed time as will or is it just setting up the basic infrastructure? Also I don't understand how you are differentiating between the middleware processing time and the planning time (for the middleware benchmark we only care for the first).
- Added some explanation. See ( #5 (comment) ) - Fixed typo of description in package.xml. See ( #5 (comment) )
…void function's documentation. See ( #5 (comment) ) See ( #5 (comment) )
…e_benchmark.launch.py - See ( #5 (comment) ) - See ( #5 (comment) )
Firstly thank you so much for your valuable review. This PR creates a json report after running launch file as indicated in this repo's documentation. But launch doesn't close itself as soon as the execution of benchmark_node finishes. You must close it (Btw i have a fix for that and i'm working this on servo_benchmarks ). For example report: https://gist.github.com/CihatAltiparmak/ab3676b0452203f36f26f110b9a5b5fe I'm planning to add some dockerfile for this PR (probably in another branch created from development) for your working on better |
… soon as benchmark node process is finished.
|
Hello @sjahr , i think i'm ready for a second review. When you have any chance etc. to take a second look at this PR, could you review new commits? Btw i'm trying not to merge other branches to development branch so that you can review easier. Additionaly, I've created a github action for creating and pushing docker image from github registry. You will also have a chance to pull this docker image approximately in this morning :) But leave this merging stuffs to me at the moment. Because for example this commit(e61a02a) may not solve my problem completely. |
…ith unsoundly - First insights: It seems benhcmark nodes shouldn't store the shared ptrs of ros nodes etc. while rclcpp::shutdown is running Otherwise, benchmark sometimes finishes unsoundly like below. [ERROR] [1713850255.212319171] [bla_bla_node.rclcpp]: Error in destruction of rcl subscription handle: Failed to delete datareader, at ./src/subscription.cpp:52, at ./src/rcl/subscription.c:184 cannot publish data, at ./src/rmw_publish.cpp:62 during '__function__' Fail in delete datareader, at ./src/rmw_service.cpp:104 during '__function__'
There was a problem hiding this comment.
@henningkayser Would you like to review it too before we merge it?
No description provided.