Conversation
There was a problem hiding this comment.
Pull request overview
Housekeeping cleanup across the core EDSL, parts library, and examples: fixes invalid regex escape sequences, tightens builder context handling, and reorganizes example schematic resources (plus removing/relocating some special-purpose parts).
Changes:
- Fix regex syntax warnings by converting patterns to raw strings across JLC part parsers and parsing utilities.
- Introduce
builder.block()(asserting current context) and migrate connection helpers away fromget_enclosing_block(). - Move example schematics into per-example directories and update example imports; remove special-purpose
BufferedSupply/Supercapfrom the library and switch Datalogger toWaveshare_Epd.
Reviewed changes
Copilot reviewed 46 out of 52 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/test_robotowl.py | Update KiCad schematic import path to per-example directory. |
| examples/test_multimeter.py | Update KiCad schematic import paths to examples/Multimeter/. |
| examples/test_fcml.py | Update KiCad schematic import paths to examples/Fcml/. |
| examples/test_datalogger.py | Move Supercap / BufferedSupply definitions into the example and switch E-Ink block to Waveshare_Epd. |
| examples/test_blinky.py | Update schematic import paths to examples/TestBlinkyWithSchematicImport/. |
| examples/resources/upgrade_pcb.py | Remove unused/legacy upgrade script from examples resources. |
| examples/resources/.gitignore | Remove resources-specific ignore file (resource layout changed). |
| examples/RobotOwl/PhotodiodeSensor.kicad_sch | Add RobotOwl per-example schematic. |
| examples/Multimeter/MultimeterCurrentDriver.kicad_sch | Add Multimeter per-example schematic. |
| examples/Multimeter/MultimeterAnalog.kicad_sch | Add Multimeter per-example schematic. |
| examples/Fcml/MultilevelSwitchingCell_True.kicad_sch | Add FCML per-example schematic for is_first=True. |
| examples/Fcml/MultilevelSwitchingCell_False.kicad_sch | Add FCML per-example schematic for is_first=False. |
| examples/Datalogger/Datalogger.svgpcb.js | Update generated layout/net naming to match the new Waveshare EPD hierarchy. |
| examples/Datalogger/Datalogger.net | Update netlist metadata paths for moved blocks and renamed E-Ink hierarchy. |
| edg/parts/init.py | Stop exporting removed special-purpose parts; drop deprecated E-Ink wrapper export. |
| edg/parts/PowerConditioning.py | Remove BufferedSupply / Supercap; use builder.block() in helper connection API. |
| edg/parts/JlcResistorArray.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcResistor.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcPptcFuse.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcPart.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcOscillator.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcInductor.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcFet.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcFerriteBead.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcElectrolyticCapacitor.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcDiode.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcCrystal.py | Fix regex string escapes via raw strings. |
| edg/parts/JlcBjt.py | Fix regex string escapes via raw strings. |
| edg/parts/EInk_E2154fs091.py | Remove deprecated E-Ink wrapper implementation file. |
| edg/electronics_model/PartParserUtil.py | Fix regex string escapes via raw f-string. |
| edg/electronics_model/DigitalPorts.py | Use builder.block() to avoid optional enclosing context. |
| edg/electronics_model/ConnectedGenerator.py | Use builder.block() instead of optional enclosing context + assert. |
| edg/core/Ports.py | Switch context capture/bind checks to new builder APIs. |
| edg/core/HierarchyBlock.py | Switch top-level construction check to current builder context API. |
| edg/core/ConstraintExpr.py | Switch expression context capture/bind checks to new builder APIs. |
| edg/core/Builder.py | Add block() and _current_block(), deprecate optional get_enclosing_block(). |
| edg/core/Blocks.py | Capture block creation context via current builder context API. |
| edg/core/Array.py | Update allocation APIs to use builder.block() / _current_block() context checks. |
| edg/abstract_parts/UsbBitBang.py | Use builder.block() in fluent connect helper. |
| edg/abstract_parts/PassiveFilters.py | Use builder.block() in fluent connect helper. |
| edg/abstract_parts/PassiveConnector.py | Use builder.block() in fluent connect helper. |
| edg/abstract_parts/MergedBlocks.py | Use builder.block() in fluent connect helper. |
| edg/abstract_parts/I2cBitBang.py | Use builder.block() in fluent connect helper. |
| edg/abstract_parts/AbstractTestPoint.py | Use builder.block() in fluent connect helper(s). |
| edg/abstract_parts/AbstractResistor.py | Fix regex string escapes; use builder.block() in fluent connect helper(s). |
| edg/abstract_parts/AbstractPowerConverters.py | Use builder.block() in fluent connect helper(s). |
| edg/abstract_parts/AbstractInductor.py | Use builder.block() in fluent connect helper(s). |
| edg/abstract_parts/AbstractFuse.py | Use builder.block() in fluent connect helper(s). |
| edg/abstract_parts/AbstractFerriteBead.py | Use builder.block() in fluent connect helper(s). |
| edg/abstract_parts/AbstractCapacitor.py | Fix regex string escapes; use builder.block() in fluent connect helper(s). |
| edg/abstract_parts/AbstractAnalogSwitch.py | Use builder.block() in mux/demux helper(s). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
+47
| def block(self) -> BaseBlock: | ||
| """Returns the current block context, throwing an error if there is no block context.""" | ||
| current_block = self._current_block() | ||
| if current_block is None: | ||
| raise Exception("no block context") | ||
| return current_block |
There was a problem hiding this comment.
Builder.block() raises a generic Exception when no context exists. Since this is an EDSL user error, it would be more consistent (and test-friendly) to raise EdgContextError (from edg/core/HdlUserExceptions.py) with a message that explains what operations require a block context (eg, calling .connect() / .request() only inside a block’s __init__, contents(), or generate()).
Comment on lines
277
to
281
| assert ( | ||
| builder.get_enclosing_block() is block_parent._parent or builder.get_enclosing_block() is None | ||
| block_parent._parent is None or builder.block() is block_parent._parent | ||
| ), "can only allocate ports of internal blocks" # None case is to allow elaborating in unit tests | ||
| # self._elts is ignored, since that defines the inner-facing behavior, which this is outer-facing behavior | ||
| allocated = type(self._tpe).empty()._bind(self) |
There was a problem hiding this comment.
Vector.request() now calls builder.block() as part of its context assertion. request_vector() below still allows a None builder context (via _current_block() is None, per the comment). This makes the two APIs enforce different context rules. Consider making both methods consistent (either both allow the unit-test/elaboration None context, or both require an active block context) and ensure they raise the same error type/message when misused.
Comment on lines
44
to
48
| self._parent: Optional[PortParentTypes] # refined from Optional[Refable] in base LibraryElement | ||
| self._block_context: Optional["Refable"] # set by metaclass, as lexical scope available pre-binding | ||
| self._initializer_args: Tuple[Tuple[Any, ...], Dict[str, Any]] # set by metaclass | ||
| self._block_context = builder.get_enclosing_block() | ||
| self._block_context = builder._current_block() | ||
|
|
There was a problem hiding this comment.
Several modules now call builder._current_block() directly. Since this is a private API, it increases coupling between Builder and the rest of the core. Consider adding a public optional accessor (eg builder.maybe_block() / builder.current_block() -> Optional[BaseBlock]) and using that instead, reserving underscore-prefixed helpers for Builder internals.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #473, fix regex syntax
Resolves #470, move special-purpose BufferedSupply and non-generic Supercap out of the library
Resolves #469, defines builder.block() that asserts the context exists
Resolves #425, moves schematic-defined blocks from examples/resources to per-example directories
Removes the really old eink display without a deprecation path.