Skip to content

Allow legacy field names in rosidl_generate_interfaces [humble] #834

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 10 additions & 3 deletions rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@
# @public
#
function(rosidl_adapt_interfaces idl_var arguments_file)
cmake_parse_arguments(ARG "" "TARGET" ""
${ARGN})
set(options ALLOW_LEGACY_FIELD_NAMES)
set(oneValueArgs TARGET)
set(multiValueArgs "")
cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
if(ARG_UNPARSED_ARGUMENTS)
message(FATAL_ERROR "rosidl_adapt_interfaces() called with unused "
"arguments: ${ARG_UNPARSED_ARGUMENTS}")
"arguments: ${ARG_UNPARSED_ARGUMENTS}. ALLOW_LEGACY? '${ARG_ALLOW_LEGACY_FIELD_NAMES}'")
endif()

find_package(ament_cmake_core REQUIRED) # for get_executable_path
Expand All @@ -48,6 +50,11 @@ function(rosidl_adapt_interfaces idl_var arguments_file)
--arguments-file "${arguments_file}"
--output-dir "${CMAKE_CURRENT_BINARY_DIR}/rosidl_adapter/${PROJECT_NAME}"
--output-file "${idl_output}")
if(ARG_ALLOW_LEGACY_FIELD_NAMES)
list(APPEND cmd --allow-legacy-field-naming)
MESSAGE(WARNING Allowing legacy arguments.)
endif()

execute_process(
COMMAND ${cmd}
OUTPUT_QUIET
Expand Down
6 changes: 4 additions & 2 deletions rosidl_adapter/rosidl_adapter/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from rosidl_adapter.parser import DEFAULT_ALLOW_LEGACY_FIELD_NAMES

def convert_to_idl(package_dir, package_name, interface_file, output_dir):
def convert_to_idl(package_dir, package_name, interface_file, output_dir, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES):
if interface_file.suffix == '.msg':
from rosidl_adapter.msg import convert_msg_to_idl

return convert_msg_to_idl(
package_dir, package_name, interface_file, output_dir / 'msg')
package_dir, package_name, interface_file, output_dir / 'msg', allow_legacy_field_naming=allow_legacy_field_naming)

if interface_file.suffix == '.srv':
from rosidl_adapter.srv import convert_srv_to_idl
Expand Down
8 changes: 7 additions & 1 deletion rosidl_adapter/rosidl_adapter/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@


from rosidl_adapter import convert_to_idl
from rosidl_adapter.parser import DEFAULT_ALLOW_LEGACY_FIELD_NAMES


def main(argv=sys.argv[1:]):
Expand All @@ -38,6 +39,10 @@ def main(argv=sys.argv[1:]):
'--output-file', required=True,
help='The output file containing the tuples for the generated .idl '
'files')
legacy_field_name_action = "store_true" if not DEFAULT_ALLOW_LEGACY_FIELD_NAMES else "store_false"
parser.add_argument(
'--allow-legacy-field-naming', required=False, action=legacy_field_name_action,
help='Allow legacy ROS1 style field names that use PascalCase, camelCase, and Pascal_With_Underscores')
args = parser.parse_args(argv)
output_dir = pathlib.Path(args.output_dir)
output_file = pathlib.Path(args.output_file)
Expand All @@ -52,7 +57,8 @@ def main(argv=sys.argv[1:]):
basepath, relative_path = non_idl_tuple.rsplit(':', 1)
abs_idl_file = convert_to_idl(
pathlib.Path(basepath), args.package_name,
pathlib.Path(relative_path), output_dir)
pathlib.Path(relative_path), output_dir,
allow_legacy_field_naming=args.allow_legacy_field_naming)
idl_tuples.append((output_dir, abs_idl_file.relative_to(output_dir)))

output_file.parent.mkdir(exist_ok=True)
Expand Down
7 changes: 4 additions & 3 deletions rosidl_adapter/rosidl_adapter/msg/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from rosidl_adapter.parser import parse_message_string
from rosidl_adapter.parser import parse_message_string, DEFAULT_ALLOW_LEGACY_FIELD_NAMES
from rosidl_adapter.resource import expand_template


def convert_msg_to_idl(package_dir, package_name, input_file, output_dir):
def convert_msg_to_idl(package_dir, package_name, input_file, output_dir, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES):

assert package_dir.is_absolute()
assert not input_file.is_absolute()
assert input_file.suffix == '.msg'
Expand All @@ -25,7 +26,7 @@ def convert_msg_to_idl(package_dir, package_name, input_file, output_dir):
print(f'Reading input file: {abs_input_file}')
abs_input_file = package_dir / input_file
content = abs_input_file.read_text(encoding='utf-8')
msg = parse_message_string(package_name, input_file.stem, content)
msg = parse_message_string(package_name, input_file.stem, content, allow_legacy_field_naming=allow_legacy_field_naming)

output_file = output_dir / input_file.with_suffix('.idl').name
abs_output_file = output_file.absolute()
Expand Down
25 changes: 19 additions & 6 deletions rosidl_adapter/rosidl_adapter/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,15 @@
'$')
VALID_FIELD_NAME_PATTERN = VALID_PACKAGE_NAME_PATTERN
# relaxed patterns used for compatibility with ROS 1 messages
# VALID_FIELD_NAME_PATTERN = re.compile('^[A-Za-z][A-Za-z0-9_]*$')
RELAXED_FIELD_NAME_PATTERN = re.compile('^[A-Za-z][A-Za-z0-9_]*$')
VALID_MESSAGE_NAME_PATTERN = re.compile('^[A-Z][A-Za-z0-9]*$')
# relaxed patterns used for compatibility with ROS 1 messages
# VALID_MESSAGE_NAME_PATTERN = re.compile('^[A-Za-z][A-Za-z0-9]*$')
VALID_CONSTANT_NAME_PATTERN = re.compile('^[A-Z]([A-Z0-9_]?[A-Z0-9]+)*$')

# By default, ROS 2 does not allow legacy field names.
# https://docs.ros.org/en/rolling/Concepts/Basic/About-Interfaces.html#field-names
DEFAULT_ALLOW_LEGACY_FIELD_NAMES=False

class InvalidSpecification(Exception):
pass
Expand Down Expand Up @@ -115,8 +118,16 @@ def is_valid_package_name(name):
raise InvalidResourceName(name)
return m is not None and m.group(0) == name

def is_valid_legacy_field_name(name):
try:
m = RELAXED_FIELD_NAME_PATTERN.match(name)
except TypeError:
raise InvalidResourceName(name)
return m is not None and m.group(0) == name

def is_valid_field_name(name):
def is_valid_field_name(name, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES):
if allow_legacy_field_naming:
return is_valid_legacy_field_name(name)
try:
m = VALID_FIELD_NAME_PATTERN.match(name)
except TypeError:
Expand Down Expand Up @@ -345,12 +356,12 @@ def __str__(self):

class Field:

def __init__(self, type_, name, default_value_string=None):
def __init__(self, type_, name, default_value_string=None, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES):
if not isinstance(type_, Type):
raise TypeError(
"the field type '%s' must be a 'Type' instance" % type_)
self.type = type_
if not is_valid_field_name(name):
if not is_valid_field_name(name, allow_legacy_field_naming=allow_legacy_field_naming):
raise NameError(
"'{}' is an invalid field name. It should have the pattern '{}'".format(
name, VALID_FIELD_NAME_PATTERN.pattern))
Expand Down Expand Up @@ -462,7 +473,7 @@ def extract_file_level_comments(message_string):
return file_level_comments, file_content


def parse_message_string(pkg_name, msg_name, message_string):
def parse_message_string(pkg_name, msg_name, message_string, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES):
fields = []
constants = []
last_element = None # either a field or a constant
Expand Down Expand Up @@ -518,7 +529,9 @@ def parse_message_string(pkg_name, msg_name, message_string):
try:
fields.append(Field(
Type(type_string, context_package_name=pkg_name),
field_name, default_value_string))
field_name, default_value_string,
allow_legacy_field_naming=allow_legacy_field_naming),
)
except Exception as err:
print(
"Error processing '{line}' of '{pkg}/{msg}': '{err}'".format(
Expand Down
14 changes: 13 additions & 1 deletion rosidl_cmake/cmake/rosidl_generate_interfaces.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
#
macro(rosidl_generate_interfaces target)
cmake_parse_arguments(_ARG
"ADD_LINTER_TESTS;SKIP_INSTALL;SKIP_GROUP_MEMBERSHIP_CHECK"
"ADD_LINTER_TESTS;SKIP_INSTALL;SKIP_GROUP_MEMBERSHIP_CHECK;ALLOW_LEGACY_FIELD_NAMES"
"LIBRARY_NAME" "DEPENDENCIES"
${ARGN})
if(NOT _ARG_UNPARSED_ARGUMENTS)
Expand Down Expand Up @@ -127,9 +127,21 @@ macro(rosidl_generate_interfaces target)
PACKAGE_NAME "${PROJECT_NAME}"
NON_IDL_TUPLES "${_non_idl_tuples}"
)
set(_rosidl_apt_interfaces_opts)
if(_ARG_ALLOW_LEGACY_FIELD_NAMES)
set(_rosidl_apt_interfaces_opts ALLOW_LEGACY_FIELD_NAMES)
#list(APPEND _arg ALLOW_LEGACY_FIELD_NAMES)
message(WARNING "Allowing legacy field names")
else()
message(FATAL_ERROR NO Legacy field names)
endif()

#${_rosidl_apt_interfaces_opts}

rosidl_adapt_interfaces(
_idl_adapter_tuples
"${_adapter_arguments_file}"
${_rosidl_apt_interfaces_opts}
TARGET ${target}
)
endif()
Expand Down