feat(back): create packages for ROS2 exec#3
Open
KelianHalleray wants to merge 1 commit intodevelopmentfrom
Open
feat(back): create packages for ROS2 exec#3KelianHalleray wants to merge 1 commit intodevelopmentfrom
KelianHalleray wants to merge 1 commit intodevelopmentfrom
Conversation
Review Summary by QodoCreate ROS2 packages for robocoop backend and bringup
WalkthroughsDescription• Create Python package setup for robocoop_backend with dependencies • Create ROS2 bringup package with launch and config files • Define package metadata and build configuration for both packages Diagramflowchart LR
A["Project Structure"] --> B["robocoop_backend Package"]
A --> C["robocoop_bringup Package"]
B --> D["Python Setup Configuration"]
C --> E["ROS2 CMake Build"]
C --> F["Package Metadata"]
D --> G["Dependencies: websockets, pyyaml, rclpy"]
E --> H["Install launch/config/scripts"]
F --> I["ROS2 Message Dependencies"]
File Changes1. src/robocoop_backend/setup.py
|
Code Review by Qodo
1. Broken package.xml manifest
|
Member
|
/copilot |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+3
to
+6
| <n>robocoop_bringup</n> | ||
| <version>0.1.0</version> | ||
| <description>Launch files and parameter configs for Robocoop.</description> | ||
| <maintainer>Robocoop Team</maintainer> |
There was a problem hiding this comment.
1. Broken package.xml manifest 🐞 Bug ≡ Correctness
src/robocoop_bringup/package.xml uses an invalid tag (<n> instead of <name>) and declares a <maintainer> without the required email attribute, which will prevent ROS2 tooling (ament/colcon) from parsing/building the package. This blocks building/using robocoop_bringup in the repo’s ROS2 environment.
Agent Prompt
## Issue description
`src/robocoop_bringup/package.xml` is not valid for ROS2 tooling because it uses `<n>` instead of `<name>` and the maintainer tag omits the required `email` attribute.
## Issue Context
This will prevent `ament/colcon` from parsing the package manifest, blocking builds and installs.
## Fix Focus Areas
- src/robocoop_bringup/package.xml[1-8]
## Suggested change
- Replace `<n>robocoop_bringup</n>` with `<name>robocoop_bringup</name>`
- Change maintainer to include email, e.g. `<maintainer email="dev@robocoop.example">Robocoop Team</maintainer>` (use the real email)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Pull request overview
Introduces initial scaffolding for ROS2 execution by adding a robocoop_bringup package (launch/config/scripts) and adding packaging metadata for the Python robocoop_backend.
Changes:
- Add
robocoop_bringupROS2 package skeleton (CMake/package.xml) and install rules. - Add placeholder launch entrypoints/includes and environment-specific config placeholders.
- Add
setup.pyto packagerobocoop_backendfor pip installation.
Reviewed changes
Copilot reviewed 3 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/robocoop_bringup/scripts/run_sim.sh | Placeholder script intended to launch sim backend. |
| src/robocoop_bringup/scripts/run_real.sh | Placeholder script intended to launch real backend. |
| src/robocoop_bringup/scripts/run_mock.sh | Placeholder script intended to launch mock backend. |
| src/robocoop_bringup/package.xml | New ROS2 package manifest for bringup package. |
| src/robocoop_bringup/launch/includes/websocket.launch.py | Placeholder include for WebSocket node launch. |
| src/robocoop_bringup/launch/includes/ros_bridges.launch.py | Placeholder include for ROS bridge nodes. |
| src/robocoop_bringup/launch/includes/robot_runtime.launch.py | Placeholder include for hardware-only runtime nodes. |
| src/robocoop_bringup/launch/includes/monitoring.launch.py | Placeholder include for monitoring/observability nodes. |
| src/robocoop_bringup/launch/backend_sim.launch.py | Placeholder sim backend launch entrypoint. |
| src/robocoop_bringup/launch/backend_real.launch.py | Placeholder real backend launch entrypoint. |
| src/robocoop_bringup/launch/backend_mock.launch.py | Placeholder mock backend launch entrypoint. |
| src/robocoop_bringup/launch/backend_debug.launch.py | Placeholder debug backend launch entrypoint. |
| src/robocoop_bringup/config/sim.params.yaml | Placeholder sim configuration spec. |
| src/robocoop_bringup/config/security.params.yaml | Placeholder security configuration spec. |
| src/robocoop_bringup/config/real.params.yaml | Placeholder real-hardware configuration spec. |
| src/robocoop_bringup/config/mock.params.yaml | Placeholder mock configuration spec. |
| src/robocoop_bringup/config/m3pro_topics.yaml | Placeholder topic mapping spec. |
| src/robocoop_bringup/config/common.params.yaml | Placeholder common configuration spec. |
| src/robocoop_bringup/config/backend.params.yaml | Placeholder backend/WebSocket configuration spec. |
| src/robocoop_bringup/CMakeLists.txt | Adds ament package definition + installs launch/config/scripts. |
| src/robocoop_backend/setup.py | Adds setuptools packaging for Python backend. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setup( | ||
| name="robocoop_backend", | ||
| version="0.1.0", | ||
| packages=find_packages(exclude=["tests*"]), |
There was a problem hiding this comment.
find_packages(exclude=["tests*"]) will not exclude robocoop_backend.tests... because the pattern only matches packages that start with tests. As a result, the test packages are likely to be included in the distribution; use an exclude pattern like robocoop_backend.tests* (or drop the exclude if you intend to ship tests).
Suggested change
| packages=find_packages(exclude=["tests*"]), | |
| packages=find_packages(exclude=["robocoop_backend.tests*"]), |
| <n>robocoop_bringup</n> | ||
| <version>0.1.0</version> | ||
| <description>Launch files and parameter configs for Robocoop.</description> | ||
| <maintainer>Robocoop Team</maintainer> |
There was a problem hiding this comment.
<maintainer> is missing the required email attribute (e.g., <maintainer email="...">...). Many ROS2/ament checks treat this as an error, and it also makes it hard for users to contact the maintainer.
Suggested change
| <maintainer>Robocoop Team</maintainer> | |
| <maintainer email="team@robocoop.example.com">Robocoop Team</maintainer> |
| <exec_depend>geometry_msgs</exec_depend> | ||
| <exec_depend>nav_msgs</exec_depend> | ||
| <exec_depend>sensor_msgs</exec_depend> | ||
| <exec_depend>std_msgs</exec_depend> |
There was a problem hiding this comment.
This bringup package contains Python launch files, but package.xml does not declare runtime dependencies on launch/launch_ros. Without these, rosdep may not install the required launch system packages when building from source.
Suggested change
| <exec_depend>std_msgs</exec_depend> | |
| <exec_depend>std_msgs</exec_depend> | |
| <exec_depend>launch</exec_depend> | |
| <exec_depend>launch_ros</exec_depend> |
Comment on lines
+7
to
+10
| DIRECTORY launch config scripts | ||
| DESTINATION share/${PROJECT_NAME} | ||
| ) | ||
|
|
There was a problem hiding this comment.
install(DIRECTORY ... scripts ...) may install the shell scripts without executable permissions depending on source file modes. If these are meant to be runnable entrypoints, prefer install(PROGRAMS scripts/*.sh DESTINATION lib/${PROJECT_NAME}) (or similar) to ensure they are executable after installation.
Suggested change
| DIRECTORY launch config scripts | |
| DESTINATION share/${PROJECT_NAME} | |
| ) | |
| DIRECTORY launch config | |
| DESTINATION share/${PROJECT_NAME} | |
| ) | |
| install( | |
| PROGRAMS scripts/*.sh | |
| DESTINATION lib/${PROJECT_NAME} | |
| ) |
| @@ -0,0 +1,16 @@ | |||
| <?xml version="1.0"?> | |||
| <package format="3"> | |||
| <n>robocoop_bringup</n> | |||
There was a problem hiding this comment.
package.xml uses <n> instead of the required <name> tag. This makes the manifest invalid and will cause ROS2 tooling (colcon/ament) to fail parsing the package.
Suggested change
| <n>robocoop_bringup</n> | |
| <name>robocoop_bringup</name> |
This file contains hidden or 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
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.
No description provided.