Make stricter writing, align Addresses fields to the library style

Author: vanous

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

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)

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,38 @@ 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 []
+        # Use plural names as primary fields
+        # Accept legacy keyword names if passed
+        if addresses is None and "address" in kwargs:
+            addresses = kwargs.pop("address")
+        if networks is None and "network" in kwargs:
+            networks = kwargs.pop("network")
+        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 +590,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 +634,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 +651,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 +662,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 +765,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 +859,13 @@ 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
+            if self.unit_number is None:
+                self.unit_number = 0
         self.populate_xml(element)
 
         if self.focus:
@@ -1416,6 +1450,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 +1498,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 +1543,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 +1579,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 +1805,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 +1870,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 +1897,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 +1954,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

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]
 

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]
 

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)

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]
 

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)