-
Notifications
You must be signed in to change notification settings - Fork 88
[OSS] [ROS2OV_Team1] Refactor topic and service names to relative for namespace compatibility #327
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
base: ros2
Are you sure you want to change the base?
Conversation
This commit updates topic and service names from absolute to relative paths across the repository by removing prefixed '/' from ros2 names This enables the correct appending of namespaces to nodes, topics, and services, enhancing flexibility and modularity of the system. - Modified topic and service paths in C++ source files and headers. - Updated corresponding Python scripts to reflect relative naming. - Adjusted unit tests to align with the new naming convention. Signed-off-by: Arshad Mehmood <[email protected]>
|
This fix will work well for issue #303. Here is my question. In a big system, if the namespaces are used without a clear structure or understanding, the relative topic names could lead to communication issues. |
Per ROS the recommended solution is to use relative namespace. As we want topics to be shared across several ros nodes, it gives the flexibility for publishers and subscribers efficiently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please have a check with my comments.
…vino_toolkit/image_raw in all the launch files
* Refactor topic and service names to relative for namespace compatibility This commit updates topic and service names from absolute to relative paths across the repository by removing prefixed '/' from ros2 names This enables the correct appending of namespaces to nodes, topics, and services, enhancing flexibility and modularity of the system. - Modified topic and service paths in C++ source files and headers. - Updated corresponding Python scripts to reflect relative naming. - Adjusted unit tests to align with the new naming convention. Signed-off-by: Arshad Mehmood <[email protected]> * Fix review comments and replace /openvino_toolkit/image_raw with openvino_toolkit/image_raw in all the launch files * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> --------- Signed-off-by: Arshad Mehmood <[email protected]> Co-authored-by: Arshad Mehmood <[email protected]> Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors ROS2 topic and service names from absolute paths to relative paths across the codebase by removing the leading '/' prefix. This change improves namespace compatibility by allowing proper namespace appending to nodes, topics, and services.
- Converted absolute topic paths (e.g.,
/ros2_openvino_toolkit/detected_objects) to relative paths (ros2_openvino_toolkit/detected_objects) - Updated service names from absolute to relative paths for improved modularity
- Modified launch files to use relative remapping paths
Reviewed Changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/src/topic/*.cpp | Updated test subscription topic names to use relative paths |
| tests/src/service/*.cpp | Changed service client creation to use relative service names |
| sample/src/*.cpp | Modified sample clients to use relative service paths |
| sample/launch/*.py | Updated launch file remappings to use relative topic names |
| script/viewer/service.py | Changed Python service client to use relative path |
| openvino_wrapper_lib/src/services/*.cpp | Updated service creation to use relative names |
| openvino_wrapper_lib/src/outputs/*.cpp | Modified publisher topic names to use relative paths |
| openvino_wrapper_lib/src/inputs/image_topic.cpp | Changed input topic macro to relative path |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Refactor Topics to Use Relative Paths for Improved Namespace Compatibility
This commit updates topic and service names from absolute to relative paths across the repository by removing prefixed '/' from ros2 names This enables the correct appending of namespaces to nodes, topics, and services, enhancing flexibility and modularity of the system.
Fixes : #303