Skip to content

Swapped usage of rosidl_cmake over to the new rosidl_pycommon. #297

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

Merged
merged 8 commits into from
Apr 6, 2023
Merged

Swapped usage of rosidl_cmake over to the new rosidl_pycommon. #297

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
05383e2
Swapped usage of rosidl_cmake over to the new rosidl_pycommon.
maspe36 Dec 12, 2022
c6173ac
Support builds from older ROS 2 distros.
maspe36 Dec 12, 2022
ae86ce2
Fixed build for rolling
maspe36 Dec 12, 2022
f231114
Added `test_depend` conditional inclusion as well.
maspe36 Dec 12, 2022
8ee4517
Swap to a more elegant check
maspe36 Feb 24, 2023
46c0e54
Merge branch 'main' into fix_deprecated_rosidl_cmake_warning
maspe36 Mar 12, 2023
084fea3
PR Feedback
maspe36 Mar 17, 2023
0a530d2
Merge branch 'ros2-rust:main' into fix_deprecated_rosidl_cmake_warning
maspe36 Mar 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions rosidl_generator_rs/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ project(rosidl_generator_rs)

find_package(ament_cmake REQUIRED)
find_package(ament_cmake_python REQUIRED)
find_package(rosidl_cmake REQUIRED)
find_package(rosidl_generator_c REQUIRED)
find_package(rosidl_typesupport_interface REQUIRED)
find_package(rosidl_typesupport_introspection_c REQUIRED)

ament_export_dependencies(rosidl_cmake)
if("$ENV{ROS_DISTRO}" STRLESS "rolling")
find_package(rosidl_cmake REQUIRED)
ament_export_dependencies(rosidl_cmake)
endif()

ament_export_dependencies(rmw)
ament_export_dependencies(rosidl_generator_c)

Expand Down
6 changes: 4 additions & 2 deletions rosidl_generator_rs/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
<buildtool_depend>rosidl_runtime_rs</buildtool_depend>

<buildtool_export_depend>ament_cmake</buildtool_export_depend>
<buildtool_export_depend>rosidl_cmake</buildtool_export_depend>
<buildtool_export_depend condition="$ROS_DISTRO != rolling">rosidl_cmake</buildtool_export_depend>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I may be interpreting REP 149 incorrectly, but to me it reads as if I should be able to use a less/greater than for ROS distros. Unfortunately that doesn't work, and the xml fails to parse. Not sure what to do if rosidl_cmake 3.3.0+ gets back ported to Humble or when Iron comes out

Copy link
Contributor

Choose a reason for hiding this comment

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

Reads that way to me too. Did you try only > or also >=?

Copy link
Collaborator Author

@maspe36 maspe36 Feb 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Coming back to this now, looks to only be a problem with less than operators.

Fails to parse
$ROS_DISTRO <= rolling
$ROS_DISTRO < rolling

Successfully parses
$ROS_DISTRO >= rolling
$ROS_DISTRO > rolling

Thankfully this means we can handle this a bit more concisely in the package.xml

<buildtool_export_depend condition="humble >= $ROS_DISTRO">rosidl_cmake</buildtool_export_depend>
<buildtool_export_depend condition="$ROS_DISTRO > humble">rosidl_pycommon</buildtool_export_depend>

We know this change is happening after humble (but only on rolling right now), so we can assume anything that's newer than humble will use rosidl_pycommon

<buildtool_export_depend condition="$ROS_DISTRO == rolling">rosidl_pycommon</buildtool_export_depend>
<buildtool_export_depend>rosidl_runtime_rs</buildtool_export_depend>
<buildtool_export_depend>rosidl_typesupport_c</buildtool_export_depend>
<buildtool_export_depend>rosidl_typesupport_interface</buildtool_export_depend>
Expand All @@ -26,7 +27,8 @@
<test_depend>ament_cmake_gtest</test_depend>
<test_depend>ament_lint_auto</test_depend>
<test_depend>ament_lint_common</test_depend>
<test_depend>rosidl_cmake</test_depend>
<test_depend condition="$ROS_DISTRO != rolling">rosidl_cmake</test_depend>
<test_depend condition="$ROS_DISTRO == rolling">rosidl_pycommon</test_depend>
<test_depend>rosidl_generator_c</test_depend>

<member_of_group>rosidl_generator_packages</member_of_group>
Expand Down
25 changes: 12 additions & 13 deletions rosidl_generator_rs/rosidl_generator_rs/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@

from pathlib import Path

from rosidl_cmake import convert_camel_case_to_lower_case_underscore
from rosidl_cmake import expand_template
from rosidl_cmake import generate_files
from rosidl_cmake import get_newest_modification_time
from rosidl_cmake import read_generator_arguments
if os.environ['ROS_DISTRO'] < 'rolling':
import rosidl_cmake as rosidl_pycommon
else:
import rosidl_pycommon

from rosidl_parser.definition import AbstractGenericString
from rosidl_parser.definition import AbstractNestedType
Expand Down Expand Up @@ -53,7 +52,7 @@ def convert_lower_case_underscore_to_camel_case(word):


def generate_rs(generator_arguments_file, typesupport_impls):
args = read_generator_arguments(generator_arguments_file)
args = rosidl_pycommon.read_generator_arguments(generator_arguments_file)
package_name = args['package_name']

# expand init modules for each directory
Expand Down Expand Up @@ -108,7 +107,7 @@ def generate_rs(generator_arguments_file, typesupport_impls):
'constant_value_to_rs': constant_value_to_rs,
'value_to_rs': value_to_rs,
'convert_camel_case_to_lower_case_underscore':
convert_camel_case_to_lower_case_underscore,
rosidl_pycommon.convert_camel_case_to_lower_case_underscore,
'convert_lower_case_underscore_to_camel_case':
convert_lower_case_underscore_to_camel_case,
'msg_specs': [],
Expand All @@ -118,7 +117,7 @@ def generate_rs(generator_arguments_file, typesupport_impls):
'interface_path': idl_rel_path,
}

latest_target_timestamp = get_newest_modification_time(
latest_target_timestamp = rosidl_pycommon.get_newest_modification_time(
args['target_dependencies'])

for message in idl_content.get_elements_of_type(Message):
Expand All @@ -132,7 +131,7 @@ def generate_rs(generator_arguments_file, typesupport_impls):
for generated_filename in generated_filenames:
generated_file = os.path.join(args['output_dir'],
generated_filename % 'msg')
expand_template(
rosidl_pycommon.expand_template(
os.path.join(template_dir, template_file),
data.copy(),
generated_file,
Expand All @@ -143,13 +142,13 @@ def generate_rs(generator_arguments_file, typesupport_impls):
for generated_filename in generated_filenames:
generated_file = os.path.join(args['output_dir'],
generated_filename % 'srv')
expand_template(
rosidl_pycommon.expand_template(
os.path.join(template_dir, template_file),
data.copy(),
generated_file,
minimum_timestamp=latest_target_timestamp)

expand_template(
rosidl_pycommon.expand_template(
os.path.join(template_dir, 'lib.rs.em'),
data.copy(),
os.path.join(args['output_dir'], 'rust/src/lib.rs'),
Expand All @@ -160,13 +159,13 @@ def generate_rs(generator_arguments_file, typesupport_impls):
'package_name': args['package_name'],
'package_version': args['package_version'],
}
expand_template(
rosidl_pycommon.expand_template(
os.path.join(template_dir, 'Cargo.toml.em'),
cargo_toml_data,
os.path.join(args['output_dir'], 'rust/Cargo.toml'),
minimum_timestamp=latest_target_timestamp)

expand_template(
rosidl_pycommon.expand_template(
os.path.join(template_dir, 'build.rs.em'),
{},
os.path.join(args['output_dir'], 'rust/build.rs'),
Expand Down