From 067df9fb9f9e36b8efebd6900adb432e54b4b182 Mon Sep 17 00:00:00 2001 From: Matthew Wildoer Date: Fri, 13 Sep 2024 11:28:47 -0700 Subject: [PATCH] Core: Schematic round-trips work to KiCAD without visual artifacts --- src/faebryk/libs/kicad/fileformats_common.py | 21 ++++- src/faebryk/libs/kicad/fileformats_sch.py | 94 +++++++++++++------- src/faebryk/libs/sexp/dataclass_sexp.py | 33 +++++-- test/libs/kicad/test_fileformats.py | 2 +- 4 files changed, 107 insertions(+), 43 deletions(-) diff --git a/src/faebryk/libs/kicad/fileformats_common.py b/src/faebryk/libs/kicad/fileformats_common.py index 4df90f71..8d4c066f 100644 --- a/src/faebryk/libs/kicad/fileformats_common.py +++ b/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) diff --git a/src/faebryk/libs/kicad/fileformats_sch.py b/src/faebryk/libs/kicad/fileformats_sch.py index 816d3db9..3ec49732 100644 --- a/src/faebryk/libs/kicad/fileformats_sch.py +++ b/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,9 +166,8 @@ 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: @@ -169,10 +175,11 @@ class C_power: 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,24 +230,28 @@ 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 @@ -247,8 +259,9 @@ class C_pin: 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 @@ -256,14 +269,27 @@ class C_pin: @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 diff --git a/src/faebryk/libs/sexp/dataclass_sexp.py b/src/faebryk/libs/sexp/dataclass_sexp.py index 15b246b2..ccd6c2b7 100644 --- a/src/faebryk/libs/sexp/dataclass_sexp.py +++ b/src/faebryk/libs/sexp/dataclass_sexp.py @@ -38,6 +38,8 @@ 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 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 @@ -45,6 +47,8 @@ class sexp_field(dict[str, Any]): 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 +81,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 +90,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 +138,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 +190,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,7 +244,8 @@ 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: @@ -242,7 +253,8 @@ def _decode[T]( 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 +272,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 +301,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 +383,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: diff --git a/test/libs/kicad/test_fileformats.py b/test/libs/kicad/test_fileformats.py index 7a70b88d..c6e101c3 100644 --- a/test/libs/kicad/test_fileformats.py +++ b/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", )