Refactor Neopixel to add decoupling caps, use consistent pwr port naming #428
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request updates the Neopixel LED components to include decoupling capacitors for each individual LED. The main changes involve:
- Refactoring Neopixel LED arrays to include capacitor blocks for each LED element
- Correcting a path specification from 'vdd' to 'pwr' in test configuration
- Updating SvgPcbTemplateBlock assertions to support multiple blocks matching a path (changed from
== 1to> 0) - Regenerating PCB layout files (
.svgpcb.jsand.netfiles) for multiple example projects
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/test_deskcontroller.py | Corrects path from 'vdd' to 'pwr' for npx current draw specification |
| examples/SevenSegment/SevenSegment.svgpcb.js | Regenerated PCB layout with new component positions and capacitor additions for 64 LED devices |
| examples/SevenSegment/SevenSegment.net | Regenerated netlist with capacitor components and updated sheet paths |
| examples/RobotCrawler/RobotCrawler.svgpcb.js | Regenerated PCB layout with repositioned components and capacitor additions for 10 RGB LEDs |
| examples/RobotCrawler/RobotCrawler.net | Regenerated netlist with capacitor components and updated connectivity |
| examples/PcbBot/PcbBot.svgpcb.js | Regenerated PCB layout with repositioned components and capacitor addition for npx_key LED |
| examples/PcbBot/PcbBot.net | Regenerated netlist with capacitor component and updated net assignments |
| edg/electronics_model/SvgPcbTemplateBlock.py | Relaxed assertions from requiring exactly one block to allowing multiple blocks per path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| candidate_blocks = [block for block in self._svgpcb_netlist.blocks | ||
| if block.full_path.startswith(block_path)] | ||
| assert len(candidate_blocks) == 1 | ||
| assert len(candidate_blocks) > 0 |
There was a problem hiding this comment.
The assertion change from == 1 to > 0 allows multiple matching blocks but then unconditionally uses candidate_blocks[0]. This could mask issues where multiple blocks exist but different ones are expected. Consider adding a warning or selecting based on specific criteria when multiple blocks are found.
| candidate_blocks = [block for block in self._svgpcb_netlist.blocks | ||
| if block.full_path.startswith(block_path)] | ||
| assert len(candidate_blocks) == 1 | ||
| assert len(candidate_blocks) > 0 |
There was a problem hiding this comment.
Similar to the previous assertion, this change allows multiple blocks but always returns the first one. The updated docstring says 'Picks the first one, which is assumed to be the main / anchor device' but this assumption may not always hold. Consider adding validation or making the selection more explicit.
The old .vdd port naming is kept as an alias, eventually needs a proper deprecation warning on access. Cleans up Neopixel docstrings to be less redundant.
Also changes svg-pcb generation to use the first matching footprint if there are multiple in a subtree. This unbreaks examples from changes. Convention is the first block (recursively) is the anchor block, eg the main chip.
Updates generated netlists.
Resolves #424