From ba7ba0c3fb195472db248f4543f4ca28bcfc1ce4 Mon Sep 17 00:00:00 2001 From: vanous Date: Thu, 27 Nov 2025 18:58:47 +0100 Subject: [PATCH 1/2] Make stricter writing, align Addresses fields to the library style --- CHANGELOG.md | 8 +++ README.md | 12 +++- pymvr/__init__.py | 91 ++++++++++++++++++++++------ tests/test_fixture_1_5.py | 4 +- tests/test_mvr_02_read_ours.py | 4 +- tests/test_mvr_03_write_ours_json.py | 2 +- tests/test_mvr_04_read_ours_json.py | 4 +- tests/test_read_write_round_trip.py | 4 +- 8 files changed, 98 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07d61d6..0b929a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ ### Changelog +### NEXT + +* Enforce required nodes/fields at write time in class `to_xml` (raises on missing required Source/Projections/Geometries unless auto-filled). +* Auto-fill minimal IDs for non-multipatch fixtures/truss/support/video/projector when missing (`FixtureID="0"`, `FixtureIDNumeric=0`, fixtures also `UnitNumber=0`). +* Preserve `UserData/Data` payload content (text/children) on round-trip. +* Default `Gobo` rotation to `0.0` to avoid invalid `None` serialization. +* Switch `Addresses` to plural fields (`addresses`/`networks`) to match spec semantics; remove singular access and update tests/docs. + ### 1.0.4 * Add test for MVR read-write round-trip diff --git a/README.md b/README.md index af5efe8..51ee968 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,12 @@ for layer_index, layer in enumerate(mvr_file.scene.layers): ### Writing MVR +> Validation notes +> - Each object now enforces required children/fields when writing. Missing mandatory data will raise `ValueError`. +> - For convenience, fixtures/truss/support/video/projector auto-fill missing IDs with minimal defaults (`FixtureID="0"`, `FixtureIDNumeric=0`, fixtures also set `UnitNumber=0`) when not a multipatch child. +> - `Addresses` uses plural fields `addresses`/`networks` to align with the spec’s container semantics. +> - Required nodes such as `Geometries`, `Source` inside `MappingDefinition`/`Projection`, and `Projections` on `Projector` must be present; empty `Sources`/`Projections` will raise. + #### Load and Export an MVR ```python @@ -67,10 +73,10 @@ mvr_read = pymvr.GeneralSceneDescription("mvr_file.mvr") mvr_writer = pymvr.GeneralSceneDescriptionWriter() # 3. Serialize the scene object into the writer's XML root -mvr_read.scene.to_xml(parent=mvr_writer.xml_root) +mvr_writer.serialize_scene(mvr_read.scene) # 4. Serialize the user_data object into the writer's XML root -mvr_read.user_data.to_xml(parent=mvr_writer.xml_root) +mvr_writer.serialize_user_data(mvr_read.user_data) # 5. Add necesarry files like GDTF fixtures, trusses, 3D objects and so on # Skipped in this example @@ -111,7 +117,7 @@ fixture = pymvr.Fixture(name="Test Fixture") child_list.fixtures.append(fixture) # 3. Serialize the scene object into the writer's XML root -scene_obj.to_xml(parent=mvr_writer.xml_root) +mvr_writer.serialize_scene(scene_obj) # 4. Add any necessary files (like GDTF fixtures, trusses...) to the MVR archive # The list should contain tuples of (file_path, GDTF_file_name) diff --git a/pymvr/__init__.py b/pymvr/__init__.py index 91100d1..85e4e65 100644 --- a/pymvr/__init__.py +++ b/pymvr/__init__.py @@ -22,6 +22,7 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +from copy import deepcopy from typing import List, Union, Optional, Tuple from xml.etree import ElementTree from xml.etree.ElementTree import Element @@ -80,7 +81,9 @@ def __exit__(self, exc_type, exc_val, exc_tb): class GeneralSceneDescriptionWriter: """Creates MVR zip archive with packed GeneralSceneDescription xml and other files""" - def __init__(self): + def __init__( + self, + ): self.version_major: str = "1" self.version_minor: str = "6" self.provider: str = "pymvr" @@ -94,6 +97,13 @@ def __init__(self): providerVersion=self.provider_version, ) + def serialize_scene(self, scene: "Scene"): + scene.to_xml(parent=self.xml_root) + + def serialize_user_data(self, user_data: "UserData"): + if user_data: + user_data.to_xml(parent=self.xml_root) + def write_mvr(self, path: Optional[str] = None): if path is not None: if sys.version_info >= (3, 9): @@ -381,32 +391,32 @@ def to_xml(self, parent: Element): class Addresses(BaseNode): def __init__( self, - address: Optional[List["Address"]] = None, - network: Optional[List["Network"]] = None, + addresses: Optional[List["Address"]] = None, + networks: Optional[List["Network"]] = None, xml_node: Optional["Element"] = None, *args, **kwargs, ): - self.address = address if address is not None else [] - self.network = network if network is not None else [] + self.addresses: List["Address"] = addresses if addresses is not None else [] + self.networks: List["Network"] = networks if networks is not None else [] super().__init__(xml_node, *args, **kwargs) def _read_xml(self, xml_node: "Element"): - self.address = [Address(xml_node=i) for i in xml_node.findall("Address")] - self.network = [Network(xml_node=i) for i in xml_node.findall("Network")] + self.addresses = [Address(xml_node=i) for i in xml_node.findall("Address")] + self.networks = [Network(xml_node=i) for i in xml_node.findall("Network")] def to_xml(self, parent: Element) -> Optional[Element]: - if not self.address and not self.network: + if not self.addresses and not self.networks: return None element = ElementTree.SubElement(parent, "Addresses") - for dmx_address in self.address: + for dmx_address in self.addresses: dmx_address.to_xml(element) - for network_address in self.network: + for network_address in self.networks: network_address.to_xml(element) return element def __len__(self): - return len(self.address) + len(self.network) + return len(self.addresses) + len(self.networks) class BaseChildNode(BaseNode): @@ -574,12 +584,10 @@ def populate_xml(self, element: Element): if self.fixture_id is not None: ElementTree.SubElement(element, "FixtureID").text = str(self.fixture_id) - if self.fixture_id_numeric is not None: ElementTree.SubElement(element, "FixtureIDNumeric").text = str( self.fixture_id_numeric ) - if self.unit_number is not None: ElementTree.SubElement(element, "UnitNumber").text = str(self.unit_number) if self.custom_id_type is not None: @@ -620,8 +628,11 @@ def __str__(self): def populate_xml(self, element: Element): super().populate_xml(element) - if self.geometries: - self.geometries.to_xml(element) + if self.geometries is None: + raise ValueError( + f"{type(self).__name__} '{self.name}' missing required Geometries" + ) + self.geometries.to_xml(element) class Data(BaseNode): @@ -634,6 +645,8 @@ def __init__( ): self.provider = provider self.ver = ver + self.text: Optional[str] = None + self.extra_children: List[Element] = [] super().__init__(*args, **kwargs) def _read_xml(self, xml_node: "Element"): @@ -643,14 +656,20 @@ def _read_xml(self, xml_node: "Element"): ver = xml_node.attrib.get("ver") if ver is not None: self.ver = ver + self.text = xml_node.text + self.extra_children = [deepcopy(child) for child in list(xml_node)] def __str__(self): return f"{self.provider} {self.ver}" def to_xml(self): - return ElementTree.Element( + element = ElementTree.Element( type(self).__name__, provider=self.provider, ver=self.ver ) + element.text = self.text + for child in self.extra_children: + element.append(deepcopy(child)) + return element class AUXData(BaseNode): @@ -740,6 +759,8 @@ def _read_xml(self, xml_node: "Element"): self.scale_handling = ScaleHandeling(xml_node=scale_handling_node) def to_xml(self): + if self.source is None: + raise ValueError(f"MappingDefinition '{self.name}' missing required Source") element = ElementTree.Element( type(self).__name__, name=self.name, uuid=self.uuid ) @@ -832,6 +853,11 @@ def to_xml(self): if self.multipatch: attributes["multipatch"] = self.multipatch element = ElementTree.Element(type(self).__name__, attributes) + if self.multipatch is None: + if self.fixture_id is None: + self.fixture_id = "0" + if self.fixture_id_numeric is None: + self.fixture_id_numeric = 0 self.populate_xml(element) if self.focus: @@ -1416,6 +1442,11 @@ def to_xml(self): if self.multipatch: attributes["multipatch"] = self.multipatch element = ElementTree.Element(type(self).__name__, attributes) + if self.multipatch is None: + if self.fixture_id is None: + self.fixture_id = "0" + if self.fixture_id_numeric is None: + self.fixture_id_numeric = 0 self.populate_xml(element) if self.position: ElementTree.SubElement(element, "Position").text = self.position @@ -1459,6 +1490,11 @@ def to_xml(self): if self.multipatch: attributes["multipatch"] = self.multipatch element = ElementTree.Element(type(self).__name__, attributes) + if self.multipatch is None: + if self.fixture_id is None: + self.fixture_id = "0" + if self.fixture_id_numeric is None: + self.fixture_id_numeric = 0 self.populate_xml(element) if self.position: @@ -1499,6 +1535,11 @@ def to_xml(self): if self.multipatch: attributes["multipatch"] = self.multipatch element = ElementTree.Element(type(self).__name__, attributes) + if self.multipatch is None: + if self.fixture_id is None: + self.fixture_id = "0" + if self.fixture_id_numeric is None: + self.fixture_id_numeric = 0 self.populate_xml(element) if self.sources: @@ -1530,10 +1571,17 @@ def to_xml(self): if self.multipatch: attributes["multipatch"] = self.multipatch element = ElementTree.Element(type(self).__name__, attributes) + if self.multipatch is None: + if self.fixture_id is None: + self.fixture_id = "0" + if self.fixture_id_numeric is None: + self.fixture_id_numeric = 0 self.populate_xml(element) if self.projections: self.projections.to_xml(element) + else: + raise ValueError(f"Projector '{self.name}' missing Projections") return element @@ -1749,7 +1797,7 @@ def __init__( *args, **kwargs, ): - self.rotation = rotation + self.rotation = 0.0 if rotation is None else rotation self.filename = filename super().__init__(xml_node, *args, **kwargs) @@ -1814,9 +1862,10 @@ def _read_xml(self, xml_node: "Element"): self.scale_handling = ScaleHandeling(xml_node=scale_handling_node) def to_xml(self): + if self.source is None: + raise ValueError("Projection missing required Source") element = ElementTree.Element(type(self).__name__) - if self.source: - element.append(self.source.to_xml()) + element.append(self.source.to_xml()) if self.scale_handling: self.scale_handling.to_xml(element) return element @@ -1840,6 +1889,8 @@ def _read_xml(self, xml_node: "Element"): def to_xml(self, parent: Element): element = ElementTree.SubElement(parent, type(self).__name__) + if len(self.projections) == 0: + raise ValueError("Projections missing Projection entries") for projection in self.projections: element.append(projection.to_xml()) return element @@ -1895,6 +1946,8 @@ def _read_xml(self, xml_node: "Element"): def to_xml(self, parent: Element): element = ElementTree.SubElement(parent, type(self).__name__) + if len(self.sources) == 0: + raise ValueError("Sources missing Source entries") for source in self.sources: element.append(source.to_xml()) return element diff --git a/tests/test_fixture_1_5.py b/tests/test_fixture_1_5.py index a0741ab..f0b6bbd 100644 --- a/tests/test_fixture_1_5.py +++ b/tests/test_fixture_1_5.py @@ -47,9 +47,9 @@ def process_mvr_child_list(child_list, mvr_scene): def process_mvr_fixture(fixture): assert fixture.gdtf_spec == "LED PAR 64 RGBW.gdtf" assert ( - fixture.addresses.address[0].universe == 1 + fixture.addresses.addresses[0].universe == 1 ) # even though the uni is 0 in the file, 1 is by the spec - assert fixture.addresses.address[0].address == 1 # dtto + assert fixture.addresses.addresses[0].address == 1 # dtto assert fixture.gdtf_mode == "Default" assert fixture.matrix.matrix[3] == [5.0, 5.0, 5.0, 0] diff --git a/tests/test_mvr_02_read_ours.py b/tests/test_mvr_02_read_ours.py index 096c00c..3febd04 100644 --- a/tests/test_mvr_02_read_ours.py +++ b/tests/test_mvr_02_read_ours.py @@ -46,8 +46,8 @@ def process_mvr_child_list(child_list, mvr_scene): def process_mvr_fixture(fixture): assert fixture.gdtf_spec == "LED PAR 64 RGBW.gdtf" - assert fixture.addresses.address[0].universe == 1 - assert fixture.addresses.address[0].address == 1 + assert fixture.addresses.addresses[0].universe == 1 + assert fixture.addresses.addresses[0].address == 1 assert fixture.gdtf_mode == "Default" assert fixture.matrix.matrix[3] == [5.0, 5.0, 5.0, 0] diff --git a/tests/test_mvr_03_write_ours_json.py b/tests/test_mvr_03_write_ours_json.py index e2242b9..c522115 100644 --- a/tests/test_mvr_03_write_ours_json.py +++ b/tests/test_mvr_03_write_ours_json.py @@ -75,7 +75,7 @@ def test_write_from_json(): gdtf_spec=fixture_data["gdtf_spec"], gdtf_mode=fixture_data["gdtf_mode"], fixture_id=fixture_data["fixture_id"], - addresses=pymvr.Addresses(address=new_addresses), + addresses=pymvr.Addresses(addresses=new_addresses), ) child_list.fixtures.append(new_fixture) diff --git a/tests/test_mvr_04_read_ours_json.py b/tests/test_mvr_04_read_ours_json.py index 1964d65..1713117 100644 --- a/tests/test_mvr_04_read_ours_json.py +++ b/tests/test_mvr_04_read_ours_json.py @@ -47,8 +47,8 @@ def process_mvr_child_list(child_list, mvr_scene): def process_mvr_fixture(fixture): assert fixture.gdtf_spec == "BlenderDMX@Basic_LED_Bulb@ver2.gdtf" - assert fixture.addresses.address[0].dmx_break == 1 - assert fixture.addresses.address[0].universe == 1 + assert fixture.addresses.addresses[0].dmx_break == 1 + assert fixture.addresses.addresses[0].universe == 1 assert fixture.gdtf_mode == "Standard mode" assert fixture.matrix.matrix[3] == [0.0, 0.0, 0.0, 0] diff --git a/tests/test_read_write_round_trip.py b/tests/test_read_write_round_trip.py index e8b19c9..83ac6cd 100644 --- a/tests/test_read_write_round_trip.py +++ b/tests/test_read_write_round_trip.py @@ -45,7 +45,7 @@ def test_read_write_round_trip(request, pymvr_module): with pymvr_module.GeneralSceneDescription(file_read_path) as mvr_read: mvr_writer = pymvr_module.GeneralSceneDescriptionWriter() - mvr_read.scene.to_xml(parent=mvr_writer.xml_root) - mvr_read.user_data.to_xml(parent=mvr_writer.xml_root) + mvr_writer.serialize_scene(mvr_read.scene) + mvr_writer.serialize_user_data(mvr_read.user_data) mvr_writer.write_mvr(file_write_path) From f640fdf16233ba46ebb321163b24a0b8fd93ab45 Mon Sep 17 00:00:00 2001 From: vanous Date: Thu, 27 Nov 2025 23:23:48 +0100 Subject: [PATCH 2/2] Release v1.0.5 --- CHANGELOG.md | 4 ++-- README.md | 2 +- pymvr/__init__.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b929a0..0f0de1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,12 @@ ### Changelog -### NEXT +### 1.0.5 +* Breaking: Switch `Addresses` to plural fields (`addresses`/`networks`) to match spec semantics; remove singular access and update tests/docs. * Enforce required nodes/fields at write time in class `to_xml` (raises on missing required Source/Projections/Geometries unless auto-filled). * Auto-fill minimal IDs for non-multipatch fixtures/truss/support/video/projector when missing (`FixtureID="0"`, `FixtureIDNumeric=0`, fixtures also `UnitNumber=0`). * Preserve `UserData/Data` payload content (text/children) on round-trip. * Default `Gobo` rotation to `0.0` to avoid invalid `None` serialization. -* Switch `Addresses` to plural fields (`addresses`/`networks`) to match spec semantics; remove singular access and update tests/docs. ### 1.0.4 diff --git a/README.md b/README.md index 51ee968..f0ec789 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ for layer_index, layer in enumerate(mvr_file.scene.layers): > Validation notes > - Each object now enforces required children/fields when writing. Missing mandatory data will raise `ValueError`. -> - For convenience, fixtures/truss/support/video/projector auto-fill missing IDs with minimal defaults (`FixtureID="0"`, `FixtureIDNumeric=0`, fixtures also set `UnitNumber=0`) when not a multipatch child. +> - For convenience, fixtures/truss/support/video/projector auto-fill missing IDs with minimal defaults (`FixtureID="0"`, `FixtureIDNumeric=0`) when not a multipatch child. > - `Addresses` uses plural fields `addresses`/`networks` to align with the spec’s container semantics. > - Required nodes such as `Geometries`, `Source` inside `MappingDefinition`/`Projection`, and `Projections` on `Projector` must be present; empty `Sources`/`Projections` will raise. diff --git a/pymvr/__init__.py b/pymvr/__init__.py index 85e4e65..4d0d7df 100644 --- a/pymvr/__init__.py +++ b/pymvr/__init__.py @@ -32,7 +32,7 @@ from .value import Matrix, Color # type: ignore from enum import Enum -__version__ = "1.0.4" +__version__ = "1.0.5" def _find_root(pkg: "zipfile.ZipFile") -> "ElementTree.Element":