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.

Sign up for GitHub

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

Jump to bottom

Create package rosidl_core_generators #22

Merged
merged 4 commits into from
Aug 23, 2022
Merged

Create package rosidl_core_generators #22

merged 4 commits into from
Aug 23, 2022

Conversation

jacobperron
Copy link
Member

@jacobperron jacobperron commented Aug 2, 2022 •
edited
Loading

Move contents of rosidl_default_generators into the new package.
This package contains dependencies for code generation of ROS messages.
rosidl_default_generators now depends on the new package, plus message definitions required for Actions (namely action_msgs).

This allows users to avoid having to explictly depend on action_msgs.

Connected to

Downstream PRs removing the dependency:

Documentation PRs:

@jacobperron
Create rosidl_core_generators
8a103ff 
Move contents of rosidl_default_generators into the new package.
This package is contains dependencies for code generation of ROS messages.
rosidl_default_generators now depends on the new package, plus message definitions required for Actions (namely action_msgs).

This allows users to avoid having to explictly depend on action_msgs.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2/rcl_interfaces that referenced this pull request Aug 2, 2022
@jacobperron
Depend on rosidl_core_generators for packages required by actions
ba50a5c 
This allows us to forward a dependency on action_msgs for users and avoid a dependency cycle.

Connected to ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron mentioned this pull request Aug 2, 2022
Merged
@jacobperron jacobperron changed the title Create rosidl_core_generators Create package rosidl_core_generators Aug 2, 2022
@jacobperron
Copy link
Member Author

Though this avoids a dependency cycle with action_msgs, I just realized this creates a cycle at the repository level. So, if this is an approved solution we should consider moving the new package to it's own repository.

@jacobperron
Copy link
Member Author

Note, I think we also require ros2/rosidl#696 in order to avoid the dependency cycle.

This was referenced Aug 2, 2022
Merged
Merged
Merged
Merged
Merged
@ihasdapie ihasdapie mentioned this pull request Aug 8, 2022
Closed
This was referenced Aug 9, 2022
Closed
Closed
ivanpauno
ivanpauno requested changes Aug 16, 2022
View reviewed changes
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

About the name of the new package, I don't have suggestions, but the distinction between "code" and "defaults" is confusing.

LGTM when the new package is moved to another repo (if not, it will create a circular dependency when bloom-releasing).

@ivanpauno ivanpauno added the enhancement New feature or request label Aug 16, 2022
@jacobperron jacobperron mentioned this pull request Aug 16, 2022
Merged
jacobperron added a commit to ros2/rosidl_core that referenced this pull request Aug 18, 2022
@jacobperron
Add generators and runtime configuration packages
2e845a1 
Moved (and renamed) from rosidl_defaults.

Related PR: ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Move core generators to a separate repository
de5bb3d 
https://github.com/ros2/rosidl_core.git
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Move and rename rosidl_default_runtime
67209ed 
Move rosidl_default_runtime to rosidl_core_runtime, depend on rosidl_core_runtime, and depend on action_msgs.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron mentioned this pull request Aug 18, 2022
Merged
jacobperron added a commit to ros2/ros2 that referenced this pull request Aug 18, 2022
@jacobperron
Add repository rosidl_core
07ab292 
Connected to ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron mentioned this pull request Aug 18, 2022
Merged
jacobperron added a commit to ros2/unique_identifier_msgs that referenced this pull request Aug 18, 2022
@jacobperron
Depend on rosidl_core instead of rosidl_defaults
fc65b23 
This allows us to forward a dependency on action_msgs for users and avoid a dependency cycle.

Connected to ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron mentioned this pull request Aug 18, 2022
Merged
@jacobperron
Copy link
Member Author

@ivanpauno I've updated this PR, moving the new "core" packages their own repositories. I've updated this PRs description with an updated list of connected PRs.

ivanpauno
ivanpauno reviewed Aug 18, 2022
View reviewed changes
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM when the only open discussion is resolved (and CI is green)

rosidl_default_generators/CMakeLists.txt Show resolved Hide resolved
@jacobperron
export rosidl_default_runtime dependencies
07dd659 
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Member Author

jacobperron commented Aug 18, 2022 •
edited
Loading

Full CI with all connected PRs:

jacobperron added a commit to ros2/rosidl_core that referenced this pull request Aug 23, 2022
@jacobperron
Add generators and runtime configuration packages (#1)
ee10bb2 
* Add generators and runtime configuration packages

Moved (and renamed) from rosidl_defaults.

Related PR: ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Add README files

and update the rosidl_core_runtime README

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron merged commit 718c8c0 into rolling Aug 23, 2022
@delete-merged-branch delete-merged-branch bot deleted the jacob/action_msgs_dep branch August 23, 2022 17:55
jacobperron added a commit to ros2/ros2 that referenced this pull request Aug 23, 2022
@jacobperron
Add repository rosidl_core (#1316)
4b92be6 
Connected to ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2/rcl_interfaces that referenced this pull request Aug 23, 2022
@jacobperron
Depend on rosidl_core_generators for packages required by actions (#144)
e6c063a 
* Depend on rosidl_core_generators for packages required by actions

This allows us to forward a dependency on action_msgs for users and avoid a dependency cycle.

Connected to ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Remove action_msgs dependency from test_msgs

It is now properly included in the rosidl dependency chain.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Depend on rosidl_core_runtime for packages depended on by actions

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

* Update quality declarations

Signed-off-by: Jacob Perron <jacob@openrobotics.org>

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron added a commit to ros2/unique_identifier_msgs that referenced this pull request Aug 23, 2022
@jacobperron
Depend on rosidl_core instead of rosidl_defaults (#24)
916f346 
This allows us to forward a dependency on action_msgs for users and avoid a dependency cycle.

Connected to ros2/rosidl_defaults#22

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@nnmm nnmm mentioned this pull request Aug 31, 2022
Merged
@jacobperron jacobperron mentioned this pull request Sep 13, 2022
Open
hoffmann-stefan added a commit to hoffmann-stefan/ros2_dotnet that referenced this pull request May 18, 2023
@hoffmann-stefan
add rosidl_core repo to ros2_dotnet_rolling.repos
6de0216 
In ros2/rosidl_defaults#22, the rosidl_core_generators package was split off from rosidl_defaults into a separate repo, which needed to be added.
Without this repository the dotnet code for the interfaces is not generated in the workspace.

see PR on ros2-rust: ros2-rust/ros2_rust#251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivanpauno ivanpauno ivanpauno approved these changes

@ihasdapie ihasdapie ihasdapie approved these changes

Assignees

@jacobperron jacobperron

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jacobperron @ivanpauno @ihasdapie