From a5f8b7d64521f8b6e901758647fad278edc2b596 Mon Sep 17 00:00:00 2001 From: emerson Date: Fri, 22 May 2026 15:45:44 -0700 Subject: [PATCH 1/2] Add fragments composition: register_fragment macro, schema, resolver Schema (nodl_schema/schemas/nodl.schema.yaml): Top-level 'base' (enum of node / lifecycle_node) declares the built-in node interface to inherit. Top-level 'fragments' (array of fragment_ref) declares additional fragment interfaces to compose in, by relative path or 'nodl://pkg/name'. fragment_ref definition has required 'ref' and optional 'name' (label used for merge ordering and documentation). Generated models pick these up automatically; FragmentRef and the Base enum are added to NodlDocument. nodl_schema.validator: validate_fragment(data) runs the regular schema validation, then rejects the presence of top-level 'base' or 'fragments'. Fragments are flat ingredients; nested composition is intentionally disallowed in v2 and can be lifted later without a schema change. load_fragment(source) wraps load_nodl with this stricter check. python -m nodl_schema gains a --fragment flag for build-time hooks. nodl_schema.resolve: resolve(doc, source_path=None) returns a LayeredDocument carrying the resolved base, the resolved fragments (by label), and the main document. LayeredDocument.merged() flattens into a single NodlDocument with later layers winning on duplicate names; order is base -> fragments (insertion order) -> main. Every load path goes through load_fragment, so a fragment that declares its own base or fragments is rejected at resolve time with a clear error. Bundled fragments: node.nodl.yaml: declares use_sim_time. lifecycle_node.nodl.yaml: adds the standard transition services and the transition_event publisher on top of the node fragment. ament_nodl_register_fragment: Sibling of register_node, following the same install RENAME pattern, rosidl-style docstring, and "NoDL file" wording. Build-time validation via add_custom_command/target that runs 'python -m nodl_schema --fragment ' against the source. The stamp target is ALL, so install won't proceed without it. Tests: test_ament_nodl gains register_fragment tests (5 cases) covering default and explicit PACKAGE, byte-for-byte resource vs source, and source-file install layout. nodl_schema/test/test_resolve.py covers no-base, both bases, relative fragment resolution (success / missing / no source path), layer precedence (main > fragments > base), and explicit rejection of a fragment that declares its own base or fragments. nodl_schema/test/test_schema.py: the previously inverted base/fragments rejection tests become positive accept tests, plus 'unknown base rejected' and 'fragment_ref missing ref rejected' negative cases. nodl_schema/test/test_validator_cli.py: adds --fragment cases for valid fragments, rejection of base/fragments at the top level, and combined invalid-schema + fragment cases. Smoke-tested end-to-end by appending 'base: node' to a fragment fixture: the build fails at the validate_fragment target with the expected error and restoring brings it back green. Signed-off-by: Emerson Knapp --- ament_nodl/CMakeLists.txt | 1 + ament_nodl/ament_nodl-extras.cmake.in | 1 + .../cmake/ament_nodl_register_fragment.cmake | 80 +++++++ nodl_schema/nodl_schema/__init__.py | 21 +- nodl_schema/nodl_schema/models.py | 32 +++ nodl_schema/nodl_schema/resolve.py | 150 +++++++++++++ .../fragments/lifecycle_node.nodl.yaml | 26 +++ .../schemas/fragments/node.nodl.yaml | 6 + .../nodl_schema/schemas/nodl.schema.yaml | 25 +++ nodl_schema/nodl_schema/validator.py | 50 ++++- nodl_schema/setup.py | 9 +- nodl_schema/test/test_resolve.py | 202 ++++++++++++++++++ nodl_schema/test/test_schema.py | 20 +- nodl_schema/test/test_validator_cli.py | 50 ++++- test_ament_nodl/CMakeLists.txt | 9 + .../test/fixtures/extra_telemetry.nodl.yaml | 10 + .../test/fixtures/tf_listener.nodl.yaml | 17 ++ .../test/test_register_fragment.py | 64 ++++++ 18 files changed, 757 insertions(+), 16 deletions(-) create mode 100644 ament_nodl/cmake/ament_nodl_register_fragment.cmake create mode 100644 nodl_schema/nodl_schema/resolve.py create mode 100644 nodl_schema/nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml create mode 100644 nodl_schema/nodl_schema/schemas/fragments/node.nodl.yaml create mode 100644 nodl_schema/test/test_resolve.py create mode 100644 test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml create mode 100644 test_ament_nodl/test/fixtures/tf_listener.nodl.yaml create mode 100644 test_ament_nodl/test/test_register_fragment.py diff --git a/ament_nodl/CMakeLists.txt b/ament_nodl/CMakeLists.txt index 489fc28..7f3423f 100644 --- a/ament_nodl/CMakeLists.txt +++ b/ament_nodl/CMakeLists.txt @@ -6,6 +6,7 @@ project(ament_nodl) find_package(ament_cmake REQUIRED) install(FILES + cmake/ament_nodl_register_fragment.cmake cmake/ament_nodl_register_node.cmake DESTINATION share/${PROJECT_NAME}/cmake) diff --git a/ament_nodl/ament_nodl-extras.cmake.in b/ament_nodl/ament_nodl-extras.cmake.in index 1dd4afb..88ade3e 100644 --- a/ament_nodl/ament_nodl-extras.cmake.in +++ b/ament_nodl/ament_nodl-extras.cmake.in @@ -1,4 +1,5 @@ # Resolves ${Python3_EXECUTABLE} for the build-time validation step the macros invoke. find_package(Python3 REQUIRED COMPONENTS Interpreter) +include("${CMAKE_CURRENT_LIST_DIR}/ament_nodl_register_fragment.cmake") include("${CMAKE_CURRENT_LIST_DIR}/ament_nodl_register_node.cmake") diff --git a/ament_nodl/cmake/ament_nodl_register_fragment.cmake b/ament_nodl/cmake/ament_nodl_register_fragment.cmake new file mode 100644 index 0000000..893252b --- /dev/null +++ b/ament_nodl/cmake/ament_nodl_register_fragment.cmake @@ -0,0 +1,80 @@ +# SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. +# SPDX-License-Identifier: Apache-2.0 +# +# Register a NoDL fragment in the ament resource index. +# +# Publishes the contents of a NoDL file under the ``nodl_fragments`` resource type. +# The resource key is ``__``. +# NoDL documents reference the fragment by ``nodl:///``. +# +# Consumers retrieve the content via:: +# +# ament_index_python.packages.get_resource('nodl_fragments', '__') +# +# The source file is also installed under ``share//nodl/fragments/`` for direct filesystem access. +# +# Example:: +# +# ament_nodl_register_fragment(tf_listener +# FILE nodl/tf_listener.nodl.yaml +# ) +# +# :param fragment_name: name of the fragment. +# Combined with PACKAGE to form the resource key and the ``nodl:///`` URI. +# :type fragment_name: string +# :param FILE: path to the NoDL file containing the fragment definition. +# May be absolute or relative to ``CMAKE_CURRENT_SOURCE_DIR``. +# :type FILE: string +# :param PACKAGE: package name to use in the resource key. +# Defaults to ``${PROJECT_NAME}``. +# :type PACKAGE: string +# +# @public +# +function(ament_nodl_register_fragment fragment_name) + cmake_parse_arguments(_ARGS "" "FILE;PACKAGE" "" ${ARGN}) + + if(NOT _ARGS_FILE) + message(FATAL_ERROR "ament_nodl_register_fragment: FILE is required") + endif() + if(NOT _ARGS_PACKAGE) + set(_ARGS_PACKAGE "${PROJECT_NAME}") + endif() + + get_filename_component(_abs_file "${_ARGS_FILE}" ABSOLUTE + BASE_DIR "${CMAKE_CURRENT_SOURCE_DIR}") + + if(NOT EXISTS "${_abs_file}") + message(WARNING + "ament_nodl_register_fragment: file not found at configure time: ${_abs_file}") + endif() + + # Validate the fragment at build time so authoring errors surface when registering, not downstream when consuming. + # --fragment additionally rejects nested base/fragments, which are disallowed in v2. + # This only runs when ${_abs_file} changes. + set(_stamp_dir "${CMAKE_CURRENT_BINARY_DIR}/ament_nodl/nodl_fragments") + set(_stamp "${_stamp_dir}/${_ARGS_PACKAGE}__${fragment_name}.valid.stamp") + file(MAKE_DIRECTORY "${_stamp_dir}") + add_custom_command( + OUTPUT "${_stamp}" + DEPENDS "${_abs_file}" + COMMAND "${Python3_EXECUTABLE}" -m nodl_schema --fragment "${_abs_file}" + COMMAND "${CMAKE_COMMAND}" -E touch "${_stamp}" + COMMENT "Validating NoDL fragment ${_ARGS_PACKAGE}/${fragment_name}" + VERBATIM + ) + add_custom_target(_ament_nodl_validate_fragment_${_ARGS_PACKAGE}__${fragment_name} ALL + DEPENDS "${_stamp}" + ) + + # Install to ament index + install( + FILES "${_abs_file}" + DESTINATION "share/ament_index/resource_index/nodl_fragments" + RENAME "${_ARGS_PACKAGE}__${fragment_name}") + + # Install to package's share directory + install( + FILES "${_abs_file}" + DESTINATION "share/${_ARGS_PACKAGE}/nodl/fragments") +endfunction() diff --git a/nodl_schema/nodl_schema/__init__.py b/nodl_schema/nodl_schema/__init__.py index 4df564c..f29a678 100644 --- a/nodl_schema/nodl_schema/__init__.py +++ b/nodl_schema/nodl_schema/__init__.py @@ -2,6 +2,23 @@ # SPDX-License-Identifier: Apache-2.0 """NoDL schema, in-memory models, and validation helpers.""" -from nodl_schema.validator import dump_nodl, load_nodl, load_schema, validate +from nodl_schema.resolve import LayeredDocument, resolve +from nodl_schema.validator import ( + dump_nodl, + load_fragment, + load_nodl, + load_schema, + validate, + validate_fragment, +) -__all__ = ['dump_nodl', 'load_nodl', 'load_schema', 'validate'] +__all__ = [ + 'LayeredDocument', + 'dump_nodl', + 'load_fragment', + 'load_nodl', + 'load_schema', + 'resolve', + 'validate', + 'validate_fragment', +] diff --git a/nodl_schema/nodl_schema/models.py b/nodl_schema/nodl_schema/models.py index 9dcab80..146cbb7 100644 --- a/nodl_schema/nodl_schema/models.py +++ b/nodl_schema/nodl_schema/models.py @@ -14,6 +14,15 @@ from pydantic import BaseModel, Extra, Field, conint, constr +class Base(Enum): + """ + Built-in ROS 2 base node type this node inherits from. + """ + + node = 'node' + lifecycle_node = 'lifecycle_node' + + class ScalarType(Enum): """ Scalar types: @@ -48,6 +57,24 @@ class ArrayType(Enum): string_array = 'string_array' +class FragmentRef(BaseModel): + """ + Reference to a NoDL fragment. + """ + + class Config: + extra = Extra.forbid + + ref: str = Field( + ..., + description="'nodl://pkg/name' for ament-index fragments, or a relative file path.\n", + ) + name: Optional[str] = Field( + None, + description='Optional label for this fragment in documentation and merge order.', + ) + + class History(Enum): """ History policy. @@ -434,6 +461,11 @@ class Config: nodl_version: int = Field(2, const=True, description='NoDL schema major version this document targets.') description: Optional[str] = Field(None, description='Human-readable description of what this node does.') + base: Optional[Base] = Field(None, description='Built-in ROS 2 base node type this node inherits from.') + fragments: Optional[list[FragmentRef]] = Field( + None, + description='NoDL fragments composed into this node (single-level, not recursive).', + ) parameters: Optional[dict[str, ParameterDefinition]] = Field( None, description='ROS parameters declared by this node, keyed by parameter name.\nParameter shape is borrowed from ``generate_parameter_library``\n(see ``parameter.schema.yaml``).\n', diff --git a/nodl_schema/nodl_schema/resolve.py b/nodl_schema/nodl_schema/resolve.py new file mode 100644 index 0000000..b413410 --- /dev/null +++ b/nodl_schema/nodl_schema/resolve.py @@ -0,0 +1,150 @@ +# SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. +# SPDX-License-Identifier: Apache-2.0 +"""Resolution of NoDL composition (base + fragments) into a LayeredDocument.""" + +from __future__ import annotations + +import importlib.resources as ir +from dataclasses import dataclass, field +from pathlib import Path +from typing import Dict, List, Optional + +from nodl_schema.models import ( + ActionEndpoint, + NodlDocument, + ParameterDefinition, + ServiceEndpoint, + TopicEndpoint, +) + +_BUILTIN_BASES = frozenset(['node', 'lifecycle_node']) + + +def _load_builtin(base: str) -> NodlDocument: + from nodl_schema.validator import load_fragment + + path = ir.files('nodl_schema') / 'schemas' / 'fragments' / f'{base}.nodl.yaml' + return load_fragment(path.read_text(encoding='utf-8')) + + +def _load_ament_fragment(pkg: str, name: str) -> NodlDocument: + try: + from ament_index_python.packages import get_resource + except ImportError as exc: + raise ImportError('ament_index_python is required to resolve nodl:// URIs') from exc + # Key format mirrors ament_nodl_register_fragment: pkg__name + resource_key = f'{pkg}__{name}' + try: + content, _ = get_resource('nodl_fragments', resource_key) + except KeyError: + raise FileNotFoundError( + f'NoDL fragment not found in ament index: {pkg}/{name} (looked up as nodl_fragments/{resource_key})' + ) + from nodl_schema.validator import load_fragment + + return load_fragment(content) + + +def _resolve_ref(ref: str, source_path: Optional[Path] = None) -> NodlDocument: + if ref.startswith('nodl://'): + rest = ref[len('nodl://') :] + parts = rest.split('/', 1) + if len(parts) != 2: + raise ValueError(f'Invalid nodl:// URI: {ref!r} -- expected nodl://pkg/name') + return _load_ament_fragment(parts[0], parts[1]) + if source_path is None: + raise ValueError(f'Cannot resolve relative fragment ref {ref!r} without a source path') + full_path = (source_path.parent / ref).resolve() + if not full_path.exists(): + raise FileNotFoundError(f'Fragment file not found: {full_path}') + from nodl_schema.validator import load_fragment + + return load_fragment(full_path.read_text(encoding='utf-8')) + + +@dataclass +class LayeredDocument: + """A NoDL document with its resolved composition layers, keyed by label.""" + + main: NodlDocument + base: Optional[NodlDocument] = None + base_name: Optional[str] = None + fragments: Dict[str, NodlDocument] = field(default_factory=dict) + + def merged(self) -> NodlDocument: + """Merge all layers into a flat NodlDocument. + + Layers applied in order: base -> named fragments (insertion order) -> main. + Later layers win on duplicate names. + """ + layers: List[NodlDocument] = [] + if self.base is not None: + layers.append(self.base) + layers.extend(self.fragments.values()) + layers.append(self.main) + + publishers: Dict[str, TopicEndpoint] = {} + subscriptions: Dict[str, TopicEndpoint] = {} + service_servers: Dict[str, ServiceEndpoint] = {} + service_clients: Dict[str, ServiceEndpoint] = {} + action_servers: Dict[str, ActionEndpoint] = {} + action_clients: Dict[str, ActionEndpoint] = {} + parameters: Dict[str, ParameterDefinition] = {} + + for doc in layers: + for ep in doc.publishers or []: + publishers[ep.name] = ep + for ep in doc.subscriptions or []: + subscriptions[ep.name] = ep + for ep in doc.service_servers or []: + service_servers[ep.name] = ep + for ep in doc.service_clients or []: + service_clients[ep.name] = ep + for ep in doc.action_servers or []: + action_servers[ep.name] = ep + for ep in doc.action_clients or []: + action_clients[ep.name] = ep + for param_name, param in (doc.parameters or {}).items(): + parameters[param_name] = param + + return NodlDocument( + nodl_version=self.main.nodl_version, + parameters=parameters or None, + publishers=list(publishers.values()) or None, + subscriptions=list(subscriptions.values()) or None, + service_servers=list(service_servers.values()) or None, + service_clients=list(service_clients.values()) or None, + action_servers=list(action_servers.values()) or None, + action_clients=list(action_clients.values()) or None, + ) + + +def resolve(doc: NodlDocument, source_path: Optional[Path] = None) -> LayeredDocument: + """Resolve a NodlDocument's base and fragments into a LayeredDocument. + + source_path: filesystem path to the NoDL file, used to resolve relative refs. + Fragments are single-level only -- fragment files are not themselves resolved. + """ + # doc.base is an enum on pydantic v2 and may be either enum or string on v1 + # depending on how the document was constructed. Normalize to the string value. + base_name: Optional[str] = None + if doc.base is not None: + base_name = doc.base.value if hasattr(doc.base, 'value') else doc.base + + base_doc: Optional[NodlDocument] = None + if base_name is not None: + if base_name not in _BUILTIN_BASES: + raise ValueError(f'Unknown base {base_name!r}. Must be one of: {sorted(_BUILTIN_BASES)}') + base_doc = _load_builtin(base_name) + + fragment_docs: Dict[str, NodlDocument] = {} + for frag_ref in doc.fragments or []: + label = frag_ref.name or frag_ref.ref + fragment_docs[label] = _resolve_ref(frag_ref.ref, source_path=source_path) + + return LayeredDocument( + main=doc, + base=base_doc, + base_name=base_name, + fragments=fragment_docs, + ) diff --git a/nodl_schema/nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml b/nodl_schema/nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml new file mode 100644 index 0000000..e0582ec --- /dev/null +++ b/nodl_schema/nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml @@ -0,0 +1,26 @@ +nodl_version: 2 +parameters: + use_sim_time: + type: bool + default_value: false + description: Use simulation (clock) time instead of wall time + +service_servers: + - name: ~/change_state + type: lifecycle_msgs/srv/ChangeState + - name: ~/get_state + type: lifecycle_msgs/srv/GetState + - name: ~/get_available_states + type: lifecycle_msgs/srv/GetAvailableStates + - name: ~/get_available_transitions + type: lifecycle_msgs/srv/GetAvailableTransitions + - name: ~/get_transition_graph + type: lifecycle_msgs/srv/GetTransitionGraph + +publishers: + - name: ~/transition_event + type: lifecycle_msgs/msg/TransitionEvent + qos: + history: KEEP_LAST + depth: 10 + reliability: RELIABLE diff --git a/nodl_schema/nodl_schema/schemas/fragments/node.nodl.yaml b/nodl_schema/nodl_schema/schemas/fragments/node.nodl.yaml new file mode 100644 index 0000000..1f5e741 --- /dev/null +++ b/nodl_schema/nodl_schema/schemas/fragments/node.nodl.yaml @@ -0,0 +1,6 @@ +nodl_version: 2 +parameters: + use_sim_time: + type: bool + default_value: false + description: Use simulation (clock) time instead of wall time diff --git a/nodl_schema/nodl_schema/schemas/nodl.schema.yaml b/nodl_schema/nodl_schema/schemas/nodl.schema.yaml index 6c152f5..d6712b1 100644 --- a/nodl_schema/nodl_schema/schemas/nodl.schema.yaml +++ b/nodl_schema/nodl_schema/schemas/nodl.schema.yaml @@ -28,6 +28,17 @@ properties: type: string description: Human-readable description of what this node does. + base: + type: string + enum: [node, lifecycle_node] + description: Built-in ROS 2 base node type this node inherits from. + + fragments: + type: array + description: NoDL fragments composed into this node (single-level, not recursive). + items: + $ref: '#/definitions/fragment_ref' + parameters: type: object description: | @@ -74,6 +85,20 @@ properties: $ref: '#/definitions/action_endpoint' definitions: + fragment_ref: + type: object + description: Reference to a NoDL fragment. + required: [ref] + additionalProperties: false + properties: + ref: + type: string + description: | + 'nodl://pkg/name' for ament-index fragments, or a relative file path. + name: + type: string + description: Optional label for this fragment in documentation and merge order. + rosName: type: string description: | diff --git a/nodl_schema/nodl_schema/validator.py b/nodl_schema/nodl_schema/validator.py index 5af466d..a95cb14 100644 --- a/nodl_schema/nodl_schema/validator.py +++ b/nodl_schema/nodl_schema/validator.py @@ -55,6 +55,31 @@ def validate(data: dict) -> None: _make_validator().validate(data) +# Top-level keys that a fragment document must not declare. +# Fragments are flat ingredients; nested composition is intentionally disallowed in v2. +_FRAGMENT_FORBIDDEN_KEYS = ('base', 'fragments') + + +def validate_fragment(data: dict) -> None: + """Validate a NoDL fragment. + + A fragment must pass the standard schema and additionally must not declare + a ``base`` or ``fragments`` key. + Nested fragment composition is intentionally disallowed in v2; if a real + use case emerges, the constraint can be lifted without a schema change. + Raises jsonschema.ValidationError on schema failure or ValueError on a + forbidden key. + """ + validate(data) + forbidden = [k for k in _FRAGMENT_FORBIDDEN_KEYS if k in data] + if forbidden: + raise ValueError( + 'NoDL fragment must not declare ' + + ', '.join(repr(k) for k in forbidden) + + '; nested fragment composition is not supported in v2.' + ) + + def load_nodl(source: Union[str, bytes, IO]) -> NodlDocument: """Load and validate a NoDL document from a string, bytes, or file-like object. @@ -62,11 +87,22 @@ def load_nodl(source: Union[str, bytes, IO]) -> NodlDocument: Raises jsonschema.ValidationError on schema error or pydantic.ValidationError on type error. """ + return _load(source, validate) + + +def load_fragment(source: Union[str, bytes, IO]) -> NodlDocument: + """Load and validate a NoDL fragment document. + + Same as load_nodl but enforces the no-base / no-fragments constraint. + """ + return _load(source, validate_fragment) + + +def _load(source, validator) -> NodlDocument: data = yaml.safe_load(source) if not isinstance(data, dict): raise ValueError('NoDL document must be a YAML/JSON mapping at the top level') - - validate(data) + validator(data) # parse_obj is pydantic v1 API, retained as a deprecated alias in v2. # Used so this module works against both rosdep-shipped pydantic v1 # (humble/jazzy/kilted) and v2 (lyrical+). @@ -105,11 +141,19 @@ def main(argv: list[str] | None = None) -> int: description='Validate a NoDL file against the schema.', ) parser.add_argument('file', type=Path, help='Path to the NoDL file to validate.') + parser.add_argument( + '--fragment', + action='store_true', + help='Validate the file as a fragment (rejects nested base/fragments).', + ) args = parser.parse_args(argv) try: with args.file.open('r') as f: - load_nodl(f) + if args.fragment: + load_fragment(f) + else: + load_nodl(f) except Exception as exc: print(f'{args.file}: {exc}', file=sys.stderr) return 1 diff --git a/nodl_schema/setup.py b/nodl_schema/setup.py index c1dd8d0..272ec32 100644 --- a/nodl_schema/setup.py +++ b/nodl_schema/setup.py @@ -8,7 +8,7 @@ name=package_name, version='0.0.0', packages=[package_name], - package_data={package_name: ['schemas/*.yaml']}, + package_data={package_name: ['schemas/*.yaml', 'schemas/fragments/*.yaml']}, data_files=[ ('share/ament_index/resource_index/packages', ['resource/' + package_name]), ('share/' + package_name, ['package.xml']), @@ -19,6 +19,13 @@ 'nodl_schema/schemas/parameter.schema.yaml', ], ), + ( + 'share/' + package_name + '/schemas/fragments', + [ + 'nodl_schema/schemas/fragments/node.nodl.yaml', + 'nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml', + ], + ), ], zip_safe=True, extras_require={'test': ['pytest']}, diff --git a/nodl_schema/test/test_resolve.py b/nodl_schema/test/test_resolve.py new file mode 100644 index 0000000..2321fce --- /dev/null +++ b/nodl_schema/test/test_resolve.py @@ -0,0 +1,202 @@ +# SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. +# SPDX-License-Identifier: Apache-2.0 +"""Unit tests for nodl_schema.resolve -- no ament_index or live ROS required.""" + +from __future__ import annotations + +import textwrap +from pathlib import Path + +import pytest + +from nodl_schema.models import FragmentRef, NodlDocument, ParameterDefinition, QosProfile, TopicEndpoint +from nodl_schema.resolve import resolve + +_SYS_QOS = QosProfile(history='SYSTEM_DEFAULT', reliability='SYSTEM_DEFAULT') + + +# --------------------------------------------------------------------------- +# Built-in base resolution +# --------------------------------------------------------------------------- + + +def test_resolve_no_base_or_fragments(): + doc = NodlDocument(nodl_version=2) + layered = resolve(doc) + assert layered.base is None + assert layered.fragments == {} + assert layered.merged() == NodlDocument(nodl_version=2) + + +def test_resolve_base_node_loads_use_sim_time(): + doc = NodlDocument(nodl_version=2, base='node') + layered = resolve(doc) + assert layered.base is not None + assert layered.base_name == 'node' + merged = layered.merged() + assert merged.parameters is not None + assert 'use_sim_time' in merged.parameters + p = merged.parameters['use_sim_time'] + assert (p.type.value if hasattr(p.type, 'value') else p.type) == 'bool' + + +def test_resolve_base_lifecycle_node_has_use_sim_time(): + doc = NodlDocument(nodl_version=2, base='lifecycle_node') + merged = resolve(doc).merged() + assert 'use_sim_time' in (merged.parameters or {}) + + +def test_resolve_base_lifecycle_node_has_change_state_service(): + merged = resolve(NodlDocument(nodl_version=2, base='lifecycle_node')).merged() + names = [s.name for s in (merged.service_servers or [])] + assert '~/change_state' in names + + +def test_resolve_base_lifecycle_node_has_transition_event_publisher(): + merged = resolve(NodlDocument(nodl_version=2, base='lifecycle_node')).merged() + topics = [p.name for p in (merged.publishers or [])] + assert '~/transition_event' in topics + + +def test_resolve_unknown_base_raises(): + # Pydantic rejects unknown bases at model construction time. + with pytest.raises(Exception): + NodlDocument(nodl_version=2, base='unknown_base') + + +# --------------------------------------------------------------------------- +# Fragment resolution -- relative path +# --------------------------------------------------------------------------- + + +def test_resolve_relative_fragment(tmp_path: Path): + frag_file = tmp_path / 'my_frag.nodl.yaml' + frag_file.write_text( + textwrap.dedent("""\ + nodl_version: 2 + publishers: + - name: /extra + type: std_msgs/msg/String + qos: + history: SYSTEM_DEFAULT + reliability: SYSTEM_DEFAULT + """) + ) + + doc = NodlDocument( + nodl_version=2, + fragments=[FragmentRef(ref='my_frag.nodl.yaml', name='extra')], + ) + layered = resolve(doc, source_path=tmp_path / 'root.nodl.yaml') + assert 'extra' in layered.fragments + topics = [p.name for p in (layered.merged().publishers or [])] + assert '/extra' in topics + + +def test_resolve_relative_fragment_missing_raises(tmp_path: Path): + doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='nonexistent.nodl.yaml')]) + with pytest.raises(FileNotFoundError): + resolve(doc, source_path=tmp_path / 'root.nodl.yaml') + + +def test_resolve_relative_fragment_without_source_raises(): + doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='./something.nodl.yaml')]) + with pytest.raises(ValueError, match='source path'): + resolve(doc) + + +def test_resolve_fragment_declaring_base_raises(tmp_path: Path): + # A fragment may not declare base; nested composition is disallowed in v2. + frag = tmp_path / 'frag.nodl.yaml' + frag.write_text('nodl_version: 2\nbase: node\n') + doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='frag.nodl.yaml')]) + with pytest.raises(ValueError, match="'base'"): + resolve(doc, source_path=tmp_path / 'root.nodl.yaml') + + +def test_resolve_fragment_declaring_fragments_raises(tmp_path: Path): + # A fragment may not declare its own fragments either. + frag = tmp_path / 'frag.nodl.yaml' + frag.write_text('nodl_version: 2\nfragments:\n - ref: nodl://pkg/other\n') + doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='frag.nodl.yaml')]) + with pytest.raises(ValueError, match="'fragments'"): + resolve(doc, source_path=tmp_path / 'root.nodl.yaml') + + +# --------------------------------------------------------------------------- +# LayeredDocument.merged() -- layer precedence +# --------------------------------------------------------------------------- + + +def test_merged_main_wins_over_base(): + """Main document's parameter overrides the base's.""" + doc = NodlDocument( + nodl_version=2, + base='node', + parameters={'use_sim_time': ParameterDefinition(type='bool', default_value=True)}, + ) + merged = resolve(doc).merged() + # Main sets default_value=True; base has default_value=False + assert merged.parameters['use_sim_time'].default_value is True + + +def test_merged_fragment_wins_over_base(tmp_path: Path): + frag_file = tmp_path / 'frag.nodl.yaml' + frag_file.write_text( + textwrap.dedent("""\ + nodl_version: 2 + parameters: + use_sim_time: + type: bool + default_value: true + """) + ) + doc = NodlDocument( + nodl_version=2, + base='node', + fragments=[FragmentRef(ref='frag.nodl.yaml', name='frag')], + ) + merged = resolve(doc, source_path=tmp_path / 'root.nodl.yaml').merged() + assert merged.parameters['use_sim_time'].default_value is True + + +def test_merged_combines_publishers_from_all_layers(tmp_path: Path): + frag_file = tmp_path / 'frag.nodl.yaml' + frag_file.write_text( + textwrap.dedent("""\ + nodl_version: 2 + publishers: + - name: /from_frag + type: std_msgs/msg/String + qos: + history: SYSTEM_DEFAULT + reliability: SYSTEM_DEFAULT + """) + ) + doc = NodlDocument( + nodl_version=2, + publishers=[TopicEndpoint(name='/from_main', type='std_msgs/msg/String', qos=_SYS_QOS)], + fragments=[FragmentRef(ref='frag.nodl.yaml', name='f')], + ) + merged = resolve(doc, source_path=tmp_path / 'root.nodl.yaml').merged() + topics = {p.name for p in (merged.publishers or [])} + assert '/from_main' in topics + assert '/from_frag' in topics + + +def test_fragment_label_defaults_to_ref(tmp_path: Path): + frag_file = tmp_path / 'f.nodl.yaml' + frag_file.write_text( + textwrap.dedent("""\ + nodl_version: 2 + publishers: + - name: /t + type: std_msgs/msg/String + qos: + history: SYSTEM_DEFAULT + reliability: SYSTEM_DEFAULT + """) + ) + doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='f.nodl.yaml')]) + layered = resolve(doc, source_path=tmp_path / 'root.nodl.yaml') + assert 'f.nodl.yaml' in layered.fragments diff --git a/nodl_schema/test/test_schema.py b/nodl_schema/test/test_schema.py index 99a5b1f..bfc9490 100644 --- a/nodl_schema/test/test_schema.py +++ b/nodl_schema/test/test_schema.py @@ -235,14 +235,26 @@ def test_string_nodl_version_rejected(): validate({'nodl_version': '2'}) -def test_fragments_no_longer_allowed(): +def test_base_node_accepted(): + validate({'nodl_version': 2, 'base': 'node'}) + + +def test_base_lifecycle_node_accepted(): + validate({'nodl_version': 2, 'base': 'lifecycle_node'}) + + +def test_base_unknown_rejected(): with pytest.raises(ValidationError): - validate({'nodl_version': 2, 'fragments': [{'ref': 'nodl://pkg/x'}]}) + validate({'nodl_version': 2, 'base': 'unknown_base'}) + + +def test_fragments_accepted(): + validate({'nodl_version': 2, 'fragments': [{'ref': 'nodl://pkg/x'}]}) -def test_base_no_longer_allowed(): +def test_fragment_missing_ref_rejected(): with pytest.raises(ValidationError): - validate({'nodl_version': 2, 'base': 'lifecycle_node'}) + validate({'nodl_version': 2, 'fragments': [{'name': 'no_ref'}]}) def test_invalid_parameter_type(): diff --git a/nodl_schema/test/test_validator_cli.py b/nodl_schema/test/test_validator_cli.py index 3b7621b..dd0dd72 100644 --- a/nodl_schema/test/test_validator_cli.py +++ b/nodl_schema/test/test_validator_cli.py @@ -29,12 +29,12 @@ _INVALID_NOT_A_MAPPING = '- just a list\n' -def _run(file: Path) -> subprocess.CompletedProcess: - return subprocess.run( - [sys.executable, '-m', 'nodl_schema', str(file)], - capture_output=True, - text=True, - ) +def _run(file: Path, *, fragment: bool = False) -> subprocess.CompletedProcess: + cmd = [sys.executable, '-m', 'nodl_schema'] + if fragment: + cmd.append('--fragment') + cmd.append(str(file)) + return subprocess.run(cmd, capture_output=True, text=True) def test_valid_file_exits_zero(tmp_path: Path): @@ -72,3 +72,41 @@ def test_json_frontend_supported(tmp_path: Path): f.write_text('{"nodl_version": 2}\n') result = _run(f) assert result.returncode == 0, result.stderr + + +# --------------------------------------------------------------------------- +# --fragment mode +# --------------------------------------------------------------------------- + + +def test_valid_fragment_exits_zero(tmp_path: Path): + # A fragment with publishers / subscriptions but no base/fragments passes. + f = tmp_path / 'frag.nodl.yaml' + f.write_text(_VALID) + result = _run(f, fragment=True) + assert result.returncode == 0, result.stderr + + +def test_fragment_with_base_rejected(tmp_path: Path): + f = tmp_path / 'bad_frag.nodl.yaml' + f.write_text('nodl_version: 2\nbase: node\n') + result = _run(f, fragment=True) + assert result.returncode == 1 + assert str(f) in result.stderr + assert "'base'" in result.stderr + + +def test_fragment_with_fragments_rejected(tmp_path: Path): + f = tmp_path / 'bad_frag.nodl.yaml' + f.write_text('nodl_version: 2\nfragments:\n - ref: nodl://pkg/other\n') + result = _run(f, fragment=True) + assert result.returncode == 1 + assert "'fragments'" in result.stderr + + +def test_fragment_with_invalid_schema_still_rejected(tmp_path: Path): + # Schema validation happens before the fragment-only check. + f = tmp_path / 'doubly_bad.nodl.yaml' + f.write_text(_INVALID_BAD_PARAM_TYPE) + result = _run(f, fragment=True) + assert result.returncode == 1 diff --git a/test_ament_nodl/CMakeLists.txt b/test_ament_nodl/CMakeLists.txt index 05f68d4..3d64351 100644 --- a/test_ament_nodl/CMakeLists.txt +++ b/test_ament_nodl/CMakeLists.txt @@ -17,9 +17,18 @@ ament_nodl_register_node(custom_exe # Non-yaml frontend. Make sure the macro is extension-agnostic. ament_nodl_register_node(json_node FILE test/fixtures/json_node.nodl.json) +# Default PACKAGE for the fragment register macro. +ament_nodl_register_fragment(tf_listener FILE test/fixtures/tf_listener.nodl.yaml) + +# Explicit PACKAGE override for fragments. +ament_nodl_register_fragment(extra_telemetry + FILE test/fixtures/extra_telemetry.nodl.yaml + PACKAGE custom_pkg) + if(BUILD_TESTING) find_package(ament_cmake_pytest REQUIRED) ament_add_pytest_test(register_node_tests test/test_register_node.py) + ament_add_pytest_test(register_fragment_tests test/test_register_fragment.py) ament_add_pytest_test(macro_rejection_tests test/test_macro_rejection.py) endif() diff --git a/test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml b/test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml new file mode 100644 index 0000000..3191b0e --- /dev/null +++ b/test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml @@ -0,0 +1,10 @@ +--- +nodl_version: 2 +description: Test fragment registered under an explicit PACKAGE override. +publishers: + - name: ~/diagnostics + type: diagnostic_msgs/msg/DiagnosticArray + qos: + history: KEEP_LAST + depth: 10 + reliability: RELIABLE diff --git a/test_ament_nodl/test/fixtures/tf_listener.nodl.yaml b/test_ament_nodl/test/fixtures/tf_listener.nodl.yaml new file mode 100644 index 0000000..e00de75 --- /dev/null +++ b/test_ament_nodl/test/fixtures/tf_listener.nodl.yaml @@ -0,0 +1,17 @@ +--- +nodl_version: 2 +description: Test fragment that adds the standard tf2 listener subscriptions. +subscriptions: + - name: /tf + type: tf2_msgs/msg/TFMessage + qos: + history: KEEP_LAST + depth: 100 + reliability: RELIABLE + - name: /tf_static + type: tf2_msgs/msg/TFMessage + qos: + history: KEEP_LAST + depth: 100 + reliability: RELIABLE + durability: TRANSIENT_LOCAL diff --git a/test_ament_nodl/test/test_register_fragment.py b/test_ament_nodl/test/test_register_fragment.py new file mode 100644 index 0000000..16b9c0f --- /dev/null +++ b/test_ament_nodl/test/test_register_fragment.py @@ -0,0 +1,64 @@ +# SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. +# SPDX-License-Identifier: Apache-2.0 +"""Integration tests for the ament_nodl_register_fragment CMake macro. + +The macro is exercised at configure / install time by this package's +CMakeLists.txt; here we assert on the resulting ament index and install +tree without re-invoking CMake. +""" + +from pathlib import Path + +from ament_index_python.packages import PackageNotFoundError, get_package_share_directory +from ament_index_python.resources import get_resource + + +def _share(pkg: str = 'test_ament_nodl') -> Path: + # Real packages resolve through the ament index. + # Virtual packages used only as a PACKAGE override aren't registered, so we + # resolve them as siblings under the registering package's install prefix. + try: + return Path(get_package_share_directory(pkg)) + except PackageNotFoundError: + return Path(get_package_share_directory('test_ament_nodl')).parent / pkg + + +# --------------------------------------------------------------------------- +# Resource registration +# --------------------------------------------------------------------------- + + +def test_default_package_uses_project_name(): + # No PACKAGE arg means the macro defaults to ${PROJECT_NAME}, so the key is test_ament_nodl__tf_listener. + content, prefix = get_resource('nodl_fragments', 'test_ament_nodl__tf_listener') + assert prefix + assert 'tf2 listener' in content + + +def test_explicit_package_override(): + # PACKAGE custom_pkg makes the key custom_pkg__extra_telemetry, matching what nodl://custom_pkg/extra_telemetry + # would resolve to. + content, _ = get_resource('nodl_fragments', 'custom_pkg__extra_telemetry') + assert 'explicit PACKAGE override' in content + + +def test_resource_content_matches_source(): + # The registered resource should be byte-identical to the source file. + content, _ = get_resource('nodl_fragments', 'test_ament_nodl__tf_listener') + on_disk = (_share() / 'nodl' / 'fragments' / 'tf_listener.nodl.yaml').read_text() + assert content == on_disk + + +# --------------------------------------------------------------------------- +# Source file installation +# --------------------------------------------------------------------------- + + +def test_source_file_installed_under_registering_package(): + # Default PACKAGE installs the source under share/test_ament_nodl/nodl/fragments/. + assert (_share() / 'nodl' / 'fragments' / 'tf_listener.nodl.yaml').is_file() + + +def test_source_file_installed_under_override_package(): + # PACKAGE override redirects the source-file install to share//nodl/fragments/. + assert (_share('custom_pkg') / 'nodl' / 'fragments' / 'extra_telemetry.nodl.yaml').is_file() From d4a32c7f7a691d4dae62851f9fa01915390c8160 Mon Sep 17 00:00:00 2001 From: Emerson Knapp Date: Wed, 10 Jun 2026 16:24:36 -0700 Subject: [PATCH 2/2] Redesign Fragments to Node+Documents Signed-off-by: Emerson Knapp --- ament_nodl/CMakeLists.txt | 2 +- ament_nodl/ament_nodl-extras.cmake.in | 2 +- ...ake => ament_nodl_register_document.cmake} | 44 ++--- .../cmake/ament_nodl_register_node.cmake | 9 +- nodl/doc/concepts.md | 34 +++- nodl/doc/conf.py | 4 + nodl/doc/schema.md | 16 +- nodl/doc/schema_reference.py | 16 +- nodl_schema/nodl_schema/__init__.py | 11 +- nodl_schema/nodl_schema/composition.py | 50 ++++++ nodl_schema/nodl_schema/models.py | 46 +---- nodl_schema/nodl_schema/resolve.py | 87 +++++---- .../lifecycle_node.nodl.yaml | 1 + .../{fragments => bases}/node.nodl.yaml | 1 + .../nodl_schema/schemas/node.schema.yaml | 55 ++++++ .../nodl_schema/schemas/nodl.schema.yaml | 39 +--- nodl_schema/nodl_schema/validator.py | 116 ++++++------ nodl_schema/setup.py | 9 +- nodl_schema/test/test_resolve.py | 168 ++++++++---------- nodl_schema/test/test_schema.py | 50 ++++-- nodl_schema/test/test_validator_cli.py | 61 ++++--- test_ament_nodl/CMakeLists.txt | 11 +- .../test/fixtures/alt_pkg_node.nodl.yaml | 4 +- .../test/fixtures/basic_node.nodl.yaml | 21 ++- .../test/fixtures/extra_telemetry.nodl.yaml | 2 +- .../test/fixtures/json_node.nodl.json | 5 +- .../test/fixtures/tf_listener.nodl.yaml | 2 +- test_ament_nodl/test/test_macro_rejection.py | 8 +- ..._fragment.py => test_register_document.py} | 18 +- 29 files changed, 521 insertions(+), 371 deletions(-) rename ament_nodl/cmake/{ament_nodl_register_fragment.cmake => ament_nodl_register_document.cmake} (51%) create mode 100644 nodl_schema/nodl_schema/composition.py rename nodl_schema/nodl_schema/schemas/{fragments => bases}/lifecycle_node.nodl.yaml (99%) rename nodl_schema/nodl_schema/schemas/{fragments => bases}/node.nodl.yaml (97%) create mode 100644 nodl_schema/nodl_schema/schemas/node.schema.yaml rename test_ament_nodl/test/{test_register_fragment.py => test_register_document.py} (81%) diff --git a/ament_nodl/CMakeLists.txt b/ament_nodl/CMakeLists.txt index 7f3423f..9f491cf 100644 --- a/ament_nodl/CMakeLists.txt +++ b/ament_nodl/CMakeLists.txt @@ -6,7 +6,7 @@ project(ament_nodl) find_package(ament_cmake REQUIRED) install(FILES - cmake/ament_nodl_register_fragment.cmake + cmake/ament_nodl_register_document.cmake cmake/ament_nodl_register_node.cmake DESTINATION share/${PROJECT_NAME}/cmake) diff --git a/ament_nodl/ament_nodl-extras.cmake.in b/ament_nodl/ament_nodl-extras.cmake.in index 88ade3e..fb3e740 100644 --- a/ament_nodl/ament_nodl-extras.cmake.in +++ b/ament_nodl/ament_nodl-extras.cmake.in @@ -1,5 +1,5 @@ # Resolves ${Python3_EXECUTABLE} for the build-time validation step the macros invoke. find_package(Python3 REQUIRED COMPONENTS Interpreter) -include("${CMAKE_CURRENT_LIST_DIR}/ament_nodl_register_fragment.cmake") +include("${CMAKE_CURRENT_LIST_DIR}/ament_nodl_register_document.cmake") include("${CMAKE_CURRENT_LIST_DIR}/ament_nodl_register_node.cmake") diff --git a/ament_nodl/cmake/ament_nodl_register_fragment.cmake b/ament_nodl/cmake/ament_nodl_register_document.cmake similarity index 51% rename from ament_nodl/cmake/ament_nodl_register_fragment.cmake rename to ament_nodl/cmake/ament_nodl_register_document.cmake index 893252b..1f867aa 100644 --- a/ament_nodl/cmake/ament_nodl_register_fragment.cmake +++ b/ament_nodl/cmake/ament_nodl_register_document.cmake @@ -1,28 +1,28 @@ # SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. # SPDX-License-Identifier: Apache-2.0 # -# Register a NoDL fragment in the ament resource index. +# Register a reusable NoDL document in the ament resource index. # -# Publishes the contents of a NoDL file under the ``nodl_fragments`` resource type. +# Publishes the contents of a NoDL document under the ``nodl_documents`` resource type. # The resource key is ``__``. -# NoDL documents reference the fragment by ``nodl:///``. +# NoDL node compositions reference the document as a mixin by ``nodl:///``. # # Consumers retrieve the content via:: # -# ament_index_python.packages.get_resource('nodl_fragments', '__') +# ament_index_python.packages.get_resource('nodl_documents', '__') # -# The source file is also installed under ``share//nodl/fragments/`` for direct filesystem access. +# The source file is also installed under ``share//nodl/documents/`` for direct filesystem access. # # Example:: # -# ament_nodl_register_fragment(tf_listener +# ament_nodl_register_document(tf_listener # FILE nodl/tf_listener.nodl.yaml # ) # -# :param fragment_name: name of the fragment. +# :param document_name: name of the document. # Combined with PACKAGE to form the resource key and the ``nodl:///`` URI. -# :type fragment_name: string -# :param FILE: path to the NoDL file containing the fragment definition. +# :type document_name: string +# :param FILE: path to the NoDL file containing the document. # May be absolute or relative to ``CMAKE_CURRENT_SOURCE_DIR``. # :type FILE: string # :param PACKAGE: package name to use in the resource key. @@ -31,11 +31,11 @@ # # @public # -function(ament_nodl_register_fragment fragment_name) +function(ament_nodl_register_document document_name) cmake_parse_arguments(_ARGS "" "FILE;PACKAGE" "" ${ARGN}) if(NOT _ARGS_FILE) - message(FATAL_ERROR "ament_nodl_register_fragment: FILE is required") + message(FATAL_ERROR "ament_nodl_register_document: FILE is required") endif() if(NOT _ARGS_PACKAGE) set(_ARGS_PACKAGE "${PROJECT_NAME}") @@ -46,35 +46,35 @@ function(ament_nodl_register_fragment fragment_name) if(NOT EXISTS "${_abs_file}") message(WARNING - "ament_nodl_register_fragment: file not found at configure time: ${_abs_file}") + "ament_nodl_register_document: file not found at configure time: ${_abs_file}") endif() - # Validate the fragment at build time so authoring errors surface when registering, not downstream when consuming. - # --fragment additionally rejects nested base/fragments, which are disallowed in v2. + # Validate the document at build time so authoring errors surface when registering, not downstream when consuming. + # The document schema forbids composition keys (base/main/mixins), so a node composition cannot be registered here. # This only runs when ${_abs_file} changes. - set(_stamp_dir "${CMAKE_CURRENT_BINARY_DIR}/ament_nodl/nodl_fragments") - set(_stamp "${_stamp_dir}/${_ARGS_PACKAGE}__${fragment_name}.valid.stamp") + set(_stamp_dir "${CMAKE_CURRENT_BINARY_DIR}/ament_nodl/nodl_documents") + set(_stamp "${_stamp_dir}/${_ARGS_PACKAGE}__${document_name}.valid.stamp") file(MAKE_DIRECTORY "${_stamp_dir}") add_custom_command( OUTPUT "${_stamp}" DEPENDS "${_abs_file}" - COMMAND "${Python3_EXECUTABLE}" -m nodl_schema --fragment "${_abs_file}" + COMMAND "${Python3_EXECUTABLE}" -m nodl_schema "${_abs_file}" COMMAND "${CMAKE_COMMAND}" -E touch "${_stamp}" - COMMENT "Validating NoDL fragment ${_ARGS_PACKAGE}/${fragment_name}" + COMMENT "Validating NoDL document ${_ARGS_PACKAGE}/${document_name}" VERBATIM ) - add_custom_target(_ament_nodl_validate_fragment_${_ARGS_PACKAGE}__${fragment_name} ALL + add_custom_target(_ament_nodl_validate_document_${_ARGS_PACKAGE}__${document_name} ALL DEPENDS "${_stamp}" ) # Install to ament index install( FILES "${_abs_file}" - DESTINATION "share/ament_index/resource_index/nodl_fragments" - RENAME "${_ARGS_PACKAGE}__${fragment_name}") + DESTINATION "share/ament_index/resource_index/nodl_documents" + RENAME "${_ARGS_PACKAGE}__${document_name}") # Install to package's share directory install( FILES "${_abs_file}" - DESTINATION "share/${_ARGS_PACKAGE}/nodl/fragments") + DESTINATION "share/${_ARGS_PACKAGE}/nodl/documents") endfunction() diff --git a/ament_nodl/cmake/ament_nodl_register_node.cmake b/ament_nodl/cmake/ament_nodl_register_node.cmake index 999a3b8..c2a6dcd 100644 --- a/ament_nodl/cmake/ament_nodl_register_node.cmake +++ b/ament_nodl/cmake/ament_nodl_register_node.cmake @@ -1,9 +1,12 @@ # SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. # SPDX-License-Identifier: Apache-2.0 # -# Register a NoDL document for an executable in the ament resource index. +# Register a NoDL node composition for an executable in the ament resource index. # -# Publishes the contents of a NoDL file under the ``nodl_nodes`` resource type. +# The file is a NoDL node (``node.schema.yaml``): a ``base`` + ``mixins`` + ``main`` +# composition describing the executable's whole interface. +# +# Publishes the contents of the file under the ``nodl_nodes`` resource type. # The resource key is ``__``. # Tools like ``nodl_test`` and ``nodl_docgen`` use this to locate the spec by package and executable name. # @@ -57,7 +60,7 @@ function(ament_nodl_register_node executable_name) add_custom_command( OUTPUT "${_stamp}" DEPENDS "${_abs_file}" - COMMAND "${Python3_EXECUTABLE}" -m nodl_schema "${_abs_file}" + COMMAND "${Python3_EXECUTABLE}" -m nodl_schema --node "${_abs_file}" COMMAND "${CMAKE_COMMAND}" -E touch "${_stamp}" COMMENT "Validating NoDL node ${_ARGS_PACKAGE}/${executable_name}" VERBATIM diff --git a/nodl/doc/concepts.md b/nodl/doc/concepts.md index 458b67b..6433dd3 100644 --- a/nodl/doc/concepts.md +++ b/nodl/doc/concepts.md @@ -1,12 +1,17 @@ # NoDL Concepts -A NoDL document declares a ROS 2 node's public interface. +A NoDL **document** declares a ROS 2 node's public interface. It can be consumed: 1. at **runtime** by conformance testing and health monitoring 1. at **build time** by code generators 1. at **documentation time** to describe a node for its users +NoDL distinguishes two things: + +- a **document** — an interface, which may be only _part_ of a node's interface. +- a **node** — a _whole_ node's interface, [composed](#composition-documents-into-nodes) from documents. + ## Node identity A NoDL document does _not_ declare its own name, package, or namespace. @@ -25,6 +30,26 @@ Those interfaces are: See the [schema reference](schema.md) for field-by-field detail. +## Composition: documents into nodes + +A single document is often not the whole story: a node inherits interfaces from its ROS base type and may share reusable interfaces with other nodes. +A NoDL **node** composes a whole interface from three layers: + +- **`base`** — a built-in ROS 2 node type (`node` or `lifecycle_node`) whose interface is inherited. It resolves to a built-in document shipped with `nodl_schema` (e.g. `use_sim_time`, and for lifecycle nodes the state-management services and transition event). +- **`main`** — the interface this node's implementation _owns_, written in place as a document. +- **`mixins`** — additional documents merged in, each either a reference (`nodl:///` for a document registered in the ament index, or a path relative to the node file) or an in-place document. + +Layers merge in order **base → mixins (as listed) → main**; later layers win on a name collision, so `main` always has the final say. + +Mixins are single-level: a referenced document is merged as-is and cannot itself declare a `base` or further `mixins`. The document schema forbids those keys, so this is enforced, not just convention. + +### Forward and backward + +This split lines up with the two [usage models](#usage-models): + +- Working **forward** (code generation), a generator implements `base` + `main` and **ignores `mixins`** — mixins describe interfaces owned by other code, not this node's implementation. +- Working **backward** (observation), `base` can be deduced and the full observed non-base interface placed in `main`. `mixins` cannot be deduced and are simply absent; conformance and documentation still work against the merged result. + ## Syntax & Frontend(s) The schema is JSON Schema. @@ -33,9 +58,14 @@ Tools should not assume any particular extension (such as `.nodl.yaml`) or a sin ## Validation -NoDL files are validated both at build time (by the `ament_nodl` CMake macros, before install) and at runtime (by `nodl_schema.load_nodl`). +NoDL files are validated both at build time (by the `ament_nodl` CMake macros, before install) and at runtime (by `nodl_schema.load_nodl` / `load_node`). Authoring errors surface during the build of the package that owns the file, not at runtime, so a misconfigured node never ships. +The `ament_nodl` package registers files into the ament index and validates them as it does: + +- `ament_nodl_register_node( FILE ...)` registers a node composition (under the `nodl_nodes` resource type) for an executable. +- `ament_nodl_register_document( FILE ...)` registers a reusable document (under `nodl_documents`) that other nodes pull in as a mixin via `nodl:///`. + ## Usage Models The NoDL project aims to support two directional modes of use for NoDL documents, which it calls "forward" and "backward". diff --git a/nodl/doc/conf.py b/nodl/doc/conf.py index cd6fabf..abef967 100644 --- a/nodl/doc/conf.py +++ b/nodl/doc/conf.py @@ -48,6 +48,10 @@ 'tasklist', ] +# Generate slug anchors for headings (h1-h3) so `[...](#slug)` and +# `[...](other.md#slug)` cross-references resolve. +myst_heading_anchors = 3 + source_suffix = { '.md': 'markdown', '.rst': 'restructuredtext', diff --git a/nodl/doc/schema.md b/nodl/doc/schema.md index 48ac3f8..57f7ffc 100644 --- a/nodl/doc/schema.md +++ b/nodl/doc/schema.md @@ -1,6 +1,11 @@ # NoDL Schema reference -The following reference is generated from {repo}`nodl_schema/nodl_schema/schemas/nodl.schema.yaml`, which is the canonical source. +NoDL has two schemas, both generated below from their canonical sources: + +- a **node** ({repo}`nodl_schema/nodl_schema/schemas/node.schema.yaml`) -- a whole-node composition of `base` + `mixins` + `main`. +- a **document** ({repo}`nodl_schema/nodl_schema/schemas/nodl.schema.yaml`) -- a (possibly partial) node interface, used for each composition layer. + +See [Concepts](concepts.md#composition-documents-into-nodes) for how the two relate. ## Schema Version @@ -8,6 +13,15 @@ The NoDL schema is [JSON Schema Draft 7](https://json-schema.org/draft-07). This was chosen chosen to trivially support all live ROS 2 distributions - the key limitation being the system packages available on Ubuntu 22.04 Jammy with ROS 2 Humble. After Humble EOL in May 2027, we will consider updating to a newer JSON Schema draft version. +## Node composition + +A node composes a whole interface from a built-in `base`, the node's own `main` document, and zero or more `mixins`. `main` is a [node document](#node-document); each mixin is a reference (`nodl:///` or a relative path) or an in-place document. + +```{eval-rst} +.. json:schema:: Node + :title: NoDL node +``` + ## Node document ```{eval-rst} diff --git a/nodl/doc/schema_reference.py b/nodl/doc/schema_reference.py index edc1dff..5fdf225 100644 --- a/nodl/doc/schema_reference.py +++ b/nodl/doc/schema_reference.py @@ -15,6 +15,7 @@ SCHEMA_GLOB = '_generated/schemas/*.yaml' _DEFINITION_REF = re.compile(r'#/definitions/(?P\w+)$') +_FILE_REF = re.compile(r'^(?P\w+)\.schema\.yaml$') def _short_id(name: str) -> str: @@ -23,13 +24,20 @@ def _short_id(name: str) -> str: def _rewrite_refs(node, ref_map: dict) -> None: - """Replace every ``#/definitions/`` $ref with its short id, in place.""" + """Rewrite each $ref to a short display id, in place. + + Handles both ``#/definitions/`` (a definition in any schema) and a + whole-file ref like ``nodl.schema.yaml`` (e.g. the node schema's ``main``). + """ if isinstance(node, dict): for key, value in node.items(): if key == '$ref' and isinstance(value, str): - match = _DEFINITION_REF.search(value) - if match: - node[key] = ref_map[match.group('name')] + definition = _DEFINITION_REF.search(value) + whole_file = _FILE_REF.match(value) + if definition: + node[key] = ref_map[definition.group('name')] + elif whole_file: + node[key] = _short_id(whole_file.group('stem')) else: _rewrite_refs(value, ref_map) elif isinstance(node, list): diff --git a/nodl_schema/nodl_schema/__init__.py b/nodl_schema/nodl_schema/__init__.py index f29a678..b8db117 100644 --- a/nodl_schema/nodl_schema/__init__.py +++ b/nodl_schema/nodl_schema/__init__.py @@ -2,23 +2,26 @@ # SPDX-License-Identifier: Apache-2.0 """NoDL schema, in-memory models, and validation helpers.""" +from nodl_schema.composition import Base, Node from nodl_schema.resolve import LayeredDocument, resolve from nodl_schema.validator import ( dump_nodl, - load_fragment, + load_node, load_nodl, load_schema, validate, - validate_fragment, + validate_node, ) __all__ = [ + 'Base', 'LayeredDocument', + 'Node', 'dump_nodl', - 'load_fragment', + 'load_node', 'load_nodl', 'load_schema', 'resolve', 'validate', - 'validate_fragment', + 'validate_node', ] diff --git a/nodl_schema/nodl_schema/composition.py b/nodl_schema/nodl_schema/composition.py new file mode 100644 index 0000000..aefc32d --- /dev/null +++ b/nodl_schema/nodl_schema/composition.py @@ -0,0 +1,50 @@ +# SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. +# SPDX-License-Identifier: Apache-2.0 +"""Models for the Node composition schema (``node.schema.yaml``). + +These are hand-written rather than emitted into the generated ``models.py`` +because the Node schema's ``main``/``mixins`` reference the document schema +across files, which datamodel-codegen can only express as a multi-module +package -- not the single ``models.py`` the rest of nodl_schema imports. Keep +this in sync with ``node.schema.yaml`` by hand; it is a small, stable surface. +""" + +from __future__ import annotations + +from enum import Enum +from typing import Optional, Union + +try: + from pydantic.v1 import BaseModel, Extra, Field +except ImportError: + from pydantic import BaseModel, Extra, Field + +from nodl_schema.models import NodlDocument + + +class Base(Enum): + """Built-in ROS 2 base node type whose interface a node inherits.""" + + node = 'node' + lifecycle_node = 'lifecycle_node' + + +class Node(BaseModel): + """A NoDL Node: a whole ROS 2 node interface composed from base + mixins + main.""" + + class Config: + extra = Extra.forbid + + nodl_version: int = Field(2, const=True, description='NoDL schema major version this document targets.') + base: Optional[Base] = Field( + None, + description='Built-in ROS 2 base node type whose interface this node inherits.', + ) + main: NodlDocument = Field( + ..., + description="The interface this node's implementation owns, as an in-place NoDL document.", + ) + mixins: Optional[list[Union[str, NodlDocument]]] = Field( + None, + description='NoDL documents merged in for documentation and conformance; ignored by code generation.', + ) diff --git a/nodl_schema/nodl_schema/models.py b/nodl_schema/nodl_schema/models.py index 146cbb7..34268a4 100644 --- a/nodl_schema/nodl_schema/models.py +++ b/nodl_schema/nodl_schema/models.py @@ -14,15 +14,6 @@ from pydantic import BaseModel, Extra, Field, conint, constr -class Base(Enum): - """ - Built-in ROS 2 base node type this node inherits from. - """ - - node = 'node' - lifecycle_node = 'lifecycle_node' - - class ScalarType(Enum): """ Scalar types: @@ -57,24 +48,6 @@ class ArrayType(Enum): string_array = 'string_array' -class FragmentRef(BaseModel): - """ - Reference to a NoDL fragment. - """ - - class Config: - extra = Extra.forbid - - ref: str = Field( - ..., - description="'nodl://pkg/name' for ament-index fragments, or a relative file path.\n", - ) - name: Optional[str] = Field( - None, - description='Optional label for this fragment in documentation and merge order.', - ) - - class History(Enum): """ History policy. @@ -442,13 +415,15 @@ class Config: class NodlDocument(BaseModel): """ - NoDL (Node Definition Language) v2 schema. + NoDL (Node Definition Language) v2 document schema. - Describes the public ROS interface of a single ROS 2 node: - parameters, publishers, subscriptions, service servers/clients, - and action servers/clients. Field names align with ``rcl_interfaces`` - and ``rosgraph_msgs`` (``Topic``, ``Service``, ``Action``) so a NoDL document - maps cleanly onto runtime introspection types. + Describes a ROS 2 node interface -- but not necessarily a *whole* + node's interface: parameters, publishers, subscriptions, service + servers/clients, and action servers/clients. A document may be + partial; whole nodes are assembled from documents by the Node schema + (``node.schema.yaml``) via base + mixins + main. Field names align + with ``rcl_interfaces`` and ``rosgraph_msgs`` (``Topic``, ``Service``, + ``Action``) so a document maps cleanly onto runtime introspection types. Node identity (name, package, namespace) is established by the enclosing package and the file's location within it, not declared @@ -461,11 +436,6 @@ class Config: nodl_version: int = Field(2, const=True, description='NoDL schema major version this document targets.') description: Optional[str] = Field(None, description='Human-readable description of what this node does.') - base: Optional[Base] = Field(None, description='Built-in ROS 2 base node type this node inherits from.') - fragments: Optional[list[FragmentRef]] = Field( - None, - description='NoDL fragments composed into this node (single-level, not recursive).', - ) parameters: Optional[dict[str, ParameterDefinition]] = Field( None, description='ROS parameters declared by this node, keyed by parameter name.\nParameter shape is borrowed from ``generate_parameter_library``\n(see ``parameter.schema.yaml``).\n', diff --git a/nodl_schema/nodl_schema/resolve.py b/nodl_schema/nodl_schema/resolve.py index b413410..fb91bec 100644 --- a/nodl_schema/nodl_schema/resolve.py +++ b/nodl_schema/nodl_schema/resolve.py @@ -1,6 +1,6 @@ # SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. # SPDX-License-Identifier: Apache-2.0 -"""Resolution of NoDL composition (base + fragments) into a LayeredDocument.""" +"""Resolution of NoDL node composition (base + mixins + main) into a LayeredDocument.""" from __future__ import annotations @@ -9,6 +9,7 @@ from pathlib import Path from typing import Dict, List, Optional +from nodl_schema.composition import Node from nodl_schema.models import ( ActionEndpoint, NodlDocument, @@ -17,70 +18,69 @@ TopicEndpoint, ) -_BUILTIN_BASES = frozenset(['node', 'lifecycle_node']) - def _load_builtin(base: str) -> NodlDocument: - from nodl_schema.validator import load_fragment + from nodl_schema.validator import load_nodl - path = ir.files('nodl_schema') / 'schemas' / 'fragments' / f'{base}.nodl.yaml' - return load_fragment(path.read_text(encoding='utf-8')) + path = ir.files('nodl_schema') / 'schemas' / 'bases' / f'{base}.nodl.yaml' + return load_nodl(path.read_text(encoding='utf-8')) -def _load_ament_fragment(pkg: str, name: str) -> NodlDocument: +def _load_ament_document(pkg: str, name: str) -> NodlDocument: try: from ament_index_python.packages import get_resource except ImportError as exc: raise ImportError('ament_index_python is required to resolve nodl:// URIs') from exc - # Key format mirrors ament_nodl_register_fragment: pkg__name + # Key format mirrors ament_nodl_register_document: pkg__name resource_key = f'{pkg}__{name}' try: - content, _ = get_resource('nodl_fragments', resource_key) + content, _ = get_resource('nodl_documents', resource_key) except KeyError: raise FileNotFoundError( - f'NoDL fragment not found in ament index: {pkg}/{name} (looked up as nodl_fragments/{resource_key})' + f'NoDL document not found in ament index: {pkg}/{name} (looked up as nodl_documents/{resource_key})' ) - from nodl_schema.validator import load_fragment + from nodl_schema.validator import load_nodl - return load_fragment(content) + return load_nodl(content) def _resolve_ref(ref: str, source_path: Optional[Path] = None) -> NodlDocument: + """Resolve a mixin reference string to a NoDL document.""" if ref.startswith('nodl://'): rest = ref[len('nodl://') :] parts = rest.split('/', 1) if len(parts) != 2: raise ValueError(f'Invalid nodl:// URI: {ref!r} -- expected nodl://pkg/name') - return _load_ament_fragment(parts[0], parts[1]) + return _load_ament_document(parts[0], parts[1]) if source_path is None: - raise ValueError(f'Cannot resolve relative fragment ref {ref!r} without a source path') + raise ValueError(f'Cannot resolve relative mixin ref {ref!r} without a source path') full_path = (source_path.parent / ref).resolve() if not full_path.exists(): - raise FileNotFoundError(f'Fragment file not found: {full_path}') - from nodl_schema.validator import load_fragment + raise FileNotFoundError(f'Mixin document not found: {full_path}') + from nodl_schema.validator import load_nodl - return load_fragment(full_path.read_text(encoding='utf-8')) + return load_nodl(full_path.read_text(encoding='utf-8')) @dataclass class LayeredDocument: - """A NoDL document with its resolved composition layers, keyed by label.""" + """A NoDL node with its resolved composition layers.""" main: NodlDocument base: Optional[NodlDocument] = None base_name: Optional[str] = None - fragments: Dict[str, NodlDocument] = field(default_factory=dict) + mixins: List[NodlDocument] = field(default_factory=list) def merged(self) -> NodlDocument: """Merge all layers into a flat NodlDocument. - Layers applied in order: base -> named fragments (insertion order) -> main. + Layers applied in order: base -> mixins (declared order) -> main. Later layers win on duplicate names. """ layers: List[NodlDocument] = [] if self.base is not None: layers.append(self.base) - layers.extend(self.fragments.values()) + layers.extend(self.mixins) layers.append(self.main) publishers: Dict[str, TopicEndpoint] = {} @@ -119,32 +119,25 @@ def merged(self) -> NodlDocument: ) -def resolve(doc: NodlDocument, source_path: Optional[Path] = None) -> LayeredDocument: - """Resolve a NodlDocument's base and fragments into a LayeredDocument. +def resolve(node: Node, source_path: Optional[Path] = None) -> LayeredDocument: + """Resolve a Node's base and mixins into a LayeredDocument. - source_path: filesystem path to the NoDL file, used to resolve relative refs. - Fragments are single-level only -- fragment files are not themselves resolved. + source_path: filesystem path to the node file, used to resolve relative + mixin refs. Mixins are single-level only -- referenced documents are not + themselves resolved (a document cannot declare base/mixins). """ - # doc.base is an enum on pydantic v2 and may be either enum or string on v1 - # depending on how the document was constructed. Normalize to the string value. + # node.base is an enum on pydantic v2 and may be enum or str on v1. base_name: Optional[str] = None - if doc.base is not None: - base_name = doc.base.value if hasattr(doc.base, 'value') else doc.base - - base_doc: Optional[NodlDocument] = None - if base_name is not None: - if base_name not in _BUILTIN_BASES: - raise ValueError(f'Unknown base {base_name!r}. Must be one of: {sorted(_BUILTIN_BASES)}') - base_doc = _load_builtin(base_name) - - fragment_docs: Dict[str, NodlDocument] = {} - for frag_ref in doc.fragments or []: - label = frag_ref.name or frag_ref.ref - fragment_docs[label] = _resolve_ref(frag_ref.ref, source_path=source_path) - - return LayeredDocument( - main=doc, - base=base_doc, - base_name=base_name, - fragments=fragment_docs, - ) + if node.base is not None: + base_name = node.base.value if hasattr(node.base, 'value') else node.base + + base_doc = _load_builtin(base_name) if base_name is not None else None + + mixin_docs: List[NodlDocument] = [] + for mixin in node.mixins or []: + if isinstance(mixin, NodlDocument): + mixin_docs.append(mixin) + else: + mixin_docs.append(_resolve_ref(mixin, source_path=source_path)) + + return LayeredDocument(main=node.main, base=base_doc, base_name=base_name, mixins=mixin_docs) diff --git a/nodl_schema/nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml b/nodl_schema/nodl_schema/schemas/bases/lifecycle_node.nodl.yaml similarity index 99% rename from nodl_schema/nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml rename to nodl_schema/nodl_schema/schemas/bases/lifecycle_node.nodl.yaml index e0582ec..9acac33 100644 --- a/nodl_schema/nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml +++ b/nodl_schema/nodl_schema/schemas/bases/lifecycle_node.nodl.yaml @@ -1,3 +1,4 @@ +--- nodl_version: 2 parameters: use_sim_time: diff --git a/nodl_schema/nodl_schema/schemas/fragments/node.nodl.yaml b/nodl_schema/nodl_schema/schemas/bases/node.nodl.yaml similarity index 97% rename from nodl_schema/nodl_schema/schemas/fragments/node.nodl.yaml rename to nodl_schema/nodl_schema/schemas/bases/node.nodl.yaml index 1f5e741..ca8d459 100644 --- a/nodl_schema/nodl_schema/schemas/fragments/node.nodl.yaml +++ b/nodl_schema/nodl_schema/schemas/bases/node.nodl.yaml @@ -1,3 +1,4 @@ +--- nodl_version: 2 parameters: use_sim_time: diff --git a/nodl_schema/nodl_schema/schemas/node.schema.yaml b/nodl_schema/nodl_schema/schemas/node.schema.yaml new file mode 100644 index 0000000..87ab0e2 --- /dev/null +++ b/nodl_schema/nodl_schema/schemas/node.schema.yaml @@ -0,0 +1,55 @@ +--- +$schema: http://json-schema.org/draft-07/schema# +$id: node.schema.yaml +title: NoDL Node +description: | + A NoDL Node composes a *whole* ROS 2 node interface from layers: + + - ``base`` -- an optional built-in ROS 2 node type whose interface is inherited. + - ``main`` -- the interface this node's implementation owns (a NoDL document). + - ``mixins`` -- zero or more additional NoDL documents merged in by reference + or in place. + + Code generation works *forward* from ``base`` + ``main`` and ignores + ``mixins``. Documentation and conformance testing use the fully merged + interface (base, then mixins in order, then main; later layers win). + + When working *backward* from an observed node, ``base`` can be deduced and + the full non-base interface placed in ``main``; ``mixins`` cannot be deduced + and are simply absent. +type: object +additionalProperties: false +required: + - nodl_version + - main +properties: + nodl_version: + description: NoDL schema major version this document targets. + const: 2 + + base: + type: string + enum: [node, lifecycle_node] + description: | + Built-in ROS 2 base node type whose interface this node inherits. + Resolves to a built-in NoDL document shipped with ``nodl_schema``. + + main: + description: | + The interface this node's implementation owns, as an in-place NoDL + document. Code generation implements exactly this layer. + $ref: nodl.schema.yaml + + mixins: + type: array + description: | + Additional NoDL documents merged into this node's interface for + documentation and conformance, but ignored by code generation. + Each entry is either a reference -- an ament-index locator + ``nodl:///`` or a path relative to this file -- or an + in-place NoDL document (an escape hatch when no registered document exists). + items: + oneOf: + - type: string + description: Reference to a NoDL document -- ``nodl:///`` or a relative path. + - $ref: nodl.schema.yaml diff --git a/nodl_schema/nodl_schema/schemas/nodl.schema.yaml b/nodl_schema/nodl_schema/schemas/nodl.schema.yaml index d6712b1..8816a21 100644 --- a/nodl_schema/nodl_schema/schemas/nodl.schema.yaml +++ b/nodl_schema/nodl_schema/schemas/nodl.schema.yaml @@ -3,13 +3,15 @@ $schema: http://json-schema.org/draft-07/schema# $id: nodl.schema.yaml title: NoDL Node Definition description: | - NoDL (Node Definition Language) v2 schema. + NoDL (Node Definition Language) v2 document schema. - Describes the public ROS interface of a single ROS 2 node: - parameters, publishers, subscriptions, service servers/clients, - and action servers/clients. Field names align with ``rcl_interfaces`` - and ``rosgraph_msgs`` (``Topic``, ``Service``, ``Action``) so a NoDL document - maps cleanly onto runtime introspection types. + Describes a ROS 2 node interface -- but not necessarily a *whole* + node's interface: parameters, publishers, subscriptions, service + servers/clients, and action servers/clients. A document may be + partial; whole nodes are assembled from documents by the Node schema + (``node.schema.yaml``) via base + mixins + main. Field names align + with ``rcl_interfaces`` and ``rosgraph_msgs`` (``Topic``, ``Service``, + ``Action``) so a document maps cleanly onto runtime introspection types. Node identity (name, package, namespace) is established by the enclosing package and the file's location within it, not declared @@ -28,17 +30,6 @@ properties: type: string description: Human-readable description of what this node does. - base: - type: string - enum: [node, lifecycle_node] - description: Built-in ROS 2 base node type this node inherits from. - - fragments: - type: array - description: NoDL fragments composed into this node (single-level, not recursive). - items: - $ref: '#/definitions/fragment_ref' - parameters: type: object description: | @@ -85,20 +76,6 @@ properties: $ref: '#/definitions/action_endpoint' definitions: - fragment_ref: - type: object - description: Reference to a NoDL fragment. - required: [ref] - additionalProperties: false - properties: - ref: - type: string - description: | - 'nodl://pkg/name' for ament-index fragments, or a relative file path. - name: - type: string - description: Optional label for this fragment in documentation and merge order. - rosName: type: string description: | diff --git a/nodl_schema/nodl_schema/validator.py b/nodl_schema/nodl_schema/validator.py index a95cb14..8af6cda 100644 --- a/nodl_schema/nodl_schema/validator.py +++ b/nodl_schema/nodl_schema/validator.py @@ -1,6 +1,13 @@ # SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. # SPDX-License-Identifier: Apache-2.0 -"""NoDL schema loading, validation, and serialization.""" +"""NoDL schema loading, validation, and serialization. + +Two schemas are validated here: + +* a NoDL **document** (``nodl.schema.yaml``) -- a possibly-partial node interface. +* a NoDL **node** (``node.schema.yaml``) -- a whole-node composition of + ``base`` + ``mixins`` + ``main``, where each layer is a document. +""" from __future__ import annotations @@ -13,10 +20,12 @@ from jsonschema import RefResolver from jsonschema.validators import Draft7Validator +from nodl_schema.composition import Node from nodl_schema.models import NodlDocument +_document_validator: Draft7Validator | None = None +_node_validator: Draft7Validator | None = None _schema_cache: dict | None = None -_validator_cache: Draft7Validator | None = None def _load_resource(name: str) -> dict: @@ -25,59 +34,51 @@ def _load_resource(name: str) -> dict: def load_schema() -> dict: - """Load and cache the NoDL JSON schema.""" + """Load and cache the NoDL document JSON schema.""" global _schema_cache if _schema_cache is None: _schema_cache = _load_resource('nodl.schema.yaml') return _schema_cache -def _make_validator() -> Draft7Validator: - """Build a validator with the parameter schema pre-loaded so $refs resolve.""" - global _validator_cache - if _validator_cache is None: - schema = load_schema() - param_schema = _load_resource('parameter.schema.yaml') - store = { - 'parameter.schema.yaml': param_schema, - param_schema.get('$id', ''): param_schema, - } - resolver = RefResolver.from_schema(schema, store=store) - _validator_cache = Draft7Validator(schema, resolver=resolver) - return _validator_cache +def _resource_store() -> dict: + """Build a $ref store so cross-file refs (node -> document -> parameter) resolve.""" + document = load_schema() + parameter = _load_resource('parameter.schema.yaml') + return { + 'nodl.schema.yaml': document, + document.get('$id', ''): document, + 'parameter.schema.yaml': parameter, + parameter.get('$id', ''): parameter, + } + + +def _make_validator(schema: dict) -> Draft7Validator: + store = _resource_store() + resolver = RefResolver.from_schema(schema, store=store) + return Draft7Validator(schema, resolver=resolver) def validate(data: dict) -> None: - """Validate a plain dict against the NoDL JSON schema. + """Validate a plain dict against the NoDL document schema. Raises jsonschema.ValidationError on failure. """ - _make_validator().validate(data) + global _document_validator + if _document_validator is None: + _document_validator = _make_validator(load_schema()) + _document_validator.validate(data) -# Top-level keys that a fragment document must not declare. -# Fragments are flat ingredients; nested composition is intentionally disallowed in v2. -_FRAGMENT_FORBIDDEN_KEYS = ('base', 'fragments') +def validate_node(data: dict) -> None: + """Validate a plain dict against the NoDL node (composition) schema. - -def validate_fragment(data: dict) -> None: - """Validate a NoDL fragment. - - A fragment must pass the standard schema and additionally must not declare - a ``base`` or ``fragments`` key. - Nested fragment composition is intentionally disallowed in v2; if a real - use case emerges, the constraint can be lifted without a schema change. - Raises jsonschema.ValidationError on schema failure or ValueError on a - forbidden key. + Raises jsonschema.ValidationError on failure. """ - validate(data) - forbidden = [k for k in _FRAGMENT_FORBIDDEN_KEYS if k in data] - if forbidden: - raise ValueError( - 'NoDL fragment must not declare ' - + ', '.join(repr(k) for k in forbidden) - + '; nested fragment composition is not supported in v2.' - ) + global _node_validator + if _node_validator is None: + _node_validator = _make_validator(_load_resource('node.schema.yaml')) + _node_validator.validate(data) def load_nodl(source: Union[str, bytes, IO]) -> NodlDocument: @@ -87,29 +88,27 @@ def load_nodl(source: Union[str, bytes, IO]) -> NodlDocument: Raises jsonschema.ValidationError on schema error or pydantic.ValidationError on type error. """ - return _load(source, validate) + return NodlDocument.parse_obj(_load(source, validate)) -def load_fragment(source: Union[str, bytes, IO]) -> NodlDocument: - """Load and validate a NoDL fragment document. +def load_node(source: Union[str, bytes, IO]) -> Node: + """Load and validate a NoDL node (composition) document. - Same as load_nodl but enforces the no-base / no-fragments constraint. + Same input handling as load_nodl, but validates and parses the + base/main/mixins composition schema. """ - return _load(source, validate_fragment) + return Node.parse_obj(_load(source, validate_node)) -def _load(source, validator) -> NodlDocument: +def _load(source, validator) -> dict: data = yaml.safe_load(source) if not isinstance(data, dict): raise ValueError('NoDL document must be a YAML/JSON mapping at the top level') validator(data) - # parse_obj is pydantic v1 API, retained as a deprecated alias in v2. - # Used so this module works against both rosdep-shipped pydantic v1 - # (humble/jazzy/kilted) and v2 (lyrical+). - return NodlDocument.parse_obj(data) + return data -def _to_plain_dict(doc: NodlDocument) -> dict: +def _to_plain_dict(doc: Union[NodlDocument, Node]) -> dict: """Serialize a model to a JSON-compatible dict that drops Nones and unwraps enums. Goes via .json() so the result is a plain dict on both pydantic v1 and v2; @@ -118,9 +117,9 @@ def _to_plain_dict(doc: NodlDocument) -> dict: return json.loads(doc.json(exclude_none=True)) -def dump_nodl(doc: Union[NodlDocument, dict], *, format: str = 'yaml') -> str: - """Serialize a NodlDocument (or plain dict) to YAML or JSON string.""" - data = _to_plain_dict(doc) if isinstance(doc, NodlDocument) else doc +def dump_nodl(doc: Union[NodlDocument, Node, dict], *, format: str = 'yaml') -> str: + """Serialize a NodlDocument or Node (or plain dict) to a YAML or JSON string.""" + data = _to_plain_dict(doc) if not isinstance(doc, dict) else doc if format == 'json': return json.dumps(data, indent=2) return yaml.dump(data, default_flow_style=False, allow_unicode=True) @@ -131,7 +130,8 @@ def main(argv: list[str] | None = None) -> int: Exits 0 on success, 1 on validation failure or I/O error. Designed for invocation from CMake macros (ament_nodl_register_node and - siblings) so files are checked at build time, not at runtime. + ament_nodl_register_document) so files are checked at build time, not at + runtime. """ import argparse import sys @@ -142,16 +142,16 @@ def main(argv: list[str] | None = None) -> int: ) parser.add_argument('file', type=Path, help='Path to the NoDL file to validate.') parser.add_argument( - '--fragment', + '--node', action='store_true', - help='Validate the file as a fragment (rejects nested base/fragments).', + help='Validate the file as a NoDL node composition (base/main/mixins) rather than a document.', ) args = parser.parse_args(argv) try: with args.file.open('r') as f: - if args.fragment: - load_fragment(f) + if args.node: + load_node(f) else: load_nodl(f) except Exception as exc: diff --git a/nodl_schema/setup.py b/nodl_schema/setup.py index 272ec32..c569140 100644 --- a/nodl_schema/setup.py +++ b/nodl_schema/setup.py @@ -8,22 +8,23 @@ name=package_name, version='0.0.0', packages=[package_name], - package_data={package_name: ['schemas/*.yaml', 'schemas/fragments/*.yaml']}, + package_data={package_name: ['schemas/*.yaml', 'schemas/bases/*.yaml']}, data_files=[ ('share/ament_index/resource_index/packages', ['resource/' + package_name]), ('share/' + package_name, ['package.xml']), ( 'share/' + package_name + '/schemas', [ + 'nodl_schema/schemas/node.schema.yaml', 'nodl_schema/schemas/nodl.schema.yaml', 'nodl_schema/schemas/parameter.schema.yaml', ], ), ( - 'share/' + package_name + '/schemas/fragments', + 'share/' + package_name + '/schemas/bases', [ - 'nodl_schema/schemas/fragments/node.nodl.yaml', - 'nodl_schema/schemas/fragments/lifecycle_node.nodl.yaml', + 'nodl_schema/schemas/bases/node.nodl.yaml', + 'nodl_schema/schemas/bases/lifecycle_node.nodl.yaml', ], ), ], diff --git a/nodl_schema/test/test_resolve.py b/nodl_schema/test/test_resolve.py index 2321fce..b27070d 100644 --- a/nodl_schema/test/test_resolve.py +++ b/nodl_schema/test/test_resolve.py @@ -9,28 +9,33 @@ import pytest -from nodl_schema.models import FragmentRef, NodlDocument, ParameterDefinition, QosProfile, TopicEndpoint +from nodl_schema.composition import Node +from nodl_schema.models import NodlDocument, ParameterDefinition, QosProfile, TopicEndpoint from nodl_schema.resolve import resolve _SYS_QOS = QosProfile(history='SYSTEM_DEFAULT', reliability='SYSTEM_DEFAULT') +def _node(**kwargs) -> Node: + """Build a Node, defaulting main to an empty document.""" + kwargs.setdefault('main', NodlDocument(nodl_version=2)) + return Node(nodl_version=2, **kwargs) + + # --------------------------------------------------------------------------- # Built-in base resolution # --------------------------------------------------------------------------- -def test_resolve_no_base_or_fragments(): - doc = NodlDocument(nodl_version=2) - layered = resolve(doc) +def test_resolve_no_base_or_mixins(): + layered = resolve(_node()) assert layered.base is None - assert layered.fragments == {} + assert layered.mixins == [] assert layered.merged() == NodlDocument(nodl_version=2) def test_resolve_base_node_loads_use_sim_time(): - doc = NodlDocument(nodl_version=2, base='node') - layered = resolve(doc) + layered = resolve(_node(base='node')) assert layered.base is not None assert layered.base_name == 'node' merged = layered.merged() @@ -41,19 +46,18 @@ def test_resolve_base_node_loads_use_sim_time(): def test_resolve_base_lifecycle_node_has_use_sim_time(): - doc = NodlDocument(nodl_version=2, base='lifecycle_node') - merged = resolve(doc).merged() + merged = resolve(_node(base='lifecycle_node')).merged() assert 'use_sim_time' in (merged.parameters or {}) def test_resolve_base_lifecycle_node_has_change_state_service(): - merged = resolve(NodlDocument(nodl_version=2, base='lifecycle_node')).merged() + merged = resolve(_node(base='lifecycle_node')).merged() names = [s.name for s in (merged.service_servers or [])] assert '~/change_state' in names def test_resolve_base_lifecycle_node_has_transition_event_publisher(): - merged = resolve(NodlDocument(nodl_version=2, base='lifecycle_node')).merged() + merged = resolve(_node(base='lifecycle_node')).merged() topics = [p.name for p in (merged.publishers or [])] assert '~/transition_event' in topics @@ -61,17 +65,17 @@ def test_resolve_base_lifecycle_node_has_transition_event_publisher(): def test_resolve_unknown_base_raises(): # Pydantic rejects unknown bases at model construction time. with pytest.raises(Exception): - NodlDocument(nodl_version=2, base='unknown_base') + _node(base='unknown_base') # --------------------------------------------------------------------------- -# Fragment resolution -- relative path +# Mixin resolution -- references and in-place documents # --------------------------------------------------------------------------- -def test_resolve_relative_fragment(tmp_path: Path): - frag_file = tmp_path / 'my_frag.nodl.yaml' - frag_file.write_text( +def test_resolve_relative_mixin(tmp_path: Path): + mixin_file = tmp_path / 'my_mixin.nodl.yaml' + mixin_file.write_text( textwrap.dedent("""\ nodl_version: 2 publishers: @@ -83,44 +87,46 @@ def test_resolve_relative_fragment(tmp_path: Path): """) ) - doc = NodlDocument( - nodl_version=2, - fragments=[FragmentRef(ref='my_frag.nodl.yaml', name='extra')], - ) - layered = resolve(doc, source_path=tmp_path / 'root.nodl.yaml') - assert 'extra' in layered.fragments + node = _node(mixins=['my_mixin.nodl.yaml']) + layered = resolve(node, source_path=tmp_path / 'root.nodl.yaml') + assert len(layered.mixins) == 1 topics = [p.name for p in (layered.merged().publishers or [])] assert '/extra' in topics -def test_resolve_relative_fragment_missing_raises(tmp_path: Path): - doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='nonexistent.nodl.yaml')]) - with pytest.raises(FileNotFoundError): - resolve(doc, source_path=tmp_path / 'root.nodl.yaml') +def test_resolve_inplace_mixin_document(): + # The escape hatch: a mixin may be an in-place NoDL document instead of a ref. + node = _node( + mixins=[ + NodlDocument( + nodl_version=2, publishers=[TopicEndpoint(name='/inline', type='std_msgs/msg/String', qos=_SYS_QOS)] + ) + ] + ) + merged = resolve(node).merged() + assert '/inline' in {p.name for p in (merged.publishers or [])} -def test_resolve_relative_fragment_without_source_raises(): - doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='./something.nodl.yaml')]) - with pytest.raises(ValueError, match='source path'): - resolve(doc) +def test_resolve_relative_mixin_missing_raises(tmp_path: Path): + node = _node(mixins=['nonexistent.nodl.yaml']) + with pytest.raises(FileNotFoundError): + resolve(node, source_path=tmp_path / 'root.nodl.yaml') -def test_resolve_fragment_declaring_base_raises(tmp_path: Path): - # A fragment may not declare base; nested composition is disallowed in v2. - frag = tmp_path / 'frag.nodl.yaml' - frag.write_text('nodl_version: 2\nbase: node\n') - doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='frag.nodl.yaml')]) - with pytest.raises(ValueError, match="'base'"): - resolve(doc, source_path=tmp_path / 'root.nodl.yaml') +def test_resolve_relative_mixin_without_source_raises(): + node = _node(mixins=['./something.nodl.yaml']) + with pytest.raises(ValueError, match='source path'): + resolve(node) -def test_resolve_fragment_declaring_fragments_raises(tmp_path: Path): - # A fragment may not declare its own fragments either. - frag = tmp_path / 'frag.nodl.yaml' - frag.write_text('nodl_version: 2\nfragments:\n - ref: nodl://pkg/other\n') - doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='frag.nodl.yaml')]) - with pytest.raises(ValueError, match="'fragments'"): - resolve(doc, source_path=tmp_path / 'root.nodl.yaml') +def test_resolve_mixin_declaring_composition_keys_rejected(tmp_path: Path): + # A mixin document is validated as a NoDL document, which forbids composition + # keys -- so a referenced file that declares base/main/mixins is rejected. + mixin = tmp_path / 'bad.nodl.yaml' + mixin.write_text('nodl_version: 2\nmain:\n nodl_version: 2\n') + node = _node(mixins=['bad.nodl.yaml']) + with pytest.raises(Exception): + resolve(node, source_path=tmp_path / 'root.nodl.yaml') # --------------------------------------------------------------------------- @@ -130,73 +136,51 @@ def test_resolve_fragment_declaring_fragments_raises(tmp_path: Path): def test_merged_main_wins_over_base(): """Main document's parameter overrides the base's.""" - doc = NodlDocument( - nodl_version=2, + node = _node( base='node', - parameters={'use_sim_time': ParameterDefinition(type='bool', default_value=True)}, + main=NodlDocument( + nodl_version=2, + parameters={'use_sim_time': ParameterDefinition(type='bool', default_value=True)}, + ), ) - merged = resolve(doc).merged() + merged = resolve(node).merged() # Main sets default_value=True; base has default_value=False assert merged.parameters['use_sim_time'].default_value is True -def test_merged_fragment_wins_over_base(tmp_path: Path): - frag_file = tmp_path / 'frag.nodl.yaml' - frag_file.write_text( - textwrap.dedent("""\ - nodl_version: 2 - parameters: - use_sim_time: - type: bool - default_value: true - """) - ) - doc = NodlDocument( - nodl_version=2, - base='node', - fragments=[FragmentRef(ref='frag.nodl.yaml', name='frag')], +def test_merged_main_wins_over_mixin(): + node = _node( + main=NodlDocument( + nodl_version=2, + parameters={'p': ParameterDefinition(type='int', default_value=2)}, + ), + mixins=[NodlDocument(nodl_version=2, parameters={'p': ParameterDefinition(type='int', default_value=1)})], ) - merged = resolve(doc, source_path=tmp_path / 'root.nodl.yaml').merged() - assert merged.parameters['use_sim_time'].default_value is True + merged = resolve(node).merged() + assert merged.parameters['p'].default_value == 2 def test_merged_combines_publishers_from_all_layers(tmp_path: Path): - frag_file = tmp_path / 'frag.nodl.yaml' - frag_file.write_text( + mixin_file = tmp_path / 'mixin.nodl.yaml' + mixin_file.write_text( textwrap.dedent("""\ nodl_version: 2 publishers: - - name: /from_frag + - name: /from_mixin type: std_msgs/msg/String qos: history: SYSTEM_DEFAULT reliability: SYSTEM_DEFAULT """) ) - doc = NodlDocument( - nodl_version=2, - publishers=[TopicEndpoint(name='/from_main', type='std_msgs/msg/String', qos=_SYS_QOS)], - fragments=[FragmentRef(ref='frag.nodl.yaml', name='f')], + node = _node( + main=NodlDocument( + nodl_version=2, + publishers=[TopicEndpoint(name='/from_main', type='std_msgs/msg/String', qos=_SYS_QOS)], + ), + mixins=['mixin.nodl.yaml'], ) - merged = resolve(doc, source_path=tmp_path / 'root.nodl.yaml').merged() + merged = resolve(node, source_path=tmp_path / 'root.nodl.yaml').merged() topics = {p.name for p in (merged.publishers or [])} assert '/from_main' in topics - assert '/from_frag' in topics - - -def test_fragment_label_defaults_to_ref(tmp_path: Path): - frag_file = tmp_path / 'f.nodl.yaml' - frag_file.write_text( - textwrap.dedent("""\ - nodl_version: 2 - publishers: - - name: /t - type: std_msgs/msg/String - qos: - history: SYSTEM_DEFAULT - reliability: SYSTEM_DEFAULT - """) - ) - doc = NodlDocument(nodl_version=2, fragments=[FragmentRef(ref='f.nodl.yaml')]) - layered = resolve(doc, source_path=tmp_path / 'root.nodl.yaml') - assert 'f.nodl.yaml' in layered.fragments + assert '/from_mixin' in topics diff --git a/nodl_schema/test/test_schema.py b/nodl_schema/test/test_schema.py index bfc9490..172b821 100644 --- a/nodl_schema/test/test_schema.py +++ b/nodl_schema/test/test_schema.py @@ -8,7 +8,7 @@ import pytest from jsonschema import ValidationError -from nodl_schema import dump_nodl, load_nodl, load_schema, validate +from nodl_schema import dump_nodl, load_nodl, load_schema, validate, validate_node from nodl_schema.models import NodlDocument _MIN_QOS = {'history': 'SYSTEM_DEFAULT', 'reliability': 'SYSTEM_DEFAULT'} @@ -235,26 +235,54 @@ def test_string_nodl_version_rejected(): validate({'nodl_version': '2'}) -def test_base_node_accepted(): - validate({'nodl_version': 2, 'base': 'node'}) +def test_document_rejects_composition_keys(): + # Composition lives on the node schema; documents must not carry base/main/mixins. + for key, value in (('base', 'node'), ('main', {'nodl_version': 2}), ('mixins', ['nodl://pkg/x'])): + with pytest.raises(ValidationError): + validate({'nodl_version': 2, key: value}) -def test_base_lifecycle_node_accepted(): - validate({'nodl_version': 2, 'base': 'lifecycle_node'}) +# --------------------------------------------------------------------------- +# validate_node -- the Node composition schema +# --------------------------------------------------------------------------- + + +def test_node_minimal_accepted(): + validate_node({'nodl_version': 2, 'main': {'nodl_version': 2}}) + + +def test_node_with_base_and_mixins_accepted(): + validate_node({ + 'nodl_version': 2, + 'base': 'lifecycle_node', + 'main': { + 'nodl_version': 2, + 'publishers': [{'name': '~/s', 'type': 'std_msgs/msg/String', 'qos': _MIN_QOS}], + }, + 'mixins': ['nodl://pkg/telemetry', {'nodl_version': 2}], + }) -def test_base_unknown_rejected(): +def test_node_missing_main_rejected(): with pytest.raises(ValidationError): - validate({'nodl_version': 2, 'base': 'unknown_base'}) + validate_node({'nodl_version': 2, 'base': 'node'}) -def test_fragments_accepted(): - validate({'nodl_version': 2, 'fragments': [{'ref': 'nodl://pkg/x'}]}) +def test_node_unknown_base_rejected(): + with pytest.raises(ValidationError): + validate_node({'nodl_version': 2, 'main': {'nodl_version': 2}, 'base': 'nope'}) + + +def test_node_main_must_be_valid_document(): + # main is validated against the document schema; an unknown key fails. + with pytest.raises(ValidationError): + validate_node({'nodl_version': 2, 'main': {'nodl_version': 2, 'bogus': 1}}) -def test_fragment_missing_ref_rejected(): +def test_node_mixin_scalar_rejected(): + # A mixin entry must be a ref string or an in-place document, not a bare scalar. with pytest.raises(ValidationError): - validate({'nodl_version': 2, 'fragments': [{'name': 'no_ref'}]}) + validate_node({'nodl_version': 2, 'main': {'nodl_version': 2}, 'mixins': [5]}) def test_invalid_parameter_type(): diff --git a/nodl_schema/test/test_validator_cli.py b/nodl_schema/test/test_validator_cli.py index dd0dd72..adc7934 100644 --- a/nodl_schema/test/test_validator_cli.py +++ b/nodl_schema/test/test_validator_cli.py @@ -29,10 +29,27 @@ _INVALID_NOT_A_MAPPING = '- just a list\n' -def _run(file: Path, *, fragment: bool = False) -> subprocess.CompletedProcess: +_VALID_NODE = """\ +nodl_version: 2 +base: lifecycle_node +main: + nodl_version: 2 + publishers: + - name: ~/status + type: std_msgs/msg/String + qos: + history: KEEP_LAST + depth: 10 + reliability: RELIABLE +mixins: + - nodl://other_pkg/telemetry +""" + + +def _run(file: Path, *, node: bool = False) -> subprocess.CompletedProcess: cmd = [sys.executable, '-m', 'nodl_schema'] - if fragment: - cmd.append('--fragment') + if node: + cmd.append('--node') cmd.append(str(file)) return subprocess.run(cmd, capture_output=True, text=True) @@ -75,38 +92,36 @@ def test_json_frontend_supported(tmp_path: Path): # --------------------------------------------------------------------------- -# --fragment mode +# --node mode (composition schema) # --------------------------------------------------------------------------- -def test_valid_fragment_exits_zero(tmp_path: Path): - # A fragment with publishers / subscriptions but no base/fragments passes. - f = tmp_path / 'frag.nodl.yaml' - f.write_text(_VALID) - result = _run(f, fragment=True) +def test_valid_node_exits_zero(tmp_path: Path): + f = tmp_path / 'node.nodl.yaml' + f.write_text(_VALID_NODE) + result = _run(f, node=True) assert result.returncode == 0, result.stderr -def test_fragment_with_base_rejected(tmp_path: Path): - f = tmp_path / 'bad_frag.nodl.yaml' +def test_node_without_main_rejected(tmp_path: Path): + f = tmp_path / 'no_main.nodl.yaml' f.write_text('nodl_version: 2\nbase: node\n') - result = _run(f, fragment=True) + result = _run(f, node=True) assert result.returncode == 1 assert str(f) in result.stderr - assert "'base'" in result.stderr -def test_fragment_with_fragments_rejected(tmp_path: Path): - f = tmp_path / 'bad_frag.nodl.yaml' - f.write_text('nodl_version: 2\nfragments:\n - ref: nodl://pkg/other\n') - result = _run(f, fragment=True) +def test_document_rejected_in_node_mode(tmp_path: Path): + # A plain document (no main) is not a valid node composition. + f = tmp_path / 'doc.nodl.yaml' + f.write_text(_VALID) + result = _run(f, node=True) assert result.returncode == 1 - assert "'fragments'" in result.stderr -def test_fragment_with_invalid_schema_still_rejected(tmp_path: Path): - # Schema validation happens before the fragment-only check. - f = tmp_path / 'doubly_bad.nodl.yaml' - f.write_text(_INVALID_BAD_PARAM_TYPE) - result = _run(f, fragment=True) +def test_node_rejected_in_document_mode(tmp_path: Path): + # A node composition has a `main` key the document schema forbids. + f = tmp_path / 'node.nodl.yaml' + f.write_text(_VALID_NODE) + result = _run(f) assert result.returncode == 1 diff --git a/test_ament_nodl/CMakeLists.txt b/test_ament_nodl/CMakeLists.txt index 3d64351..44a7bd3 100644 --- a/test_ament_nodl/CMakeLists.txt +++ b/test_ament_nodl/CMakeLists.txt @@ -17,18 +17,19 @@ ament_nodl_register_node(custom_exe # Non-yaml frontend. Make sure the macro is extension-agnostic. ament_nodl_register_node(json_node FILE test/fixtures/json_node.nodl.json) -# Default PACKAGE for the fragment register macro. -ament_nodl_register_fragment(tf_listener FILE test/fixtures/tf_listener.nodl.yaml) +# Default PACKAGE for the document register macro. basic_node mixes this in by +# nodl://test_ament_nodl/tf_listener. +ament_nodl_register_document(tf_listener FILE test/fixtures/tf_listener.nodl.yaml) -# Explicit PACKAGE override for fragments. -ament_nodl_register_fragment(extra_telemetry +# Explicit PACKAGE override for documents. +ament_nodl_register_document(extra_telemetry FILE test/fixtures/extra_telemetry.nodl.yaml PACKAGE custom_pkg) if(BUILD_TESTING) find_package(ament_cmake_pytest REQUIRED) ament_add_pytest_test(register_node_tests test/test_register_node.py) - ament_add_pytest_test(register_fragment_tests test/test_register_fragment.py) + ament_add_pytest_test(register_document_tests test/test_register_document.py) ament_add_pytest_test(macro_rejection_tests test/test_macro_rejection.py) endif() diff --git a/test_ament_nodl/test/fixtures/alt_pkg_node.nodl.yaml b/test_ament_nodl/test/fixtures/alt_pkg_node.nodl.yaml index 6d3463b..2af6622 100644 --- a/test_ament_nodl/test/fixtures/alt_pkg_node.nodl.yaml +++ b/test_ament_nodl/test/fixtures/alt_pkg_node.nodl.yaml @@ -1,3 +1,5 @@ --- nodl_version: 2 -description: Test node registered under an explicit PACKAGE override. +main: + nodl_version: 2 + description: Test node registered under an explicit PACKAGE override. diff --git a/test_ament_nodl/test/fixtures/basic_node.nodl.yaml b/test_ament_nodl/test/fixtures/basic_node.nodl.yaml index cdde5b8..9275573 100644 --- a/test_ament_nodl/test/fixtures/basic_node.nodl.yaml +++ b/test_ament_nodl/test/fixtures/basic_node.nodl.yaml @@ -1,10 +1,15 @@ --- nodl_version: 2 -description: Basic test node used to exercise ament_nodl_register_node default args. -publishers: - - name: /chatter - type: std_msgs/msg/String - qos: - history: KEEP_LAST - depth: 10 - reliability: RELIABLE +base: node +main: + nodl_version: 2 + description: Basic test node used to exercise ament_nodl_register_node default args. + publishers: + - name: /chatter + type: std_msgs/msg/String + qos: + history: KEEP_LAST + depth: 10 + reliability: RELIABLE +mixins: + - nodl://test_ament_nodl/tf_listener diff --git a/test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml b/test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml index 3191b0e..668b3fe 100644 --- a/test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml +++ b/test_ament_nodl/test/fixtures/extra_telemetry.nodl.yaml @@ -1,6 +1,6 @@ --- nodl_version: 2 -description: Test fragment registered under an explicit PACKAGE override. +description: Test mixin document registered under an explicit PACKAGE override. publishers: - name: ~/diagnostics type: diagnostic_msgs/msg/DiagnosticArray diff --git a/test_ament_nodl/test/fixtures/json_node.nodl.json b/test_ament_nodl/test/fixtures/json_node.nodl.json index b104a19..4cd521d 100644 --- a/test_ament_nodl/test/fixtures/json_node.nodl.json +++ b/test_ament_nodl/test/fixtures/json_node.nodl.json @@ -1,4 +1,7 @@ { "nodl_version": 2, - "description": "Test node using the JSON frontend; the macro should treat it the same as YAML." + "main": { + "nodl_version": 2, + "description": "Test node using the JSON frontend; the macro should treat it the same as YAML." + } } diff --git a/test_ament_nodl/test/fixtures/tf_listener.nodl.yaml b/test_ament_nodl/test/fixtures/tf_listener.nodl.yaml index e00de75..6d987f1 100644 --- a/test_ament_nodl/test/fixtures/tf_listener.nodl.yaml +++ b/test_ament_nodl/test/fixtures/tf_listener.nodl.yaml @@ -1,6 +1,6 @@ --- nodl_version: 2 -description: Test fragment that adds the standard tf2 listener subscriptions. +description: Test mixin document that adds the standard tf2 listener subscriptions. subscriptions: - name: /tf type: tf2_msgs/msg/TFMessage diff --git a/test_ament_nodl/test/test_macro_rejection.py b/test_ament_nodl/test/test_macro_rejection.py index df515e2..fe173e9 100644 --- a/test_ament_nodl/test/test_macro_rejection.py +++ b/test_ament_nodl/test/test_macro_rejection.py @@ -44,9 +44,11 @@ _INVALID_NODL = textwrap.dedent(""" nodl_version: 2 - parameters: - bad: - type: not_a_real_type + main: + nodl_version: 2 + parameters: + bad: + type: not_a_real_type """).lstrip() diff --git a/test_ament_nodl/test/test_register_fragment.py b/test_ament_nodl/test/test_register_document.py similarity index 81% rename from test_ament_nodl/test/test_register_fragment.py rename to test_ament_nodl/test/test_register_document.py index 16b9c0f..8e3e2ea 100644 --- a/test_ament_nodl/test/test_register_fragment.py +++ b/test_ament_nodl/test/test_register_document.py @@ -1,6 +1,6 @@ # SPDX-FileCopyrightText: 2026 Open Source Robotics Foundation, Inc. # SPDX-License-Identifier: Apache-2.0 -"""Integration tests for the ament_nodl_register_fragment CMake macro. +"""Integration tests for the ament_nodl_register_document CMake macro. The macro is exercised at configure / install time by this package's CMakeLists.txt; here we assert on the resulting ament index and install @@ -30,7 +30,7 @@ def _share(pkg: str = 'test_ament_nodl') -> Path: def test_default_package_uses_project_name(): # No PACKAGE arg means the macro defaults to ${PROJECT_NAME}, so the key is test_ament_nodl__tf_listener. - content, prefix = get_resource('nodl_fragments', 'test_ament_nodl__tf_listener') + content, prefix = get_resource('nodl_documents', 'test_ament_nodl__tf_listener') assert prefix assert 'tf2 listener' in content @@ -38,14 +38,14 @@ def test_default_package_uses_project_name(): def test_explicit_package_override(): # PACKAGE custom_pkg makes the key custom_pkg__extra_telemetry, matching what nodl://custom_pkg/extra_telemetry # would resolve to. - content, _ = get_resource('nodl_fragments', 'custom_pkg__extra_telemetry') + content, _ = get_resource('nodl_documents', 'custom_pkg__extra_telemetry') assert 'explicit PACKAGE override' in content def test_resource_content_matches_source(): # The registered resource should be byte-identical to the source file. - content, _ = get_resource('nodl_fragments', 'test_ament_nodl__tf_listener') - on_disk = (_share() / 'nodl' / 'fragments' / 'tf_listener.nodl.yaml').read_text() + content, _ = get_resource('nodl_documents', 'test_ament_nodl__tf_listener') + on_disk = (_share() / 'nodl' / 'documents' / 'tf_listener.nodl.yaml').read_text() assert content == on_disk @@ -55,10 +55,10 @@ def test_resource_content_matches_source(): def test_source_file_installed_under_registering_package(): - # Default PACKAGE installs the source under share/test_ament_nodl/nodl/fragments/. - assert (_share() / 'nodl' / 'fragments' / 'tf_listener.nodl.yaml').is_file() + # Default PACKAGE installs the source under share/test_ament_nodl/nodl/documents/. + assert (_share() / 'nodl' / 'documents' / 'tf_listener.nodl.yaml').is_file() def test_source_file_installed_under_override_package(): - # PACKAGE override redirects the source-file install to share//nodl/fragments/. - assert (_share('custom_pkg') / 'nodl' / 'fragments' / 'extra_telemetry.nodl.yaml').is_file() + # PACKAGE override redirects the source-file install to share//nodl/documents/. + assert (_share('custom_pkg') / 'nodl' / 'documents' / 'extra_telemetry.nodl.yaml').is_file()