Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the remaining touch-sense port/link model to use compositional Passive sub-ports, continuing the migration away from CircuitPort/CircuitLink while keeping legacy types around (deprecated) for transitional compatibility.
Changes:
- Convert
TouchPortto useLink+ compositionalPassivenets (HasPassivePort) instead ofCircuitLink/CircuitPort. - Update footprint/pin-mapping-related type signatures across parts/abstract parts to accept
PassiveandHasPassivePort. - Reintroduce/retain deprecated
CircuitPort/CircuitLinkinCircuitBlockfor backward compatibility while updatingPassiveLink/PassiveBridgeto participate in netlisting vianetsmetadata.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| edg/parts/SpeakerDriver_Max98357a.py | Update footprint pinning typing to Passive/HasPassivePort. |
| edg/parts/Microcontroller_nRF52840.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Stm32l432.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Stm32g431.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Stm32g031.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Stm32f303.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Stm32f103.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Rp2040.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Lpc1549.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Esp32s3.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Esp32c3.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/Microcontroller_Esp32.py | Update _system_pinmap typing away from CircuitPort. |
| edg/parts/LinearRegulators.py | Narrow pinning typing to HasPassivePort for regulator footprint mapping. |
| edg/parts/IoExpander_Pcf8574.py | Update pin map dict typings to Passive/HasPassivePort. |
| edg/parts/IoExpander_Pca9554.py | Update pin map dict typings to Passive/HasPassivePort. |
| edg/parts/Fpga_Ice40up.py | Update _system_pinmap typing away from CircuitPort. |
| edg/electronics_model/TouchPort.py | Implement touch ports as HasPassivePort with internal Passive net and link-level net connection. |
| edg/electronics_model/PinAssignmentUtil.py | Switch pin-assignment leaf type from CircuitPort to Passive. |
| edg/electronics_model/PassivePort.py | Make Passive compositional (no longer a CircuitPort), and mark PassiveLink/PassiveBridge as netlisted via nets metadata. |
| edg/electronics_model/KiCadSchematicBlock.py | Update conversions typing and remove legacy CircuitPort auto-adapt path. |
| edg/electronics_model/CircuitBlock.py | Keep deprecated CircuitPort/CircuitLink while updating FootprintBlock.footprint to accept Passive. |
| edg/electronics_model/AnalogPort.py | Remove now-unused import of legacy adapter types. |
| edg/abstract_parts/VariantPinRemapper.py | Update remapper typing to Passive/HasPassivePort. |
| edg/abstract_parts/StandardFootprint.py | Update pinning function typing to allow Passive/HasPassivePort. |
| edg/abstract_parts/PinMappable.py | Update PinResource typing to Passive/HasPassivePort. |
| edg/abstract_parts/PassiveConnector.py | Update connector connected() typing/assertions for Passive/HasPassivePort. |
| edg/abstract_parts/OpampCircuits.py | Remove legacy CircuitPort union typing in conversion model variable. |
| edg/abstract_parts/IoController.py | Update pinmap typing to Passive/HasPassivePort and widen runtime assertions accordingly. |
| edg/abstract_parts/DigitalAmplifiers.py | Narrow KiCad import conversions typing to HasPassivePort. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def leaf_circuit_ports(prefix: str, port: Port) -> Iterable[Tuple[str, Passive]]: | ||
| if isinstance(port, Passive): | ||
| return [(prefix, port)] | ||
| else: | ||
| assert port._ports, "must be CircuitPort or Port bundle with sub-Ports" | ||
| assert port._ports, "must be Passive or Port bundle with sub-Ports" |
There was a problem hiding this comment.
leaf_circuit_ports no longer treats legacy CircuitPort instances as leaves. If a caller passes a deprecated CircuitPort (which previously worked), this now hits the assert port._ports path and fails at runtime because CircuitPort has no sub-ports. If backward compatibility for CircuitPort is still intended, consider accepting CircuitPort here (eg treat it as a leaf like Passive) or raising a clearer error with a deprecation message instead of asserting on _ports.
| super().__init__() | ||
| self.driver = self.Port(TouchDriver()) | ||
| self.pad = self.Port(TouchPadPort()) | ||
|
|
||
| self.net = self.connect(self.driver.net, self.pad.net) |
There was a problem hiding this comment.
This refactor changes Touch connectivity from a CircuitLink/CircuitPort model to compositional Passive nets, but there doesn't appear to be any unit test coverage for Touch links/ports (eg netlisting shows driver+pad on the same net, footprint pinning resolves through .net, and IoControllerTouchDriver pin mapping still works). Adding a small electronics_model/test_touch_port.py (or similar) would help prevent regressions since this is the last port type being migrated in #114.
Refactors the last leftover port (touch sense ports) to compositional passive.
Refactoring to remove references to CircuitPort, except to preserve backward compatibility (for now). Type annotations do not preserve compatibility.
Resolves #114.