diff --git a/CHANGELOG b/CHANGELOG index 5815f9e..3c7a607 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/). --- +## [Unreleased] + +### Added +- Navigation support for `FindPackageShare` and `get_package_share_directory` with path resolution +- Added `resolved_path` field to `FindPackageShare` and `get_package_share_directory` outputs +- Unit tests for package share directory resolution functionality + +### Changed +- `FindPackageShare` handler now includes resolved filesystem path when package is available +- `get_package_share_directory` resolver now returns structured dict with resolved path +- Enhanced postprocessing to properly format Python function calls vs launch constructs + +--- + ## [0.2.0] - 2025-08-23 ### Added diff --git a/parser/parser/postprocessing.py b/parser/parser/postprocessing.py index 7418cec..8f27202 100644 --- a/parser/parser/postprocessing.py +++ b/parser/parser/postprocessing.py @@ -35,7 +35,19 @@ def simplify_launch_configurations(obj): def format_symbolic_part(part): if isinstance(part, dict): - return simplify_launch_configurations(part) + type_key = part.get("type", "") + simplified = simplify_launch_configurations(part) + # Add quotes for Python function calls (lowercase type names), + # but not for launch constructs (capitalized type names like FindPackageShare) + if ( + isinstance(simplified, str) + and simplified.startswith("${") + and simplified.endswith("}") + and type_key + and type_key[0].islower() + ): + return f"'{simplified}'" + return simplified elif isinstance(part, str): return f"'{part}'" else: @@ -62,6 +74,11 @@ def _simplify_find_package(obj): return f"${{FindPackageShare:{package}}}" +def _simplify_get_package_share_directory(obj): + package = obj.get("package") + return f"${{get_package_share_directory:{package}}}" + + def _simplify_find_executable(obj): name = obj.get("name") return f"${{FindExecutable:{name}}}" @@ -91,6 +108,7 @@ def _simplify_custom_handler(obj): "EnvironmentVariable": _simplify_environment_variable, "PathJoinSubstitution": _simplify_path_join, "FindPackageShare": _simplify_find_package, + "get_package_share_directory": _simplify_get_package_share_directory, "Command": _simplify_command, "FindExecutable": _simplify_find_executable, "ThisLaunchFileDir": _simplify_this_launch_file_dir, diff --git a/parser/parser/python/handlers/find_package_share.py b/parser/parser/python/handlers/find_package_share.py index c1ae00b..258354a 100644 --- a/parser/parser/python/handlers/find_package_share.py +++ b/parser/parser/python/handlers/find_package_share.py @@ -16,6 +16,7 @@ from parser.context import ParseContext from parser.parser.registry import register_handler +from parser.resolution.resolvers.package_path_resolver import resolve_package_share_path from parser.resolution.utils import resolve_call_signature @@ -25,4 +26,12 @@ def handle_find_package_share(node: ast.Call, context: ParseContext) -> dict: if not args: raise ValueError("FindPackageShare requires a package name.") - return {"type": "FindPackageShare", "package": args[0]} + package_name = args[0] + result = {"type": "FindPackageShare", "package": package_name} + + # Attempt to resolve the actual path + resolved_path = resolve_package_share_path(package_name) + if resolved_path is not None: + result["resolved_path"] = resolved_path + + return result diff --git a/parser/resolution/resolvers/get_package_share_directory.py b/parser/resolution/resolvers/get_package_share_directory.py index 40649d6..c28d65e 100644 --- a/parser/resolution/resolvers/get_package_share_directory.py +++ b/parser/resolution/resolvers/get_package_share_directory.py @@ -16,6 +16,7 @@ from parser.parser.postprocessing import simplify_launch_configurations from parser.resolution.resolution_registry import register_resolver +from parser.resolution.resolvers.package_path_resolver import resolve_package_share_path @register_resolver(ast.Call, priority=10) @@ -28,4 +29,16 @@ def resolve_get_package_share_directory(node: ast.Call, engine): raise ValueError("get_package_share_directory expects 1 argument") arg = engine.resolve(node.args[0]) - return f"${{get_package_share_directory:{simplify_launch_configurations(arg)}}}" + package_name = simplify_launch_configurations(arg) + + result = { + "type": "get_package_share_directory", + "package": package_name, + } + + # Attempt to resolve the actual path + resolved_path = resolve_package_share_path(package_name) + if resolved_path is not None: + result["resolved_path"] = resolved_path + + return result diff --git a/parser/resolution/resolvers/package_path_resolver.py b/parser/resolution/resolvers/package_path_resolver.py new file mode 100644 index 0000000..79f5f8c --- /dev/null +++ b/parser/resolution/resolvers/package_path_resolver.py @@ -0,0 +1,42 @@ +# Copyright (c) 2025 Kodo Robotics +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import warnings + + +def resolve_package_share_path(package_name: str) -> str | None: + """ + Resolve the actual filesystem path for a ROS 2 package share directory. + + Args: + package_name: The name of the ROS 2 package + + Returns: + The resolved path as a string, or None if the package cannot be found + """ + try: + from ament_index_python.packages import get_package_share_directory + + resolved_path = get_package_share_directory(package_name) + return resolved_path + except ImportError: + warnings.warn( + f"ament_index_python not available - cannot resolve package path for '{package_name}'" + ) + return None + except Exception as e: + warnings.warn( + f"Failed to resolve package share directory for '{package_name}': {e}" + ) + return None diff --git a/parser/tests/test_cases/python/package_resolution_tests.yaml b/parser/tests/test_cases/python/package_resolution_tests.yaml new file mode 100644 index 0000000..42bb334 --- /dev/null +++ b/parser/tests/test_cases/python/package_resolution_tests.yaml @@ -0,0 +1,97 @@ +tests: + - name: find_package_share_basic + description: Basic FindPackageShare call with package name + input: | + from launch import LaunchDescription + from launch_ros.substitutions import FindPackageShare + + def generate_launch_description(): + pkg_share = FindPackageShare("test_package") + return LaunchDescription([]) + expected: + {} + + - name: find_package_share_in_path_join + description: FindPackageShare used within PathJoinSubstitution + input: | + from launch import LaunchDescription + from launch.actions import IncludeLaunchDescription + from launch.launch_description_sources import PythonLaunchDescriptionSource + from launch.substitutions import PathJoinSubstitution + from launch_ros.substitutions import FindPackageShare + + def generate_launch_description(): + return LaunchDescription([ + IncludeLaunchDescription( + PythonLaunchDescriptionSource( + PathJoinSubstitution([ + FindPackageShare("test_package"), + "launch", + "test.launch.py" + ]) + ) + ) + ]) + expected: + includes: + - launch_description_source: ${PathJoinSubstitution:[${FindPackageShare:test_package}, 'launch', 'test.launch.py']} + launch_arguments: {} + included: {} + + - name: get_package_share_directory_basic + description: Basic get_package_share_directory call + input: | + from launch import LaunchDescription + from ament_index_python.packages import get_package_share_directory + + def generate_launch_description(): + pkg_dir = get_package_share_directory("test_package") + return LaunchDescription([]) + expected: + {} + + - name: get_package_share_directory_with_os_path_join + description: get_package_share_directory used with os.path.join + input: | + from launch import LaunchDescription + from launch.actions import IncludeLaunchDescription + from launch.launch_description_sources import PythonLaunchDescriptionSource + from ament_index_python.packages import get_package_share_directory + import os + + def generate_launch_description(): + launch_file = os.path.join( + get_package_share_directory("test_package"), + "launch", + "test.launch.py" + ) + return LaunchDescription([ + IncludeLaunchDescription( + PythonLaunchDescriptionSource(launch_file) + ) + ]) + expected: + includes: + - launch_description_source: ${os.path.join:['${get_package_share_directory:test_package}', 'launch', 'test.launch.py']} + launch_arguments: {} + included: {} + + - name: get_package_share_directory_with_launch_configuration + description: get_package_share_directory with LaunchConfiguration parameter + input: | + from launch import LaunchDescription + from launch.actions import DeclareLaunchArgument + from launch.substitutions import LaunchConfiguration + from ament_index_python.packages import get_package_share_directory + import os + + def generate_launch_description(): + pkg_name = LaunchConfiguration("package_name") + # This won't work at symbolic resolution time, but tests the structure + return LaunchDescription([ + DeclareLaunchArgument("package_name", default_value="test_package") + ]) + expected: + arguments: + - name: package_name + default_value: test_package diff --git a/parser/tests/test_from_yaml_python.py b/parser/tests/test_from_yaml_python.py index 5afd603..4e76f1c 100644 --- a/parser/tests/test_from_yaml_python.py +++ b/parser/tests/test_from_yaml_python.py @@ -120,3 +120,11 @@ def test_custom_handlers_parsing(code, expected): result = parse_launch_string(code, suffix=".py") for key in ["arguments", "nodes", "launch_argument_usages", "custom_components"]: assert result.get(key, []) == expected.get(key, []) + + +@pytest.mark.parametrize("code,expected", + load_yaml_tests("test_cases/python/package_resolution_tests.yaml")) +def test_package_resolution(code, expected): + result = parse_launch_string(code, suffix=".py") + for key in ["arguments", "includes", "launch_argument_usages"]: + assert result.get(key, []) == expected.get(key, []) diff --git a/parser/tests/test_package_resolution.py b/parser/tests/test_package_resolution.py new file mode 100644 index 0000000..bc0983a --- /dev/null +++ b/parser/tests/test_package_resolution.py @@ -0,0 +1,149 @@ +# Copyright (c) 2025 Kodo Robotics +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import warnings + +import pytest + +from parser.resolution.resolvers.package_path_resolver import resolve_package_share_path + + +def test_resolve_package_share_path_nonexistent(): + """Test that resolving a non-existent package returns None and warns.""" + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + result = resolve_package_share_path("nonexistent_package_12345") + assert result is None + assert len(w) > 0 + # Warning message depends on whether ament_index_python is available + msg = str(w[-1].message) + assert ( + "Failed to resolve package share directory" in msg + or "ament_index_python not available" in msg + ) + + +def test_resolve_package_share_path_ament_index_available(): + """Test that the function works when ament_index_python is available.""" + # Try to import ament_index_python + try: + import ament_index_python.packages # noqa: F401 + + # If we can import it, test with a known package (if available) + # We can't guarantee any specific package exists, so we just verify + # the function doesn't crash when ament_index is available + result = resolve_package_share_path("certainly_nonexistent_package_xyz") + # Should return None for non-existent package, but not crash + assert result is None or isinstance(result, str) + except ImportError: + # If ament_index_python is not available, skip this test + pytest.skip("ament_index_python not available") + + +def test_find_package_share_handler(): + """Test that FindPackageShare handler adds resolved_path field.""" + from parser.tests.test_helpers import parse_launch_string + + code = """ +from launch import LaunchDescription +from launch_ros.substitutions import FindPackageShare + +def generate_launch_description(): + pkg = FindPackageShare("test_package") + return LaunchDescription([]) +""" + result = parse_launch_string(code, suffix=".py") + # The test just verifies the code doesn't crash + # resolved_path may or may not be present depending on package availability + assert isinstance(result, dict) + + +def test_get_package_share_directory_resolver(): + """Test that get_package_share_directory resolver adds resolved_path field.""" + from parser.tests.test_helpers import parse_launch_string + + code = """ +from launch import LaunchDescription +from ament_index_python.packages import get_package_share_directory + +def generate_launch_description(): + pkg_dir = get_package_share_directory("test_package") + return LaunchDescription([]) +""" + result = parse_launch_string(code, suffix=".py") + # The test just verifies the code doesn't crash + # resolved_path may or may not be present depending on package availability + assert isinstance(result, dict) + + +def test_find_package_share_with_path_join(): + """Test FindPackageShare within PathJoinSubstitution.""" + from parser.tests.test_helpers import parse_launch_string + + code = """ +from launch import LaunchDescription +from launch.actions import IncludeLaunchDescription +from launch.launch_description_sources import PythonLaunchDescriptionSource +from launch.substitutions import PathJoinSubstitution +from launch_ros.substitutions import FindPackageShare + +def generate_launch_description(): + return LaunchDescription([ + IncludeLaunchDescription( + PythonLaunchDescriptionSource( + PathJoinSubstitution([ + FindPackageShare("test_package"), + "launch", + "test.launch.py" + ]) + ) + ) + ]) +""" + result = parse_launch_string(code, suffix=".py") + assert "includes" in result + assert len(result["includes"]) == 1 + # Verify the symbolic expression is correct + assert "FindPackageShare:test_package" in result["includes"][0]["launch_description_source"] + + +def test_get_package_share_directory_with_os_path_join(): + """Test get_package_share_directory with os.path.join.""" + from parser.tests.test_helpers import parse_launch_string + + code = """ +from launch import LaunchDescription +from launch.actions import IncludeLaunchDescription +from launch.launch_description_sources import PythonLaunchDescriptionSource +from ament_index_python.packages import get_package_share_directory +import os + +def generate_launch_description(): + launch_file = os.path.join( + get_package_share_directory("test_package"), + "launch", + "test.launch.py" + ) + return LaunchDescription([ + IncludeLaunchDescription( + PythonLaunchDescriptionSource(launch_file) + ) + ]) +""" + result = parse_launch_string(code, suffix=".py") + assert "includes" in result + assert len(result["includes"]) == 1 + # Verify the symbolic expression is correct + launch_src = result["includes"][0]["launch_description_source"] + assert "get_package_share_directory:test_package" in launch_src