From 24ba9d33bedc9401b8884915ed10e8d91933c88f Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Fri, 8 Aug 2025 21:14:24 +0200 Subject: [PATCH 01/25] Detect missing test dependencies --- .../package_xml_validator.py | 79 +++++++++++-------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index 23e5fb9..ea2dd74 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -176,44 +176,53 @@ def extract_launch_deps(folder_names: List[str]) -> List[str]: launch_deps.extend(scan_files(launch_dir)) return launch_deps - launch_folder_names = ["launch", "components"] - launch_deps = extract_launch_deps(launch_folder_names) - if not launch_deps: - self.logger.debug( - f"No launch dependencies found in {package_name}/package.xml." - ) - return True - - missing_deps = [ - dep for dep in launch_deps if dep not in exec_deps and dep != package_name - ] - if missing_deps: - sep = "\n\t - " - self.logger.warning( - f"Missing launch dependencies in {package_name}/package.xml: {sep}{sep.join(missing_deps)}" - ) - - if self.check_only: - return False - else: - self.logger.info( - f"Auto-filling {len(missing_deps)} missing launch dependencies in {package_name}/package.xml." + def validate_launch_folders(launch_folder_names: List[str], depend_tag) -> bool: + launch_deps = extract_launch_deps(launch_folder_names) + if not launch_deps: + self.logger.debug( + f"No launch dependencies found in {package_name}/package.xml." ) - # before adding dependencies make sure they are valid rosdeps - if self.check_rosdeps: - invalid_deps = self.rosdep_validator.check_rosdeps_and_local_pkgs( - missing_deps + return True + + missing_deps = [ + dep + for dep in launch_deps + if dep not in exec_deps and dep != package_name + ] + if missing_deps: + sep = "\n\t - " + self.logger.warning( + f"Missing <{depend_tag}> dependencies in {package_name}/package.xml: {sep}{sep.join(missing_deps)}" + ) + + if self.check_only: + return False + else: + self.logger.info( + f"Auto-filling {len(missing_deps)} missing <{depend_tag}> dependencies in {package_name}/package.xml." ) - valid_deps = [d for d in missing_deps if d not in invalid_deps] - if invalid_deps: - self.logger.error( - f"Cannot auto-fill invalid launch dependencies: {', '.join(invalid_deps)}" + # before adding dependencies make sure they are valid rosdeps + if self.check_rosdeps: + invalid_deps = ( + self.rosdep_validator.check_rosdeps_and_local_pkgs( + missing_deps + ) ) - return False - missing_deps = valid_deps - self.formatter.add_dependencies(root, missing_deps, "exec_depend") - return False - return True + valid_deps = [d for d in missing_deps if d not in invalid_deps] + if invalid_deps: + self.logger.error( + f"Cannot auto-fill invalid launch dependencies: {', '.join(invalid_deps)}" + ) + return False + missing_deps = valid_deps + self.formatter.add_dependencies(root, missing_deps, depend_tag) + return False + return True + + launch_folder_names = ["launch", "components"] + launch_deps_valid = validate_launch_folders(launch_folder_names, "exec_depend") + test_deps_valid = validate_launch_folders(["test"], "test_depend") + return launch_deps_valid and test_deps_valid def validate_ament_exports(self, root, xml_file: str): """Validate ament_export tags in the package.xml file. From a0e48b29f3330e704e275b9de1931f3388c579aa Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Fri, 8 Aug 2025 21:21:52 +0200 Subject: [PATCH 02/25] Updated Readme --- Readme.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Readme.md b/Readme.md index b5c1e01..1125608 100644 --- a/Readme.md +++ b/Readme.md @@ -14,6 +14,7 @@ Validates and formats `package.xml` files to enforce consistency and ROS 2 schem - Launch-File Dependency Validation - Scans Python (.py), YAML (.yaml/.yml), and XML (.xml) launch files for package references - validates and corrects that all referenced pkgs are declared in the package xml (as `` or ``) + - similarly the test folder is parsed to extract missing `` dependencies - Rosdep Key Checking - verifies that all declared pkgs exist as rodsdep key (optional) - CMakeFile Comparison and Synchronization From ed6b4b33f2d2716acd92220297d8c906e7d50ca2 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Fri, 8 Aug 2025 21:28:33 +0200 Subject: [PATCH 03/25] Bugifx --- .../helpers/pkg_xml_formatter.py | 2 +- .../package_xml_validator.py | 20 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/package_xml_validation/helpers/pkg_xml_formatter.py b/package_xml_validation/helpers/pkg_xml_formatter.py index 7c20938..f035ece 100644 --- a/package_xml_validation/helpers/pkg_xml_formatter.py +++ b/package_xml_validation/helpers/pkg_xml_formatter.py @@ -363,7 +363,7 @@ def retrieve_build_dependencies(self, root): def retrieve_test_dependencies(self, root): """Retrieve all test dependencies from the XML file.""" test_dependencies = [] - test_deps = ["test_depend"] + test_deps = ["test_depend", "depend"] for elem in root: if isinstance(elem.tag, str) and elem.tag in test_deps and elem.text: test_dependencies.append(elem.text.strip()) diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index ea2dd74..78266c8 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -163,7 +163,12 @@ def validate_xml_with_xmllint(self, xml_file): return False def validate_launch_dependencies( - self, root, package_xml_file: str, package_name: str, exec_deps: List[str] + self, + root, + package_xml_file: str, + package_name: str, + exec_deps: List[str], + test_deps: List[str] = [], ): """Validate launch dependencies in the package.xml file.""" @@ -176,7 +181,9 @@ def extract_launch_deps(folder_names: List[str]) -> List[str]: launch_deps.extend(scan_files(launch_dir)) return launch_deps - def validate_launch_folders(launch_folder_names: List[str], depend_tag) -> bool: + def validate_launch_folders( + launch_folder_names: List[str], xml_deps: List[str], depend_tag: str + ) -> bool: launch_deps = extract_launch_deps(launch_folder_names) if not launch_deps: self.logger.debug( @@ -187,7 +194,7 @@ def validate_launch_folders(launch_folder_names: List[str], depend_tag) -> bool: missing_deps = [ dep for dep in launch_deps - if dep not in exec_deps and dep != package_name + if dep not in xml_deps and dep != package_name ] if missing_deps: sep = "\n\t - " @@ -220,8 +227,10 @@ def validate_launch_folders(launch_folder_names: List[str], depend_tag) -> bool: return True launch_folder_names = ["launch", "components"] - launch_deps_valid = validate_launch_folders(launch_folder_names, "exec_depend") - test_deps_valid = validate_launch_folders(["test"], "test_depend") + launch_deps_valid = validate_launch_folders( + launch_folder_names, exec_deps, "exec_depend" + ) + test_deps_valid = validate_launch_folders(["test"], test_deps, "test_depend") return launch_deps_valid and test_deps_valid def validate_ament_exports(self, root, xml_file: str): @@ -368,6 +377,7 @@ def check_and_format_files(self, package_xml_files): xml_file, self.formatter.get_package_name(root), self.formatter.retrieve_exec_dependencies(root), + self.formatter.retrieve_test_dependencies(root), ) self.perform_check( From c36d1b8c501f08de0ba67d7328710fb9ddc9da41 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 15:09:18 +0200 Subject: [PATCH 04/25] Implemented buildtool depend checks and improved general formatting --- .../helpers/pkg_xml_formatter.py | 185 +++++++++++-- package_xml_validation/helpers/workspace.py | 1 + .../package_xml_validator.py | 256 +++++++++++++----- .../corrected_01_fail.xml | 9 +- .../corrected_13_fail.xml | 28 ++ .../original_02_correct.xml | 11 +- .../package_xml_examples/original_04_fail.xml | 3 +- .../package_xml_examples/original_07_fail.xml | 6 +- .../package_xml_examples/original_13_fail.xml | 26 ++ ...test_package_xml_linting_and_formatting.py | 3 + 10 files changed, 424 insertions(+), 104 deletions(-) create mode 100644 tests/examples/package_xml_examples/corrected_13_fail.xml create mode 100644 tests/examples/package_xml_examples/original_13_fail.xml diff --git a/package_xml_validation/helpers/pkg_xml_formatter.py b/package_xml_validation/helpers/pkg_xml_formatter.py index f035ece..a366b9d 100644 --- a/package_xml_validation/helpers/pkg_xml_formatter.py +++ b/package_xml_validation/helpers/pkg_xml_formatter.py @@ -1,11 +1,15 @@ from lxml import etree as ET from copy import deepcopy + try: from .logger import get_logger except ImportError: from helpers.logger import get_logger +# tuple of (element_name, min_occurrences, max_occurrences) +# min_occurrences is 1 if the element is required, 0 if it can be missing +# max_occurrences is None if there is no limit, otherwise it is a positive integer ELEMENTS = [ ("name", 1, 1), ("version", 1, 1), @@ -27,6 +31,18 @@ ("export", 0, 1), ] +NEW_LINE_BEFORE = [ + "buildtool_depend", + "build_depend", + "depend", + "exec_depend", + "doc_depend", + "test_depend", + "group_depend", + "member_of_group", + "export", +] + class PackageXmlFormatter: def __init__( @@ -48,7 +64,7 @@ def __init__( def check_dependency_order(self, root, xml_file): """Check and optionally correct the order of dependencies in the package.xml file (with comment preservation using lxml).""" - dependency_order = [elm[0] for elm in ELEMENTS if "depend" in elm[0]] + dependency_order = [elm[0] for elm in ELEMENTS] dependencies_with_comments = {dep: [] for dep in dependency_order} current_order = [] @@ -96,42 +112,37 @@ def check_dependency_order(self, root, xml_file): if not order_mismatch: return True + indentation = root[0].tail.replace("\n", "") # Remove old dependency elements from root for dep_type in dependency_order: for elem in dependencies_with_comments[dep_type]: for comment in elem[1]: root.remove(comment) root.remove(elem[0]) - - # Find index of or append at end - export_index = next( - (i for i, elem in enumerate(root) if elem.tag == "export"), len(root) - ) - member_of_group_index = next( - (i for i, elem in enumerate(root) if elem.tag == "member_of_group"), - len(root), - ) - group_dep_index = next( - (i for i, elem in enumerate(root) if elem.tag == "group_depend"), len(root) - ) - - indendantion = root[0].tail.replace("\n", "") - # Reinsert sorted dependencies before , , or - insert_index = min(export_index, member_of_group_index, group_dep_index) - for dep_type in dependency_order: + # filter out empty lists from dependency order + dependency_order = [ + dep for dep in dependency_order if dependencies_with_comments[dep] + ] + insert_index = 0 + for index, dep_type in enumerate(dependency_order): sorted_elems = sorted( dependencies_with_comments[dep_type], key=lambda x: x[0].text ) for i, elem_with_comment in enumerate(sorted_elems): - if i != len(sorted_elems) - 1: - elem_with_comment[0].tail = "\n" + indendantion + if ( + i == len(sorted_elems) - 1 + and index + 1 < len(dependency_order) + and dependency_order[index + 1] in NEW_LINE_BEFORE + ): + elem_with_comment[0].tail = "\n\n" + indentation else: - elem_with_comment[0].tail = "\n\n" + indendantion + elem_with_comment[0].tail = "\n" + indentation for comment in elem_with_comment[1]: root.insert(insert_index, comment) insert_index += 1 root.insert(insert_index, elem_with_comment[0]) insert_index += 1 + root[-1].tail = "\n" self.logger.info(f"Corrected dependency order in {xml_file}.") return False @@ -251,13 +262,26 @@ def sort_key(elem): elements_with_comments = [] current_comments = [] + last_tail = "" + indentation = root[0].tail.replace("\n", "") for elem in root: if elem.tag is ET.Comment: - current_comments.append(deepcopy(elem)) self.logger.error(f"Found comment: {elem.text}") + if last_tail and last_tail[-1] == "\n": + # inline comment -> append to previous element + elements_with_comments[-1][0].tail = elem.tail + elem.tail = "\n" + indentation + elements_with_comments[-1][1].append(deepcopy(elem)) + else: + # Ensure only one '\n' in elem.tail + elem.tail = "\n" + ( + elem.tail.replace("\n", "") if elem.tail else "" + ) + current_comments.append(deepcopy(elem)) else: elements_with_comments.append((deepcopy(elem), current_comments)) current_comments = [] + last_tail = elem.tail if elem.tail else "" # Sort the elements based on the expected order elements_with_comments.sort(key=lambda x: sort_key(x[0])) @@ -323,6 +347,80 @@ def remove_inner_newlines(s): elm.tail = "\n" + indendantion return False + def check_indentation(self, root, level=1, indentation=" "): + """ + Check if the indentation of the XML file is correct. + recursively checks the indentation of each element. + """ + is_correct = True + + def check_indentation_string(string, expected_indent) -> bool: + """The string should be indented with the expected_indent and contain a newline.""" + if not string or not isinstance(string, str): + return False + parsed_indentation = string.replace("\n", "") + return parsed_indentation == expected_indent and "\n" in string + + def fix_indentation(string, expected_indent) -> str: + """Fix the indentation of the string to match the expected_indent.""" + indent = string.replace(" ", "") if string and "\n" in string else "\n" + return indent + expected_indent + + def check_and_correct(string, expected_indent, name): + """Check and correct the indentation of the string.""" + if not check_indentation_string(string, expected_indent): + self.logger.error( + f"Incorrect indentation for element '{name}'. Expected: '{expected_indent}', Found: '{string.replace('\n', '') if string else 'None'}'" + ) + is_correct = False + if not self.check_only: + string = fix_indentation(string, expected_indent) + + for index, elem in enumerate(root): + is_last = index == len(root) - 1 + expected_indent = ( + indentation * (level - 1) if is_last else indentation * level + ) + if not check_indentation_string(elem.tail, expected_indent): + print( + f"elem.text: {elem.text if elem.text else 'None'}, elem.tail: {elem.tail if elem.tail else 'None'}, expected_indent: {expected_indent}" + ) + self.logger.error( + f"Incorrect indentation for element '{elem.tag}'. Expected: '{expected_indent}', Found: '{elem.tail.replace('\n', '') if elem.tail else 'None'}'" + ) + is_correct = False + if not self.check_only: + elem.tail = fix_indentation(elem.tail, expected_indent) + if len(elem) > 0: # has children + # Check indentation of first child (indentation will be in text of parent element) + expected_indent = indentation * (level + 1) + if not check_indentation_string(elem.text, expected_indent): + self.logger.error( + f"Incorrect indentation for element '{elem.tag}'. Expected: '{expected_indent}', Found: '{elem.text.replace('\n', '')}'" + ) + is_correct = False + if not self.check_only: + elem.text = fix_indentation(elem.text, expected_indent) + # check children recursively + if not self.check_indentation(elem, level + 1, indentation): + is_correct = False + else: + # make sure there are no new lines in texts + if elem.text and "\n" in elem.text: + self.logger.error( + f"Element '{elem.tag}' has new lines in its text: '{elem.text}'" + ) + is_correct = False + if not self.check_only: + elem.text = elem.text.replace("\n", " ").strip() + + def prettyprint(element, **kwargs): + xml = ET.tostring(element, pretty_print=True, **kwargs) + print(xml.decode(), end="") + + prettyprint(root) + return is_correct + def check_for_non_existing_tags(self, root, xml_file): """Check for non-existing tags in the XML file.""" non_existing_tags = [] @@ -460,6 +558,49 @@ def add_build_type_export(self, root, build_type: str): build_type_elem.text = build_type export.text = "\n" + 2 * indendantion + def add_buildtool_depends(self, root, buildtool: list[str]): + """ + Add the buildtool_depend to the XML file. + If the buildtool_depend already exists, it will be updated. + If it does not exist, it will be created. + """ + indendantion = root[0].tail.replace("\n", "") + # 1. clear existing buildtool_depend elements + for elem in root.findall("buildtool_depend"): + root.remove(elem) + # 2. insertposition -> after license, url, or author element + insert_position = 0 + for i, elem in enumerate(root): + if isinstance(elem.tag, str) and elem.tag == "license": + insert_position = i + 1 + elif isinstance(elem.tag, str) and elem.tag == "url": + insert_position = i + 1 + elif isinstance(elem.tag, str) and elem.tag == "author": + insert_position = i + 1 + # 3. add buildtool_depend elements + for i, tool in enumerate(buildtool): + is_last = i == len(buildtool) - 1 + new_elem = ET.Element("buildtool_depend") + new_elem.text = tool + new_elem.tail = "\n\n" if is_last else "\n" + new_elem.tail += indendantion + root.insert(insert_position, new_elem) + insert_position += 1 + + def add_member_of_group(self, root, group_name: str): + """Add member_of_group element to the XML file.""" + indendantion = root[0].tail.replace("\n", "") + member_of_group = ET.Element("member_of_group") + member_of_group.text = group_name + member_of_group.tail = "\n\n" + indendantion + # insert position -> right before export or at the end + insert_position = len(root) + for i, elem in enumerate(root): + if isinstance(elem.tag, str) and elem.tag == "export": + insert_position = i + break + root.insert(insert_position, member_of_group) + if __name__ == "__main__": # Example usage diff --git a/package_xml_validation/helpers/workspace.py b/package_xml_validation/helpers/workspace.py index 30a46a5..9d4ebed 100644 --- a/package_xml_validation/helpers/workspace.py +++ b/package_xml_validation/helpers/workspace.py @@ -112,6 +112,7 @@ def get_pkgs_in_wrs(path: Path) -> List[str]: path = Path(path).resolve(strict=True) if not path.exists(): raise ValueError(f"Path does not exist: {path}") + pkg_dir = None try: pkg_dir = find_package_dir(path) ws_root = find_workspace_root(pkg_dir) diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index 78266c8..e4cf5b0 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -2,6 +2,7 @@ import os from typing import List from lxml import etree as ET +from enum import Enum try: from .helpers.logger import get_logger @@ -16,6 +17,13 @@ from helpers.cmake_parsers import read_deps_from_cmake_file from helpers.find_launch_dependencies import scan_files import subprocess +import re + + +class PackageType(Enum): + CMAKE_PKG = "ament_cmake" + PYTHON_PKG = "ament_python" + MSG_PKG = "rosidl_default_generators" class PackageXmlValidator: @@ -46,7 +54,7 @@ def __init__( self.encountered_unresolvable_error = False # calculate num checks - self.num_checks = 8 + self.num_checks = 11 self.check_count = 1 if self.check_rosdeps: self.num_checks += 1 @@ -73,6 +81,25 @@ def find_package_xml_files(self, paths): package_xml_files = list(set(package_xml_files)) return package_xml_files + def get_package_type(self, xml_file: str) -> tuple[PackageType, bool]: + """Determine the package type based on the presence of CMakeLists.txt or setup.py. + Returns where bool indicates if the package is a message package.""" + cmake_file = os.path.join(os.path.dirname(xml_file), "CMakeLists.txt") + setup_file = os.path.join(os.path.dirname(xml_file), "setup.py") + is_msg_pkg = False + pkg_type = PackageType.CMAKE_PKG + if os.path.exists(cmake_file): + # check if rosidl_generate_interfaces is in CMakeLists.txt using regex + regex = r"rosidl_generate_interfaces\s*\(\s*.*?\)" + with open(cmake_file, "r") as f: + content = f.read() + if re.search(regex, content, re.DOTALL): + is_msg_pkg = True + pkg_type = PackageType.CMAKE_PKG + elif os.path.exists(setup_file): + pkg_type = PackageType.PYTHON_PKG + return pkg_type, is_msg_pkg + def check_for_rosdeps(self, rosdeps: List[str], xml_file: str): """extract list of rosdeps and check if they are valid""" if not rosdeps: @@ -186,9 +213,6 @@ def validate_launch_folders( ) -> bool: launch_deps = extract_launch_deps(launch_folder_names) if not launch_deps: - self.logger.debug( - f"No launch dependencies found in {package_name}/package.xml." - ) return True missing_deps = [ @@ -204,6 +228,12 @@ def validate_launch_folders( if self.check_only: return False + elif not self.auto_fill_missing_deps: + self.encountered_unresolvable_error = True + self.logger.error( + f"Cannot auto-fill missing <{depend_tag}> dependencies: {missing_deps} in {package_name}/package.xml. Please add them manually." + ) + return False else: self.logger.info( f"Auto-filling {len(missing_deps)} missing <{depend_tag}> dependencies in {package_name}/package.xml." @@ -233,41 +263,96 @@ def validate_launch_folders( test_deps_valid = validate_launch_folders(["test"], test_deps, "test_depend") return launch_deps_valid and test_deps_valid - def validate_ament_exports(self, root, xml_file: str): - """Validate ament_export tags in the package.xml file. - if a CMakeLists.txt file exists: ament_cmake must be present - if a setup.py file exists: ament_python must be present + def validate_buildtool_depend(self, root, xml_file: str): + """Validate build_tool depend tags in the package.xml file. + Note: for interface packages 2 buildtool_depend tags are required: ament_cmake and rosidl_default_generators """ - cmake_file = os.path.join(os.path.dirname(xml_file), "CMakeLists.txt") - setup_file = os.path.join(os.path.dirname(xml_file), "setup.py") - cmake_present = os.path.exists(cmake_file) - setup_present = os.path.exists(setup_file) + pkg_type, is_msg_pkg = self.get_package_type(xml_file) + buildtool = root.findall("buildtool_depend") + buildtool = [tool.text for tool in buildtool] + is_buildtool_correct = ( + len(buildtool) > 0 + and ( + ( + pkg_type == PackageType.CMAKE_PKG + and PackageType.CMAKE_PKG.value in buildtool + ) + or ( + pkg_type == PackageType.PYTHON_PKG + and PackageType.PYTHON_PKG.value in buildtool + ) + ) + and (not is_msg_pkg or PackageType.MSG_PKG.value in buildtool) + ) + + if is_buildtool_correct: + return True + else: + corrected_buildtool_str = f"{'ament_cmake' if pkg_type == PackageType.CMAKE_PKG else 'ament_python'}" + if is_msg_pkg: + corrected_buildtool_str += ( + f" {PackageType.MSG_PKG.value}" + ) + if len(buildtool) == 0: + self.logger.error( + f"Missing tag in package.xml. Please include {corrected_buildtool_str}." + ) + else: + self.logger.error( + f"Incorrect in package.xml. Expected {corrected_buildtool_str}, found {buildtool if buildtool is not None else 'None'}." + ) + if self.check_only: + return False + if not self.auto_fill_missing_deps: + self.logger.error( + f"Cannot auto-fill missing in {os.path.basename(os.path.dirname(xml_file))}/package.xml. Please add it manually." + ) + self.encountered_unresolvable_error = True + return False + else: + self.formatter.add_buildtool_depends( + root, + [pkg_type.value, PackageType.MSG_PKG.value] + if is_msg_pkg + else [pkg_type.value], + ) + self.logger.warning( + f"Auto-filling {corrected_buildtool_str} in {os.path.basename(os.path.dirname(xml_file))}/package.xml." + ) + return False # Indicate that changes were made + + def validate_ament_exports(self, root, xml_file: str): + """Validate ament_export tags in the package.xml file.""" + pkg_type, _ = self.get_package_type(xml_file) export = root.find("export") export_exists = export is not None build_type = export.find("build_type") if export_exists else None build_type_correct = ( ( - cmake_present + pkg_type == PackageType.CMAKE_PKG and build_type is not None and build_type.text == "ament_cmake" ) or ( - setup_present + pkg_type == PackageType.PYTHON_PKG and build_type is not None and build_type.text == "ament_python" ) - or (not cmake_present and not setup_present) + or ( + not pkg_type == PackageType.CMAKE_PKG + and not pkg_type == PackageType.PYTHON_PKG + ) ) if build_type_correct: return True else: if not export_exists: self.logger.error( - f"Missing tag in package.xml. Please include {'ament_cmake' if cmake_present else 'ament_python'}." + f"Missing tag in package.xml. Please include {'ament_cmake' if pkg_type == PackageType.CMAKE_PKG else 'ament_python'}." ) else: self.logger.error( - f"Incorrect in tag in package.xml. Expected {'ament_cmake' if cmake_present else 'ament_python'}, found {build_type.text if build_type is not None else 'None'}." + f"Incorrect in tag in package.xml. Expected {'ament_cmake' if pkg_type == PackageType.CMAKE_PKG else 'ament_python'}, found {build_type.text if build_type is not None else 'None'}." ) if self.check_only: return False @@ -276,13 +361,40 @@ def validate_ament_exports(self, root, xml_file: str): return False else: self.formatter.add_build_type_export( - root, "ament_cmake" if cmake_present else "ament_python" + root, + "ament_cmake" if pkg_type == PackageType.CMAKE_PKG else "ament_python", ) self.logger.warning( - f"Auto-filling {'ament_cmake' if cmake_present else 'ament_python'} in {os.path.basename(os.path.dirname(xml_file))}/package.xml." + f"Auto-filling {'ament_cmake' if pkg_type == PackageType.CMAKE_PKG else 'ament_python'} in {os.path.basename(os.path.dirname(xml_file))}/package.xml." ) return False # Indicate that changes were made + def validate_member_of_group(self, root, xml_file: str): + """Validate member_of_group tag in the package.xml file. + -> interface packages must include rosidl_interface_packages + """ + member_of_group = root.find("member_of_group") + _, is_msg_pkg = self.get_package_type(xml_file) + if is_msg_pkg and ( + member_of_group is None + or member_of_group.text != "rosidl_interface_packages" + ): + self.logger.error( + f"Missing or incorrect in package.xml. Expected rosidl_interface_packages, found {member_of_group.text if member_of_group is not None else 'None'}." + ) + if self.check_only: + return False + if not self.auto_fill_missing_deps: + self.encountered_unresolvable_error = True + return False + else: + self.formatter.add_member_of_group(root, "rosidl_interface_packages") + self.logger.warning( + f"Auto-filling rosidl_interface_packages in {os.path.basename(os.path.dirname(xml_file))}/package.xml." + ) + return False # Indicate that changes were made + return True + def log_check_result(self, check_name, result): """Log the result of a check.""" if result: @@ -369,51 +481,70 @@ def check_and_format_files(self, package_xml_files): root, xml_file, ) - self.perform_check( - "Check launch dependencies", - self.validate_launch_dependencies, + "Check indentation", + self.formatter.check_indentation, root, - xml_file, - self.formatter.get_package_name(root), - self.formatter.retrieve_exec_dependencies(root), - self.formatter.retrieve_test_dependencies(root), ) - self.perform_check( - "Check build type export", - self.validate_ament_exports, - root, - xml_file, - ) - - # Check rosdeps if enabled - if self.check_rosdeps: - rosdeps = self.formatter.retrieve_all_dependencies(root) - valid = self.perform_check( - "Check ROS dependencies", self.check_for_rosdeps, rosdeps, xml_file - ) - self.encountered_unresolvable_error |= not valid - # Check with xmllint if enabled - if self.check_with_xmllint: - valid = self.perform_check( - "Check with xmllint", self.validate_xml_with_xmllint, xml_file - ) - self.encountered_unresolvable_error |= not valid - # Check for CMake dependencies if enabled - if self.compare_with_cmake: - build_deps = self.formatter.retrieve_build_dependencies(root) - test_deps = self.formatter.retrieve_test_dependencies(root) - valid = self.perform_check( - "Check CMake dependencies", - self.check_for_cmake, - build_deps, - test_deps, - xml_file, - root, - ) - if not self.auto_fill_missing_deps: - self.encountered_unresolvable_error |= not valid + # self.perform_check( + # "Check launch dependencies", + # self.validate_launch_dependencies, + # root, + # xml_file, + # self.formatter.get_package_name(root), + # self.formatter.retrieve_exec_dependencies(root), + # self.formatter.retrieve_test_dependencies(root), + # ) + + # self.perform_check( + # "Check build tool depend", + # self.validate_buildtool_depend, + # root, + # xml_file, + # ) + + # self.perform_check( + # "Check member of group", + # self.validate_member_of_group, + # root, + # xml_file, + # ) + + # self.perform_check( + # "Check build type export", + # self.validate_ament_exports, + # root, + # xml_file, + # ) + + # # Check rosdeps if enabled + # if self.check_rosdeps: + # rosdeps = self.formatter.retrieve_all_dependencies(root) + # valid = self.perform_check( + # "Check ROS dependencies", self.check_for_rosdeps, rosdeps, xml_file + # ) + # self.encountered_unresolvable_error |= not valid + # # Check with xmllint if enabled + # if self.check_with_xmllint: + # valid = self.perform_check( + # "Check with xmllint", self.validate_xml_with_xmllint, xml_file + # ) + # self.encountered_unresolvable_error |= not valid + # # Check for CMake dependencies if enabled + # if self.compare_with_cmake: + # build_deps = self.formatter.retrieve_build_dependencies(root) + # test_deps = self.formatter.retrieve_test_dependencies(root) + # valid = self.perform_check( + # "Check CMake dependencies", + # self.check_for_cmake, + # build_deps, + # test_deps, + # xml_file, + # root, + # ) + # if not self.auto_fill_missing_deps: + # self.encountered_unresolvable_error |= not valid # Write back to file if not in check-only mode if not self.xml_valid and not self.check_only: @@ -512,11 +643,6 @@ def main(): print("Cannot use --compare-with-cmake with --skip-rosdep-key-validation.") args.compare_with_cmake = False - # --auto-fill-missing-deps is only possible with --compare-with-cmake - if not args.compare_with_cmake and args.auto_fill_missing_deps: - print("Cannot use --auto-fill-missing-deps without --compare-with-cmake.") - args.auto_fill_missing_deps = False - formatter = PackageXmlValidator( check_only=args.check_only, verbose=args.verbose, diff --git a/tests/examples/package_xml_examples/corrected_01_fail.xml b/tests/examples/package_xml_examples/corrected_01_fail.xml index 362b82a..86723bd 100644 --- a/tests/examples/package_xml_examples/corrected_01_fail.xml +++ b/tests/examples/package_xml_examples/corrected_01_fail.xml @@ -4,16 +4,14 @@ srdfdom 2.0.7 Parser for Semantic Robot Description Format (SRDF). - MoveIt Release Team - BSD http://ros.org/wiki/srdfdom - https://github.com/ros-planning/srdfdom/issues https://github.com/ros-planning/srdfdom - - Ioan Sucan + https://github.com/ros-planning/srdfdom/issues Guillaume Walck + Ioan Sucan + ament_cmake ament_cmake_python @@ -34,7 +32,6 @@ ament_cmake_pytest ament_lint_auto ament_lint_cmake - ament_cmake diff --git a/tests/examples/package_xml_examples/corrected_13_fail.xml b/tests/examples/package_xml_examples/corrected_13_fail.xml new file mode 100644 index 0000000..26b80fd --- /dev/null +++ b/tests/examples/package_xml_examples/corrected_13_fail.xml @@ -0,0 +1,28 @@ + + + moveit_state_server_msgs + 0.0.0 + Message and action definitions for MoveIt state server interaction. + Aljoscha Schmidt + + MIT + Aljoscha Schmidt + + ament_cmake + + rosidl_default_generators + + action_msgs + geometry_msgs + sensor_msgs + std_msgs + + rosidl_default_runtime + + +rosidl_interface_packages + + + ament_cmake + + diff --git a/tests/examples/package_xml_examples/original_02_correct.xml b/tests/examples/package_xml_examples/original_02_correct.xml index 362b82a..e79b868 100644 --- a/tests/examples/package_xml_examples/original_02_correct.xml +++ b/tests/examples/package_xml_examples/original_02_correct.xml @@ -4,18 +4,16 @@ srdfdom 2.0.7 Parser for Semantic Robot Description Format (SRDF). - MoveIt Release Team - BSD http://ros.org/wiki/srdfdom - https://github.com/ros-planning/srdfdom/issues https://github.com/ros-planning/srdfdom - - Ioan Sucan + https://github.com/ros-planning/srdfdom/issues Guillaume Walck + Ioan Sucan + ament_cmake - ament_cmake_python + rosidl_default_generators console_bridge_vendor libboost-dev @@ -34,6 +32,7 @@ ament_cmake_pytest ament_lint_auto ament_lint_cmake + rosidl_interface_packages ament_cmake diff --git a/tests/examples/package_xml_examples/original_04_fail.xml b/tests/examples/package_xml_examples/original_04_fail.xml index b97c33e..be37fcc 100644 --- a/tests/examples/package_xml_examples/original_04_fail.xml +++ b/tests/examples/package_xml_examples/original_04_fail.xml @@ -9,8 +9,7 @@ ament_cmake - ament_lint_auto - ament_lint_common + ament_lint_autoament_lint_common pluginlib rclcpp diff --git a/tests/examples/package_xml_examples/original_07_fail.xml b/tests/examples/package_xml_examples/original_07_fail.xml index 3e2cea7..f045a6a 100644 --- a/tests/examples/package_xml_examples/original_07_fail.xml +++ b/tests/examples/package_xml_examples/original_07_fail.xml @@ -6,8 +6,8 @@ 0.0.0 TODO TODO: License declaration - - ament_cmake + +ament_cmake pluginlib rclcpp @@ -19,4 +19,4 @@ ament_cmake - + diff --git a/tests/examples/package_xml_examples/original_13_fail.xml b/tests/examples/package_xml_examples/original_13_fail.xml new file mode 100644 index 0000000..9aaf0ae --- /dev/null +++ b/tests/examples/package_xml_examples/original_13_fail.xml @@ -0,0 +1,26 @@ + + + moveit_state_server_msgs + 0.0.0 + Message and action definitions for MoveIt state server interaction. + + Aljoscha Schmidt + Aljoscha Schmidt + MIT + + ament_cmake + + rosidl_default_generators + rosidl_default_runtime + + std_msgs + geometry_msgs + sensor_msgs + action_msgs + + +rosidl_interface_packages + + ament_cmake + + diff --git a/tests/test_package_xml_linting_and_formatting.py b/tests/test_package_xml_linting_and_formatting.py index c2fc7ef..21a474b 100644 --- a/tests/test_package_xml_linting_and_formatting.py +++ b/tests/test_package_xml_linting_and_formatting.py @@ -142,6 +142,9 @@ def test_xml_formatting(self): all_valid_check = formatter.check_and_format_files([original_path]) if self._is_correct_file(fname): + if not all_valid_check: + with open(original_path, "r") as f_after: + print(f"File content after check:\n{f_after.read()}") self.assertTrue( all_valid_check, f"Expected correct file {fname} to pass in check-only mode.", From 7bbf79095b6ed1fb3159e26081d6716583f2b2c3 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 15:32:14 +0200 Subject: [PATCH 05/25] Bugfix general formtting tests --- .../helpers/pkg_xml_formatter.py | 120 +++++++++--------- .../package_xml_validator.py | 116 ++++++++--------- .../corrected_01_fail.xml | 3 +- .../corrected_06_fail.xml | 55 ++++---- .../corrected_07_fail.xml | 2 +- .../corrected_13_fail.xml | 2 +- .../original_02_correct.xml | 2 +- 7 files changed, 148 insertions(+), 152 deletions(-) diff --git a/package_xml_validation/helpers/pkg_xml_formatter.py b/package_xml_validation/helpers/pkg_xml_formatter.py index a366b9d..1012029 100644 --- a/package_xml_validation/helpers/pkg_xml_formatter.py +++ b/package_xml_validation/helpers/pkg_xml_formatter.py @@ -43,6 +43,8 @@ "export", ] +NEW_LINE = "\n" + class PackageXmlFormatter: def __init__( @@ -61,6 +63,10 @@ def __init__( ) self.encountered_unresolvable_error = False + def prettyprint(self, element, **kwargs): + xml = ET.tostring(element, pretty_print=True, **kwargs) + print(xml.decode(), end="") + def check_dependency_order(self, root, xml_file): """Check and optionally correct the order of dependencies in the package.xml file (with comment preservation using lxml).""" @@ -112,7 +118,7 @@ def check_dependency_order(self, root, xml_file): if not order_mismatch: return True - indentation = root[0].tail.replace("\n", "") + indentation = root[0].tail.replace(NEW_LINE, "") # Remove old dependency elements from root for dep_type in dependency_order: for elem in dependencies_with_comments[dep_type]: @@ -136,13 +142,13 @@ def check_dependency_order(self, root, xml_file): ): elem_with_comment[0].tail = "\n\n" + indentation else: - elem_with_comment[0].tail = "\n" + indentation + elem_with_comment[0].tail = NEW_LINE + indentation for comment in elem_with_comment[1]: root.insert(insert_index, comment) insert_index += 1 root.insert(insert_index, elem_with_comment[0]) insert_index += 1 - root[-1].tail = "\n" + root[-1].tail = NEW_LINE self.logger.info(f"Corrected dependency order in {xml_file}.") return False @@ -263,19 +269,19 @@ def sort_key(elem): current_comments = [] last_tail = "" - indentation = root[0].tail.replace("\n", "") + indentation = root[0].tail.replace(NEW_LINE, "") for elem in root: if elem.tag is ET.Comment: self.logger.error(f"Found comment: {elem.text}") - if last_tail and last_tail[-1] == "\n": + if last_tail and last_tail[-1] == NEW_LINE: # inline comment -> append to previous element elements_with_comments[-1][0].tail = elem.tail - elem.tail = "\n" + indentation + elem.tail = NEW_LINE + indentation elements_with_comments[-1][1].append(deepcopy(elem)) else: - # Ensure only one '\n' in elem.tail - elem.tail = "\n" + ( - elem.tail.replace("\n", "") if elem.tail else "" + # Ensure only one NEW_LINE in elem.tail + elem.tail = NEW_LINE + ( + elem.tail.replace(NEW_LINE, "") if elem.tail else "" ) current_comments.append(deepcopy(elem)) else: @@ -305,27 +311,27 @@ def check_for_empty_lines(self, root, xml_file): """ def remove_inner_newlines(s): - first_newline_pos = s.find("\n") - last_newline_pos = s.rfind("\n") + first_newline_pos = s.find(NEW_LINE) + last_newline_pos = s.rfind(NEW_LINE) if first_newline_pos == -1 or first_newline_pos == last_newline_pos: return s start = s[: first_newline_pos + 1] - middle = s[first_newline_pos + 1 : last_newline_pos].replace("\n", "") + middle = s[first_newline_pos + 1 : last_newline_pos].replace(NEW_LINE, "") end = s[last_newline_pos:] return start + middle + end found_empty_lines = False for elm in root: - if elm.tail and elm.tail.count("\n") > 2: + if elm.tail and elm.tail.count(NEW_LINE) > 2: self.logger.info( f"Error: More than one empty line found in {xml_file}." ) found_empty_lines = True if self.check_only: return False - if elm.tail is None or elm.tail.count("\n") == 0: + if elm.tail is None or elm.tail.count(NEW_LINE) == 0: found_empty_lines = True self.logger.info(f"Error: Two Elements in the sane line in {xml_file}.") if self.check_only: @@ -336,15 +342,15 @@ def remove_inner_newlines(s): if not found_empty_lines: return True # elements after last \n - indendantion = root[0].tail[root[0].tail.rfind("\n") + 1 :] + indendantion = root[0].tail[root[0].tail.rfind(NEW_LINE) + 1 :] # correct the empty lines & missing newlines for elm in root: - if elm.tail and elm.tail.count("\n") > 2: + if elm.tail and elm.tail.count(NEW_LINE) > 2: elm.tail = remove_inner_newlines(elm.tail) - elif elm.tail and elm.tail.count("\n") == 0: - elm.tail += "\n" + elif elm.tail and elm.tail.count(NEW_LINE) == 0: + elm.tail += NEW_LINE elif elm.tail is None: - elm.tail = "\n" + indendantion + elm.tail = NEW_LINE + indendantion return False def check_indentation(self, root, level=1, indentation=" "): @@ -358,67 +364,57 @@ def check_indentation_string(string, expected_indent) -> bool: """The string should be indented with the expected_indent and contain a newline.""" if not string or not isinstance(string, str): return False - parsed_indentation = string.replace("\n", "") - return parsed_indentation == expected_indent and "\n" in string + parsed_indentation = string.replace(NEW_LINE, "") + return parsed_indentation == expected_indent and NEW_LINE in string def fix_indentation(string, expected_indent) -> str: """Fix the indentation of the string to match the expected_indent.""" - indent = string.replace(" ", "") if string and "\n" in string else "\n" + indent = ( + string.replace(" ", "") if string and NEW_LINE in string else NEW_LINE + ) return indent + expected_indent - def check_and_correct(string, expected_indent, name): + def check_and_correct(string, expected_indent, name) -> tuple[str, bool]: """Check and correct the indentation of the string.""" if not check_indentation_string(string, expected_indent): self.logger.error( - f"Incorrect indentation for element '{name}'. Expected: '{expected_indent}', Found: '{string.replace('\n', '') if string else 'None'}'" + f"Incorrect indentation for element '{name}'. Expected: '{expected_indent}', Found: '{string.replace(NEW_LINE, '') if string else 'None'}'" ) - is_correct = False if not self.check_only: string = fix_indentation(string, expected_indent) + print(f"Corrected indentation for element '{name}': {string}") + return string, True + return string, False + + root.text, corrected = check_and_correct( + root.text, indentation * level, f"{root.tag}-text" + ) + is_correct &= not corrected for index, elem in enumerate(root): is_last = index == len(root) - 1 expected_indent = ( indentation * (level - 1) if is_last else indentation * level ) - if not check_indentation_string(elem.tail, expected_indent): - print( - f"elem.text: {elem.text if elem.text else 'None'}, elem.tail: {elem.tail if elem.tail else 'None'}, expected_indent: {expected_indent}" - ) - self.logger.error( - f"Incorrect indentation for element '{elem.tag}'. Expected: '{expected_indent}', Found: '{elem.tail.replace('\n', '') if elem.tail else 'None'}'" - ) - is_correct = False - if not self.check_only: - elem.tail = fix_indentation(elem.tail, expected_indent) + elem.tail, corrected = check_and_correct( + elem.tail, expected_indent, f"{elem.tag}-{elem.text[:15]}" + ) + is_correct &= not corrected if len(elem) > 0: # has children - # Check indentation of first child (indentation will be in text of parent element) - expected_indent = indentation * (level + 1) - if not check_indentation_string(elem.text, expected_indent): - self.logger.error( - f"Incorrect indentation for element '{elem.tag}'. Expected: '{expected_indent}', Found: '{elem.text.replace('\n', '')}'" - ) - is_correct = False - if not self.check_only: - elem.text = fix_indentation(elem.text, expected_indent) # check children recursively if not self.check_indentation(elem, level + 1, indentation): is_correct = False else: # make sure there are no new lines in texts - if elem.text and "\n" in elem.text: + if elem.text and NEW_LINE in elem.text: self.logger.error( f"Element '{elem.tag}' has new lines in its text: '{elem.text}'" ) is_correct = False if not self.check_only: - elem.text = elem.text.replace("\n", " ").strip() - - def prettyprint(element, **kwargs): - xml = ET.tostring(element, pretty_print=True, **kwargs) - print(xml.decode(), end="") + elem.text = elem.text.replace(NEW_LINE, " ").strip() - prettyprint(root) + self.logger.info(f"Checked indentation for XML file. Correct: {is_correct}") return is_correct def check_for_non_existing_tags(self, root, xml_file): @@ -489,11 +485,11 @@ def add_dependencies(self, root, dependencies, dep_type): elements = [dep[0] for dep in ELEMENTS] if dep_type not in dep_types: raise ValueError(f"Invalid dependency type: {dep_type}") - indendantion = root[0].tail.replace("\n", "") + indendantion = root[0].tail.replace(NEW_LINE, "") for dep in dependencies: new_elem = ET.Element(dep_type) new_elem.text = dep - new_elem.tail = "\n" + indendantion + new_elem.tail = NEW_LINE + indendantion # add element to root at correct position -> correct dep group and alphabetical order # case 1: dependency group is empty if not root.findall(dep_type): @@ -523,9 +519,9 @@ def add_dependencies(self, root, dependencies, dep_type): # adapt empty lines -> in case element prior ends with empty line move it to the new element if insert_position > 0 and insert_position > first_of_group: previous_element = root[insert_position - 1] - if previous_element.tail and previous_element.tail.count("\n") > 1: + if previous_element.tail and previous_element.tail.count(NEW_LINE) > 1: new_elem.tail = previous_element.tail - previous_element.tail = "\n" + indendantion + previous_element.tail = NEW_LINE + indendantion if insert_position < len(root) - 1: # if next tag is different than the new element, add empty line next_element = root[insert_position + 1] @@ -539,7 +535,7 @@ def add_build_type_export(self, root, build_type: str): If it does not exist, it will be created. Other exports will not be changed(besides the build_type export). """ - indendantion = root[0].tail.replace("\n", "") + indendantion = root[0].tail.replace(NEW_LINE, "") export = root.find("export") if export is None: export = ET.Element("export") @@ -548,15 +544,15 @@ def add_build_type_export(self, root, build_type: str): last_element = root[-1] if last_element.tail: last_element.tail = "\n\n" + indendantion - export.tail = "\n" + export.tail = NEW_LINE root.append(export) build_type_elem = export.find("build_type") if build_type_elem is None: build_type_elem = ET.Element("build_type") - build_type_elem.tail = "\n" + indendantion + build_type_elem.tail = NEW_LINE + indendantion export.append(build_type_elem) build_type_elem.text = build_type - export.text = "\n" + 2 * indendantion + export.text = NEW_LINE + 2 * indendantion def add_buildtool_depends(self, root, buildtool: list[str]): """ @@ -564,7 +560,7 @@ def add_buildtool_depends(self, root, buildtool: list[str]): If the buildtool_depend already exists, it will be updated. If it does not exist, it will be created. """ - indendantion = root[0].tail.replace("\n", "") + indendantion = root[0].tail.replace(NEW_LINE, "") # 1. clear existing buildtool_depend elements for elem in root.findall("buildtool_depend"): root.remove(elem) @@ -582,14 +578,14 @@ def add_buildtool_depends(self, root, buildtool: list[str]): is_last = i == len(buildtool) - 1 new_elem = ET.Element("buildtool_depend") new_elem.text = tool - new_elem.tail = "\n\n" if is_last else "\n" + new_elem.tail = "\n\n" if is_last else NEW_LINE new_elem.tail += indendantion root.insert(insert_position, new_elem) insert_position += 1 def add_member_of_group(self, root, group_name: str): """Add member_of_group element to the XML file.""" - indendantion = root[0].tail.replace("\n", "") + indendantion = root[0].tail.replace(NEW_LINE, "") member_of_group = ET.Element("member_of_group") member_of_group.text = group_name member_of_group.tail = "\n\n" + indendantion diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index e4cf5b0..6c7087f 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -487,64 +487,64 @@ def check_and_format_files(self, package_xml_files): root, ) - # self.perform_check( - # "Check launch dependencies", - # self.validate_launch_dependencies, - # root, - # xml_file, - # self.formatter.get_package_name(root), - # self.formatter.retrieve_exec_dependencies(root), - # self.formatter.retrieve_test_dependencies(root), - # ) - - # self.perform_check( - # "Check build tool depend", - # self.validate_buildtool_depend, - # root, - # xml_file, - # ) - - # self.perform_check( - # "Check member of group", - # self.validate_member_of_group, - # root, - # xml_file, - # ) - - # self.perform_check( - # "Check build type export", - # self.validate_ament_exports, - # root, - # xml_file, - # ) - - # # Check rosdeps if enabled - # if self.check_rosdeps: - # rosdeps = self.formatter.retrieve_all_dependencies(root) - # valid = self.perform_check( - # "Check ROS dependencies", self.check_for_rosdeps, rosdeps, xml_file - # ) - # self.encountered_unresolvable_error |= not valid - # # Check with xmllint if enabled - # if self.check_with_xmllint: - # valid = self.perform_check( - # "Check with xmllint", self.validate_xml_with_xmllint, xml_file - # ) - # self.encountered_unresolvable_error |= not valid - # # Check for CMake dependencies if enabled - # if self.compare_with_cmake: - # build_deps = self.formatter.retrieve_build_dependencies(root) - # test_deps = self.formatter.retrieve_test_dependencies(root) - # valid = self.perform_check( - # "Check CMake dependencies", - # self.check_for_cmake, - # build_deps, - # test_deps, - # xml_file, - # root, - # ) - # if not self.auto_fill_missing_deps: - # self.encountered_unresolvable_error |= not valid + self.perform_check( + "Check launch dependencies", + self.validate_launch_dependencies, + root, + xml_file, + self.formatter.get_package_name(root), + self.formatter.retrieve_exec_dependencies(root), + self.formatter.retrieve_test_dependencies(root), + ) + + self.perform_check( + "Check build tool depend", + self.validate_buildtool_depend, + root, + xml_file, + ) + + self.perform_check( + "Check member of group", + self.validate_member_of_group, + root, + xml_file, + ) + + self.perform_check( + "Check build type export", + self.validate_ament_exports, + root, + xml_file, + ) + + # Check rosdeps if enabled + if self.check_rosdeps: + rosdeps = self.formatter.retrieve_all_dependencies(root) + valid = self.perform_check( + "Check ROS dependencies", self.check_for_rosdeps, rosdeps, xml_file + ) + self.encountered_unresolvable_error |= not valid + # Check with xmllint if enabled + if self.check_with_xmllint: + valid = self.perform_check( + "Check with xmllint", self.validate_xml_with_xmllint, xml_file + ) + self.encountered_unresolvable_error |= not valid + # Check for CMake dependencies if enabled + if self.compare_with_cmake: + build_deps = self.formatter.retrieve_build_dependencies(root) + test_deps = self.formatter.retrieve_test_dependencies(root) + valid = self.perform_check( + "Check CMake dependencies", + self.check_for_cmake, + build_deps, + test_deps, + xml_file, + root, + ) + if not self.auto_fill_missing_deps: + self.encountered_unresolvable_error |= not valid # Write back to file if not in check-only mode if not self.xml_valid and not self.check_only: diff --git a/tests/examples/package_xml_examples/corrected_01_fail.xml b/tests/examples/package_xml_examples/corrected_01_fail.xml index 86723bd..2019cdb 100644 --- a/tests/examples/package_xml_examples/corrected_01_fail.xml +++ b/tests/examples/package_xml_examples/corrected_01_fail.xml @@ -32,7 +32,8 @@ ament_cmake_pytest ament_lint_auto ament_lint_cmake + - ament_cmake + ament_cmake diff --git a/tests/examples/package_xml_examples/corrected_06_fail.xml b/tests/examples/package_xml_examples/corrected_06_fail.xml index b2a9fc7..5ec3a13 100644 --- a/tests/examples/package_xml_examples/corrected_06_fail.xml +++ b/tests/examples/package_xml_examples/corrected_06_fail.xml @@ -1,33 +1,32 @@ - gazebo_robot_sim_athena - 0.0.1 - - Gazebo Sim (Harmonic) simulation package for the Athena robot. - - todo - MIT - ament_cmake + gazebo_robot_sim_athena + 0.0.1 + Gazebo Sim (Harmonic) simulation package for the Athena robot. + todo + MIT - athena_description - athena_launch_config - gz_ros2_control - hector_gazebo_plugins - hector_gazebo_simulation - hector_launch_manager - hector_ros_controllers - launch - launch_ros - rclpy - robot_state_publisher - ros2_control - ros2_controllers - ros_gz_bridge - ros_gz_sim - srdf_publisher - xacro + ament_cmake - - ament_cmake - + athena_description + athena_launch_config + gz_ros2_control + hector_gazebo_plugins + hector_gazebo_simulation + hector_launch_manager + hector_ros_controllers + launch + launch_ros + rclpy + robot_state_publisher + ros2_control + ros2_controllers + ros_gz_bridge + ros_gz_sim + srdf_publisher + xacro + + + ament_cmake + diff --git a/tests/examples/package_xml_examples/corrected_07_fail.xml b/tests/examples/package_xml_examples/corrected_07_fail.xml index ce4f195..6d7a8dc 100644 --- a/tests/examples/package_xml_examples/corrected_07_fail.xml +++ b/tests/examples/package_xml_examples/corrected_07_fail.xml @@ -5,8 +5,8 @@ 0.0.0 Package for managing gamepad inputs TODO + TODO: License declaration - ament_cmake pluginlib diff --git a/tests/examples/package_xml_examples/corrected_13_fail.xml b/tests/examples/package_xml_examples/corrected_13_fail.xml index 26b80fd..0736082 100644 --- a/tests/examples/package_xml_examples/corrected_13_fail.xml +++ b/tests/examples/package_xml_examples/corrected_13_fail.xml @@ -20,7 +20,7 @@ rosidl_default_runtime -rosidl_interface_packages + rosidl_interface_packages ament_cmake diff --git a/tests/examples/package_xml_examples/original_02_correct.xml b/tests/examples/package_xml_examples/original_02_correct.xml index e79b868..ae7d398 100644 --- a/tests/examples/package_xml_examples/original_02_correct.xml +++ b/tests/examples/package_xml_examples/original_02_correct.xml @@ -35,6 +35,6 @@ rosidl_interface_packages - ament_cmake + ament_cmake From 7bcaba6669ce157899e178a0d2838ae096bbde7c Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 15:56:12 +0200 Subject: [PATCH 06/25] extended tests: buildtool_depend and msg pkg --- .../helpers/pkg_xml_formatter.py | 2 - .../pkg_false_buildtool_depend/CMakeLists.txt | 58 +++++++++++++++++++ .../pkg_false_buildtool_depend/package.xml | 28 +++++++++ .../pkg_no_buildtool_depend/CMakeLists.txt | 58 +++++++++++++++++++ .../pkg_no_buildtool_depend/package.xml | 26 +++++++++ .../msg_pkg/pkg_correct/CMakeLists.txt | 13 +++++ .../msg_pkg/pkg_correct/package.xml | 24 ++++++++ .../msg_pkg/pkg_false_export/CMakeLists.txt | 13 +++++ .../msg_pkg/pkg_false_export/package.xml | 24 ++++++++ .../CMakeLists.txt | 13 +++++ .../pkg_missing_ament_buildtool/package.xml | 23 ++++++++ .../msg_pkg/pkg_missing_export/CMakeLists.txt | 13 +++++ .../msg_pkg/pkg_missing_export/package.xml | 21 +++++++ .../pkg_missing_member_group/CMakeLists.txt | 13 +++++ .../pkg_missing_member_group/package.xml | 23 ++++++++ .../CMakeLists.txt | 13 +++++ .../pkg_missing_rosidl_buildtool/package.xml | 23 ++++++++ .../pkg_no_buildtool_depend/CMakeLists.txt | 13 +++++ .../pkg_no_buildtool_depend/package.xml | 21 +++++++ ... => test_pky_type_dependent_validation.py} | 2 +- 20 files changed, 421 insertions(+), 3 deletions(-) create mode 100644 tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/package.xml create mode 100644 tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/package.xml create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_correct/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_correct/package.xml create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_false_export/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_false_export/package.xml create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/package.xml create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/package.xml create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/package.xml create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/package.xml create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/CMakeLists.txt create mode 100644 tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/package.xml rename tests/{test_export_tag_correction.py => test_pky_type_dependent_validation.py} (98%) diff --git a/package_xml_validation/helpers/pkg_xml_formatter.py b/package_xml_validation/helpers/pkg_xml_formatter.py index 1012029..2a05430 100644 --- a/package_xml_validation/helpers/pkg_xml_formatter.py +++ b/package_xml_validation/helpers/pkg_xml_formatter.py @@ -382,7 +382,6 @@ def check_and_correct(string, expected_indent, name) -> tuple[str, bool]: ) if not self.check_only: string = fix_indentation(string, expected_indent) - print(f"Corrected indentation for element '{name}': {string}") return string, True return string, False @@ -414,7 +413,6 @@ def check_and_correct(string, expected_indent, name) -> tuple[str, bool]: if not self.check_only: elem.text = elem.text.replace(NEW_LINE, " ").strip() - self.logger.info(f"Checked indentation for XML file. Correct: {is_correct}") return is_correct def check_for_non_existing_tags(self, root, xml_file): diff --git a/tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/CMakeLists.txt b/tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/CMakeLists.txt new file mode 100644 index 0000000..8b10ece --- /dev/null +++ b/tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/CMakeLists.txt @@ -0,0 +1,58 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_gamepad_manager) + +if(NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 17) + set(CMAKE_CXX_STANDARD_REQUIRED ON) +endif() + +if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wall -Wextra -Wpedantic) +endif() + +find_package(ament_cmake REQUIRED) +set(DEPENDENCIES pluginlib rclcpp sensor_msgs yaml-cpp + hector_gamepad_plugin_interface) + +foreach(dependency IN LISTS DEPENDENCIES) + find_package(${dependency} REQUIRED) +endforeach() + +set(HEADERS include/hector_gamepad_manager/hector_gamepad_manager.hpp) + +set(SOURCES src/hector_gamepad_manager.cpp) + +add_library(${PROJECT_NAME} SHARED ${SOURCES} ${HEADERS}) +target_include_directories( + ${PROJECT_NAME} PUBLIC $ + $) +target_link_libraries(${PROJECT_NAME} PUBLIC yaml-cpp) +ament_target_dependencies(${PROJECT_NAME} PUBLIC ${DEPENDENCIES}) + +add_executable(hector_gamepad_manager_node src/hector_gamepad_manager_node.cpp) +target_link_libraries(hector_gamepad_manager_node PUBLIC ${PROJECT_NAME}) + +install( + TARGETS ${PROJECT_NAME} + EXPORT ${PROJECT_NAME}-targets + LIBRARY DESTINATION lib) +install(DIRECTORY include/ DESTINATION include) +install(TARGETS hector_gamepad_manager_node DESTINATION lib/${PROJECT_NAME}) +install(DIRECTORY launch config DESTINATION share/${PROJECT_NAME}) + +if(BUILD_TESTING) + find_package(ament_cmake_gtest REQUIRED) + find_package(ros_testing REQUIRED) + ament_add_gtest_executable(${PROJECT_NAME}_test + test/test_hector_gamepad_manager.cpp) + target_link_libraries(${PROJECT_NAME}_test ${PROJECT_NAME}) + ament_target_dependencies(${PROJECT_NAME}_test + ${THIS_PACKAGE_INCLUDE_DEPENDS}) + add_ros_test(test/test_hector_gamepad_manager.launch.py ARGS + "test_binary_dir:=${CMAKE_CURRENT_BINARY_DIR}") +endif() + +ament_export_targets(${PROJECT_NAME}-targets) +ament_export_include_directories(include) +ament_export_dependencies(${DEPENDENCIES}) +ament_package() diff --git a/tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/package.xml b/tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/package.xml new file mode 100644 index 0000000..f44d893 --- /dev/null +++ b/tests/examples/export_tag_examples/ament_cmake/pkg_false_buildtool_depend/package.xml @@ -0,0 +1,28 @@ + + + + hector_gamepad_manager + 0.0.0 + Package for managing gamepad inputs + Simon Giegerich + TODO: License declaration + + ament_python + + hector_gamepad_plugin_interface + pluginlib + rclcpp + sensor_msgs + yaml-cpp + + hector_gamepad_manager_plugins + + ament_cmake_gtest + ament_lint_auto + ament_lint_common + ros_testing + + + ament_cmake + + diff --git a/tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/CMakeLists.txt b/tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/CMakeLists.txt new file mode 100644 index 0000000..8b10ece --- /dev/null +++ b/tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/CMakeLists.txt @@ -0,0 +1,58 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_gamepad_manager) + +if(NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 17) + set(CMAKE_CXX_STANDARD_REQUIRED ON) +endif() + +if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wall -Wextra -Wpedantic) +endif() + +find_package(ament_cmake REQUIRED) +set(DEPENDENCIES pluginlib rclcpp sensor_msgs yaml-cpp + hector_gamepad_plugin_interface) + +foreach(dependency IN LISTS DEPENDENCIES) + find_package(${dependency} REQUIRED) +endforeach() + +set(HEADERS include/hector_gamepad_manager/hector_gamepad_manager.hpp) + +set(SOURCES src/hector_gamepad_manager.cpp) + +add_library(${PROJECT_NAME} SHARED ${SOURCES} ${HEADERS}) +target_include_directories( + ${PROJECT_NAME} PUBLIC $ + $) +target_link_libraries(${PROJECT_NAME} PUBLIC yaml-cpp) +ament_target_dependencies(${PROJECT_NAME} PUBLIC ${DEPENDENCIES}) + +add_executable(hector_gamepad_manager_node src/hector_gamepad_manager_node.cpp) +target_link_libraries(hector_gamepad_manager_node PUBLIC ${PROJECT_NAME}) + +install( + TARGETS ${PROJECT_NAME} + EXPORT ${PROJECT_NAME}-targets + LIBRARY DESTINATION lib) +install(DIRECTORY include/ DESTINATION include) +install(TARGETS hector_gamepad_manager_node DESTINATION lib/${PROJECT_NAME}) +install(DIRECTORY launch config DESTINATION share/${PROJECT_NAME}) + +if(BUILD_TESTING) + find_package(ament_cmake_gtest REQUIRED) + find_package(ros_testing REQUIRED) + ament_add_gtest_executable(${PROJECT_NAME}_test + test/test_hector_gamepad_manager.cpp) + target_link_libraries(${PROJECT_NAME}_test ${PROJECT_NAME}) + ament_target_dependencies(${PROJECT_NAME}_test + ${THIS_PACKAGE_INCLUDE_DEPENDS}) + add_ros_test(test/test_hector_gamepad_manager.launch.py ARGS + "test_binary_dir:=${CMAKE_CURRENT_BINARY_DIR}") +endif() + +ament_export_targets(${PROJECT_NAME}-targets) +ament_export_include_directories(include) +ament_export_dependencies(${DEPENDENCIES}) +ament_package() diff --git a/tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/package.xml b/tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/package.xml new file mode 100644 index 0000000..76da736 --- /dev/null +++ b/tests/examples/export_tag_examples/ament_cmake/pkg_no_buildtool_depend/package.xml @@ -0,0 +1,26 @@ + + + + hector_gamepad_manager + 0.0.0 + Package for managing gamepad inputs + Simon Giegerich + TODO: License declaration + + hector_gamepad_plugin_interface + pluginlib + rclcpp + sensor_msgs + yaml-cpp + + hector_gamepad_manager_plugins + + ament_cmake_gtest + ament_lint_auto + ament_lint_common + ros_testing + + + ament_cmake + + diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_correct/CMakeLists.txt b/tests/examples/export_tag_examples/msg_pkg/pkg_correct/CMakeLists.txt new file mode 100644 index 0000000..4e454e4 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_correct/CMakeLists.txt @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_transmission_interface_msgs) + +find_package(ament_cmake REQUIRED) +find_package(rosidl_default_generators REQUIRED) +find_package(sensor_msgs REQUIRED) + +rosidl_generate_interfaces(${PROJECT_NAME} srv/AdjustTransmissionOffsets.srv + DEPENDENCIES sensor_msgs) + +ament_export_dependencies(rosidl_default_runtime) + +ament_package() diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_correct/package.xml b/tests/examples/export_tag_examples/msg_pkg/pkg_correct/package.xml new file mode 100644 index 0000000..4ad2b87 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_correct/package.xml @@ -0,0 +1,24 @@ + + + + hector_transmission_interface_msgs + 0.0.0 + TODO + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + ament_cmake + rosidl_default_generators + + rosidl_default_generators + sensor_msgs + + rosidl_default_runtime + + rosidl_interface_packages + + ament_cmake + + diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_false_export/CMakeLists.txt b/tests/examples/export_tag_examples/msg_pkg/pkg_false_export/CMakeLists.txt new file mode 100644 index 0000000..4e454e4 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_false_export/CMakeLists.txt @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_transmission_interface_msgs) + +find_package(ament_cmake REQUIRED) +find_package(rosidl_default_generators REQUIRED) +find_package(sensor_msgs REQUIRED) + +rosidl_generate_interfaces(${PROJECT_NAME} srv/AdjustTransmissionOffsets.srv + DEPENDENCIES sensor_msgs) + +ament_export_dependencies(rosidl_default_runtime) + +ament_package() diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_false_export/package.xml b/tests/examples/export_tag_examples/msg_pkg/pkg_false_export/package.xml new file mode 100644 index 0000000..246d1a9 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_false_export/package.xml @@ -0,0 +1,24 @@ + + + + hector_transmission_interface_msgs + 0.0.0 + TODO + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + ament_cmake + rosidl_default_generators + + rosidl_default_generators + sensor_msgs + + rosidl_default_runtime + + rosidl_interface_packages + + rosidl_default_generators + + diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/CMakeLists.txt b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/CMakeLists.txt new file mode 100644 index 0000000..4e454e4 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/CMakeLists.txt @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_transmission_interface_msgs) + +find_package(ament_cmake REQUIRED) +find_package(rosidl_default_generators REQUIRED) +find_package(sensor_msgs REQUIRED) + +rosidl_generate_interfaces(${PROJECT_NAME} srv/AdjustTransmissionOffsets.srv + DEPENDENCIES sensor_msgs) + +ament_export_dependencies(rosidl_default_runtime) + +ament_package() diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/package.xml b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/package.xml new file mode 100644 index 0000000..18b8762 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_ament_buildtool/package.xml @@ -0,0 +1,23 @@ + + + + hector_transmission_interface_msgs + 0.0.0 + TODO + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + rosidl_default_generators + + rosidl_default_generators + sensor_msgs + + rosidl_default_runtime + + rosidl_interface_packages + + ament_cmake + + diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/CMakeLists.txt b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/CMakeLists.txt new file mode 100644 index 0000000..4e454e4 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/CMakeLists.txt @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_transmission_interface_msgs) + +find_package(ament_cmake REQUIRED) +find_package(rosidl_default_generators REQUIRED) +find_package(sensor_msgs REQUIRED) + +rosidl_generate_interfaces(${PROJECT_NAME} srv/AdjustTransmissionOffsets.srv + DEPENDENCIES sensor_msgs) + +ament_export_dependencies(rosidl_default_runtime) + +ament_package() diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/package.xml b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/package.xml new file mode 100644 index 0000000..8f4d68b --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_export/package.xml @@ -0,0 +1,21 @@ + + + + hector_transmission_interface_msgs + 0.0.0 + TODO + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + ament_cmake + rosidl_default_generators + + rosidl_default_generators + sensor_msgs + + rosidl_default_runtime + + rosidl_interface_packages + diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/CMakeLists.txt b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/CMakeLists.txt new file mode 100644 index 0000000..4e454e4 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/CMakeLists.txt @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_transmission_interface_msgs) + +find_package(ament_cmake REQUIRED) +find_package(rosidl_default_generators REQUIRED) +find_package(sensor_msgs REQUIRED) + +rosidl_generate_interfaces(${PROJECT_NAME} srv/AdjustTransmissionOffsets.srv + DEPENDENCIES sensor_msgs) + +ament_export_dependencies(rosidl_default_runtime) + +ament_package() diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/package.xml b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/package.xml new file mode 100644 index 0000000..9fac631 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_member_group/package.xml @@ -0,0 +1,23 @@ + + + + hector_transmission_interface_msgs + 0.0.0 + TODO + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + ament_cmake + rosidl_default_generators + + rosidl_default_generators + sensor_msgs + + rosidl_default_runtime + + + ament_cmake + + diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/CMakeLists.txt b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/CMakeLists.txt new file mode 100644 index 0000000..4e454e4 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/CMakeLists.txt @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_transmission_interface_msgs) + +find_package(ament_cmake REQUIRED) +find_package(rosidl_default_generators REQUIRED) +find_package(sensor_msgs REQUIRED) + +rosidl_generate_interfaces(${PROJECT_NAME} srv/AdjustTransmissionOffsets.srv + DEPENDENCIES sensor_msgs) + +ament_export_dependencies(rosidl_default_runtime) + +ament_package() diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/package.xml b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/package.xml new file mode 100644 index 0000000..c37bd14 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_missing_rosidl_buildtool/package.xml @@ -0,0 +1,23 @@ + + + + hector_transmission_interface_msgs + 0.0.0 + TODO + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + ament_cmake + + rosidl_default_generators + sensor_msgs + + rosidl_default_runtime + + rosidl_interface_packages + + ament_cmake + + diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/CMakeLists.txt b/tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/CMakeLists.txt new file mode 100644 index 0000000..4e454e4 --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/CMakeLists.txt @@ -0,0 +1,13 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_transmission_interface_msgs) + +find_package(ament_cmake REQUIRED) +find_package(rosidl_default_generators REQUIRED) +find_package(sensor_msgs REQUIRED) + +rosidl_generate_interfaces(${PROJECT_NAME} srv/AdjustTransmissionOffsets.srv + DEPENDENCIES sensor_msgs) + +ament_export_dependencies(rosidl_default_runtime) + +ament_package() diff --git a/tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/package.xml b/tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/package.xml new file mode 100644 index 0000000..cc8141f --- /dev/null +++ b/tests/examples/export_tag_examples/msg_pkg/pkg_no_buildtool_depend/package.xml @@ -0,0 +1,21 @@ + + + + hector_transmission_interface_msgs + 0.0.0 + TODO + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + rosidl_default_generators + sensor_msgs + + rosidl_default_runtime + + rosidl_interface_packages + + ament_cmake + + diff --git a/tests/test_export_tag_correction.py b/tests/test_pky_type_dependent_validation.py similarity index 98% rename from tests/test_export_tag_correction.py rename to tests/test_pky_type_dependent_validation.py index 7a3dbff..b6dd457 100644 --- a/tests/test_export_tag_correction.py +++ b/tests/test_pky_type_dependent_validation.py @@ -96,7 +96,7 @@ def test_xml_formatting(self): """ Iterate over all example packages in the test directory, """ - build_types = ["ament_cmake"] # "ament_python" + build_types = ["ament_cmake", "msg_pkg"] # "ament_python" for build_type in build_types: correct_xml = os.path.join( self.examples_dir, build_type, "pkg_correct", "package.xml" From 5a3259f1449828a551c1090d0b1a18ca086003ff Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 16:13:33 +0200 Subject: [PATCH 07/25] added launch deps test --- .../package_xml_validator.py | 3 + .../pkg_correct/CMakeLists.txt | 75 + .../launch/controller_manager.launch.yaml | 43 + .../launch/load_description.launch.py | 72 + .../pkg_correct/package.xml | 37 + .../pkg_missing_launch_deps/CMakeLists.txt | 75 + .../launch/controller_manager.launch.yaml | 43 + .../launch/load_description.launch.py | 72 + .../pkg_missing_launch_deps/package.xml | 34 + .../pkg_correct/CMakeLists.txt | 48 + .../pkg_correct/config/athena.yaml | 45 + .../hector_controller_spawner_launch.yml | 8 + .../pkg_correct/package.xml | 30 + .../pkg_correct/test/config/athena.urdf | 2110 +++++++++++++++++ .../test/config/controller_spawner.yaml | 40 + .../controller_spawner_chain_detection.yaml | 40 + .../config/controller_spawner_with_estop.yaml | 40 + .../pkg_correct/test/config/controllers.yaml | 95 + .../test/test_controller_spawner.cpp | 0 .../test/test_spawner_basic.test.py | 139 ++ .../test/test_spawner_chain_detection.test.py | 139 ++ .../test/test_spawner_estop.test.py | 381 +++ .../test/test_spawner_twice.test.py | 235 ++ .../pkg_missing_test_depends/CMakeLists.txt | 48 + .../config/athena.yaml | 45 + .../hector_controller_spawner_launch.yml | 8 + .../pkg_missing_test_depends/package.xml | 28 + .../test/config/athena.urdf | 2110 +++++++++++++++++ .../test/config/controller_spawner.yaml | 40 + .../controller_spawner_chain_detection.yaml | 40 + .../config/controller_spawner_with_estop.yaml | 40 + .../test/config/controllers.yaml | 95 + .../test/test_controller_spawner.cpp | 0 .../test/test_spawner_basic.test.py | 139 ++ .../test/test_spawner_chain_detection.test.py | 139 ++ .../test/test_spawner_estop.test.py | 381 +++ .../test/test_spawner_twice.test.py | 235 ++ tests/test_pkg_missing_launch_deps.py | 131 + 38 files changed, 7283 insertions(+) create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/CMakeLists.txt create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/controller_manager.launch.yaml create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/load_description.launch.py create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/package.xml create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/CMakeLists.txt create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/controller_manager.launch.yaml create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/load_description.launch.py create mode 100644 tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/package.xml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/CMakeLists.txt create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/config/athena.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/launch/hector_controller_spawner_launch.yml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/package.xml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/athena.urdf create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_chain_detection.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_with_estop.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controllers.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_controller_spawner.cpp create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/CMakeLists.txt create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/config/athena.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/launch/hector_controller_spawner_launch.yml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/package.xml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/athena.urdf create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_chain_detection.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_with_estop.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controllers.yaml create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_controller_spawner.cpp create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py create mode 100644 tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py create mode 100644 tests/test_pkg_missing_launch_deps.py diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index 6c7087f..60dc42f 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -206,6 +206,9 @@ def extract_launch_deps(folder_names: List[str]) -> List[str]: launch_dir = os.path.join(os.path.dirname(package_xml_file), folder) if os.path.isdir(launch_dir): launch_deps.extend(scan_files(launch_dir)) + self.logger.info( + f"Extracted launch dependencies from {package_name}/package.xml: {launch_deps}" + ) return launch_deps def validate_launch_folders( diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/CMakeLists.txt b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/CMakeLists.txt new file mode 100644 index 0000000..fc11ae2 --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/CMakeLists.txt @@ -0,0 +1,75 @@ +cmake_minimum_required(VERSION 3.8) +project(dynamixel_ros_control) + +# Default to C++17 +if (NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 17) + set(CMAKE_CXX_STANDARD_REQUIRED ON) +endif () + +if (CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wall -Wextra -Wpedantic) +endif () + +# Find dependencies +find_package(ament_cmake REQUIRED) +find_package(rclcpp REQUIRED) +find_package(rclcpp_lifecycle REQUIRED) +find_package(lifecycle_msgs REQUIRED) +find_package(hardware_interface REQUIRED) +find_package(pluginlib REQUIRED) +find_package(dynamixel_sdk REQUIRED) +find_package(yaml-cpp REQUIRED) +#find_package(dynamixel_ros_control_msgs REQUIRED) +find_package(std_msgs REQUIRED) +find_package(std_srvs REQUIRED) +find_package(ament_index_cpp REQUIRED) +find_package(transmission_interface REQUIRED) +find_package(hector_transmission_interface REQUIRED) +find_package(hector_transmission_interface_msgs REQUIRED) +find_package(controller_orchestrator REQUIRED) + + +set(HEADERS + include/${PROJECT_NAME}/common.hpp + include/${PROJECT_NAME}/joint.hpp + include/${PROJECT_NAME}/dynamixel.hpp + include/${PROJECT_NAME}/control_table.hpp + include/${PROJECT_NAME}/control_table_item.hpp + include/${PROJECT_NAME}/dynamixel_driver.hpp + include/${PROJECT_NAME}/dynamixel_hardware_interface.hpp + include/${PROJECT_NAME}/sync_read_manager.hpp + include/${PROJECT_NAME}/sync_write_manager.hpp + include/${PROJECT_NAME}/log.hpp +) + +set(SOURCES + src/common.cpp + src/joint.cpp + src/dynamixel.cpp + src/control_table.cpp + src/control_table_item.cpp + src/dynamixel_driver.cpp + src/dynamixel_hardware_interface.cpp + src/sync_read_manager.cpp + src/sync_write_manager.cpp +) +include_directories(include) + +add_library(${PROJECT_NAME} SHARED ${SOURCES} ${HEADERS}) +target_link_libraries(${PROJECT_NAME} PUBLIC yaml-cpp) +ament_target_dependencies(${PROJECT_NAME} PUBLIC rclcpp rclcpp_lifecycle lifecycle_msgs hardware_interface pluginlib + dynamixel_sdk yaml-cpp std_msgs std_srvs ament_index_cpp transmission_interface hector_transmission_interface hector_transmission_interface_msgs controller_orchestrator) + +pluginlib_export_plugin_description_file(hardware_interface dynamixel_ros_control.xml) + +# Install library and include folder +install(TARGETS ${PROJECT_NAME} EXPORT ${PROJECT_NAME}-targets LIBRARY DESTINATION lib) +install(DIRECTORY include/ DESTINATION include) + +# Install directories +install(DIRECTORY launch config devices + DESTINATION share/${PROJECT_NAME} +) + +ament_package() diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/controller_manager.launch.yaml b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/controller_manager.launch.yaml new file mode 100644 index 0000000..12541f3 --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/controller_manager.launch.yaml @@ -0,0 +1,43 @@ +launch: +# Robot description +- include: + file: "$(find-pkg-share dynamixel_ros_control)/launch/load_description.launch.py" + +# Controller manager +- node: + pkg: "controller_manager" + exec: "ros2_control_node" + name: "controller_manager" + param: + - from: "$(find-pkg-share dynamixel_ros_control)/config/test_controllers.yaml" + allow_substs: true + - name: "hardware_components_initial_state.unconfigured" + value: ['hardware_interface'] +# Controller manager lifecycle management +- node: + pkg: "controller_manager" + exec: "hardware_spawner" + name: "hardware_interface_spawner" + output: "screen" + args: "hardware_interface --activate" + respawn: "true" + respawn_delay: 5.0 + +# Controllers +- node: + pkg: "controller_manager" + exec: "spawner" + name: "joint_state_broadcaster_spawner" + output: "screen" + args: "joint_state_broadcaster" + respawn: "true" + respawn_delay: 5.0 + +- node: + pkg: "controller_manager" + exec: "spawner" + name: "position_controller_spawner" + output: "screen" + args: "position_controller" + param: + - from: "$(find-pkg-share dynamixel_ros_control)/config/test_controllers.yaml" diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/load_description.launch.py b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/load_description.launch.py new file mode 100644 index 0000000..3d4ae5c --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/launch/load_description.launch.py @@ -0,0 +1,72 @@ +from launch import LaunchDescription +from launch_ros.actions import Node +from launch.actions import DeclareLaunchArgument +from launch.substitutions import Command, PathJoinSubstitution, LaunchConfiguration +from ament_index_python.packages import get_package_share_directory +from launch_ros.parameter_descriptions import ParameterValue + + +def generate_launch_description(): + ld = LaunchDescription() + + # Parameters + port_name_arg = DeclareLaunchArgument( + name="port_name", + default_value="/dev/ttyUSB0", + description="Path to USB serial converter device", + ) + ld.add_action(port_name_arg) + port_name = LaunchConfiguration("port_name") + + baud_rate_arg = DeclareLaunchArgument( + name="baud_rate", default_value="57600", description="Baud rate" + ) + ld.add_action(baud_rate_arg) + baud_rate = LaunchConfiguration("baud_rate") + + id_arg = DeclareLaunchArgument( + name="id", default_value="1", description="ID of the dynamixel to control" + ) + ld.add_action(id_arg) + id_arg = LaunchConfiguration("id") + + # Get the package directory + urdf_file = PathJoinSubstitution( + [ + get_package_share_directory("dynamixel_ros_control"), + "config", + "test_robot.urdf.xacro", + ] + ) + + robot_description = ParameterValue( + Command( + [ + "xacro ", + urdf_file, + " port_name:=", + port_name, + " baud_rate:=", + baud_rate, + " id:=", + id_arg, + ] + ), + value_type=str, + ) + + # robot state publisher + robot_state_publisher = Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[ + { + "robot_description": robot_description, + } + ], + ) + ld.add_action(robot_state_publisher) + + return ld diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/package.xml b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/package.xml new file mode 100644 index 0000000..958de5c --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_correct/package.xml @@ -0,0 +1,37 @@ + + + + dynamixel_ros_control + 0.0.0 + Provides a hardware interface for dynamixel motors. + Martin Oehler + MIT + Martin Oehler + + ament_cmake + + ament_index_cpp + controller_orchestrator + dynamixel_sdk + hardware_interface + hector_transmission_interface + hector_transmission_interface_msgs + lifecycle_msgs + pluginlib + rclcpp + rclcpp_lifecycle + std_msgs + std_srvs + transmission_interface + yaml_cpp_vendor + + controller_manager + robot_state_publisher + + ament_lint_auto + ament_lint_common + + + ament_cmake + + diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/CMakeLists.txt b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/CMakeLists.txt new file mode 100644 index 0000000..fc11ae2 --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/CMakeLists.txt @@ -0,0 +1,75 @@ +cmake_minimum_required(VERSION 3.8) +project(dynamixel_ros_control) + +# Default to C++17 +if (NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 17) + set(CMAKE_CXX_STANDARD_REQUIRED ON) +endif () + +if (CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wall -Wextra -Wpedantic) +endif () + +# Find dependencies +find_package(ament_cmake REQUIRED) +find_package(rclcpp REQUIRED) +find_package(rclcpp_lifecycle REQUIRED) +find_package(lifecycle_msgs REQUIRED) +find_package(hardware_interface REQUIRED) +find_package(pluginlib REQUIRED) +find_package(dynamixel_sdk REQUIRED) +find_package(yaml-cpp REQUIRED) +#find_package(dynamixel_ros_control_msgs REQUIRED) +find_package(std_msgs REQUIRED) +find_package(std_srvs REQUIRED) +find_package(ament_index_cpp REQUIRED) +find_package(transmission_interface REQUIRED) +find_package(hector_transmission_interface REQUIRED) +find_package(hector_transmission_interface_msgs REQUIRED) +find_package(controller_orchestrator REQUIRED) + + +set(HEADERS + include/${PROJECT_NAME}/common.hpp + include/${PROJECT_NAME}/joint.hpp + include/${PROJECT_NAME}/dynamixel.hpp + include/${PROJECT_NAME}/control_table.hpp + include/${PROJECT_NAME}/control_table_item.hpp + include/${PROJECT_NAME}/dynamixel_driver.hpp + include/${PROJECT_NAME}/dynamixel_hardware_interface.hpp + include/${PROJECT_NAME}/sync_read_manager.hpp + include/${PROJECT_NAME}/sync_write_manager.hpp + include/${PROJECT_NAME}/log.hpp +) + +set(SOURCES + src/common.cpp + src/joint.cpp + src/dynamixel.cpp + src/control_table.cpp + src/control_table_item.cpp + src/dynamixel_driver.cpp + src/dynamixel_hardware_interface.cpp + src/sync_read_manager.cpp + src/sync_write_manager.cpp +) +include_directories(include) + +add_library(${PROJECT_NAME} SHARED ${SOURCES} ${HEADERS}) +target_link_libraries(${PROJECT_NAME} PUBLIC yaml-cpp) +ament_target_dependencies(${PROJECT_NAME} PUBLIC rclcpp rclcpp_lifecycle lifecycle_msgs hardware_interface pluginlib + dynamixel_sdk yaml-cpp std_msgs std_srvs ament_index_cpp transmission_interface hector_transmission_interface hector_transmission_interface_msgs controller_orchestrator) + +pluginlib_export_plugin_description_file(hardware_interface dynamixel_ros_control.xml) + +# Install library and include folder +install(TARGETS ${PROJECT_NAME} EXPORT ${PROJECT_NAME}-targets LIBRARY DESTINATION lib) +install(DIRECTORY include/ DESTINATION include) + +# Install directories +install(DIRECTORY launch config devices + DESTINATION share/${PROJECT_NAME} +) + +ament_package() diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/controller_manager.launch.yaml b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/controller_manager.launch.yaml new file mode 100644 index 0000000..12541f3 --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/controller_manager.launch.yaml @@ -0,0 +1,43 @@ +launch: +# Robot description +- include: + file: "$(find-pkg-share dynamixel_ros_control)/launch/load_description.launch.py" + +# Controller manager +- node: + pkg: "controller_manager" + exec: "ros2_control_node" + name: "controller_manager" + param: + - from: "$(find-pkg-share dynamixel_ros_control)/config/test_controllers.yaml" + allow_substs: true + - name: "hardware_components_initial_state.unconfigured" + value: ['hardware_interface'] +# Controller manager lifecycle management +- node: + pkg: "controller_manager" + exec: "hardware_spawner" + name: "hardware_interface_spawner" + output: "screen" + args: "hardware_interface --activate" + respawn: "true" + respawn_delay: 5.0 + +# Controllers +- node: + pkg: "controller_manager" + exec: "spawner" + name: "joint_state_broadcaster_spawner" + output: "screen" + args: "joint_state_broadcaster" + respawn: "true" + respawn_delay: 5.0 + +- node: + pkg: "controller_manager" + exec: "spawner" + name: "position_controller_spawner" + output: "screen" + args: "position_controller" + param: + - from: "$(find-pkg-share dynamixel_ros_control)/config/test_controllers.yaml" diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/load_description.launch.py b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/load_description.launch.py new file mode 100644 index 0000000..3d4ae5c --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/launch/load_description.launch.py @@ -0,0 +1,72 @@ +from launch import LaunchDescription +from launch_ros.actions import Node +from launch.actions import DeclareLaunchArgument +from launch.substitutions import Command, PathJoinSubstitution, LaunchConfiguration +from ament_index_python.packages import get_package_share_directory +from launch_ros.parameter_descriptions import ParameterValue + + +def generate_launch_description(): + ld = LaunchDescription() + + # Parameters + port_name_arg = DeclareLaunchArgument( + name="port_name", + default_value="/dev/ttyUSB0", + description="Path to USB serial converter device", + ) + ld.add_action(port_name_arg) + port_name = LaunchConfiguration("port_name") + + baud_rate_arg = DeclareLaunchArgument( + name="baud_rate", default_value="57600", description="Baud rate" + ) + ld.add_action(baud_rate_arg) + baud_rate = LaunchConfiguration("baud_rate") + + id_arg = DeclareLaunchArgument( + name="id", default_value="1", description="ID of the dynamixel to control" + ) + ld.add_action(id_arg) + id_arg = LaunchConfiguration("id") + + # Get the package directory + urdf_file = PathJoinSubstitution( + [ + get_package_share_directory("dynamixel_ros_control"), + "config", + "test_robot.urdf.xacro", + ] + ) + + robot_description = ParameterValue( + Command( + [ + "xacro ", + urdf_file, + " port_name:=", + port_name, + " baud_rate:=", + baud_rate, + " id:=", + id_arg, + ] + ), + value_type=str, + ) + + # robot state publisher + robot_state_publisher = Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[ + { + "robot_description": robot_description, + } + ], + ) + ld.add_action(robot_state_publisher) + + return ld diff --git a/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/package.xml b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/package.xml new file mode 100644 index 0000000..af85b85 --- /dev/null +++ b/tests/examples/launch_pkg_examples/dynamixel_ros_control/pkg_missing_launch_deps/package.xml @@ -0,0 +1,34 @@ + + + + dynamixel_ros_control + 0.0.0 + Provides a hardware interface for dynamixel motors. + Martin Oehler + MIT + Martin Oehler + + ament_cmake + + ament_index_cpp + controller_orchestrator + dynamixel_sdk + hardware_interface + hector_transmission_interface + hector_transmission_interface_msgs + lifecycle_msgs + pluginlib + rclcpp + rclcpp_lifecycle + std_msgs + std_srvs + transmission_interface + yaml_cpp_vendor + + ament_lint_auto + ament_lint_common + + + ament_cmake + + diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/CMakeLists.txt b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/CMakeLists.txt new file mode 100644 index 0000000..c8d94f8 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/CMakeLists.txt @@ -0,0 +1,48 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_controller_spawner) + +# Default to C++17 +if(NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 17) +endif() + +if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wall -Wextra -Wpedantic) +endif() + +find_package(ament_cmake REQUIRED) +find_package(rclcpp REQUIRED) +find_package(std_msgs REQUIRED) +find_package(controller_manager_msgs REQUIRED) + +add_executable(hector_controller_spawner src/hector_controller_spawner.cpp) + +target_include_directories( + hector_controller_spawner + PUBLIC $ + $) + +ament_target_dependencies(hector_controller_spawner rclcpp std_msgs + controller_manager_msgs) + +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + ament_lint_auto_find_test_dependencies() + + # for ROS 2 launch‐testing + find_package(launch_testing_ament_cmake REQUIRED) + # This will install and run your test/controller_spawner_launch.py + add_launch_test(test/test_spawner_basic.test.py TIMEOUT 360) + add_launch_test(test/test_spawner_twice.test.py TIMEOUT 360) + add_launch_test(test/test_spawner_estop.test.py TIMEOUT 360) + add_launch_test(test/test_spawner_chain_detection.test.py TIMEOUT 360) +endif() + +install(TARGETS hector_controller_spawner DESTINATION lib/${PROJECT_NAME}) + +install( + DIRECTORY config launch test + DESTINATION share/${PROJECT_NAME} + OPTIONAL) + +ament_package() diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/config/athena.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/config/athena.yaml new file mode 100644 index 0000000..73d30eb --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/config/athena.yaml @@ -0,0 +1,45 @@ +/**: + # Example configuration !! + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + estop_topic: "" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - self_collision_avoidance_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - moveit_twist_controller + + # Per‑controller options + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + self_collision_avoidance_controller: + activate: true + + flipper_velocity_controller: + activate: true + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true + + moveit_twist_controller: + activate: false diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/launch/hector_controller_spawner_launch.yml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/launch/hector_controller_spawner_launch.yml new file mode 100644 index 0000000..1190c83 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/launch/hector_controller_spawner_launch.yml @@ -0,0 +1,8 @@ +launch: + - node: + pkg: hector_controller_spawner + exec: hector_controller_spawner + name: multi_controller_spawner + output: screen + param: + - from: "$(find-pkg-share hector_controller_spawner)/config/athena.yaml" diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/package.xml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/package.xml new file mode 100644 index 0000000..8580eec --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/package.xml @@ -0,0 +1,30 @@ + + + + hector_controller_spawner + 0.0.0 + Robust hardware interface and controller spawning + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + ament_cmake + + controller_manager_msgs + rclcpp + std_msgs + + ament_lint_auto + controller_manager + hector_ros_controllers + joint_state_broadcaster + joint_trajectory_controller + launch_testing + launch_testing_ament_cmake + robot_state_publisher + + + ament_cmake + + diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/athena.urdf b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/athena.urdf new file mode 100644 index 0000000..35ad6e0 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/athena.urdf @@ -0,0 +1,2110 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + " + + 0 0 0 0 0 0 + /athena/front_lidar_sensor + 10 + front_lidar_sensor_laser_frame + + + + 100 + 1 + 0 + 6.28318 + + + 360 + 1 + -0.12601266555555554 + 0.9637699988888888 + + + + 0.08 + 10.0 + 0.01 + + + 1 + True + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + " + + 0 0 0 0 0 0 + /athena/back_lidar_sensor + 10 + back_lidar_sensor_laser_frame + + + + 100 + 1 + 0 + 6.28318 + + + 360 + 1 + -0.12601266555555554 + 0.9637699988888888 + + + + 0.08 + 10.0 + 0.01 + + + 1 + True + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 1 + athena/front_wideangle_camera/image_raw + front_wideangle_camera_optical_frame + + athena/front_wideangle_camera/camera_info + 4.188786666666666 + + R8G8B8 + 3840 + 3032 + + + 0.01 + 100 + + + equidistant + 2.094393333333333 + true + 512 + + + + + + + + + + + + + + + + + + + + + + 1 + athena/back_wideangle_camera/image_raw + back_wideangle_camera_optical_frame + + athena/back_wideangle_camera/camera_info + 4.188786666666666 + + R8G8B8 + 3840 + 3032 + + + 0.01 + 100 + + + equidistant + 2.094393333333333 + true + 512 + + + + + + + + true + true + 15 + /athena/front_rgbd/rgb/image_raw + front_rgbd_rgb_link + + /athena/front_rgbd/rgb/camera_info + front_rgbd_rgb_optical_frame + 1.570795 + + R8G8B8 + 720 + 480 + + + 0.05 + 10 + + + + + true + true + 15 + /athena/front_rgbd/depth/image_raw + front_rgbd_depth_link + + /athena/front_rgbd/depth/camera_info + front_rgbd_depth_optical_frame + 1.047 + + R_FLOAT32 + 640 + 480 + + + + 0.1 + 10.0 + + + + gaussian + 0.0 + 0.01 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + true + true + 15 + /athena/back_rgbd/rgb/image_raw + back_rgbd_rgb_link + + /athena/back_rgbd/rgb/camera_info + back_rgbd_rgb_optical_frame + 1.570795 + + R8G8B8 + 720 + 480 + + + 0.05 + 10 + + + + + true + true + 15 + /athena/back_rgbd/depth/image_raw + back_rgbd_depth_link + + /athena/back_rgbd/depth/camera_info + back_rgbd_depth_optical_frame + 1.047 + + R_FLOAT32 + 640 + 480 + + + + 0.1 + 10.0 + + + + gaussian + 0.0 + 0.01 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + + + + + + + + + + + + + + + + 30 + athena/gripper_cam/image_raw + gripper_cam_optical_frame + + athena/gripper_cam/camera_info + 1.085594794740473 + + R8G8B8 + 1640 + 1232 + + + 0.01 + 100 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + gripper_servo_joint + + gripper_joint_r2 + 0.909 + 0.0 + + + + gripper_joint_l2 + 0.909 + + + + gripper_joint_r1 + + + + gripper_joint_l1 + + + + + + + + + + + + + + + + + + + + + 30.0 + true + true + gripper_joint_link_l2 + /athena/force_torque_sensor/finger_l/wrench + + child + child_to_parent + + + + + + + + 30.0 + true + true + gripper_joint_link_r2 + /athena/force_torque_sensor/finger_r/wrench + + child + child_to_parent + + + + + + + + mock_components/GenericSystem + true + false + false + false + 0.0 + + + 1 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + -2.0 + 0.684 + + + + 2 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + 2.0 + -1.177 + + + + 3 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + 2.0 + -0.949 + + + + 4 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + -2.0 + -0.77 + + + + + + + + mock_components/GenericSystem + true + false + false + false + 0.0 + + + + + + 0.0 + + + + + + + + + 0.16 + + + + + + + + + -0.06 + + + + + + + + + 0.0 + + + + + + + + + -1.43815 + + + + + + + + + 0.0 + + + + + + + + + 0.0 + + + + + + + + + /home/aljoscha-schmidt/hector/install/athena_description/share/athena_description/config/athena_controllers.yaml + + athena + + /tf:=/athena/tf + /tf_static:=/athena/tf_static + + + + 20.0 + base_link + athena/odom + athena/odom_covariance + 3 + odom + athena/tf + 0.0 + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + true + + + true + + + + + + main_track_left_link + + + flipper_fl_link + + + flipper_bl_link + + + main_track_right_link + + + flipper_fr_link + + + flipper_br_link + + 0.06 + 0.215 + 0.5 + + -1.5 + 1.5 + + + + -3.0 + 3.0 + + + 50 + athena/cmd_vel_out + athena/steering_efficiency + athena/odom_wheel + + odom + base_link + + + main_track_left_link + + -1.5 + 1.5 + + + + flipper_fl_link + + -1.5 + 1.5 + + + + flipper_bl_link + 3.141592653589793 0 0 + -1.5 + 1.5 + + + + main_track_right_link + + -1.5 + 1.5 + + + + flipper_fr_link + + -1.5 + 1.5 + + + + flipper_br_link + 3.141592653589793 0 0 + -1.5 + 1.5 + + + + + Gazebo/DarkGray + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + 0.2 + 0.2 + Gazebo/DarkGrey + + diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner.yaml new file mode 100644 index 0000000..09cb989 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner.yaml @@ -0,0 +1,40 @@ +/**: + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + #estop_topic: "estop_board/hard_estop" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - vel_to_pos_controller + + # ---------- Per‑controller options --------------------------- + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + flipper_velocity_controller: + activate: true + + vel_to_pos_controller: + activate: true + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_chain_detection.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_chain_detection.yaml new file mode 100644 index 0000000..ec0bb53 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_chain_detection.yaml @@ -0,0 +1,40 @@ +/**: + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + #estop_topic: "estop_board/hard_estop" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - vel_to_pos_controller + + # ---------- Per‑controller options --------------------------- + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + flipper_velocity_controller: + activate: true + + vel_to_pos_controller: + activate: false # In test must be activated + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_with_estop.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_with_estop.yaml new file mode 100644 index 0000000..3993455 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controller_spawner_with_estop.yaml @@ -0,0 +1,40 @@ +/**: + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + estop_topic: "estop_board/hard_estop" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - vel_to_pos_controller + + # ---------- Per‑controller options --------------------------- + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + flipper_velocity_controller: + activate: true + + vel_to_pos_controller: + activate: true + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controllers.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controllers.yaml new file mode 100644 index 0000000..962d282 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/config/controllers.yaml @@ -0,0 +1,95 @@ +# THIS PARAMETERS ARE ONLY USED IN THE SIMULATION !!! +# SEE athena_driver_launch/athena_driver_launch_config/configs/controllers.yaml FOR THE REAL ROBOT CONFIGURATION +/**: + gz_ros_control: + ros__parameters: + use_sim_time: true + + controller_manager: + ros__parameters: + update_rate: 50 # Hz + hardware_components_initial_state: + - unconfigured: [ athena_flipper_interface, athena_arm_interface ] + + joint_state_broadcaster: + type: joint_state_broadcaster/JointStateBroadcaster + + flipper_velocity_controller: + type: safety_forward_controller/SafetyForwardController + + flipper_trajectory_controller: + type: joint_trajectory_controller/JointTrajectoryController + + arm_trajectory_controller: + type: joint_trajectory_controller/JointTrajectoryController + + gripper_trajectory_controller: + type: joint_trajectory_controller/JointTrajectoryController + + + vel_to_pos_controller: + type: velocity_to_position_command_controller/VelocityToPositionCommandController + + + vel_to_pos_controller: + ros__parameters: + joints: + - flipper_fl_joint + - flipper_fr_joint + - flipper_bl_joint + - flipper_br_joint + + e_stop_topic: estop_board/hard_estop + + flipper_velocity_controller: + ros__parameters: + joints: + - flipper_fl_joint + - flipper_fr_joint + - flipper_bl_joint + - flipper_br_joint + + passthrough_controller: vel_to_pos_controller + + interface_type: velocity + + flipper_trajectory_controller: + ros__parameters: + joints: + - flipper_fl_joint + - flipper_fr_joint + - flipper_bl_joint + - flipper_br_joint + + command_interfaces: + - position + state_interfaces: + - position + - velocity + + arm_trajectory_controller: + ros__parameters: + joints: + - arm_joint_1 + - arm_joint_2 + - arm_joint_3 + - arm_joint_4 + - arm_joint_5 + - arm_joint_6 + + command_interfaces: + - position + state_interfaces: + - position + - velocity + + gripper_trajectory_controller: + ros__parameters: + joints: + - gripper_servo_joint + + command_interfaces: + - position + state_interfaces: + - position + - velocity diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_controller_spawner.cpp b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_controller_spawner.cpp new file mode 100644 index 0000000..e69de29 diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py new file mode 100644 index 0000000..8c66fa5 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py @@ -0,0 +1,139 @@ +import os +import unittest +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions + +import rclpy +from rclpy.node import Node +from controller_manager_msgs.srv import ListControllers, ListHardwareComponents + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + spawner_node = launch_ros.actions.Node( + package="hector_controller_spawner", + executable="hector_controller_spawner", + output="screen", + parameters=[spawner_config], + ) + + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction(period=5.0, actions=[spawner_node]), + launch.actions.TimerAction( + period=10.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager, "spawner_node": spawner_node}, + ) + + +class TestControllerSpawner(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_controller_spawner") + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_hardware_interfaces_loaded(self): + client = self.node.create_client( + ListHardwareComponents, "/controller_manager/list_hardware_components" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListHardwareComponents.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_interfaces = ["athena_flipper_interface", "athena_arm_interface"] + loaded_interfaces = [iface.name for iface in response.component] + + self.assertGreater(len(loaded_interfaces), 0, "No hardware interfaces loaded") + for iface in expected_interfaces: + self.assertIn(iface, loaded_interfaces) + + def test_controllers_loaded_and_activated(self): + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_active = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + expected_inactive = ["flipper_trajectory_controller"] + + loaded_controllers = {ctrl.name: ctrl.state for ctrl in response.controller} + self.node.get_logger().info(f"Loaded controllers: {loaded_controllers}") + + for controller in expected_active: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "active") + + for controller in expected_inactive: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "inactive") + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # You can disable this assert if controller_manager fails to shutdown cleanly + self.assertEqual(proc_info["hector_controller_spawner"].returncode, 0) diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py new file mode 100644 index 0000000..ad6ad87 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py @@ -0,0 +1,139 @@ +import os +import unittest +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions + +import rclpy +from rclpy.node import Node +from controller_manager_msgs.srv import ListControllers, ListHardwareComponents + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner_chain_detection.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + spawner_node = launch_ros.actions.Node( + package="hector_controller_spawner", + executable="hector_controller_spawner", + output="screen", + parameters=[spawner_config], + ) + + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction(period=5.0, actions=[spawner_node]), + launch.actions.TimerAction( + period=10.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager, "spawner_node": spawner_node}, + ) + + +class TestControllerSpawner(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_controller_spawner") + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_hardware_interfaces_loaded(self): + client = self.node.create_client( + ListHardwareComponents, "/controller_manager/list_hardware_components" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListHardwareComponents.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_interfaces = ["athena_flipper_interface", "athena_arm_interface"] + loaded_interfaces = [iface.name for iface in response.component] + + self.assertGreater(len(loaded_interfaces), 0, "No hardware interfaces loaded") + for iface in expected_interfaces: + self.assertIn(iface, loaded_interfaces) + + def test_controllers_loaded_and_activated(self): + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_active = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + expected_inactive = ["flipper_trajectory_controller"] + + loaded_controllers = {ctrl.name: ctrl.state for ctrl in response.controller} + self.node.get_logger().info(f"Loaded controllers: {loaded_controllers}") + + for controller in expected_active: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "active") + + for controller in expected_inactive: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "inactive") + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # You can disable this assert if controller_manager fails to shutdown cleanly + self.assertEqual(proc_info["hector_controller_spawner"].returncode, 0) diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py new file mode 100644 index 0000000..c0346d9 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py @@ -0,0 +1,381 @@ +import os +import unittest +import time +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions +from lifecycle_msgs.msg import State +import rclpy +from rclpy.node import Node +from std_msgs.msg import Bool +from controller_manager_msgs.srv import ( + ListControllers, + ListHardwareComponents, + SwitchController, + SetHardwareComponentState, +) +from rclpy.qos import QoSProfile, DurabilityPolicy + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner_with_estop.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + # Launch the spawner alongside the controller manager + spawner_node = launch_ros.actions.Node( + package="hector_controller_spawner", + executable="hector_controller_spawner", + output="screen", + parameters=[spawner_config], + ) + + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction(period=3.0, actions=[spawner_node]), + launch.actions.TimerAction( + period=5.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager, "spawner_node": spawner_node}, + ) + + +class TestEStopFunctionality(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_estop_functionality") + + # Create e-stop publisher + estop_qos = QoSProfile( + depth=1, + durability=DurabilityPolicy.TRANSIENT_LOCAL, # Match spawner's QoS + ) + cls.estop_pub = cls.node.create_publisher( + Bool, "estop_board/hard_estop", estop_qos + ) + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_complete_estop_workflow(self): + """Test complete e-stop workflow: block loading, allow loading, block again, allow again""" + + # Step 1: Send e-stop true (active/stopped) + self.node.get_logger().info("Step 1: Activating e-stop") + self._publish_estop(True) + time.sleep(2.0) # Allow time for e-stop to be processed + + # Step 2: Check that hardware interfaces and controllers are not active + self.node.get_logger().info( + "Step 2: Checking that hardware/controllers are not active" + ) + hardware_components = self._get_hardware_components() + controllers = self._get_controller_list() + + # Verify no hardware components are active + active_hardware = [ + hw.name for hw in hardware_components if hw.state.label == "active" + ] + self.assertEqual( + len(active_hardware), + 0, + "No hardware interfaces should be active while e-stop is active", + ) + + # Verify no controllers are active + active_controllers = [c.name for c in controllers if c.state == "active"] + self.assertEqual( + len(active_controllers), + 0, + "No controllers should be active while e-stop is active", + ) + + # Step 3: Send e-stop false (inactive/running) and wait + self.node.get_logger().info("Step 3: Releasing e-stop") + self._publish_estop(False) + time.sleep(8.0) # Allow time for spawner to proceed and load everything + + # Step 4: Check that controllers and hardware interfaces are loaded and active + self.node.get_logger().info( + "Step 4: Checking that hardware/controllers are active" + ) + hardware_components = self._get_hardware_components() + controllers = self._get_controller_list() + + # Verify expected hardware components are active + expected_hardware = ["athena_flipper_interface", "athena_arm_interface"] + active_hardware = [ + hw.name for hw in hardware_components if hw.state.label == "active" + ] + + for hw_name in expected_hardware: + self.assertIn( + hw_name, + active_hardware, + f"Hardware interface {hw_name} should be active after e-stop release", + ) + + # Verify expected controllers are active + expected_active_controllers = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + + active_controllers = [c.name for c in controllers if c.state == "active"] + for controller in expected_active_controllers: + self.assertIn( + controller, + active_controllers, + f"Controller {controller} should be active after e-stop release", + ) + + # Step 5: Send e-stop true and unload hardware interfaces + self.node.get_logger().info( + "Step 5: Activating e-stop again (should deactivate hardware)" + ) + self._publish_estop(True) + self._deactivate_all_controllers() + self._unload_hardware_interfaces() + time.sleep(3.0) # Allow time for deactivation + + # Step 6: Verify hardware interfaces are deactivated + hardware_components = self._get_hardware_components() + controllers = self._get_controller_list() + + # Check that hardware is no longer active + active_hardware_after_estop = [ + hw.name for hw in hardware_components if hw.state.label == "active" + ] + self.assertEqual( + len(active_hardware_after_estop), + 0, + "Hardware interfaces should be deactivated when e-stop is activated again", + ) + + # Check that controllers are no longer active + active_controllers_after_estop = [ + c.name for c in controllers if c.state == "active" + ] + self.assertEqual( + len(active_controllers_after_estop), + 0, + "Controllers should be deactivated when e-stop is activated again", + ) + + # Step 7: Send e-stop false and wait some time + self.node.get_logger().info("Step 6: Releasing e-stop second time") + self._publish_estop(False) + time.sleep(8.0) # Allow time for reactivation + + # Step 8: Check that hardware interfaces and requested controllers are active again + self.node.get_logger().info( + "Step 7: Checking final state - should be active again" + ) + final_hardware_components = self._get_hardware_components() + final_controllers = self._get_controller_list() + + # Verify hardware interfaces are active again + final_active_hardware = [ + hw.name for hw in final_hardware_components if hw.state.label == "active" + ] + for hw_name in expected_hardware: + self.assertIn( + hw_name, + final_active_hardware, + f"Hardware interface {hw_name} should be active again after second e-stop release", + ) + + # Verify controllers are active again + final_active_controllers = [ + c.name for c in final_controllers if c.state == "active" + ] + for controller in expected_active_controllers: + self.assertIn( + controller, + final_active_controllers, + f"Controller {controller} should be active again after second e-stop release", + ) + + self.node.get_logger().info("E-stop workflow test completed successfully") + + def _publish_estop(self, active): + """Helper to publish e-stop message multiple times for reliability""" + estop_msg = Bool() + estop_msg.data = active + + # Publish multiple times to ensure delivery + for _ in range(10): + self.estop_pub.publish(estop_msg) + rclpy.spin_once(self.node, timeout_sec=0.1) + time.sleep(0.1) + + # Extra time for message propagation + time.sleep(0.5) + + def _get_hardware_components(self): + """Helper to get current hardware component list""" + client = self.node.create_client( + ListHardwareComponents, "/controller_manager/list_hardware_components" + ) + + if not client.wait_for_service(timeout_sec=3.0): + self.node.get_logger().warn("Hardware components service not available") + return [] + + request = ListHardwareComponents.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + return response.component if response else [] + + def _get_controller_list(self): + """Helper to get current controller list""" + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + if not client.wait_for_service(timeout_sec=3.0): + self.node.get_logger().warn("Controller manager service not available") + return [] + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + return response.controller if response else [] + + def _unload_hardware_interfaces(self): + """Unload/deactivate hardware interfaces by transitioning them to inactive state""" + active_hardware = ["athena_flipper_interface", "athena_arm_interface"] + # Create service client for setting hardware component state + set_hw_state_client = self.node.create_client( + SetHardwareComponentState, + "/controller_manager/set_hardware_component_state", + ) + + if not set_hw_state_client.wait_for_service(timeout_sec=5.0): + self.node.get_logger().error( + "Hardware component state service not available" + ) + return + + # Deactivate each active hardware interface + for hw_component in active_hardware: + self.node.get_logger().info( + f"Deactivating hardware interface: {hw_component}" + ) + + request = SetHardwareComponentState.Request() + request.name = hw_component + request.target_state = State() + request.target_state.id = ( + State.PRIMARY_STATE_INACTIVE + ) # Transition to inactive + request.target_state.label = "inactive" + + # Call the service + future = set_hw_state_client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + if response and response.ok: + self.node.get_logger().info( + f"Successfully deactivated hardware interface: {hw_component}" + ) + else: + self.node.get_logger().error( + f"Failed to deactivate hardware interface: {hw_component}" + ) + + def _deactivate_all_controllers(self): + """Deactivate all controllers by switching them off""" + + controllers_to_deactivate = [ + "joint_state_broadcaster", + "flipper_trajectory_controller", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + + # Create service client for switching controllers + switch_client = self.node.create_client( + SwitchController, "/controller_manager/switch_controller" + ) + + if not switch_client.wait_for_service(timeout_sec=5.0): + self.node.get_logger().error("Switch controller service not available") + return + + self.node.get_logger().info("Deactivating all controllers") + + request = SwitchController.Request() + request.activate_controllers = [] # No controllers to activate + request.deactivate_controllers = ( + controllers_to_deactivate # Controllers to deactivate + ) + request.strictness = ( + SwitchController.Request.BEST_EFFORT + ) # Strict mode - fail if any controller can't be switched + request.activate_asap = False + request.timeout = rclpy.duration.Duration(seconds=5.0).to_msg() + + # Call the service + future = switch_client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + if response and response.ok: + self.node.get_logger().info("Successfully deactivated all controllers") + else: + self.node.get_logger().error("Failed to deactivate controllers") + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # Check that the spawner completed successfully + self.assertEqual(proc_info["hector_controller_spawner"].returncode, 0) diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py new file mode 100644 index 0000000..757412a --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py @@ -0,0 +1,235 @@ +import os +import unittest +import subprocess +import time +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions + +import rclpy +from rclpy.node import Node +from controller_manager_msgs.srv import ListControllers + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + # Don't launch spawner automatically - we'll control it manually + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction( + period=5.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager}, + ) + + +class TestControllerSpawnerIdempotency(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_controller_spawner_idempotency") + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_spawner_run_twice_idempotent(self): + """Test that running the spawner twice doesn't cause problems""" + + pkg_share = get_package_share_directory("hector_controller_spawner") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner.yaml" + ) + + # Spawner command + cmd = [ + "ros2", + "run", + "hector_controller_spawner", + "hector_controller_spawner", + "--ros-args", + "--params-file", + spawner_config, + ] + + # 1. Check initial state (no controllers loaded) + initial_controllers = self._get_controller_list() + initial_count = len(initial_controllers) + self.node.get_logger().info(f"Initial controller count: {initial_count}") + + # 2. Run spawner first time + self.node.get_logger().info("Running spawner first time...") + process1 = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + # Wait for spawner to complete + return_code1 = process1.wait(timeout=30) # 30 second timeout + + stdout1, stderr1 = process1.communicate() + self.assertEqual( + return_code1, 0, f"First spawner run failed with return code {return_code1}" + ) + + # Give time for controllers to be loaded + time.sleep(2.0) + + # 3. Check state after first run + first_run_controllers = self._get_controller_list() + first_run_count = len(first_run_controllers) + self.node.get_logger().info( + f"Controller count after first run: {first_run_count}" + ) + + self.assertGreater( + first_run_count, + initial_count, + "Controllers should be loaded after first spawner run", + ) + + # Store the state for comparison + first_run_active = [ + c.name for c in first_run_controllers if c.state == "active" + ] + first_run_inactive = [ + c.name for c in first_run_controllers if c.state == "inactive" + ] + + # 4. Wait a bit, then run spawner second time + time.sleep(3.0) # Delay between runs + + self.node.get_logger().info("Running spawner second time...") + process2 = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + # Wait for second spawner to complete + return_code2 = process2.wait(timeout=30) + + stdout2, stderr2 = process2.communicate() + self.assertEqual( + return_code2, + 0, + f"Second spawner run failed with return code {return_code2}", + ) + + # Give time for any changes to take effect + time.sleep(2.0) + + # 5. Check state after second run + second_run_controllers = self._get_controller_list() + second_run_count = len(second_run_controllers) + self.node.get_logger().info( + f"Controller count after second run: {second_run_count}" + ) + + second_run_active = [ + c.name for c in second_run_controllers if c.state == "active" + ] + second_run_inactive = [ + c.name for c in second_run_controllers if c.state == "inactive" + ] + + # 6. Verify that the state is identical (idempotent behavior) + self.assertEqual( + first_run_count, + second_run_count, + "Controller count should be the same after second spawner run", + ) + + # Check that active controllers are the same + self.assertEqual( + set(first_run_active), + set(second_run_active), + "Active controllers should be identical after second run", + ) + + # Check that inactive controllers are the same + self.assertEqual( + set(first_run_inactive), + set(second_run_inactive), + "Inactive controllers should be identical after second run", + ) + + # 7. Verify expected controllers are still in correct states + expected_active = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + + for controller in expected_active: + self.assertIn( + controller, + second_run_active, + f"Controller {controller} should still be active after second run", + ) + + # Log the outputs for debugging if needed + if stdout1: + self.node.get_logger().info( + f"First spawner stdout: {stdout1.decode()[:200]}..." + ) + if stdout2: + self.node.get_logger().info( + f"Second spawner stdout: {stdout2.decode()[:200]}..." + ) + + def _get_controller_list(self): + """Helper to get current controller list""" + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + if not client.wait_for_service(timeout_sec=5.0): + return [] + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + return response.controller if response else [] + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # Controller manager should shutdown cleanly + pass # Allow flexible exit codes since we're not checking spawner here diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/CMakeLists.txt b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/CMakeLists.txt new file mode 100644 index 0000000..c8d94f8 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/CMakeLists.txt @@ -0,0 +1,48 @@ +cmake_minimum_required(VERSION 3.8) +project(hector_controller_spawner) + +# Default to C++17 +if(NOT CMAKE_CXX_STANDARD) + set(CMAKE_CXX_STANDARD 17) +endif() + +if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wall -Wextra -Wpedantic) +endif() + +find_package(ament_cmake REQUIRED) +find_package(rclcpp REQUIRED) +find_package(std_msgs REQUIRED) +find_package(controller_manager_msgs REQUIRED) + +add_executable(hector_controller_spawner src/hector_controller_spawner.cpp) + +target_include_directories( + hector_controller_spawner + PUBLIC $ + $) + +ament_target_dependencies(hector_controller_spawner rclcpp std_msgs + controller_manager_msgs) + +if(BUILD_TESTING) + find_package(ament_lint_auto REQUIRED) + ament_lint_auto_find_test_dependencies() + + # for ROS 2 launch‐testing + find_package(launch_testing_ament_cmake REQUIRED) + # This will install and run your test/controller_spawner_launch.py + add_launch_test(test/test_spawner_basic.test.py TIMEOUT 360) + add_launch_test(test/test_spawner_twice.test.py TIMEOUT 360) + add_launch_test(test/test_spawner_estop.test.py TIMEOUT 360) + add_launch_test(test/test_spawner_chain_detection.test.py TIMEOUT 360) +endif() + +install(TARGETS hector_controller_spawner DESTINATION lib/${PROJECT_NAME}) + +install( + DIRECTORY config launch test + DESTINATION share/${PROJECT_NAME} + OPTIONAL) + +ament_package() diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/config/athena.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/config/athena.yaml new file mode 100644 index 0000000..73d30eb --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/config/athena.yaml @@ -0,0 +1,45 @@ +/**: + # Example configuration !! + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + estop_topic: "" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - self_collision_avoidance_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - moveit_twist_controller + + # Per‑controller options + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + self_collision_avoidance_controller: + activate: true + + flipper_velocity_controller: + activate: true + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true + + moveit_twist_controller: + activate: false diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/launch/hector_controller_spawner_launch.yml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/launch/hector_controller_spawner_launch.yml new file mode 100644 index 0000000..1190c83 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/launch/hector_controller_spawner_launch.yml @@ -0,0 +1,8 @@ +launch: + - node: + pkg: hector_controller_spawner + exec: hector_controller_spawner + name: multi_controller_spawner + output: screen + param: + - from: "$(find-pkg-share hector_controller_spawner)/config/athena.yaml" diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/package.xml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/package.xml new file mode 100644 index 0000000..1364ece --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/package.xml @@ -0,0 +1,28 @@ + + + + hector_controller_spawner + 0.0.0 + Robust hardware interface and controller spawning + + Aljoscha Schmidt + TODO + Aljoscha Schmidt + + ament_cmake + + controller_manager_msgs + rclcpp + std_msgs + + ament_lint_auto + hector_ros_controllers + joint_state_broadcaster + joint_trajectory_controller + launch_testing + launch_testing_ament_cmake + + + ament_cmake + + diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/athena.urdf b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/athena.urdf new file mode 100644 index 0000000..35ad6e0 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/athena.urdf @@ -0,0 +1,2110 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + " + + 0 0 0 0 0 0 + /athena/front_lidar_sensor + 10 + front_lidar_sensor_laser_frame + + + + 100 + 1 + 0 + 6.28318 + + + 360 + 1 + -0.12601266555555554 + 0.9637699988888888 + + + + 0.08 + 10.0 + 0.01 + + + 1 + True + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + " + + 0 0 0 0 0 0 + /athena/back_lidar_sensor + 10 + back_lidar_sensor_laser_frame + + + + 100 + 1 + 0 + 6.28318 + + + 360 + 1 + -0.12601266555555554 + 0.9637699988888888 + + + + 0.08 + 10.0 + 0.01 + + + 1 + True + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 1 + athena/front_wideangle_camera/image_raw + front_wideangle_camera_optical_frame + + athena/front_wideangle_camera/camera_info + 4.188786666666666 + + R8G8B8 + 3840 + 3032 + + + 0.01 + 100 + + + equidistant + 2.094393333333333 + true + 512 + + + + + + + + + + + + + + + + + + + + + + 1 + athena/back_wideangle_camera/image_raw + back_wideangle_camera_optical_frame + + athena/back_wideangle_camera/camera_info + 4.188786666666666 + + R8G8B8 + 3840 + 3032 + + + 0.01 + 100 + + + equidistant + 2.094393333333333 + true + 512 + + + + + + + + true + true + 15 + /athena/front_rgbd/rgb/image_raw + front_rgbd_rgb_link + + /athena/front_rgbd/rgb/camera_info + front_rgbd_rgb_optical_frame + 1.570795 + + R8G8B8 + 720 + 480 + + + 0.05 + 10 + + + + + true + true + 15 + /athena/front_rgbd/depth/image_raw + front_rgbd_depth_link + + /athena/front_rgbd/depth/camera_info + front_rgbd_depth_optical_frame + 1.047 + + R_FLOAT32 + 640 + 480 + + + + 0.1 + 10.0 + + + + gaussian + 0.0 + 0.01 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + true + true + 15 + /athena/back_rgbd/rgb/image_raw + back_rgbd_rgb_link + + /athena/back_rgbd/rgb/camera_info + back_rgbd_rgb_optical_frame + 1.570795 + + R8G8B8 + 720 + 480 + + + 0.05 + 10 + + + + + true + true + 15 + /athena/back_rgbd/depth/image_raw + back_rgbd_depth_link + + /athena/back_rgbd/depth/camera_info + back_rgbd_depth_optical_frame + 1.047 + + R_FLOAT32 + 640 + 480 + + + + 0.1 + 10.0 + + + + gaussian + 0.0 + 0.01 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + + + + + + + + + + + + + + + + 30 + athena/gripper_cam/image_raw + gripper_cam_optical_frame + + athena/gripper_cam/camera_info + 1.085594794740473 + + R8G8B8 + 1640 + 1232 + + + 0.01 + 100 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + gripper_servo_joint + + gripper_joint_r2 + 0.909 + 0.0 + + + + gripper_joint_l2 + 0.909 + + + + gripper_joint_r1 + + + + gripper_joint_l1 + + + + + + + + + + + + + + + + + + + + + 30.0 + true + true + gripper_joint_link_l2 + /athena/force_torque_sensor/finger_l/wrench + + child + child_to_parent + + + + + + + + 30.0 + true + true + gripper_joint_link_r2 + /athena/force_torque_sensor/finger_r/wrench + + child + child_to_parent + + + + + + + + mock_components/GenericSystem + true + false + false + false + 0.0 + + + 1 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + -2.0 + 0.684 + + + + 2 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + 2.0 + -1.177 + + + + 3 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + 2.0 + -0.949 + + + + 4 + 9.0 + 255 + + + + + + + + + hector_transmission_interface/AdjustableOffsetTransmission + + + -2.0 + -0.77 + + + + + + + + mock_components/GenericSystem + true + false + false + false + 0.0 + + + + + + 0.0 + + + + + + + + + 0.16 + + + + + + + + + -0.06 + + + + + + + + + 0.0 + + + + + + + + + -1.43815 + + + + + + + + + 0.0 + + + + + + + + + 0.0 + + + + + + + + + /home/aljoscha-schmidt/hector/install/athena_description/share/athena_description/config/athena_controllers.yaml + + athena + + /tf:=/athena/tf + /tf_static:=/athena/tf_static + + + + 20.0 + base_link + athena/odom + athena/odom_covariance + 3 + odom + athena/tf + 0.0 + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + + + + + 1.0 + 50 + + 0 + 0 + + + + + Gazebo/Black + + + true + + + true + + + + + + main_track_left_link + + + flipper_fl_link + + + flipper_bl_link + + + main_track_right_link + + + flipper_fr_link + + + flipper_br_link + + 0.06 + 0.215 + 0.5 + + -1.5 + 1.5 + + + + -3.0 + 3.0 + + + 50 + athena/cmd_vel_out + athena/steering_efficiency + athena/odom_wheel + + odom + base_link + + + main_track_left_link + + -1.5 + 1.5 + + + + flipper_fl_link + + -1.5 + 1.5 + + + + flipper_bl_link + 3.141592653589793 0 0 + -1.5 + 1.5 + + + + main_track_right_link + + -1.5 + 1.5 + + + + flipper_fr_link + + -1.5 + 1.5 + + + + flipper_br_link + 3.141592653589793 0 0 + -1.5 + 1.5 + + + + + Gazebo/DarkGray + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + 0.2 + 0.2 + Gazebo/DarkGrey + + + 0.2 + 0.2 + Gazebo/DarkGrey + + diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner.yaml new file mode 100644 index 0000000..09cb989 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner.yaml @@ -0,0 +1,40 @@ +/**: + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + #estop_topic: "estop_board/hard_estop" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - vel_to_pos_controller + + # ---------- Per‑controller options --------------------------- + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + flipper_velocity_controller: + activate: true + + vel_to_pos_controller: + activate: true + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_chain_detection.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_chain_detection.yaml new file mode 100644 index 0000000..ec0bb53 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_chain_detection.yaml @@ -0,0 +1,40 @@ +/**: + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + #estop_topic: "estop_board/hard_estop" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - vel_to_pos_controller + + # ---------- Per‑controller options --------------------------- + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + flipper_velocity_controller: + activate: true + + vel_to_pos_controller: + activate: false # In test must be activated + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_with_estop.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_with_estop.yaml new file mode 100644 index 0000000..3993455 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controller_spawner_with_estop.yaml @@ -0,0 +1,40 @@ +/**: + ros__parameters: + # Optional emergency‑stop topic. Empty → start immediately. + estop_topic: "estop_board/hard_estop" + + # How long to wait between retry attempts (HW + controllers). + retry_delay: 5.0 + + # ---------- hardware interfaces (activate = always) ---------- + hardware_interfaces: + - "athena_flipper_interface" + - "athena_arm_interface" + + # ---------- controllers -------------------------------------- + controllers: + - joint_state_broadcaster + - flipper_trajectory_controller + - flipper_velocity_controller + - gripper_trajectory_controller + - arm_trajectory_controller + - vel_to_pos_controller + + # ---------- Per‑controller options --------------------------- + joint_state_broadcaster: + activate: true + + flipper_trajectory_controller: + activate: false + + flipper_velocity_controller: + activate: true + + vel_to_pos_controller: + activate: true + + gripper_trajectory_controller: + activate: true + + arm_trajectory_controller: + activate: true diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controllers.yaml b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controllers.yaml new file mode 100644 index 0000000..962d282 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/config/controllers.yaml @@ -0,0 +1,95 @@ +# THIS PARAMETERS ARE ONLY USED IN THE SIMULATION !!! +# SEE athena_driver_launch/athena_driver_launch_config/configs/controllers.yaml FOR THE REAL ROBOT CONFIGURATION +/**: + gz_ros_control: + ros__parameters: + use_sim_time: true + + controller_manager: + ros__parameters: + update_rate: 50 # Hz + hardware_components_initial_state: + - unconfigured: [ athena_flipper_interface, athena_arm_interface ] + + joint_state_broadcaster: + type: joint_state_broadcaster/JointStateBroadcaster + + flipper_velocity_controller: + type: safety_forward_controller/SafetyForwardController + + flipper_trajectory_controller: + type: joint_trajectory_controller/JointTrajectoryController + + arm_trajectory_controller: + type: joint_trajectory_controller/JointTrajectoryController + + gripper_trajectory_controller: + type: joint_trajectory_controller/JointTrajectoryController + + + vel_to_pos_controller: + type: velocity_to_position_command_controller/VelocityToPositionCommandController + + + vel_to_pos_controller: + ros__parameters: + joints: + - flipper_fl_joint + - flipper_fr_joint + - flipper_bl_joint + - flipper_br_joint + + e_stop_topic: estop_board/hard_estop + + flipper_velocity_controller: + ros__parameters: + joints: + - flipper_fl_joint + - flipper_fr_joint + - flipper_bl_joint + - flipper_br_joint + + passthrough_controller: vel_to_pos_controller + + interface_type: velocity + + flipper_trajectory_controller: + ros__parameters: + joints: + - flipper_fl_joint + - flipper_fr_joint + - flipper_bl_joint + - flipper_br_joint + + command_interfaces: + - position + state_interfaces: + - position + - velocity + + arm_trajectory_controller: + ros__parameters: + joints: + - arm_joint_1 + - arm_joint_2 + - arm_joint_3 + - arm_joint_4 + - arm_joint_5 + - arm_joint_6 + + command_interfaces: + - position + state_interfaces: + - position + - velocity + + gripper_trajectory_controller: + ros__parameters: + joints: + - gripper_servo_joint + + command_interfaces: + - position + state_interfaces: + - position + - velocity diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_controller_spawner.cpp b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_controller_spawner.cpp new file mode 100644 index 0000000..e69de29 diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py new file mode 100644 index 0000000..8c66fa5 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py @@ -0,0 +1,139 @@ +import os +import unittest +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions + +import rclpy +from rclpy.node import Node +from controller_manager_msgs.srv import ListControllers, ListHardwareComponents + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + spawner_node = launch_ros.actions.Node( + package="hector_controller_spawner", + executable="hector_controller_spawner", + output="screen", + parameters=[spawner_config], + ) + + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction(period=5.0, actions=[spawner_node]), + launch.actions.TimerAction( + period=10.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager, "spawner_node": spawner_node}, + ) + + +class TestControllerSpawner(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_controller_spawner") + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_hardware_interfaces_loaded(self): + client = self.node.create_client( + ListHardwareComponents, "/controller_manager/list_hardware_components" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListHardwareComponents.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_interfaces = ["athena_flipper_interface", "athena_arm_interface"] + loaded_interfaces = [iface.name for iface in response.component] + + self.assertGreater(len(loaded_interfaces), 0, "No hardware interfaces loaded") + for iface in expected_interfaces: + self.assertIn(iface, loaded_interfaces) + + def test_controllers_loaded_and_activated(self): + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_active = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + expected_inactive = ["flipper_trajectory_controller"] + + loaded_controllers = {ctrl.name: ctrl.state for ctrl in response.controller} + self.node.get_logger().info(f"Loaded controllers: {loaded_controllers}") + + for controller in expected_active: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "active") + + for controller in expected_inactive: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "inactive") + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # You can disable this assert if controller_manager fails to shutdown cleanly + self.assertEqual(proc_info["hector_controller_spawner"].returncode, 0) diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py new file mode 100644 index 0000000..ad6ad87 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py @@ -0,0 +1,139 @@ +import os +import unittest +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions + +import rclpy +from rclpy.node import Node +from controller_manager_msgs.srv import ListControllers, ListHardwareComponents + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner_chain_detection.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + spawner_node = launch_ros.actions.Node( + package="hector_controller_spawner", + executable="hector_controller_spawner", + output="screen", + parameters=[spawner_config], + ) + + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction(period=5.0, actions=[spawner_node]), + launch.actions.TimerAction( + period=10.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager, "spawner_node": spawner_node}, + ) + + +class TestControllerSpawner(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_controller_spawner") + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_hardware_interfaces_loaded(self): + client = self.node.create_client( + ListHardwareComponents, "/controller_manager/list_hardware_components" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListHardwareComponents.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_interfaces = ["athena_flipper_interface", "athena_arm_interface"] + loaded_interfaces = [iface.name for iface in response.component] + + self.assertGreater(len(loaded_interfaces), 0, "No hardware interfaces loaded") + for iface in expected_interfaces: + self.assertIn(iface, loaded_interfaces) + + def test_controllers_loaded_and_activated(self): + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + self.assertTrue(client.wait_for_service(timeout_sec=10.0)) + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + self.assertIsNotNone(response) + + expected_active = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + expected_inactive = ["flipper_trajectory_controller"] + + loaded_controllers = {ctrl.name: ctrl.state for ctrl in response.controller} + self.node.get_logger().info(f"Loaded controllers: {loaded_controllers}") + + for controller in expected_active: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "active") + + for controller in expected_inactive: + self.assertIn(controller, loaded_controllers) + self.assertEqual(loaded_controllers[controller], "inactive") + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # You can disable this assert if controller_manager fails to shutdown cleanly + self.assertEqual(proc_info["hector_controller_spawner"].returncode, 0) diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py new file mode 100644 index 0000000..c0346d9 --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py @@ -0,0 +1,381 @@ +import os +import unittest +import time +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions +from lifecycle_msgs.msg import State +import rclpy +from rclpy.node import Node +from std_msgs.msg import Bool +from controller_manager_msgs.srv import ( + ListControllers, + ListHardwareComponents, + SwitchController, + SetHardwareComponentState, +) +from rclpy.qos import QoSProfile, DurabilityPolicy + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner_with_estop.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + # Launch the spawner alongside the controller manager + spawner_node = launch_ros.actions.Node( + package="hector_controller_spawner", + executable="hector_controller_spawner", + output="screen", + parameters=[spawner_config], + ) + + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction(period=3.0, actions=[spawner_node]), + launch.actions.TimerAction( + period=5.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager, "spawner_node": spawner_node}, + ) + + +class TestEStopFunctionality(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_estop_functionality") + + # Create e-stop publisher + estop_qos = QoSProfile( + depth=1, + durability=DurabilityPolicy.TRANSIENT_LOCAL, # Match spawner's QoS + ) + cls.estop_pub = cls.node.create_publisher( + Bool, "estop_board/hard_estop", estop_qos + ) + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_complete_estop_workflow(self): + """Test complete e-stop workflow: block loading, allow loading, block again, allow again""" + + # Step 1: Send e-stop true (active/stopped) + self.node.get_logger().info("Step 1: Activating e-stop") + self._publish_estop(True) + time.sleep(2.0) # Allow time for e-stop to be processed + + # Step 2: Check that hardware interfaces and controllers are not active + self.node.get_logger().info( + "Step 2: Checking that hardware/controllers are not active" + ) + hardware_components = self._get_hardware_components() + controllers = self._get_controller_list() + + # Verify no hardware components are active + active_hardware = [ + hw.name for hw in hardware_components if hw.state.label == "active" + ] + self.assertEqual( + len(active_hardware), + 0, + "No hardware interfaces should be active while e-stop is active", + ) + + # Verify no controllers are active + active_controllers = [c.name for c in controllers if c.state == "active"] + self.assertEqual( + len(active_controllers), + 0, + "No controllers should be active while e-stop is active", + ) + + # Step 3: Send e-stop false (inactive/running) and wait + self.node.get_logger().info("Step 3: Releasing e-stop") + self._publish_estop(False) + time.sleep(8.0) # Allow time for spawner to proceed and load everything + + # Step 4: Check that controllers and hardware interfaces are loaded and active + self.node.get_logger().info( + "Step 4: Checking that hardware/controllers are active" + ) + hardware_components = self._get_hardware_components() + controllers = self._get_controller_list() + + # Verify expected hardware components are active + expected_hardware = ["athena_flipper_interface", "athena_arm_interface"] + active_hardware = [ + hw.name for hw in hardware_components if hw.state.label == "active" + ] + + for hw_name in expected_hardware: + self.assertIn( + hw_name, + active_hardware, + f"Hardware interface {hw_name} should be active after e-stop release", + ) + + # Verify expected controllers are active + expected_active_controllers = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + + active_controllers = [c.name for c in controllers if c.state == "active"] + for controller in expected_active_controllers: + self.assertIn( + controller, + active_controllers, + f"Controller {controller} should be active after e-stop release", + ) + + # Step 5: Send e-stop true and unload hardware interfaces + self.node.get_logger().info( + "Step 5: Activating e-stop again (should deactivate hardware)" + ) + self._publish_estop(True) + self._deactivate_all_controllers() + self._unload_hardware_interfaces() + time.sleep(3.0) # Allow time for deactivation + + # Step 6: Verify hardware interfaces are deactivated + hardware_components = self._get_hardware_components() + controllers = self._get_controller_list() + + # Check that hardware is no longer active + active_hardware_after_estop = [ + hw.name for hw in hardware_components if hw.state.label == "active" + ] + self.assertEqual( + len(active_hardware_after_estop), + 0, + "Hardware interfaces should be deactivated when e-stop is activated again", + ) + + # Check that controllers are no longer active + active_controllers_after_estop = [ + c.name for c in controllers if c.state == "active" + ] + self.assertEqual( + len(active_controllers_after_estop), + 0, + "Controllers should be deactivated when e-stop is activated again", + ) + + # Step 7: Send e-stop false and wait some time + self.node.get_logger().info("Step 6: Releasing e-stop second time") + self._publish_estop(False) + time.sleep(8.0) # Allow time for reactivation + + # Step 8: Check that hardware interfaces and requested controllers are active again + self.node.get_logger().info( + "Step 7: Checking final state - should be active again" + ) + final_hardware_components = self._get_hardware_components() + final_controllers = self._get_controller_list() + + # Verify hardware interfaces are active again + final_active_hardware = [ + hw.name for hw in final_hardware_components if hw.state.label == "active" + ] + for hw_name in expected_hardware: + self.assertIn( + hw_name, + final_active_hardware, + f"Hardware interface {hw_name} should be active again after second e-stop release", + ) + + # Verify controllers are active again + final_active_controllers = [ + c.name for c in final_controllers if c.state == "active" + ] + for controller in expected_active_controllers: + self.assertIn( + controller, + final_active_controllers, + f"Controller {controller} should be active again after second e-stop release", + ) + + self.node.get_logger().info("E-stop workflow test completed successfully") + + def _publish_estop(self, active): + """Helper to publish e-stop message multiple times for reliability""" + estop_msg = Bool() + estop_msg.data = active + + # Publish multiple times to ensure delivery + for _ in range(10): + self.estop_pub.publish(estop_msg) + rclpy.spin_once(self.node, timeout_sec=0.1) + time.sleep(0.1) + + # Extra time for message propagation + time.sleep(0.5) + + def _get_hardware_components(self): + """Helper to get current hardware component list""" + client = self.node.create_client( + ListHardwareComponents, "/controller_manager/list_hardware_components" + ) + + if not client.wait_for_service(timeout_sec=3.0): + self.node.get_logger().warn("Hardware components service not available") + return [] + + request = ListHardwareComponents.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + return response.component if response else [] + + def _get_controller_list(self): + """Helper to get current controller list""" + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + if not client.wait_for_service(timeout_sec=3.0): + self.node.get_logger().warn("Controller manager service not available") + return [] + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + return response.controller if response else [] + + def _unload_hardware_interfaces(self): + """Unload/deactivate hardware interfaces by transitioning them to inactive state""" + active_hardware = ["athena_flipper_interface", "athena_arm_interface"] + # Create service client for setting hardware component state + set_hw_state_client = self.node.create_client( + SetHardwareComponentState, + "/controller_manager/set_hardware_component_state", + ) + + if not set_hw_state_client.wait_for_service(timeout_sec=5.0): + self.node.get_logger().error( + "Hardware component state service not available" + ) + return + + # Deactivate each active hardware interface + for hw_component in active_hardware: + self.node.get_logger().info( + f"Deactivating hardware interface: {hw_component}" + ) + + request = SetHardwareComponentState.Request() + request.name = hw_component + request.target_state = State() + request.target_state.id = ( + State.PRIMARY_STATE_INACTIVE + ) # Transition to inactive + request.target_state.label = "inactive" + + # Call the service + future = set_hw_state_client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + if response and response.ok: + self.node.get_logger().info( + f"Successfully deactivated hardware interface: {hw_component}" + ) + else: + self.node.get_logger().error( + f"Failed to deactivate hardware interface: {hw_component}" + ) + + def _deactivate_all_controllers(self): + """Deactivate all controllers by switching them off""" + + controllers_to_deactivate = [ + "joint_state_broadcaster", + "flipper_trajectory_controller", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + + # Create service client for switching controllers + switch_client = self.node.create_client( + SwitchController, "/controller_manager/switch_controller" + ) + + if not switch_client.wait_for_service(timeout_sec=5.0): + self.node.get_logger().error("Switch controller service not available") + return + + self.node.get_logger().info("Deactivating all controllers") + + request = SwitchController.Request() + request.activate_controllers = [] # No controllers to activate + request.deactivate_controllers = ( + controllers_to_deactivate # Controllers to deactivate + ) + request.strictness = ( + SwitchController.Request.BEST_EFFORT + ) # Strict mode - fail if any controller can't be switched + request.activate_asap = False + request.timeout = rclpy.duration.Duration(seconds=5.0).to_msg() + + # Call the service + future = switch_client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=10.0) + + response = future.result() + if response and response.ok: + self.node.get_logger().info("Successfully deactivated all controllers") + else: + self.node.get_logger().error("Failed to deactivate controllers") + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # Check that the spawner completed successfully + self.assertEqual(proc_info["hector_controller_spawner"].returncode, 0) diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py new file mode 100644 index 0000000..757412a --- /dev/null +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py @@ -0,0 +1,235 @@ +import os +import unittest +import subprocess +import time +from ament_index_python.packages import get_package_share_directory +import launch +import launch.actions +import launch_ros.actions +import launch_testing +import launch_testing.actions + +import rclpy +from rclpy.node import Node +from controller_manager_msgs.srv import ListControllers + + +def generate_test_description(): + pkg_share = get_package_share_directory("hector_controller_spawner") + + controller_config = os.path.join(pkg_share, "test", "config", "controllers.yaml") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner.yaml" + ) + robot_description_file = os.path.join(pkg_share, "test", "config", "athena.urdf") + + for path in [controller_config, spawner_config, robot_description_file]: + if not os.path.isfile(path): + raise FileNotFoundError(f"Missing test file: {path}") + + with open(robot_description_file, "r") as f: + robot_description = f.read() + + robot_state_publisher = launch_ros.actions.Node( + package="robot_state_publisher", + executable="robot_state_publisher", + name="robot_state_publisher", + output="screen", + parameters=[{"robot_description": robot_description}], + ) + + controller_manager = launch_ros.actions.Node( + package="controller_manager", + executable="ros2_control_node", + output="screen", + parameters=[controller_config], + ) + + # Don't launch spawner automatically - we'll control it manually + return ( + launch.LaunchDescription( + [ + robot_state_publisher, + controller_manager, + launch.actions.TimerAction( + period=5.0, actions=[launch_testing.actions.ReadyToTest()] + ), + ] + ), + {"controller_manager": controller_manager}, + ) + + +class TestControllerSpawnerIdempotency(unittest.TestCase): + @classmethod + def setUpClass(cls): + rclpy.init() + cls.node = Node("test_controller_spawner_idempotency") + + @classmethod + def tearDownClass(cls): + cls.node.destroy_node() + rclpy.shutdown() + + def test_spawner_run_twice_idempotent(self): + """Test that running the spawner twice doesn't cause problems""" + + pkg_share = get_package_share_directory("hector_controller_spawner") + spawner_config = os.path.join( + pkg_share, "test", "config", "controller_spawner.yaml" + ) + + # Spawner command + cmd = [ + "ros2", + "run", + "hector_controller_spawner", + "hector_controller_spawner", + "--ros-args", + "--params-file", + spawner_config, + ] + + # 1. Check initial state (no controllers loaded) + initial_controllers = self._get_controller_list() + initial_count = len(initial_controllers) + self.node.get_logger().info(f"Initial controller count: {initial_count}") + + # 2. Run spawner first time + self.node.get_logger().info("Running spawner first time...") + process1 = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + # Wait for spawner to complete + return_code1 = process1.wait(timeout=30) # 30 second timeout + + stdout1, stderr1 = process1.communicate() + self.assertEqual( + return_code1, 0, f"First spawner run failed with return code {return_code1}" + ) + + # Give time for controllers to be loaded + time.sleep(2.0) + + # 3. Check state after first run + first_run_controllers = self._get_controller_list() + first_run_count = len(first_run_controllers) + self.node.get_logger().info( + f"Controller count after first run: {first_run_count}" + ) + + self.assertGreater( + first_run_count, + initial_count, + "Controllers should be loaded after first spawner run", + ) + + # Store the state for comparison + first_run_active = [ + c.name for c in first_run_controllers if c.state == "active" + ] + first_run_inactive = [ + c.name for c in first_run_controllers if c.state == "inactive" + ] + + # 4. Wait a bit, then run spawner second time + time.sleep(3.0) # Delay between runs + + self.node.get_logger().info("Running spawner second time...") + process2 = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + # Wait for second spawner to complete + return_code2 = process2.wait(timeout=30) + + stdout2, stderr2 = process2.communicate() + self.assertEqual( + return_code2, + 0, + f"Second spawner run failed with return code {return_code2}", + ) + + # Give time for any changes to take effect + time.sleep(2.0) + + # 5. Check state after second run + second_run_controllers = self._get_controller_list() + second_run_count = len(second_run_controllers) + self.node.get_logger().info( + f"Controller count after second run: {second_run_count}" + ) + + second_run_active = [ + c.name for c in second_run_controllers if c.state == "active" + ] + second_run_inactive = [ + c.name for c in second_run_controllers if c.state == "inactive" + ] + + # 6. Verify that the state is identical (idempotent behavior) + self.assertEqual( + first_run_count, + second_run_count, + "Controller count should be the same after second spawner run", + ) + + # Check that active controllers are the same + self.assertEqual( + set(first_run_active), + set(second_run_active), + "Active controllers should be identical after second run", + ) + + # Check that inactive controllers are the same + self.assertEqual( + set(first_run_inactive), + set(second_run_inactive), + "Inactive controllers should be identical after second run", + ) + + # 7. Verify expected controllers are still in correct states + expected_active = [ + "joint_state_broadcaster", + "flipper_velocity_controller", + "gripper_trajectory_controller", + "arm_trajectory_controller", + "vel_to_pos_controller", + ] + + for controller in expected_active: + self.assertIn( + controller, + second_run_active, + f"Controller {controller} should still be active after second run", + ) + + # Log the outputs for debugging if needed + if stdout1: + self.node.get_logger().info( + f"First spawner stdout: {stdout1.decode()[:200]}..." + ) + if stdout2: + self.node.get_logger().info( + f"Second spawner stdout: {stdout2.decode()[:200]}..." + ) + + def _get_controller_list(self): + """Helper to get current controller list""" + client = self.node.create_client( + ListControllers, "/controller_manager/list_controllers" + ) + + if not client.wait_for_service(timeout_sec=5.0): + return [] + + request = ListControllers.Request() + future = client.call_async(request) + rclpy.spin_until_future_complete(self.node, future, timeout_sec=5.0) + + response = future.result() + return response.controller if response else [] + + +@launch_testing.post_shutdown_test() +class TestProcessOutput(unittest.TestCase): + def test_exit_codes(self, proc_info): + # Controller manager should shutdown cleanly + pass # Allow flexible exit codes since we're not checking spawner here diff --git a/tests/test_pkg_missing_launch_deps.py b/tests/test_pkg_missing_launch_deps.py new file mode 100644 index 0000000..289fcc9 --- /dev/null +++ b/tests/test_pkg_missing_launch_deps.py @@ -0,0 +1,131 @@ +import os +import unittest +import tempfile +import shutil +import subprocess +from lxml import etree as ET + +from package_xml_validation.package_xml_validator import ( + PackageXmlValidator, +) + + +def validate_xml_with_xmllint(xml_file): + """Validate XML file against the ROS package_format3.xsd schema using xmllint.""" + schema_url = "http://download.ros.org/schema/package_format3.xsd" + try: + result = subprocess.run( + ["xmllint", "--noout", "--schema", schema_url, xml_file], + capture_output=True, + text=True, + ) + if result.returncode != 0: + print(f"XML validation error in {xml_file}:\n{result.stderr}") + return False + return True + except FileNotFoundError: + print( + "Error: xmllint not found. Please ensure it's installed and in your PATH." + ) + return False + + +class TestPackageXmlValidator(unittest.TestCase): + @classmethod + def setUpClass(cls): + """ + We assume the example files are in 'tests/examples'. + Adjust the directory path to match your actual setup. + """ + current_dir = os.path.dirname(__file__) + cls.examples_dir = os.path.join(current_dir, "examples", "launch_pkg_examples") + cls.formatter = PackageXmlValidator( + check_only=False, + verbose=True, + auto_fill_missing_deps=True, + ) + + def setUp(self): + """ + Create a temporary directory for each test, copy example files into it, + so we can safely modify them during tests. + """ + self.test_dir = tempfile.mkdtemp(prefix="xml_tests_") + + # copy contents of examples_dir to test_dir + shutil.copytree(self.examples_dir, self.test_dir, dirs_exist_ok=True) + + def tearDown(self): + """Clean up the temporary directory after each test.""" + shutil.rmtree(self.test_dir) + + def _compare_xml_files(self, file1: str, file2: str) -> bool: + """ + Compare two XML files for equality. + Using xml parser to ignore whitespace and comments. + file1: is modified by the formatter + file2: is the expected corrected file + """ + try: + tree1 = ET.parse(file1) + tree2 = ET.parse(file2) + + # Normalize the XML trees + root1 = tree1.getroot() + root2 = tree2.getroot() + + for elem1, elem2 in zip(root1.iter(), root2.iter()): + # Ignore comments and whitespace + if ET.iselement(elem1) and ET.iselement(elem2): + if elem1.tag != elem2.tag or elem1.text != elem2.text: + return False + elif ET.iselement(elem1) and not ET.iselement(elem2): + return False + elif not ET.iselement(elem1) and ET.iselement(elem2): + return False + # make sure there are no more than 2 \n in the tai + elif elem2.tail.count("\n") > 2: + return False + return True + except ET.XMLSyntaxError as e: + print(f"XML Syntax Error: {e}") + return False + + def test_xml_formatting(self): + """ + Iterate over all example packages in the test directory, + """ + example_pkgs = os.listdir(self.examples_dir) + for example_pkg in example_pkgs: + correct_xml = os.path.join( + self.examples_dir, example_pkg, "pkg_correct", "package.xml" + ) + build_type_dir = os.path.join(self.test_dir, example_pkg) + for pkg in os.listdir(build_type_dir): + xml_file = os.path.join(build_type_dir, pkg, "package.xml") + # apply the formatter + valid = self.formatter.check_and_format_files([xml_file]) + if not pkg == "pkg_correct": + self.assertFalse( + valid, + f"XML file {xml_file} is expected to be invalid but was valid.", + ) + else: + self.assertTrue( + valid, + f"XML file {xml_file} is expected to be valid but was invalid.", + ) + self.assertTrue( + self._compare_xml_files(xml_file, correct_xml), + f"XML files do not match: {xml_file} != {correct_xml}", + ) + # validate the XML file with xmllint + self.assertTrue( + validate_xml_with_xmllint(xml_file), + f"XML file {xml_file} failed xmllint validation.", + ) + + +if __name__ == "__main__": + # Run the tests + unittest.main() From a09e8206e6404bd7deec37066ac8a52ab9f41f69 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 16:18:13 +0200 Subject: [PATCH 08/25] Refactoring --- package_xml_validation/helpers/cmake_parsers.py | 10 ++++++---- package_xml_validation/helpers/pkg_xml_formatter.py | 7 ++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/package_xml_validation/helpers/cmake_parsers.py b/package_xml_validation/helpers/cmake_parsers.py index 4f74aed..37e929e 100644 --- a/package_xml_validation/helpers/cmake_parsers.py +++ b/package_xml_validation/helpers/cmake_parsers.py @@ -97,11 +97,11 @@ def resolve_for_each(raw_lines: List[str]) -> List[str]: return lines -def retrieve_cmake_dependencies(lines: List[str]) -> List[str]: +def retrieve_cmake_dependencies(lines: List[str]) -> tuple[List[str], List[str]]: if isinstance(lines, Path): lines = read_cmake_file(lines) - main_deps = [] - test_deps = [] + main_deps: list[str] = [] + test_deps: list[str] = [] # We'll track blocks of 'if(BUILD_TESTING)' with a small stack if_stack = [] @@ -193,8 +193,10 @@ def read_cmake_file(file_path: Path) -> List[str]: return lines -def read_deps_from_cmake_file(file_path: Path) -> List[str]: +def read_deps_from_cmake_file(file_path: Path | str) -> tuple[List[str], List[str]]: """Reads a CMake file and returns a list of dependencies.""" + if isinstance(file_path, str): + file_path = Path(file_path) lines = read_cmake_file(file_path) try: main_deps, test_deps = retrieve_cmake_dependencies(lines) diff --git a/package_xml_validation/helpers/pkg_xml_formatter.py b/package_xml_validation/helpers/pkg_xml_formatter.py index 2a05430..39c9782 100644 --- a/package_xml_validation/helpers/pkg_xml_formatter.py +++ b/package_xml_validation/helpers/pkg_xml_formatter.py @@ -484,6 +484,7 @@ def add_dependencies(self, root, dependencies, dep_type): if dep_type not in dep_types: raise ValueError(f"Invalid dependency type: {dep_type}") indendantion = root[0].tail.replace(NEW_LINE, "") + insert_position, first_of_group = 0, 0 for dep in dependencies: new_elem = ET.Element(dep_type) new_elem.text = dep @@ -515,7 +516,11 @@ def add_dependencies(self, root, dependencies, dep_type): insert_position = i + 1 root.insert(insert_position, new_elem) # adapt empty lines -> in case element prior ends with empty line move it to the new element - if insert_position > 0 and insert_position > first_of_group: + if ( + insert_position > 0 + and first_of_group is not None + and insert_position > first_of_group + ): previous_element = root[insert_position - 1] if previous_element.tail and previous_element.tail.count(NEW_LINE) > 1: new_elem.tail = previous_element.tail From b39b337ece887146da434d86025fcf8cce1cd139 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 16:33:23 +0200 Subject: [PATCH 09/25] Refactoring --- package_xml_validation/helpers/pkg_xml_formatter.py | 2 +- package_xml_validation/package_xml_validator.py | 2 +- tests/test_package_xml_linting_and_formatting.py | 2 +- tests/test_pkg_missing_launch_deps.py | 3 +-- tests/test_pky_type_dependent_validation.py | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/package_xml_validation/helpers/pkg_xml_formatter.py b/package_xml_validation/helpers/pkg_xml_formatter.py index 39c9782..410165b 100644 --- a/package_xml_validation/helpers/pkg_xml_formatter.py +++ b/package_xml_validation/helpers/pkg_xml_formatter.py @@ -1,4 +1,4 @@ -from lxml import etree as ET +import lxml.etree as ET from copy import deepcopy diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index 60dc42f..b08af16 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -1,7 +1,7 @@ import argparse import os from typing import List -from lxml import etree as ET +import lxml.etree as ET from enum import Enum try: diff --git a/tests/test_package_xml_linting_and_formatting.py b/tests/test_package_xml_linting_and_formatting.py index 21a474b..a72d93b 100644 --- a/tests/test_package_xml_linting_and_formatting.py +++ b/tests/test_package_xml_linting_and_formatting.py @@ -3,7 +3,7 @@ import tempfile import shutil import subprocess -from lxml import etree as ET +import lxml.etree as ET from package_xml_validation.package_xml_validator import ( PackageXmlValidator, diff --git a/tests/test_pkg_missing_launch_deps.py b/tests/test_pkg_missing_launch_deps.py index 289fcc9..c316459 100644 --- a/tests/test_pkg_missing_launch_deps.py +++ b/tests/test_pkg_missing_launch_deps.py @@ -3,8 +3,7 @@ import tempfile import shutil import subprocess -from lxml import etree as ET - +import lxml.etree as ET from package_xml_validation.package_xml_validator import ( PackageXmlValidator, ) diff --git a/tests/test_pky_type_dependent_validation.py b/tests/test_pky_type_dependent_validation.py index b6dd457..c419689 100644 --- a/tests/test_pky_type_dependent_validation.py +++ b/tests/test_pky_type_dependent_validation.py @@ -3,7 +3,7 @@ import tempfile import shutil import subprocess -from lxml import etree as ET +import lxml.etree as ET from package_xml_validation.package_xml_validator import ( PackageXmlValidator, From 30bf4562dc0bcada0483d7ebb5ba284a3383e4b2 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 16:44:37 +0200 Subject: [PATCH 10/25] Refactoring --- .pre-commit-config.yaml | 6 ++++++ package_xml_validation/helpers/cmake_parsers.py | 15 +++++++-------- package_xml_validation/helpers/workspace.py | 7 +++---- package_xml_validation/package_xml_validator.py | 15 +++++++-------- package_xml_validation/ros2_pkg_validator.py | 3 +-- pyproject.toml | 2 +- .../pkg_correct/test/test_spawner_basic.test.py | 2 +- .../test/test_spawner_chain_detection.test.py | 2 +- .../pkg_correct/test/test_spawner_estop.test.py | 2 +- .../pkg_correct/test/test_spawner_twice.test.py | 2 +- .../test/test_spawner_basic.test.py | 2 +- .../test/test_spawner_chain_detection.test.py | 2 +- .../test/test_spawner_estop.test.py | 2 +- .../test/test_spawner_twice.test.py | 2 +- tests/test_package_xml_linting_and_formatting.py | 4 ++-- 15 files changed, 35 insertions(+), 33 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8a5da70..af0d6b2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,6 +22,12 @@ repos: args: [ --fix ] - id: ruff-format + - repo: https://github.com/asottile/pyupgrade + rev: v3.20.0 + hooks: + - id: pyupgrade + args: [--py39-plus] + # Spellcheck - repo: https://github.com/crate-ci/typos rev: v1.34.0 diff --git a/package_xml_validation/helpers/cmake_parsers.py b/package_xml_validation/helpers/cmake_parsers.py index 37e929e..31ddef5 100644 --- a/package_xml_validation/helpers/cmake_parsers.py +++ b/package_xml_validation/helpers/cmake_parsers.py @@ -1,14 +1,13 @@ from pathlib import Path import re -from typing import List -def remove_comments(lines: List[str]) -> list[str]: +def remove_comments(lines: list[str]) -> list[str]: """Removes comments from a list of lines.""" return [line.split("#", 1)[0].strip() for line in lines] -def read_cmake_lines_with_parens_joined(raw_lines: List[str]) -> list[str]: +def read_cmake_lines_with_parens_joined(raw_lines: list[str]) -> list[str]: """ Reads a CMake file and joins lines that have an opening '(' without a matching ')' until the closing ')' is found. Returns a list of logically complete lines. @@ -41,7 +40,7 @@ def has_balanced_parens(s: str) -> bool: return lines -def resolve_for_each(raw_lines: List[str]) -> List[str]: +def resolve_for_each(raw_lines: list[str]) -> list[str]: """Expands CMake's foreach() loops in a list of lines.""" foreach_stack = [] @@ -97,7 +96,7 @@ def resolve_for_each(raw_lines: List[str]) -> List[str]: return lines -def retrieve_cmake_dependencies(lines: List[str]) -> tuple[List[str], List[str]]: +def retrieve_cmake_dependencies(lines: list[str]) -> tuple[list[str], list[str]]: if isinstance(lines, Path): lines = read_cmake_file(lines) main_deps: list[str] = [] @@ -176,14 +175,14 @@ def add_deps(dep_list: list[str], is_test: bool): return main_deps, test_deps -def read_cmake_file(file_path: Path) -> List[str]: +def read_cmake_file(file_path: Path) -> list[str]: """Reads a CMake file and returns a list of lines.""" if isinstance(file_path, str): file_path = Path(file_path) if not file_path.exists(): print(f"File not found: {file_path}") return [] - with open(file_path, "r") as f: + with open(file_path) as f: raw_lines = f.readlines() lines = remove_comments(raw_lines) lines = read_cmake_lines_with_parens_joined(lines) @@ -193,7 +192,7 @@ def read_cmake_file(file_path: Path) -> List[str]: return lines -def read_deps_from_cmake_file(file_path: Path | str) -> tuple[List[str], List[str]]: +def read_deps_from_cmake_file(file_path: Path | str) -> tuple[list[str], list[str]]: """Reads a CMake file and returns a list of dependencies.""" if isinstance(file_path, str): file_path = Path(file_path) diff --git a/package_xml_validation/helpers/workspace.py b/package_xml_validation/helpers/workspace.py index 9d4ebed..79c3f33 100644 --- a/package_xml_validation/helpers/workspace.py +++ b/package_xml_validation/helpers/workspace.py @@ -15,7 +15,6 @@ import sys import xml.etree.ElementTree as ET from pathlib import Path -from typing import Dict, List import os @@ -95,9 +94,9 @@ def parse_pkg_name(package_xml: Path) -> str: return package_xml.parent.name # fallback -def pkg_iterator(src_dir: Path) -> Dict[str, Path]: +def pkg_iterator(src_dir: Path) -> dict[str, Path]: """Yield ``{pkg_name: pkg_path}`` for all packages under *src_dir*.""" - pkgs: Dict[str, Path] = {} + pkgs: dict[str, Path] = {} for xml in src_dir.rglob("package.xml"): # Respect COLCON_IGNORE: ignore a path if any ancestor contains the file if any((parent / "COLCON_IGNORE").exists() for parent in xml.parents): @@ -106,7 +105,7 @@ def pkg_iterator(src_dir: Path) -> Dict[str, Path]: return pkgs -def get_pkgs_in_wrs(path: Path) -> List[str]: +def get_pkgs_in_wrs(path: Path) -> list[str]: """Return all package names in the workspace that contains *path*.""" if isinstance(path, str): path = Path(path).resolve(strict=True) diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index b08af16..c74fccc 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -1,6 +1,5 @@ import argparse import os -from typing import List import lxml.etree as ET from enum import Enum @@ -91,7 +90,7 @@ def get_package_type(self, xml_file: str) -> tuple[PackageType, bool]: if os.path.exists(cmake_file): # check if rosidl_generate_interfaces is in CMakeLists.txt using regex regex = r"rosidl_generate_interfaces\s*\(\s*.*?\)" - with open(cmake_file, "r") as f: + with open(cmake_file) as f: content = f.read() if re.search(regex, content, re.DOTALL): is_msg_pkg = True @@ -100,7 +99,7 @@ def get_package_type(self, xml_file: str) -> tuple[PackageType, bool]: pkg_type = PackageType.PYTHON_PKG return pkg_type, is_msg_pkg - def check_for_rosdeps(self, rosdeps: List[str], xml_file: str): + def check_for_rosdeps(self, rosdeps: list[str], xml_file: str): """extract list of rosdeps and check if they are valid""" if not rosdeps: self.logger.info(f"No ROS dependencies found in {xml_file}.") @@ -115,7 +114,7 @@ def check_for_rosdeps(self, rosdeps: List[str], xml_file: str): return True def check_for_cmake( - self, build_deps: List[str], test_deps: List[str], xml_file: str, root + self, build_deps: list[str], test_deps: list[str], xml_file: str, root ): cmake_file = os.path.join(os.path.dirname(xml_file), "CMakeLists.txt") if not os.path.exists(cmake_file): @@ -194,12 +193,12 @@ def validate_launch_dependencies( root, package_xml_file: str, package_name: str, - exec_deps: List[str], - test_deps: List[str] = [], + exec_deps: list[str], + test_deps: list[str] = [], ): """Validate launch dependencies in the package.xml file.""" - def extract_launch_deps(folder_names: List[str]) -> List[str]: + def extract_launch_deps(folder_names: list[str]) -> list[str]: """Extract launch dependencies from the folder names.""" launch_deps = [] for folder in folder_names: @@ -212,7 +211,7 @@ def extract_launch_deps(folder_names: List[str]) -> List[str]: return launch_deps def validate_launch_folders( - launch_folder_names: List[str], xml_deps: List[str], depend_tag: str + launch_folder_names: list[str], xml_deps: list[str], depend_tag: str ) -> bool: launch_deps = extract_launch_deps(launch_folder_names) if not launch_deps: diff --git a/package_xml_validation/ros2_pkg_validator.py b/package_xml_validation/ros2_pkg_validator.py index 0b6a4a0..c7690f6 100644 --- a/package_xml_validation/ros2_pkg_validator.py +++ b/package_xml_validation/ros2_pkg_validator.py @@ -12,7 +12,6 @@ import sys import os from pathlib import Path -from typing import List import xml.etree.ElementTree as ET from helpers.cmake_parsers import retrieve_cmake_dependencies from helpers.rosdep_validator import check_rosdeps @@ -66,7 +65,7 @@ def find_files_in_dir(base_dir: Path): return xml_files, cmake_files -def collect_cmake_package_pairs(cmake_files: List[Path], package_xmls: List[Path]): +def collect_cmake_package_pairs(cmake_files: list[Path], package_xmls: list[Path]): # corresponding package.xml and cmake files are in the same directory pairs = [] visited_cmake_files = set() diff --git a/pyproject.toml b/pyproject.toml index c091ad0..307736c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ name = "package_xml_validation" version = "0.1.0" description = "Validates package xml of ros2 pkgs" readme = "README.md" -requires-python = ">=3.7" +requires-python = ">=3.9" dependencies = [ "lxml", "rosdep", diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py index 8c66fa5..ff69276 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_basic.test.py @@ -25,7 +25,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py index ad6ad87..696b7f2 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_chain_detection.test.py @@ -25,7 +25,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py index c0346d9..ea4e993 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_estop.test.py @@ -33,7 +33,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py index 757412a..1777561 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_correct/test/test_spawner_twice.test.py @@ -27,7 +27,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py index 8c66fa5..ff69276 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_basic.test.py @@ -25,7 +25,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py index ad6ad87..696b7f2 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_chain_detection.test.py @@ -25,7 +25,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py index c0346d9..ea4e993 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_estop.test.py @@ -33,7 +33,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py index 757412a..1777561 100644 --- a/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py +++ b/tests/examples/launch_pkg_examples/hector_controller_spawner/pkg_missing_test_depends/test/test_spawner_twice.test.py @@ -27,7 +27,7 @@ def generate_test_description(): if not os.path.isfile(path): raise FileNotFoundError(f"Missing test file: {path}") - with open(robot_description_file, "r") as f: + with open(robot_description_file) as f: robot_description = f.read() robot_state_publisher = launch_ros.actions.Node( diff --git a/tests/test_package_xml_linting_and_formatting.py b/tests/test_package_xml_linting_and_formatting.py index a72d93b..2eed0dc 100644 --- a/tests/test_package_xml_linting_and_formatting.py +++ b/tests/test_package_xml_linting_and_formatting.py @@ -143,7 +143,7 @@ def test_xml_formatting(self): if self._is_correct_file(fname): if not all_valid_check: - with open(original_path, "r") as f_after: + with open(original_path) as f_after: print(f"File content after check:\n{f_after.read()}") self.assertTrue( all_valid_check, @@ -219,7 +219,7 @@ def test_xml_formatting(self): comparison = self._compare_xml_files(final_path, corrected_path) if not comparison: # print corrected file to console - with open(final_path, "r") as f_corrected: + with open(final_path) as f_corrected: corrected_content = f_corrected.read() print(f"Corrected file content:\n{corrected_content}") self.assertTrue( From efab3eeae304cb4780a72869c055e17a3d3238e6 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 16:45:39 +0200 Subject: [PATCH 11/25] Updated hooks --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index af0d6b2..19f8e17 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,7 @@ repos: # Standard hooks - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v5.0.0 + rev: v6.0.0 hooks: - id: check-added-large-files - id: check-merge-conflict @@ -16,7 +16,7 @@ repos: # Python hooks - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.12.7 + rev: v0.12.8 hooks: - id: ruff-check args: [ --fix ] From c91d0f92f2888feb98475ea0ffedefd7eab85729 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:10:39 +0200 Subject: [PATCH 12/25] Removed unnecessary file --- .../package_xml_validator.py | 1 + package_xml_validation/ros2_pkg_validator.py | 227 ------------------ 2 files changed, 1 insertion(+), 227 deletions(-) delete mode 100644 package_xml_validation/ros2_pkg_validator.py diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index 05a73b5..c531fa9 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -441,6 +441,7 @@ def check_and_format_files(self, package_xml_files): self.xml_valid = True pkg_name = os.path.basename(os.path.dirname(xml_file)) self.logger.info(f"Processing {pkg_name}...") + self.logger.debug(f"Checking {xml_file}...") if not os.path.exists(xml_file): raise FileNotFoundError(f"{xml_file} does not exist.") diff --git a/package_xml_validation/ros2_pkg_validator.py b/package_xml_validation/ros2_pkg_validator.py deleted file mode 100644 index c7690f6..0000000 --- a/package_xml_validation/ros2_pkg_validator.py +++ /dev/null @@ -1,227 +0,0 @@ -#!/usr/bin/env python3 -""" -ros2-dependency-validator.py - -A simple script that: -1. Finds package.xml and/or CMakeLists.txt files in user-specified locations. -2. Parses dependencies (main + test) from each. -3. Optionally (via --check) only reports findings without any attempt to fix them. -""" - -import argparse -import sys -import os -from pathlib import Path -import xml.etree.ElementTree as ET -from helpers.cmake_parsers import retrieve_cmake_dependencies -from helpers.rosdep_validator import check_rosdeps - - -def parse_package_xml(xml_path: Path): - """ - Parse for normal dependencies and test dependencies. - - Returns: - (main_deps, test_deps) - main_deps: list of strings for normal (build, etc.) dependencies - test_deps: list of strings for test_depend - """ - main_deps = [] - test_deps = [] - - if not xml_path.exists(): - return main_deps, test_deps - - tree = ET.parse(xml_path) - root = tree.getroot() - - main_tags = {"depend", "build_depend", "build_export_depend", "buildtool_depend"} - test_tags = {"test_depend"} - - for child in root: - tag_name = child.tag.lower() - text_value = (child.text or "").strip() - if tag_name in main_tags: - main_deps.append(text_value) - elif tag_name in test_tags: - test_deps.append(text_value) - - return main_deps, test_deps - - -def find_files_in_dir(base_dir: Path): - """ - Recursively find all package.xml or CMakeLists.txt files under base_dir. - Returns two sets: (xml_files, cmake_files). - """ - xml_files = set() - cmake_files = set() - for root, dirs, files in os.walk(base_dir): - for file in files: - if file == "package.xml": - xml_files.add(Path(root) / file) - elif file == "CMakeLists.txt": - cmake_files.add(Path(root) / file) - return xml_files, cmake_files - - -def collect_cmake_package_pairs(cmake_files: list[Path], package_xmls: list[Path]): - # corresponding package.xml and cmake files are in the same directory - pairs = [] - visited_cmake_files = set() - for package_xml in package_xmls: - pairs.append((package_xml, package_xml.parent / "CMakeLists.txt")) - visited_cmake_files.add(package_xml.parent / "CMakeLists.txt") - for cmake_file in cmake_files: - if cmake_file not in visited_cmake_files: - pairs.append((cmake_file.parent / "package.xml", cmake_file)) - # make sure all files e - existing_pairs = [] - for package_xml, cmake_file in pairs: - if package_xml.exists() and cmake_file.exists(): - existing_pairs.append((package_xml, cmake_file)) - return existing_pairs - - -def print_missing(source, target, missing_in_target_deps, is_test=False): - dep_name = "test " if is_test else "" - print(f"\t {source} -> {target} missing {dep_name}dependencies:") - for dep in missing_in_target_deps: - print(f"\t\t{dep}") - - -def compare_if_missing(source, target): - # in priniciple missing_in_target = set(source) - set(target), but robust to - # lib vs and -dev vs - missing_in_target = set() - for dep in source: - exists = dep in target - if exists: - continue - if dep.startswith("lib"): - exists = dep[3:] in target - else: - exists = f"lib{dep}" in target - if exists: - continue - if dep.endswith("-dev"): - exists = dep[:-4] in target - else: - exists = f"{dep}-dev" in target - if not exists: - missing_in_target.add(dep) - return missing_in_target - - -def validate(paths, check_rosdep=False, verbose=False): - # If SRC is empty or is just ['.'], treat it as "scan current directory". - if not paths or paths == ["."]: - src_paths = [Path(".").resolve()] - else: - src_paths = [Path(s).resolve() for s in paths] - - # Collect unique package.xml and CMakeLists.txt from all provided paths - all_package_xmls = set() - all_cmake_lists = set() - - for path in src_paths: - if path.is_dir(): - # Recursively scan - xmls, cmakes = find_files_in_dir(path) - all_package_xmls.update(xmls) - all_cmake_lists.update(cmakes) - elif path.is_file(): - if path.name == "package.xml": - all_package_xmls.add(path) - elif path.name == "CMakeLists.txt": - all_cmake_lists.add(path) - # else: non-existent path is ignored in this example - - # If no relevant files found, exit 0 (no error) - if not all_package_xmls and not all_cmake_lists: - print("No package.xml or CMakeLists.txt found. Exiting with code 0.") - sys.exit(0) - - # collect pairs of package.xml and CMakeLists.txt files - pairs = collect_cmake_package_pairs(list(all_cmake_lists), list(all_package_xmls)) - - for package_xml, cmake_list in pairs: - package_name = package_xml.parent.name - main_deps_xml, test_deps_xml = parse_package_xml(package_xml) - main_deps_cmake, test_deps_cmake = retrieve_cmake_dependencies(cmake_list) - if verbose: - print(f"Package.xml: {package_xml}") - print(f"CMakeLists.txt: {cmake_list}") - print(f"Main deps XML: {main_deps_xml}") - print(f"Test deps XML: {test_deps_xml}") - print(f"Main deps CMake: {main_deps_cmake}") - print(f"Test deps CMake: {test_deps_cmake}") - # Check for discrepancies between package.xml and CMakeLists.txt - deps_missing_in_cmake = compare_if_missing(main_deps_xml, main_deps_cmake) - deps_missing_in_xml = compare_if_missing(main_deps_cmake, main_deps_xml) - test_deps_missing_in_cmake = compare_if_missing(test_deps_xml, test_deps_cmake) - test_deps_missing_in_xml = compare_if_missing(test_deps_cmake, test_deps_xml) - - if ( - deps_missing_in_cmake - or deps_missing_in_xml - or test_deps_missing_in_cmake - or test_deps_missing_in_xml - ): - print(f"{package_name}") - - if deps_missing_in_cmake: - print_missing("package.xml", "CMakeLists.txt", deps_missing_in_cmake) - if deps_missing_in_xml: - print_missing("CMakeLists.txt", "package.xml", deps_missing_in_xml) - if test_deps_missing_in_cmake: - print_missing( - "package.xml", "CMakeLists.txt", test_deps_missing_in_cmake, True - ) - if test_deps_missing_in_xml: - print_missing( - "CMakeLists.txt", "package.xml", test_deps_missing_in_xml, True - ) - if ( - deps_missing_in_cmake - or deps_missing_in_xml - or test_deps_missing_in_cmake - or test_deps_missing_in_xml - ): - print("\n") - - # make sure the listed keys in the package.xml are resolvable - if check_rosdep: - unresolvable_deps = check_rosdeps(main_deps_xml + test_deps_xml) - if unresolvable_deps: - print(f"Could not resolve dependencies in {package_name}:") - for dep in unresolvable_deps: - print(f"\t{dep}") - print("\n") - - sys.exit(0) - - -def main(): - parser = argparse.ArgumentParser(description="ROS2 dependency validator.") - parser.add_argument( - "--check_rosdeps", - action="store_true", - default=False, - help="If set tests whether rosdep is able to resolve the listed dependencies.", - ) - parser.add_argument( - "SRC", - nargs="*", - help="List of files or directories to check. If empty or '.', we scan the current directory.", - ) - - args = parser.parse_args() - validate(args.SRC, args.check_rosdeps) - - -if __name__ == "__main__": - # main() - - paths = ["/home/aljoscha-schmidt/hector/src/"] - validate(paths, True, False) From ee0e66ee72b00b140a4e1d2f589dc5b70f86ae51 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:18:21 +0200 Subject: [PATCH 13/25] improved testing --- tests/test_pkg_missing_launch_deps.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_pkg_missing_launch_deps.py b/tests/test_pkg_missing_launch_deps.py index c316459..cd19198 100644 --- a/tests/test_pkg_missing_launch_deps.py +++ b/tests/test_pkg_missing_launch_deps.py @@ -58,6 +58,10 @@ def tearDown(self): """Clean up the temporary directory after each test.""" shutil.rmtree(self.test_dir) + def prettyprint(self, element, **kwargs): + xml = ET.tostring(element, pretty_print=True, **kwargs) + print(xml.decode(), end="") + def _compare_xml_files(self, file1: str, file2: str) -> bool: """ Compare two XML files for equality. @@ -105,11 +109,17 @@ def test_xml_formatting(self): # apply the formatter valid = self.formatter.check_and_format_files([xml_file]) if not pkg == "pkg_correct": + if valid: + with open(xml_file) as f: + print(f"Formatted XML file {xml_file}:\n{f.read()}") self.assertFalse( valid, f"XML file {xml_file} is expected to be invalid but was valid.", ) else: + if not valid: + with open(xml_file) as f: + print(f"Invalid XML file {xml_file}:\n{f.read()}") self.assertTrue( valid, f"XML file {xml_file} is expected to be valid but was invalid.", From 5daf36d0d7f8fae0dfd90dd70b2e84b7981b8232 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:26:45 +0200 Subject: [PATCH 14/25] CI Testing --- .github/workflows/unittests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml index 194560b..18da7f5 100644 --- a/.github/workflows/unittests.yml +++ b/.github/workflows/unittests.yml @@ -39,5 +39,7 @@ jobs: rosdep update - name: Run unit tests + env: + PYTHONUNBUFFERED: 1 run: | python -m unittest discover From be019267258b0e21213ded02d2d2c7c707f8420d Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:31:15 +0200 Subject: [PATCH 15/25] CI Testing II --- .github/workflows/unittests.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml index 18da7f5..8d4d0ac 100644 --- a/.github/workflows/unittests.yml +++ b/.github/workflows/unittests.yml @@ -39,7 +39,5 @@ jobs: rosdep update - name: Run unit tests - env: - PYTHONUNBUFFERED: 1 run: | - python -m unittest discover + python -u -m unittest discover -v From d5c476457718d4d233c8096e8f54b5c36e302d5e Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:33:57 +0200 Subject: [PATCH 16/25] CI Testing III --- .github/workflows/unittests.yml | 2 ++ tests/__init__.py | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml index 8d4d0ac..b025faa 100644 --- a/.github/workflows/unittests.yml +++ b/.github/workflows/unittests.yml @@ -39,5 +39,7 @@ jobs: rosdep update - name: Run unit tests + env: + PYTHONUNBUFFERED: 1 run: | python -u -m unittest discover -v diff --git a/tests/__init__.py b/tests/__init__.py index e69de29..1d1547b 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -0,0 +1,9 @@ +import logging +import sys + +logging.basicConfig( + level=logging.DEBUG, # show all levels + format="%(message)s", + stream=sys.stdout, # important for CI + force=True, # override any previous logging config +) From 20c5829f98e33704c496ccc74763580420011a1b Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:38:55 +0200 Subject: [PATCH 17/25] CI Testing IV --- .github/workflows/unittests.yml | 2 +- package_xml_validation/helpers/logger.py | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml index b025faa..245254c 100644 --- a/.github/workflows/unittests.yml +++ b/.github/workflows/unittests.yml @@ -19,7 +19,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v5 with: - python-version: '3.x' + python-version: '3.12' - name: Install system dependencies run: | diff --git a/package_xml_validation/helpers/logger.py b/package_xml_validation/helpers/logger.py index 1dabeaf..f867247 100644 --- a/package_xml_validation/helpers/logger.py +++ b/package_xml_validation/helpers/logger.py @@ -1,4 +1,5 @@ import logging +import sys class ColoredFormatter(logging.Formatter): @@ -30,28 +31,24 @@ def format(self, record): def get_logger(name: str = __name__, level: str = "normal") -> logging.Logger: - """ - Returns a configured logger. If level=='verbose', DEBUG logs will be shown. - Otherwise, we default to INFO (as 'normal'). - """ logger = logging.getLogger(f"{name}_{level}") - logger.setLevel(logging.DEBUG) # We'll filter later using the handler level + logger.setLevel(logging.DEBUG) + + ch = logging.StreamHandler(sys.stdout) # stdout is better for CI + ch.flush = sys.stdout.flush # force flush after each log - ch = logging.StreamHandler() - # Map "normal" to INFO and "verbose" to DEBUG if level == "verbose": ch.setLevel(logging.DEBUG) else: ch.setLevel(logging.INFO) - # Use our custom colored formatter formatter = ColoredFormatter("%(message)s") ch.setFormatter(formatter) - # Prevent adding multiple handlers if the logger already exists if not logger.handlers: logger.addHandler(ch) + logger.propagate = False # don't let root logger duplicate lines return logger From dfc7639d7c6b7a2115bb1d5ba4758606823f672b Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:44:56 +0200 Subject: [PATCH 18/25] CI Test V --- package_xml_validation/helpers/logger.py | 16 ++++++++++++++++ package_xml_validation/package_xml_validator.py | 5 +++++ 2 files changed, 21 insertions(+) diff --git a/package_xml_validation/helpers/logger.py b/package_xml_validation/helpers/logger.py index f867247..635e0ed 100644 --- a/package_xml_validation/helpers/logger.py +++ b/package_xml_validation/helpers/logger.py @@ -52,6 +52,22 @@ def get_logger(name: str = __name__, level: str = "normal") -> logging.Logger: return logger +def setup_ci_logging(): + """Setup logging specifically optimized for CI environments.""" + logging.basicConfig( + level=logging.DEBUG, + format="%(levelname)s: %(message)s", + stream=sys.stdout, + force=True, # Override any existing configuration + ) + + # Ensure immediate flushing + for handler in logging.getLogger().handlers: + handler.stream = sys.stdout + if hasattr(handler.stream, "reconfigure"): + handler.stream.reconfigure(line_buffering=True) + + if __name__ == "__main__": # Example usage: # For "normal" logs (INFO and above) diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index c531fa9..aa0669b 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -567,6 +567,11 @@ def check_and_format_files(self, package_xml_files): ) self.all_valid &= self.xml_valid + # how can i force logger to flush all messages? + for handler in self.logger.handlers: + if hasattr(handler, "flush"): + handler.flush() + print("Flushed logger handler:", handler) # Final result messages if not self.all_valid and self.check_only: self.logger.warning( From be8061d53d55b5b1625fcba0146758c1ba63aa7d Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 17:48:57 +0200 Subject: [PATCH 19/25] CI Test VI --- package_xml_validation/package_xml_validator.py | 5 ----- tests/test_pkg_missing_launch_deps.py | 9 +++++---- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index aa0669b..c531fa9 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -567,11 +567,6 @@ def check_and_format_files(self, package_xml_files): ) self.all_valid &= self.xml_valid - # how can i force logger to flush all messages? - for handler in self.logger.handlers: - if hasattr(handler, "flush"): - handler.flush() - print("Flushed logger handler:", handler) # Final result messages if not self.all_valid and self.check_only: self.logger.warning( diff --git a/tests/test_pkg_missing_launch_deps.py b/tests/test_pkg_missing_launch_deps.py index cd19198..2add510 100644 --- a/tests/test_pkg_missing_launch_deps.py +++ b/tests/test_pkg_missing_launch_deps.py @@ -108,21 +108,22 @@ def test_xml_formatting(self): xml_file = os.path.join(build_type_dir, pkg, "package.xml") # apply the formatter valid = self.formatter.check_and_format_files([xml_file]) + msg = "" if not pkg == "pkg_correct": if valid: with open(xml_file) as f: - print(f"Formatted XML file {xml_file}:\n{f.read()}") + msg = f"Formatted XML file {xml_file}:\n'{f.read()}'" self.assertFalse( valid, - f"XML file {xml_file} is expected to be invalid but was valid.", + f"XML file {xml_file} is expected to be invalid but was valid. {msg}", ) else: if not valid: with open(xml_file) as f: - print(f"Invalid XML file {xml_file}:\n{f.read()}") + msg = f"Invalid XML file {xml_file}:\n{f.read()}" self.assertTrue( valid, - f"XML file {xml_file} is expected to be valid but was invalid.", + f"XML file {xml_file} is expected to be valid but was invalid. {msg}", ) self.assertTrue( self._compare_xml_files(xml_file, correct_xml), From 95d6a7448d795209582fa9c5d65fb961d728dce6 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 18:00:21 +0200 Subject: [PATCH 20/25] Test --- tests/test_pkg_missing_launch_deps.py | 56 +++++++++++++++------------ 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/tests/test_pkg_missing_launch_deps.py b/tests/test_pkg_missing_launch_deps.py index 2add510..983bb01 100644 --- a/tests/test_pkg_missing_launch_deps.py +++ b/tests/test_pkg_missing_launch_deps.py @@ -106,34 +106,40 @@ def test_xml_formatting(self): build_type_dir = os.path.join(self.test_dir, example_pkg) for pkg in os.listdir(build_type_dir): xml_file = os.path.join(build_type_dir, pkg, "package.xml") - # apply the formatter - valid = self.formatter.check_and_format_files([xml_file]) - msg = "" - if not pkg == "pkg_correct": - if valid: - with open(xml_file) as f: - msg = f"Formatted XML file {xml_file}:\n'{f.read()}'" - self.assertFalse( - valid, - f"XML file {xml_file} is expected to be invalid but was valid. {msg}", + + # Use subTest to continue testing other files even if this one fails + with self.subTest(example_pkg=example_pkg, pkg=pkg, xml_file=xml_file): + # apply the formatter + valid = self.formatter.check_and_format_files([xml_file]) + msg = "" + + if not pkg == "pkg_correct": + if valid: + with open(xml_file) as f: + msg = f"Formatted XML file {xml_file}:\n'{f.read()}'" + self.assertFalse( + valid, + f"XML file {xml_file} is expected to be invalid but was valid. {msg}", + ) + else: + if not valid: + with open(xml_file) as f: + msg = f"Invalid XML file {xml_file}:\n{f.read()}" + self.assertTrue( + valid, + f"XML file {xml_file} is expected to be valid but was invalid. {msg}", + ) + + self.assertTrue( + self._compare_xml_files(xml_file, correct_xml), + f"XML files do not match: {xml_file} != {correct_xml}", ) - else: - if not valid: - with open(xml_file) as f: - msg = f"Invalid XML file {xml_file}:\n{f.read()}" + + # validate the XML file with xmllint self.assertTrue( - valid, - f"XML file {xml_file} is expected to be valid but was invalid. {msg}", + validate_xml_with_xmllint(xml_file), + f"XML file {xml_file} failed xmllint validation.", ) - self.assertTrue( - self._compare_xml_files(xml_file, correct_xml), - f"XML files do not match: {xml_file} != {correct_xml}", - ) - # validate the XML file with xmllint - self.assertTrue( - validate_xml_with_xmllint(xml_file), - f"XML file {xml_file} failed xmllint validation.", - ) if __name__ == "__main__": From 00dc8d1bdb0a3671a64fd8de3e5246c181d2be5b Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 18:10:01 +0200 Subject: [PATCH 21/25] Test --- package_xml_validation/helpers/logger.py | 16 ---------------- tests/test_pkg_missing_launch_deps.py | 6 ++++-- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/package_xml_validation/helpers/logger.py b/package_xml_validation/helpers/logger.py index 635e0ed..f867247 100644 --- a/package_xml_validation/helpers/logger.py +++ b/package_xml_validation/helpers/logger.py @@ -52,22 +52,6 @@ def get_logger(name: str = __name__, level: str = "normal") -> logging.Logger: return logger -def setup_ci_logging(): - """Setup logging specifically optimized for CI environments.""" - logging.basicConfig( - level=logging.DEBUG, - format="%(levelname)s: %(message)s", - stream=sys.stdout, - force=True, # Override any existing configuration - ) - - # Ensure immediate flushing - for handler in logging.getLogger().handlers: - handler.stream = sys.stdout - if hasattr(handler.stream, "reconfigure"): - handler.stream.reconfigure(line_buffering=True) - - if __name__ == "__main__": # Example usage: # For "normal" logs (INFO and above) diff --git a/tests/test_pkg_missing_launch_deps.py b/tests/test_pkg_missing_launch_deps.py index 983bb01..9318bc1 100644 --- a/tests/test_pkg_missing_launch_deps.py +++ b/tests/test_pkg_missing_launch_deps.py @@ -110,6 +110,8 @@ def test_xml_formatting(self): # Use subTest to continue testing other files even if this one fails with self.subTest(example_pkg=example_pkg, pkg=pkg, xml_file=xml_file): # apply the formatter + with open(xml_file) as f: + xml_content = f.read() valid = self.formatter.check_and_format_files([xml_file]) msg = "" @@ -119,7 +121,7 @@ def test_xml_formatting(self): msg = f"Formatted XML file {xml_file}:\n'{f.read()}'" self.assertFalse( valid, - f"XML file {xml_file} is expected to be invalid but was valid. {msg}", + f"XML file {xml_file} is expected to be invalid but was valid. {msg} \n vs original: \n{xml_content}", ) else: if not valid: @@ -127,7 +129,7 @@ def test_xml_formatting(self): msg = f"Invalid XML file {xml_file}:\n{f.read()}" self.assertTrue( valid, - f"XML file {xml_file} is expected to be valid but was invalid. {msg}", + f"XML file {xml_file} is expected to be valid but was invalid. {msg} \n vs original: \n{xml_content}", ) self.assertTrue( From c0559874bd93a6284bc24b9b49c1d2c95ba72cf4 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 18:15:05 +0200 Subject: [PATCH 22/25] Test --- tests/test_find_launch_dependencies.py | 55 --- ...test_package_xml_linting_and_formatting.py | 338 ------------------ tests/test_pky_type_dependent_validation.py | 132 ------- 3 files changed, 525 deletions(-) delete mode 100644 tests/test_find_launch_dependencies.py delete mode 100644 tests/test_package_xml_linting_and_formatting.py delete mode 100644 tests/test_pky_type_dependent_validation.py diff --git a/tests/test_find_launch_dependencies.py b/tests/test_find_launch_dependencies.py deleted file mode 100644 index de14364..0000000 --- a/tests/test_find_launch_dependencies.py +++ /dev/null @@ -1,55 +0,0 @@ -import os -import unittest - -from package_xml_validation.helpers.find_launch_dependencies import scan_file - - -class TestFindLaunchDependencies(unittest.TestCase): - # ─── Define test cases: filename → expected packages ─── - EXAMPLES = { - "python_example.launch.py": ["demo_nodes_cpp", "turtlesim"], - "xml_example.launch.xml": ["demo_nodes_cpp", "turtlesim"], - "yaml_example.launch.yml": ["demo_nodes_cpp", "turtlesim"], - "hector_launch_component.yaml": ["athena_announcer"], - "python_world.launch.py": [ - "gazebo_robot_sim_athena", - "ros_gz_sim", - "ros_gz_bridge", - ], - "python_gazebo_launch.py": [ - "simulation_scenario_robocup_gazebo", - "ros_gz_sim", - "ros_gz_bridge", - ], - "python_example_bad.launch.py": [], # ["demo_nodes_cpp", "turtlesim"], - "yaml_example_bad.launch.yml": [], # ["demo_nodes_cpp", "turtlesim"], - } - - # Directory where example launch files live - BASE_DIR = os.path.join(os.path.dirname(__file__), "examples", "launch_examples") - - def test_scan_each_file(self): - """Ensure scan_file finds exactly the expected packages in each example.""" - for filename, expected_pkgs in self.EXAMPLES.items(): - with self.subTest(filename=filename): - path = os.path.join(self.BASE_DIR, filename) - # 1) file must exist - self.assertTrue(os.path.isfile(path), f"Example file not found: {path}") - # 2) scan it - found = set() - print(f"Scanning {path} for launch dependencies...") - scan_file(path, found) - # 3) compare sets - self.assertEqual( - set(expected_pkgs), - found, - msg=( - f"For '{filename}':\n" - f" expected: {sorted(expected_pkgs)}\n" - f" found: {sorted(found)}" - ), - ) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/test_package_xml_linting_and_formatting.py b/tests/test_package_xml_linting_and_formatting.py deleted file mode 100644 index 2eed0dc..0000000 --- a/tests/test_package_xml_linting_and_formatting.py +++ /dev/null @@ -1,338 +0,0 @@ -import os -import unittest -import tempfile -import shutil -import subprocess -import lxml.etree as ET - -from package_xml_validation.package_xml_validator import ( - PackageXmlValidator, - RosdepValidator, - read_deps_from_cmake_file, - PackageXmlFormatter, -) - - -def validate_xml_with_xmllint(xml_file): - """Validate XML file against the ROS package_format3.xsd schema using xmllint.""" - schema_url = "http://download.ros.org/schema/package_format3.xsd" - try: - result = subprocess.run( - ["xmllint", "--noout", "--schema", schema_url, xml_file], - capture_output=True, - text=True, - ) - if result.returncode != 0: - print(f"XML validation error in {xml_file}:\n{result.stderr}") - return False - return True - except FileNotFoundError: - print( - "Error: xmllint not found. Please ensure it's installed and in your PATH." - ) - return False - - -class TestPackageXmlValidator(unittest.TestCase): - @classmethod - def setUpClass(cls): - """ - We assume the example files are in 'tests/examples'. - Adjust the directory path to match your actual setup. - """ - current_dir = os.path.dirname(__file__) - cls.examples_dir = os.path.join(current_dir, "examples", "package_xml_examples") - - def setUp(self): - """ - Create a temporary directory for each test, copy example files into it, - so we can safely modify them during tests. - """ - self.test_dir = tempfile.mkdtemp(prefix="xml_tests_") - - for fname in os.listdir(self.examples_dir): - if fname.startswith("original_") and fname.endswith(".xml"): - src = os.path.join(self.examples_dir, fname) - dst = os.path.join(self.test_dir, fname) - shutil.copy2(src, dst) - - # Also copy the 'corrected_XX.xml' if it exists - if fname.startswith("corrected_") and fname.endswith(".xml"): - src = os.path.join(self.examples_dir, fname) - dst = os.path.join(self.test_dir, fname) - shutil.copy2(src, dst) - - def tearDown(self): - """Clean up the temporary directory after each test.""" - shutil.rmtree(self.test_dir) - - def _is_fail_file(self, filename: str) -> bool: - """ - Returns True if the file name indicates it is an 'original_XX_fail.xml'. - """ - return filename.endswith("_fail.xml") and filename.startswith("original") - - def _is_correct_file(self, filename: str) -> bool: - """ - Returns True if the file name indicates it is an 'original_XX_correct.xml'. - """ - return filename.endswith("_correct.xml") or filename.startswith("corrected_") - - def _compare_xml_files(self, file1: str, file2: str) -> bool: - """ - Compare two XML files for equality. - Using xml parser to ignore whitespace and comments. - file1: is modified by the formatter - file2: is the expected corrected file - """ - try: - tree1 = ET.parse(file1) - tree2 = ET.parse(file2) - - # Normalize the XML trees - root1 = tree1.getroot() - root2 = tree2.getroot() - - for elem1, elem2 in zip(root1.iter(), root2.iter()): - # Ignore comments and whitespace - if ET.iselement(elem1) and ET.iselement(elem2): - if elem1.tag != elem2.tag or elem1.text != elem2.text: - return False - elif ET.iselement(elem1) and not ET.iselement(elem2): - return False - elif not ET.iselement(elem1) and ET.iselement(elem2): - return False - # make sure there are no more than 2 \n in the tai - elif elem2.tail.count("\n") > 2: - return False - return True - except ET.XMLSyntaxError as e: - print(f"XML Syntax Error: {e}") - return False - - def test_xml_formatting(self): - """ - Iterates over all 'original_XX_*.xml' in the temp test directory, - checks them with and without --check, - and compares results to expectations and corrected files. - """ - for fname in os.listdir(self.test_dir): - if ( - not fname.startswith("original_") - and not fname.startswith("corrected_") - and not fname.endswith(".xml") - ): - # Skip any non-original test files - continue - - original_path = os.path.join(self.test_dir, fname) - print("\nTesting file:", original_path) - - # --- 1) Test with --check ------------------------------------- - # We expect: - # - 'original_XX_correct.xml' => returns True (all_valid=True), file unchanged - # - 'original_XX_fail.xml' => returns False (all_valid=False), file unchanged - - formatter = PackageXmlValidator( - check_only=True, verbose=True, check_rosdeps=False - ) - with open(original_path, "rb") as f_before: - original_bytes_before_check = f_before.read() - - all_valid_check = formatter.check_and_format_files([original_path]) - - if self._is_correct_file(fname): - if not all_valid_check: - with open(original_path) as f_after: - print(f"File content after check:\n{f_after.read()}") - self.assertTrue( - all_valid_check, - f"Expected correct file {fname} to pass in check-only mode.", - ) - elif self._is_fail_file(fname): - self.assertFalse( - all_valid_check, - f"Expected fail file {fname} to fail in check-only mode.", - ) - - # Confirm no changes were made when using --check - with open(original_path, "rb") as f_after: - original_bytes_after_check = f_after.read() - self.assertEqual( - original_bytes_before_check, - original_bytes_after_check, - f"File {fname} should not have been modified in check-only mode.", - ) - - # --- 2) Test without --check (check_only=False) --------------- - # - For correct files => no changes - # - For fail files => corrected to match `corrected_XX.xml` - # Then we verify the final result is valid with xmllint. - - formatter = PackageXmlValidator( - check_only=False, verbose=True, check_rosdeps=False - ) - - # Reload the file from disk (in case some other step changed it). - with open(original_path, "rb") as f_before: - original_bytes_before_fix = f_before.read() - - all_valid_fix = formatter.check_and_format_files([original_path]) - self.assertTrue( - isinstance(all_valid_fix, bool), - "check_and_format_files should return a boolean.", - ) - - # Now check the final contents. - final_path = original_path # same file, presumably overwritten if needed - self.assertTrue( - os.path.exists(final_path), - "Expected the package.xml file to still exist after formatting.", - ) - - if self._is_correct_file(fname): - # For correct files, it should remain correct => all_valid=True - self.assertTrue( - all_valid_fix, f"Expected correct file {fname} to pass when fixing." - ) - # And the file should remain unchanged - with open(final_path, "rb") as f_final: - final_bytes = f_final.read() - self.assertEqual( - original_bytes_before_fix, - final_bytes, - f"Correct file {fname} should remain unchanged after fix.", - ) - elif self._is_fail_file(fname): - # For fail files, we expect check_and_format_files to fix them => all_valid False since something was fixed - self.assertFalse( - all_valid_fix, - f"Expected fail file {fname} to be fixed and return false to indicate that something was fixed.", - ) - - # Compare to "corrected_XX.xml" if that file exists - # We replace 'original_' with 'corrected_' in the filename - corrected_fname = fname.replace("original_", "corrected_") - corrected_path = os.path.join(self.test_dir, corrected_fname) - if os.path.exists(corrected_path): - # Compare the final file with corrected_XX.xml - comparison = self._compare_xml_files(final_path, corrected_path) - if not comparison: - # print corrected file to console - with open(final_path) as f_corrected: - corrected_content = f_corrected.read() - print(f"Corrected file content:\n{corrected_content}") - self.assertTrue( - comparison, - f"File {fname} after fixing does not match {corrected_fname}", - ) - - else: - self.fail( - f"Missing corrected file {corrected_fname} in test directory." - ) - - # --- 3) Validate the final file with xmllint + package_format3.xsd ---- - # We use the provided helper to ensure the final result is schema-valid. - self.assertTrue( - validate_xml_with_xmllint(final_path), - f"The final file {fname} is not valid against package_format3.xsd.", - ) - - def test_rosdep_checking(self): - """ - Test the rosdep checking functionality. - This is a placeholder for the actual implementation. - """ - # test if ros installed by testing whether the environment variable ROS_DISTRO is set - # if "ROS_DISTRO" not in os.environ: - # self.skipTest("ROS is not installed, skipping rosdep checking test.") - # Example dependencies to check - dependencies = ["rclcpp", "nonexistent_dependency", "sensor_msgs"] - validator = RosdepValidator() - unresolvable = validator.check_rosdeps(dependencies) - - # Check if the expected dependencies are unresolvable - self.assertIn("nonexistent_dependency", unresolvable) - self.assertNotIn("rclcpp", unresolvable) - self.assertNotIn("sensor_msgs", unresolvable) - - def test_integrated_rosdep_checking(self): - correct_rosdep = os.path.join(self.examples_dir, "no_incorrect_rosdeps.xml") - incorrect_rosdep = os.path.join(self.examples_dir, "incorrect_rosdeps.xml") - # Check the correct file - formatter = PackageXmlValidator( - check_only=True, verbose=True, check_rosdeps=True - ) - all_valid_check = formatter.check_and_format_files([correct_rosdep]) - self.assertTrue( - all_valid_check, "Expected correct file to pass in check-only mode." - ) - # Check the incorrect file - all_valid_check = formatter.check_and_format_files([incorrect_rosdep]) - self.assertFalse( - all_valid_check, "Expected fail file to fail in check-only mode." - ) - - def test_invalid_path(self): - """ - Test the behavior when an invalid path is provided. - """ - invalid_path = os.path.join(self.examples_dir, "nonexistent.xml") - formatter = PackageXmlValidator(check_only=True, verbose=True) - with self.assertRaises(FileNotFoundError): - formatter.check_and_format_files([invalid_path]) - - def test_cmake_parsing(self): - """ - Test the CMake parsing functionality. - This is a placeholder for the actual implementation. - """ - # iterate cmakes files in examples_dir/cmakes/ - cmakes_dir = os.path.join(self.examples_dir, "cmakes") - for package_name in os.listdir(cmakes_dir): - package_dir = os.path.join(cmakes_dir, package_name) - if not os.path.isdir(package_dir): - continue - for fname in os.listdir(package_dir): - if not fname.endswith("CMakeLists.txt"): - continue - cmake_file = os.path.join(package_dir, fname) - print(f"Testing CMake file: {cmake_file}") - # Here you would implement the actual CMake parsing and validation logic - main_deps, test_deps = read_deps_from_cmake_file(cmake_file) - - # read deps from package.xml - package_xml_file = os.path.join( - self.examples_dir, "cmakes", package_name, "package.xml" - ) - self.assertTrue( - os.path.exists(package_xml_file), - f"Expected package.xml file to exist at {package_xml_file}", - ) - try: - parser = ET.XMLParser() - tree = ET.parse(package_xml_file, parser) - root = tree.getroot() - except ET.XMLSyntaxError as e: - self.fail(f"XML Syntax Error in {package_xml_file}: {e}") - formatter = PackageXmlFormatter() - xml_build_deps = formatter.retrieve_build_dependencies(root) - xml_test_deps = formatter.retrieve_test_dependencies(root) - # Compare the dependencies - for dep in main_deps: - self.assertIn( - dep, - xml_build_deps, - f"Dependency {dep} not found in package.xml build dependencies in {package_name}.", - ) - for dep in test_deps: - self.assertIn( - dep, - xml_test_deps, - f"Dependency {dep} not found in package.xml test dependencies in {package_name}.", - ) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/test_pky_type_dependent_validation.py b/tests/test_pky_type_dependent_validation.py deleted file mode 100644 index c419689..0000000 --- a/tests/test_pky_type_dependent_validation.py +++ /dev/null @@ -1,132 +0,0 @@ -import os -import unittest -import tempfile -import shutil -import subprocess -import lxml.etree as ET - -from package_xml_validation.package_xml_validator import ( - PackageXmlValidator, -) - - -def validate_xml_with_xmllint(xml_file): - """Validate XML file against the ROS package_format3.xsd schema using xmllint.""" - schema_url = "http://download.ros.org/schema/package_format3.xsd" - try: - result = subprocess.run( - ["xmllint", "--noout", "--schema", schema_url, xml_file], - capture_output=True, - text=True, - ) - if result.returncode != 0: - print(f"XML validation error in {xml_file}:\n{result.stderr}") - return False - return True - except FileNotFoundError: - print( - "Error: xmllint not found. Please ensure it's installed and in your PATH." - ) - return False - - -class TestPackageXmlValidator(unittest.TestCase): - @classmethod - def setUpClass(cls): - """ - We assume the example files are in 'tests/examples'. - Adjust the directory path to match your actual setup. - """ - current_dir = os.path.dirname(__file__) - cls.examples_dir = os.path.join(current_dir, "examples", "export_tag_examples") - cls.formatter = PackageXmlValidator( - check_only=False, - verbose=True, - auto_fill_missing_deps=True, - check_rosdeps=False, - ) - - def setUp(self): - """ - Create a temporary directory for each test, copy example files into it, - so we can safely modify them during tests. - """ - self.test_dir = tempfile.mkdtemp(prefix="xml_tests_") - - # copy contents of examples_dir to test_dir - shutil.copytree(self.examples_dir, self.test_dir, dirs_exist_ok=True) - - def tearDown(self): - """Clean up the temporary directory after each test.""" - shutil.rmtree(self.test_dir) - - def _compare_xml_files(self, file1: str, file2: str) -> bool: - """ - Compare two XML files for equality. - Using xml parser to ignore whitespace and comments. - file1: is modified by the formatter - file2: is the expected corrected file - """ - try: - tree1 = ET.parse(file1) - tree2 = ET.parse(file2) - - # Normalize the XML trees - root1 = tree1.getroot() - root2 = tree2.getroot() - - for elem1, elem2 in zip(root1.iter(), root2.iter()): - # Ignore comments and whitespace - if ET.iselement(elem1) and ET.iselement(elem2): - if elem1.tag != elem2.tag or elem1.text != elem2.text: - return False - elif ET.iselement(elem1) and not ET.iselement(elem2): - return False - elif not ET.iselement(elem1) and ET.iselement(elem2): - return False - # make sure there are no more than 2 \n in the tai - elif elem2.tail.count("\n") > 2: - return False - return True - except ET.XMLSyntaxError as e: - print(f"XML Syntax Error: {e}") - return False - - def test_xml_formatting(self): - """ - Iterate over all example packages in the test directory, - """ - build_types = ["ament_cmake", "msg_pkg"] # "ament_python" - for build_type in build_types: - correct_xml = os.path.join( - self.examples_dir, build_type, "pkg_correct", "package.xml" - ) - build_type_dir = os.path.join(self.test_dir, build_type) - for pkg in os.listdir(build_type_dir): - xml_file = os.path.join(build_type_dir, pkg, "package.xml") - # apply the formatter - valid = self.formatter.check_and_format_files([xml_file]) - if not pkg == "pkg_correct": - self.assertFalse( - valid, - f"XML file {xml_file} is expected to be invalid but was valid.", - ) - else: - self.assertTrue( - valid, - f"XML file {xml_file} is expected to be valid but was invalid.", - ) - self.assertTrue( - self._compare_xml_files(xml_file, correct_xml), - f"XML files do not match: {xml_file} != {correct_xml}", - ) - # validate the XML file with xmllint - self.assertTrue( - validate_xml_with_xmllint(xml_file), - f"XML file {xml_file} failed xmllint validation.", - ) - - -if __name__ == "__main__": - # Run the tests - unittest.main() From 207c4a2d7fc03cd44ef097db2855d47475b12ae2 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 18:17:53 +0200 Subject: [PATCH 23/25] Test --- .github/workflows/unittests.yml | 2 -- tests/test_pkg_missing_launch_deps.py | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml index 245254c..265e4d5 100644 --- a/.github/workflows/unittests.yml +++ b/.github/workflows/unittests.yml @@ -39,7 +39,5 @@ jobs: rosdep update - name: Run unit tests - env: - PYTHONUNBUFFERED: 1 run: | python -u -m unittest discover -v diff --git a/tests/test_pkg_missing_launch_deps.py b/tests/test_pkg_missing_launch_deps.py index 9318bc1..ae935cf 100644 --- a/tests/test_pkg_missing_launch_deps.py +++ b/tests/test_pkg_missing_launch_deps.py @@ -42,6 +42,7 @@ def setUpClass(cls): check_only=False, verbose=True, auto_fill_missing_deps=True, + check_rosdeps=False, ) def setUp(self): From a19f96dc2266624ea56673c8b9a8baae51af211a Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 18:20:03 +0200 Subject: [PATCH 24/25] re-added tests --- tests/test_find_launch_dependencies.py | 55 +++ ...test_package_xml_linting_and_formatting.py | 338 ++++++++++++++++++ tests/test_pky_type_dependent_validation.py | 132 +++++++ 3 files changed, 525 insertions(+) create mode 100644 tests/test_find_launch_dependencies.py create mode 100644 tests/test_package_xml_linting_and_formatting.py create mode 100644 tests/test_pky_type_dependent_validation.py diff --git a/tests/test_find_launch_dependencies.py b/tests/test_find_launch_dependencies.py new file mode 100644 index 0000000..de14364 --- /dev/null +++ b/tests/test_find_launch_dependencies.py @@ -0,0 +1,55 @@ +import os +import unittest + +from package_xml_validation.helpers.find_launch_dependencies import scan_file + + +class TestFindLaunchDependencies(unittest.TestCase): + # ─── Define test cases: filename → expected packages ─── + EXAMPLES = { + "python_example.launch.py": ["demo_nodes_cpp", "turtlesim"], + "xml_example.launch.xml": ["demo_nodes_cpp", "turtlesim"], + "yaml_example.launch.yml": ["demo_nodes_cpp", "turtlesim"], + "hector_launch_component.yaml": ["athena_announcer"], + "python_world.launch.py": [ + "gazebo_robot_sim_athena", + "ros_gz_sim", + "ros_gz_bridge", + ], + "python_gazebo_launch.py": [ + "simulation_scenario_robocup_gazebo", + "ros_gz_sim", + "ros_gz_bridge", + ], + "python_example_bad.launch.py": [], # ["demo_nodes_cpp", "turtlesim"], + "yaml_example_bad.launch.yml": [], # ["demo_nodes_cpp", "turtlesim"], + } + + # Directory where example launch files live + BASE_DIR = os.path.join(os.path.dirname(__file__), "examples", "launch_examples") + + def test_scan_each_file(self): + """Ensure scan_file finds exactly the expected packages in each example.""" + for filename, expected_pkgs in self.EXAMPLES.items(): + with self.subTest(filename=filename): + path = os.path.join(self.BASE_DIR, filename) + # 1) file must exist + self.assertTrue(os.path.isfile(path), f"Example file not found: {path}") + # 2) scan it + found = set() + print(f"Scanning {path} for launch dependencies...") + scan_file(path, found) + # 3) compare sets + self.assertEqual( + set(expected_pkgs), + found, + msg=( + f"For '{filename}':\n" + f" expected: {sorted(expected_pkgs)}\n" + f" found: {sorted(found)}" + ), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_package_xml_linting_and_formatting.py b/tests/test_package_xml_linting_and_formatting.py new file mode 100644 index 0000000..2eed0dc --- /dev/null +++ b/tests/test_package_xml_linting_and_formatting.py @@ -0,0 +1,338 @@ +import os +import unittest +import tempfile +import shutil +import subprocess +import lxml.etree as ET + +from package_xml_validation.package_xml_validator import ( + PackageXmlValidator, + RosdepValidator, + read_deps_from_cmake_file, + PackageXmlFormatter, +) + + +def validate_xml_with_xmllint(xml_file): + """Validate XML file against the ROS package_format3.xsd schema using xmllint.""" + schema_url = "http://download.ros.org/schema/package_format3.xsd" + try: + result = subprocess.run( + ["xmllint", "--noout", "--schema", schema_url, xml_file], + capture_output=True, + text=True, + ) + if result.returncode != 0: + print(f"XML validation error in {xml_file}:\n{result.stderr}") + return False + return True + except FileNotFoundError: + print( + "Error: xmllint not found. Please ensure it's installed and in your PATH." + ) + return False + + +class TestPackageXmlValidator(unittest.TestCase): + @classmethod + def setUpClass(cls): + """ + We assume the example files are in 'tests/examples'. + Adjust the directory path to match your actual setup. + """ + current_dir = os.path.dirname(__file__) + cls.examples_dir = os.path.join(current_dir, "examples", "package_xml_examples") + + def setUp(self): + """ + Create a temporary directory for each test, copy example files into it, + so we can safely modify them during tests. + """ + self.test_dir = tempfile.mkdtemp(prefix="xml_tests_") + + for fname in os.listdir(self.examples_dir): + if fname.startswith("original_") and fname.endswith(".xml"): + src = os.path.join(self.examples_dir, fname) + dst = os.path.join(self.test_dir, fname) + shutil.copy2(src, dst) + + # Also copy the 'corrected_XX.xml' if it exists + if fname.startswith("corrected_") and fname.endswith(".xml"): + src = os.path.join(self.examples_dir, fname) + dst = os.path.join(self.test_dir, fname) + shutil.copy2(src, dst) + + def tearDown(self): + """Clean up the temporary directory after each test.""" + shutil.rmtree(self.test_dir) + + def _is_fail_file(self, filename: str) -> bool: + """ + Returns True if the file name indicates it is an 'original_XX_fail.xml'. + """ + return filename.endswith("_fail.xml") and filename.startswith("original") + + def _is_correct_file(self, filename: str) -> bool: + """ + Returns True if the file name indicates it is an 'original_XX_correct.xml'. + """ + return filename.endswith("_correct.xml") or filename.startswith("corrected_") + + def _compare_xml_files(self, file1: str, file2: str) -> bool: + """ + Compare two XML files for equality. + Using xml parser to ignore whitespace and comments. + file1: is modified by the formatter + file2: is the expected corrected file + """ + try: + tree1 = ET.parse(file1) + tree2 = ET.parse(file2) + + # Normalize the XML trees + root1 = tree1.getroot() + root2 = tree2.getroot() + + for elem1, elem2 in zip(root1.iter(), root2.iter()): + # Ignore comments and whitespace + if ET.iselement(elem1) and ET.iselement(elem2): + if elem1.tag != elem2.tag or elem1.text != elem2.text: + return False + elif ET.iselement(elem1) and not ET.iselement(elem2): + return False + elif not ET.iselement(elem1) and ET.iselement(elem2): + return False + # make sure there are no more than 2 \n in the tai + elif elem2.tail.count("\n") > 2: + return False + return True + except ET.XMLSyntaxError as e: + print(f"XML Syntax Error: {e}") + return False + + def test_xml_formatting(self): + """ + Iterates over all 'original_XX_*.xml' in the temp test directory, + checks them with and without --check, + and compares results to expectations and corrected files. + """ + for fname in os.listdir(self.test_dir): + if ( + not fname.startswith("original_") + and not fname.startswith("corrected_") + and not fname.endswith(".xml") + ): + # Skip any non-original test files + continue + + original_path = os.path.join(self.test_dir, fname) + print("\nTesting file:", original_path) + + # --- 1) Test with --check ------------------------------------- + # We expect: + # - 'original_XX_correct.xml' => returns True (all_valid=True), file unchanged + # - 'original_XX_fail.xml' => returns False (all_valid=False), file unchanged + + formatter = PackageXmlValidator( + check_only=True, verbose=True, check_rosdeps=False + ) + with open(original_path, "rb") as f_before: + original_bytes_before_check = f_before.read() + + all_valid_check = formatter.check_and_format_files([original_path]) + + if self._is_correct_file(fname): + if not all_valid_check: + with open(original_path) as f_after: + print(f"File content after check:\n{f_after.read()}") + self.assertTrue( + all_valid_check, + f"Expected correct file {fname} to pass in check-only mode.", + ) + elif self._is_fail_file(fname): + self.assertFalse( + all_valid_check, + f"Expected fail file {fname} to fail in check-only mode.", + ) + + # Confirm no changes were made when using --check + with open(original_path, "rb") as f_after: + original_bytes_after_check = f_after.read() + self.assertEqual( + original_bytes_before_check, + original_bytes_after_check, + f"File {fname} should not have been modified in check-only mode.", + ) + + # --- 2) Test without --check (check_only=False) --------------- + # - For correct files => no changes + # - For fail files => corrected to match `corrected_XX.xml` + # Then we verify the final result is valid with xmllint. + + formatter = PackageXmlValidator( + check_only=False, verbose=True, check_rosdeps=False + ) + + # Reload the file from disk (in case some other step changed it). + with open(original_path, "rb") as f_before: + original_bytes_before_fix = f_before.read() + + all_valid_fix = formatter.check_and_format_files([original_path]) + self.assertTrue( + isinstance(all_valid_fix, bool), + "check_and_format_files should return a boolean.", + ) + + # Now check the final contents. + final_path = original_path # same file, presumably overwritten if needed + self.assertTrue( + os.path.exists(final_path), + "Expected the package.xml file to still exist after formatting.", + ) + + if self._is_correct_file(fname): + # For correct files, it should remain correct => all_valid=True + self.assertTrue( + all_valid_fix, f"Expected correct file {fname} to pass when fixing." + ) + # And the file should remain unchanged + with open(final_path, "rb") as f_final: + final_bytes = f_final.read() + self.assertEqual( + original_bytes_before_fix, + final_bytes, + f"Correct file {fname} should remain unchanged after fix.", + ) + elif self._is_fail_file(fname): + # For fail files, we expect check_and_format_files to fix them => all_valid False since something was fixed + self.assertFalse( + all_valid_fix, + f"Expected fail file {fname} to be fixed and return false to indicate that something was fixed.", + ) + + # Compare to "corrected_XX.xml" if that file exists + # We replace 'original_' with 'corrected_' in the filename + corrected_fname = fname.replace("original_", "corrected_") + corrected_path = os.path.join(self.test_dir, corrected_fname) + if os.path.exists(corrected_path): + # Compare the final file with corrected_XX.xml + comparison = self._compare_xml_files(final_path, corrected_path) + if not comparison: + # print corrected file to console + with open(final_path) as f_corrected: + corrected_content = f_corrected.read() + print(f"Corrected file content:\n{corrected_content}") + self.assertTrue( + comparison, + f"File {fname} after fixing does not match {corrected_fname}", + ) + + else: + self.fail( + f"Missing corrected file {corrected_fname} in test directory." + ) + + # --- 3) Validate the final file with xmllint + package_format3.xsd ---- + # We use the provided helper to ensure the final result is schema-valid. + self.assertTrue( + validate_xml_with_xmllint(final_path), + f"The final file {fname} is not valid against package_format3.xsd.", + ) + + def test_rosdep_checking(self): + """ + Test the rosdep checking functionality. + This is a placeholder for the actual implementation. + """ + # test if ros installed by testing whether the environment variable ROS_DISTRO is set + # if "ROS_DISTRO" not in os.environ: + # self.skipTest("ROS is not installed, skipping rosdep checking test.") + # Example dependencies to check + dependencies = ["rclcpp", "nonexistent_dependency", "sensor_msgs"] + validator = RosdepValidator() + unresolvable = validator.check_rosdeps(dependencies) + + # Check if the expected dependencies are unresolvable + self.assertIn("nonexistent_dependency", unresolvable) + self.assertNotIn("rclcpp", unresolvable) + self.assertNotIn("sensor_msgs", unresolvable) + + def test_integrated_rosdep_checking(self): + correct_rosdep = os.path.join(self.examples_dir, "no_incorrect_rosdeps.xml") + incorrect_rosdep = os.path.join(self.examples_dir, "incorrect_rosdeps.xml") + # Check the correct file + formatter = PackageXmlValidator( + check_only=True, verbose=True, check_rosdeps=True + ) + all_valid_check = formatter.check_and_format_files([correct_rosdep]) + self.assertTrue( + all_valid_check, "Expected correct file to pass in check-only mode." + ) + # Check the incorrect file + all_valid_check = formatter.check_and_format_files([incorrect_rosdep]) + self.assertFalse( + all_valid_check, "Expected fail file to fail in check-only mode." + ) + + def test_invalid_path(self): + """ + Test the behavior when an invalid path is provided. + """ + invalid_path = os.path.join(self.examples_dir, "nonexistent.xml") + formatter = PackageXmlValidator(check_only=True, verbose=True) + with self.assertRaises(FileNotFoundError): + formatter.check_and_format_files([invalid_path]) + + def test_cmake_parsing(self): + """ + Test the CMake parsing functionality. + This is a placeholder for the actual implementation. + """ + # iterate cmakes files in examples_dir/cmakes/ + cmakes_dir = os.path.join(self.examples_dir, "cmakes") + for package_name in os.listdir(cmakes_dir): + package_dir = os.path.join(cmakes_dir, package_name) + if not os.path.isdir(package_dir): + continue + for fname in os.listdir(package_dir): + if not fname.endswith("CMakeLists.txt"): + continue + cmake_file = os.path.join(package_dir, fname) + print(f"Testing CMake file: {cmake_file}") + # Here you would implement the actual CMake parsing and validation logic + main_deps, test_deps = read_deps_from_cmake_file(cmake_file) + + # read deps from package.xml + package_xml_file = os.path.join( + self.examples_dir, "cmakes", package_name, "package.xml" + ) + self.assertTrue( + os.path.exists(package_xml_file), + f"Expected package.xml file to exist at {package_xml_file}", + ) + try: + parser = ET.XMLParser() + tree = ET.parse(package_xml_file, parser) + root = tree.getroot() + except ET.XMLSyntaxError as e: + self.fail(f"XML Syntax Error in {package_xml_file}: {e}") + formatter = PackageXmlFormatter() + xml_build_deps = formatter.retrieve_build_dependencies(root) + xml_test_deps = formatter.retrieve_test_dependencies(root) + # Compare the dependencies + for dep in main_deps: + self.assertIn( + dep, + xml_build_deps, + f"Dependency {dep} not found in package.xml build dependencies in {package_name}.", + ) + for dep in test_deps: + self.assertIn( + dep, + xml_test_deps, + f"Dependency {dep} not found in package.xml test dependencies in {package_name}.", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_pky_type_dependent_validation.py b/tests/test_pky_type_dependent_validation.py new file mode 100644 index 0000000..c419689 --- /dev/null +++ b/tests/test_pky_type_dependent_validation.py @@ -0,0 +1,132 @@ +import os +import unittest +import tempfile +import shutil +import subprocess +import lxml.etree as ET + +from package_xml_validation.package_xml_validator import ( + PackageXmlValidator, +) + + +def validate_xml_with_xmllint(xml_file): + """Validate XML file against the ROS package_format3.xsd schema using xmllint.""" + schema_url = "http://download.ros.org/schema/package_format3.xsd" + try: + result = subprocess.run( + ["xmllint", "--noout", "--schema", schema_url, xml_file], + capture_output=True, + text=True, + ) + if result.returncode != 0: + print(f"XML validation error in {xml_file}:\n{result.stderr}") + return False + return True + except FileNotFoundError: + print( + "Error: xmllint not found. Please ensure it's installed and in your PATH." + ) + return False + + +class TestPackageXmlValidator(unittest.TestCase): + @classmethod + def setUpClass(cls): + """ + We assume the example files are in 'tests/examples'. + Adjust the directory path to match your actual setup. + """ + current_dir = os.path.dirname(__file__) + cls.examples_dir = os.path.join(current_dir, "examples", "export_tag_examples") + cls.formatter = PackageXmlValidator( + check_only=False, + verbose=True, + auto_fill_missing_deps=True, + check_rosdeps=False, + ) + + def setUp(self): + """ + Create a temporary directory for each test, copy example files into it, + so we can safely modify them during tests. + """ + self.test_dir = tempfile.mkdtemp(prefix="xml_tests_") + + # copy contents of examples_dir to test_dir + shutil.copytree(self.examples_dir, self.test_dir, dirs_exist_ok=True) + + def tearDown(self): + """Clean up the temporary directory after each test.""" + shutil.rmtree(self.test_dir) + + def _compare_xml_files(self, file1: str, file2: str) -> bool: + """ + Compare two XML files for equality. + Using xml parser to ignore whitespace and comments. + file1: is modified by the formatter + file2: is the expected corrected file + """ + try: + tree1 = ET.parse(file1) + tree2 = ET.parse(file2) + + # Normalize the XML trees + root1 = tree1.getroot() + root2 = tree2.getroot() + + for elem1, elem2 in zip(root1.iter(), root2.iter()): + # Ignore comments and whitespace + if ET.iselement(elem1) and ET.iselement(elem2): + if elem1.tag != elem2.tag or elem1.text != elem2.text: + return False + elif ET.iselement(elem1) and not ET.iselement(elem2): + return False + elif not ET.iselement(elem1) and ET.iselement(elem2): + return False + # make sure there are no more than 2 \n in the tai + elif elem2.tail.count("\n") > 2: + return False + return True + except ET.XMLSyntaxError as e: + print(f"XML Syntax Error: {e}") + return False + + def test_xml_formatting(self): + """ + Iterate over all example packages in the test directory, + """ + build_types = ["ament_cmake", "msg_pkg"] # "ament_python" + for build_type in build_types: + correct_xml = os.path.join( + self.examples_dir, build_type, "pkg_correct", "package.xml" + ) + build_type_dir = os.path.join(self.test_dir, build_type) + for pkg in os.listdir(build_type_dir): + xml_file = os.path.join(build_type_dir, pkg, "package.xml") + # apply the formatter + valid = self.formatter.check_and_format_files([xml_file]) + if not pkg == "pkg_correct": + self.assertFalse( + valid, + f"XML file {xml_file} is expected to be invalid but was valid.", + ) + else: + self.assertTrue( + valid, + f"XML file {xml_file} is expected to be valid but was invalid.", + ) + self.assertTrue( + self._compare_xml_files(xml_file, correct_xml), + f"XML files do not match: {xml_file} != {correct_xml}", + ) + # validate the XML file with xmllint + self.assertTrue( + validate_xml_with_xmllint(xml_file), + f"XML file {xml_file} failed xmllint validation.", + ) + + +if __name__ == "__main__": + # Run the tests + unittest.main() From 3244116375c649f1e1eff9ac8c28f4d498ab9b4c Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Tue, 12 Aug 2025 18:28:35 +0200 Subject: [PATCH 25/25] Changed Logging --- .../helpers/pkg_xml_formatter.py | 17 ++++++++++++----- package_xml_validation/package_xml_validator.py | 3 --- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/package_xml_validation/helpers/pkg_xml_formatter.py b/package_xml_validation/helpers/pkg_xml_formatter.py index 410165b..a4e89e8 100644 --- a/package_xml_validation/helpers/pkg_xml_formatter.py +++ b/package_xml_validation/helpers/pkg_xml_formatter.py @@ -377,9 +377,9 @@ def fix_indentation(string, expected_indent) -> str: def check_and_correct(string, expected_indent, name) -> tuple[str, bool]: """Check and correct the indentation of the string.""" if not check_indentation_string(string, expected_indent): - self.logger.error( - f"Incorrect indentation for element '{name}'. Expected: '{expected_indent}', Found: '{string.replace(NEW_LINE, '') if string else 'None'}'" - ) + # self.logger.error( + # f"Incorrect indentation for element '{name}'. Expected: '{expected_indent}', Found: '{string.replace(NEW_LINE, '') if string else 'None'}'" + # ) if not self.check_only: string = fix_indentation(string, expected_indent) return string, True @@ -396,7 +396,9 @@ def check_and_correct(string, expected_indent, name) -> tuple[str, bool]: indentation * (level - 1) if is_last else indentation * level ) elem.tail, corrected = check_and_correct( - elem.tail, expected_indent, f"{elem.tag}-{elem.text[:15]}" + elem.tail, + expected_indent, + f"{elem.tag}-{elem.text[:15] if elem.text else 'None'}", ) is_correct &= not corrected if len(elem) > 0: # has children @@ -412,7 +414,12 @@ def check_and_correct(string, expected_indent, name) -> tuple[str, bool]: is_correct = False if not self.check_only: elem.text = elem.text.replace(NEW_LINE, " ").strip() - + if not is_correct and self.check_only: + self.logger.error( + "Incorrect indentation found in package.xml. Please fix the indentations." + ) + elif not is_correct: + self.logger.warning("Auto-corrected indentation in package.xml.") return is_correct def check_for_non_existing_tags(self, root, xml_file): diff --git a/package_xml_validation/package_xml_validator.py b/package_xml_validation/package_xml_validator.py index c531fa9..0e1b6e0 100644 --- a/package_xml_validation/package_xml_validator.py +++ b/package_xml_validation/package_xml_validator.py @@ -205,9 +205,6 @@ def extract_launch_deps(folder_names: list[str]) -> list[str]: launch_dir = os.path.join(os.path.dirname(package_xml_file), folder) if os.path.isdir(launch_dir): launch_deps.extend(scan_files(launch_dir)) - self.logger.info( - f"Extracted launch dependencies from {package_name}/package.xml: {launch_deps}" - ) return launch_deps def validate_launch_folders(