Core: Schematic round-trips work to KiCAD without visual artifacts

src/faebryk/libs/kicad/fileformats_common.py

@@ -4,7 +4,7 @@
 from enum import auto
 from typing import Optional
 
-from faebryk.libs.sexp.dataclass_sexp import SymEnum, sexp_field
+from faebryk.libs.sexp.dataclass_sexp import SymEnum, sexp_field, Symbol
 from faebryk.libs.util import KeyErrorAmbiguous
 
 logger = logging.getLogger(__name__)
@@ -105,6 +105,25 @@ class E_justify(SymEnum):
 
     font: C_font
 
+    @staticmethod
+    def _hide_input_converter(x: Symbol) -> bool:
+        if x == Symbol("hide"):
+            return True
+        raise ValueError(f"Unexpected token {x}")
+
+    @staticmethod
+    def _hide_output_converter(x: bool) -> Symbol | None:
+        return [Symbol("hide"), Symbol("yes") if x else Symbol("no")]
+
+    hide: bool = field(
+        **sexp_field(
+            positional=True,
+            input_converter=_hide_input_converter,
+            output_converter=_hide_output_converter,
+        ),
+        default=False
+    )
+
     # Legal:
     # (justify mirror right)
     # (justify bottom)

src/faebryk/libs/kicad/fileformats_sch.py

@@ -1,6 +1,6 @@
 import logging
 from dataclasses import dataclass, field
-from enum import StrEnum, auto
+from enum import auto
 from typing import Optional
 
 from faebryk.libs.kicad.fileformats_common import (
@@ -9,7 +9,6 @@
     C_pts,
     C_xy,
     C_xyr,
-    gen_uuid,
 )
 from faebryk.libs.sexp.dataclass_sexp import SEXP_File, SymEnum, sexp_field
 
@@ -22,9 +21,9 @@
 class C_property:
     name: str = field(**sexp_field(positional=True))
     value: str = field(**sexp_field(positional=True))
+    id: Optional[int] = None
     at: Optional[C_xyr] = None
     effects: Optional[C_effects] = None
-    id: Optional[int] = None
 
 
 @dataclass(kw_only=True)  # TODO: when to use kw_only?
@@ -53,7 +52,6 @@ class C_circle:
     end: C_xy
     stroke: C_stroke
     fill: C_fill
-    uuid: UUID = field(default_factory=gen_uuid)
 
 
 @dataclass(kw_only=True)
@@ -63,7 +61,6 @@ class C_arc:
     end: C_xy
     stroke: C_stroke
     fill: C_fill
-    uuid: UUID = field(default_factory=gen_uuid)
 
 
 @dataclass(kw_only=True)
@@ -72,7 +69,6 @@ class C_rect:
     end: C_xy
     stroke: C_stroke
     fill: C_fill
-    uuid: UUID = field(default_factory=gen_uuid)
 
 
 @dataclass(kw_only=True)
@@ -109,21 +105,32 @@ class C_pin_names:
                 class C_symbol:
                     @dataclass
                     class C_pin:
-                        class E_type(StrEnum):
+                        class E_type(SymEnum):
+                            # sorted alphabetically
+                            bidirectional = "bidirectional"
+                            free = "free"
                             input = "input"
+                            no_connect = "no_connect"
+                            open_collector = "open_collector"
+                            open_emitter = "open_emitter"
                             output = "output"
                             passive = "passive"
                             power_in = "power_in"
                             power_out = "power_out"
-                            bidirectional = "bidirectional"
-
-                        class E_style(StrEnum):
-                            line = "line"
+                            tri_state = "tri_state"
+                            unspecified = "unspecified"
+
+                        class E_style(SymEnum):
+                            # sorted alphabetically
+                            clock = "clock"
+                            clock_low = "clock_low"
+                            edge_clock_high = "edge_clock_high"
+                            input_low = "input_low"
                             inverted = "inverted"
-                            # Unvalidated
-                            # arrow = "arrow"
-                            # dot = "dot"
-                            # none = "none"
+                            inverted_clock = "inverted_clock"
+                            line = "line"
+                            non_logic = "non_logic"
+                            output_low = "output_low"
 
                         @dataclass
                         class C_name:
@@ -159,20 +166,20 @@ class C_number:
                         **sexp_field(multidict=True), default_factory=list
                     )
 
-                class E_show_hide(SymEnum):
+                class E_hide(SymEnum):
                     hide = "hide"
-                    show = "show"
 
                 @dataclass
                 class C_power:
                     pass
 
                 name: str = field(**sexp_field(positional=True))
                 power: Optional[C_power] = None
-                propertys: list[C_property] = field(
-                    **sexp_field(multidict=True), default_factory=list
+                propertys: dict[str, C_property] = field(
+                    **sexp_field(multidict=True, key=lambda x: x.name),
+                    default_factory=dict,
                 )
-                pin_numbers: E_show_hide = field(default=E_show_hide.show)
+                pin_numbers: Optional[E_hide] = None
                 pin_names: Optional[C_pin_names] = None
                 in_bom: Optional[bool] = None
                 on_board: Optional[bool] = None
@@ -199,8 +206,9 @@ class C_pin:
             in_bom: bool
             on_board: bool
             fields_autoplaced: bool = True
-            propertys: list[C_property] = field(
-                **sexp_field(multidict=True), default_factory=list
+            propertys: dict[str, C_property] = field(
+                **sexp_field(multidict=True, key=lambda x: x.name),
+                default_factory=dict,
             )
             pins: list[C_pin] = field(
                 **sexp_field(multidict=True), default_factory=list
@@ -222,48 +230,66 @@ class C_wire:
 
         @dataclass
         class C_text:
-            at: C_xy
+            at: C_xyr
             effects: C_effects
             uuid: UUID
             text: str = field(**sexp_field(positional=True))
 
         @dataclass
         class C_sheet:
-            @dataclass
-            class C_fill:
-                color: Optional[str] = None
-
             @dataclass
             class C_pin:
+                class E_type(SymEnum):
+                    # sorted alphabetically
+                    bidirectional = "bidirectional"
+                    input = "input"
+                    output = "output"
+                    passive = "passive"
+                    tri_state = "tri_state"
+
                 at: C_xyr
                 effects: C_effects
                 uuid: UUID
                 name: str = field(**sexp_field(positional=True))
-                type: str = field(**sexp_field(positional=True))
+                type: E_type = field(**sexp_field(positional=True))
 
             at: C_xy
             size: C_xy
             stroke: C_stroke
             fill: C_fill
             uuid: UUID
             fields_autoplaced: bool = True
-            propertys: list[C_property] = field(
-                **sexp_field(multidict=True), default_factory=list
+            propertys: dict[str, C_property] = field(
+                **sexp_field(multidict=True, key=lambda x: x.name),
+                default_factory=dict,
             )
             pins: list[C_pin] = field(
                 **sexp_field(multidict=True), default_factory=list
             )
 
         @dataclass
         class C_global_label:
-            shape: str
+            class E_shape(SymEnum):
+                # sorted alphabetically
+                input = "input"
+                output = "output"
+                bidirectional = "bidirectional"
+                tri_state = "tri_state"
+                passive = "passive"
+                dot = "dot"
+                round = "round"
+                diamond = "diamond"
+                rectangle = "rectangle"
+
+            shape: E_shape
             at: C_xyr
             effects: C_effects
             uuid: UUID
             text: str = field(**sexp_field(positional=True))
             fields_autoplaced: bool = True
-            propertys: list[C_property] = field(
-                **sexp_field(multidict=True), default_factory=list
+            propertys: dict[str, C_property] = field(
+                **sexp_field(multidict=True, key=lambda x: x.name),
+                default_factory=dict,
             )
 
         # TODO: inheritance
@@ -293,7 +319,7 @@ class C_bus_entry:
             stroke: C_stroke
             uuid: UUID
 
-        version: str
+        version: int = field(**sexp_field(assert_value=20211123))
         generator: str
         uuid: UUID
         paper: str

src/faebryk/libs/sexp/dataclass_sexp.py

@@ -38,13 +38,19 @@ class sexp_field(dict[str, Any]):
     :param Any assert_value: Assert that the value is equal to this value
     :param int order: Order of the field in the sexp, lower is first,
     can be less than 0. Only used if not positional.
+    :param Callable[[Any], Any] | None input_converter: Function to convert the value.
+    Don't use this unless you really need to. There's only one case in KiCAD 8.
+    :param Callable[[Any], Any] | None output_converter: Function to convert the value.
+    Don't use this unless you really need to. There's only one case in KiCAD 8.
     """
 
     positional: bool = False
     multidict: bool = False
     key: Callable[[Any], Any] | None = None
     assert_value: Any | None = None
     order: int = 0
+    input_converter: Callable[[Any], Any] | None = None
+    output_converter: Callable[[Any], Any] | None = None
 
     def __post_init__(self):
         super().__init__({"metadata": {"sexp": self}})
@@ -77,6 +83,7 @@ def _convert(
     t,
     stack: list[tuple[str, type]] | None = None,
     name: str | None = None,
+    converter: Callable[[Any], Any] | None = None,
 ):
     if name is None:
         name = "<" + t.__name__ + ">"
@@ -85,6 +92,10 @@ def _convert(
     substack = stack + [(name, t)]
 
     try:
+        # Run the overriding converter if set
+        if converter:
+            return converter(val)
+
         # Recurse (GenericAlias e.g list[])
         if (origin := get_origin(t)) is not None:
             args = get_args(t)
@@ -129,7 +140,7 @@ def _convert(
             if val == []:
                 return None
 
-            assert False, f"Invalid value for bool: {val}"
+            raise ValueError(f"Invalid value for bool: {val}")
 
         if isinstance(val, Symbol):
             return t(str(val))
@@ -181,6 +192,7 @@ def _decode[T](
                     if str(key) + "s" in key_fields or str(key) in key_fields:
                         ungrouped_key_values.append(val)
                         continue
+
         unprocessed_indices.add(i)
 
     key_values = groupby(
@@ -234,15 +246,17 @@ def _decode[T](
             if origin is list:
                 val_t = args[0]
                 value_dict[name] = [
-                    _convert(_val[1:], val_t, stack, name) for _val in values
+                    _convert(_val[1:], val_t, stack, name, sp.input_converter)
+                    for _val in values
                 ]
             elif origin is dict:
                 if not sp.key:
                     raise ValueError(f"Key function required for multidict: {f.name}")
                 key_t = args[0]
                 val_t = args[1]
                 converted_values = [
-                    _convert(_val[1:], val_t, stack, name) for _val in values
+                    _convert(_val[1:], val_t, stack, name, sp.input_converter)
+                    for _val in values
                 ]
                 values_with_key = [(sp.key(_val), _val) for _val in converted_values]
 
@@ -260,17 +274,20 @@ def _decode[T](
                 )
         else:
             assert len(values) == 1, f"Duplicate key: {name}"
-            out = _convert(values[0][1:], f.type, stack, name)
+            out = _convert(values[0][1:], f.type, stack, name, sp.input_converter)
             # if val is None, use default
             if out is not None:
                 value_dict[name] = out
 
     # Positional
     for f, v in (it := zip_non_locked(positional_fields.values(), pos_values.values())):
+        sp = sexp_field.from_field(f)
         # special case for missing positional empty StrEnum fields
         if isinstance(f.type, type) and issubclass(f.type, StrEnum):
             if "" in f.type and not isinstance(v, Symbol):
-                value_dict[f.name] = _convert(Symbol(""), f.type, stack, f.name)
+                value_dict[f.name] = _convert(
+                    Symbol(""), f.type, stack, f.name, sp.input_converter
+                )
                 # only advance field iterator
                 # if no more positional fields, there shouldn't be any more values
                 if it.next(0) is None:
@@ -286,9 +303,9 @@ def _decode[T](
             while next_val is not None:
                 vs.append(next_val)
                 next_val = it.next(1, None)
-            out = _convert(vs, f.type, stack, f.name)
+            out = _convert(vs, f.type, stack, f.name, sp.input_converter)
         else:
-            out = _convert(v, f.type, stack, f.name)
+            out = _convert(v, f.type, stack, f.name, sp.input_converter)
 
         value_dict[f.name] = out
 
@@ -368,6 +385,10 @@ def _append(_val):
         name = f.name
         val = getattr(t, name)
 
+        if sp.output_converter:
+            _append(sp.output_converter(val))
+            continue
+
         if sp.positional:
             if isinstance(val, list):
                 for v in val:

test/libs/kicad/test_fileformats.py

@@ -117,7 +117,7 @@ def test_parser(self):
 
         self.assertEqual(
             sch.kicad_sch.lib_symbols.symbol["Amplifier_Audio:LM4990ITL"]
-            .propertys[3]
+            .propertys["Datasheet"]
             .value,
             "http://www.ti.com/lit/ds/symlink/lm4990.pdf",
         )