Conversation
There was a problem hiding this comment.
Pull request overview
Adds OpenDRIVE 1.4 export support by introducing a version selection that suppresses 1.8-only XML features when exporting older revisions.
Changes:
- Add
opendrive_versionplumbing from GUI → export API → writer/builders. - Emit OpenDRIVE root/header differently for 1.4 vs 1.8 (namespace +
revMinor), and gate 1.8-only junction/lane features. - Add a unit test asserting 1.8-only features are present in 1.8 exports and suppressed in 1.4 exports.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/unit/test_export/test_opendrive_writer.py |
Adds coverage for 1.4 vs 1.8 feature suppression behavior. |
orbit/gui/dialogs/export_dialog.py |
Adds OpenDRIVE version selector and passes it into export call. |
orbit/export/opendrive_writer.py |
Adds opendrive_version option, switches namespace/header revMinor, and gates 1.8-only junction features. |
orbit/export/lane_builder.py |
Gates 1.8-only lane attributes (direction, advisory) based on selected version. |
Comments suppressed due to low confidence (2)
orbit/export/opendrive_writer.py:1677
export_to_opendrive()now acceptsopendrive_version, but the docstring’s Args section doesn’t document this new parameter. Please add an entry explaining supported values (e.g. "1.8" and "1.4"), what changes between versions, and what the default means for callers.
def export_to_opendrive(
project: Project,
transformer: CoordinateTransformer,
output_path: str,
line_tolerance: float = 0.5,
arc_tolerance: float = 1.0,
preserve_geometry: bool = True,
right_hand_traffic: bool = True,
country_code: str = "se",
use_tmerc: bool = False,
use_german_codes: bool = False,
opendrive_version: str = "1.8",
offset_x: float = 0.0,
offset_y: float = 0.0,
geo_reference_string: Optional[str] = None,
export_object_types: Optional[set] = None
orbit/gui/dialogs/export_dialog.py:516
- Now that the export supports OpenDRIVE 1.4, schema validation should likely be conditioned on the selected version. As written, the dialog will validate the exported file against whatever
xodr_schema_pathpoints to (often a 1.8 schema), which will produce noisy/incorrect validation warnings for 1.4 exports. Consider skipping validation for 1.4 unless a matching schema is provided, or selecting the schema based onopendrive_versionand updating the user-facing message accordingly.
success = export_to_opendrive(
self.project,
export_transformer,
str(self.output_path),
line_tolerance=self.line_tolerance_spin.value(),
arc_tolerance=self.arc_tolerance_spin.value(),
preserve_geometry=self.preserve_geometry_checkbox.isChecked(),
right_hand_traffic=self.project.right_hand_traffic,
country_code=country_code,
use_tmerc=use_tmerc,
use_german_codes=self.use_german_codes_checkbox.isChecked(),
opendrive_version=self.opendrive_version_combo.currentData(),
offset_x=offset_x,
offset_y=offset_y,
geo_reference_string=geo_reference_string,
export_object_types=self._get_export_object_types()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _create_opendrive_root(self) -> etree.Element: | ||
| """Create the root OpenDRIVE element with all content.""" | ||
| # OpenDRIVE 1.8 namespace | ||
| nsmap = {None: "http://code.asam.net/simulation/standard/opendrive_schema"} | ||
| root = etree.Element('OpenDRIVE', nsmap=nsmap) | ||
| # OpenDRIVE namespace (only for 1.8) | ||
| if self.opendrive_version == "1.8": | ||
| nsmap = {None: "http://code.asam.net/simulation/standard/opendrive_schema"} | ||
| root = etree.Element('OpenDRIVE', nsmap=nsmap) | ||
| else: | ||
| root = etree.Element('OpenDRIVE') | ||
|
|
There was a problem hiding this comment.
opendrive_version is treated as a free-form string and any value other than "1.8" silently falls back to 1.4 behavior (no namespace, revMinor set to 4, and 1.8-only elements omitted). This makes typos like "1.18" or "1.8.0" produce an incorrect 1.4 file without any warning. Consider validating the value early (e.g., in __init__) and raising a clear error or normalizing accepted aliases.
| lane_1_8 = root_1_8.find('.//od:lane', ns) | ||
| assert lane_1_8 is not None | ||
| assert lane_1_8.get('direction') == 'samedir' | ||
| assert lane_1_8.get('advisory') == 'advisory1' | ||
|
|
||
| # Parse 1.4 and assert features are suppressed | ||
| tree_1_4 = ET.parse(output_path_1_4) | ||
| root_1_4 = tree_1_4.getroot() | ||
|
|
||
| # In 1.4, no namespace is used | ||
| header_1_4 = root_1_4.find('header') | ||
| assert header_1_4.attrib['revMinor'] == '4' | ||
|
|
||
| junction_1_4 = root_1_4.find('.//junction') | ||
| assert junction_1_4 is not None | ||
| assert junction_1_4.find('boundary') is None | ||
| assert junction_1_4.find('elevationGrid') is None | ||
|
|
||
| lane_1_4 = root_1_4.find('.//lane') | ||
| assert lane_1_4 is not None | ||
| assert lane_1_4.get('direction') is None | ||
| assert lane_1_4.get('advisory') is None |
There was a problem hiding this comment.
This test relies on the first .//od:lane match being the left lane with direction/advisory set (i.e., it depends on element ordering). To make the test robust to any future XML ordering/refactors, consider selecting the specific lane by id (e.g., the lane with id="1") before asserting on direction/advisory, and likewise for the 1.4 document.
Overview
Adds support for OpenDrive 1.4 export during OpenDrive export.
Tests
Functionality tested visually with ODRViewer, as well as by successful import into the CARLA simulator.
Closes #16.