Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors pick-and-place rotation/offset handling by moving it into FootprintBlock.footprint() so the BoM can directly carry per-footprint PNP adjustments, allowing the JLCPCBA postprocess script to consume those values without being manually updated for each new part.
Changes:
- Add optional
pnp_rot/pnp_offsettoFootprintBlock.footprint()and propagate them into BoM CSV output columns. - Update the JLCPCBA postprocess script to read rotation/offset overrides from the BoM instead of maintaining large per-part tables.
- Update many parts/connectors to specify their PNP rotation/offset at footprint definition time, and add a unit test for BoM generation.
Reviewed changes
Copilot reviewed 71 out of 72 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/jlcpcb_pcba_postprocess.py | Read PNP rotation/offset overrides from BoM columns; reduce hardcoded part tables. |
| edg/parts/UsbUart_Cp2102.py | Add pnp_rot to CP2102 footprint definition. |
| edg/parts/UsbPorts.py | Add pnp_rot/pnp_offset to USB-C receptacle; add pnp_rot to USB ESD part. |
| edg/parts/SwitchedCap_TexasInstruments.py | Add pnp_rot to LM2664 footprint definition. |
| edg/parts/SpiMemory_W25q.py | Add pnp_rot to W25Q footprint definition. |
| edg/parts/SpiMemory_93Lc.py | Add pnp_rot to 93LC footprint definition. |
| edg/parts/SpeakerDriver_Max98357a.py | Add per-package pnp_rot and pass it through to footprint. |
| edg/parts/SpeakerDriver_Analog.py | Add pnp_rot for speaker driver footprints. |
| edg/parts/SdCards.py | Add pnp_rot for microSD connector footprint. |
| edg/parts/RotaryEncoder_Alps.py | Add pnp_rot / pnp_offset for rotary encoder variants. |
| edg/parts/Rf_Sx1262.py | Add pnp_rot for PE4259 and SX1262 footprints. |
| edg/parts/Rf_Pn7160.py | Add pnp_rot for PN7160 footprint. |
| edg/parts/PassiveConnector_Header.py | Add connector-family part_footprint_pnp_rot/part_footprint_pnp_offset overrides. |
| edg/parts/PassiveConnector_Fpc.py | Add connector-family part_footprint_pnp_rot/part_footprint_pnp_offset overrides. |
| edg/parts/Opamp_Tlv915x.py | Add pnp_rot for TLV9152 footprint. |
| edg/parts/Opamp_Tlv9061.py | Add pnp_rot for TLV9061 footprint. |
| edg/parts/Opamp_Opax333.py | Add pnp_rot for OPA2333 footprint. |
| edg/parts/Opamp_Opax197.py | Add pnp_rot for OPA197/OPA2197 footprints. |
| edg/parts/Opamp_Opax189.py | Add pnp_rot for OPA189/OPA2197 footprints. |
| edg/parts/Opamp_Opax171.py | Add pnp_rot for OPA2171 footprint. |
| edg/parts/Opamp_Mcp6001.py | Add pnp_rot for MCP6001 footprint. |
| edg/parts/Opamp_Lmv321.py | Add pnp_rot for LMV321 footprint. |
| edg/parts/Neopixel.py | Add pnp_rot for SK6805 footprint. |
| edg/parts/MotorDriver_Drv8833.py | Add pnp_rot for DRV8833 footprint. |
| edg/parts/Microcontroller_Stm32g031.py | Add pnp_rot for STM32G031 footprint. |
| edg/parts/Microcontroller_Stm32f103.py | Add pnp_rot for STM32F103 footprint. |
| edg/parts/Microcontroller_Esp32c3.py | Add pnp_rot for ESP32-C3 footprint. |
| edg/parts/Microcontroller_Esp32.py | Add pnp_rot / pnp_offset for ESP32-WROOM-32 footprint. |
| edg/parts/Mag_Qmc5883l.py | Add pnp_rot for QMC5883L footprint. |
| edg/parts/Logic_74Lvc.py | Add pnp_rot for SN74LVC1G74 footprint. |
| edg/parts/Logic_74Ahct.py | Add pnp_rot for 74AHCT1G125 footprint. |
| edg/parts/LinearRegulators.py | Add pnp_rot / pnp_offset for several regulator footprints. |
| edg/parts/Leds.py | Add pnp_rot for RGB LED footprints. |
| edg/parts/LedDriver_Tps92200.py | Add pnp_rot for TPS92200 footprint. |
| edg/parts/LedDriver_Al8861.py | Add pnp_rot for AL8861 footprint. |
| edg/parts/JlcSwitches.py | Add pnp_offset for SKRTLAE010 switch footprint. |
| edg/parts/Isolator_Cbmud1200.py | Add pnp_rot for CBMuD1200 footprint. |
| edg/parts/IoExpander_Pcf8574.py | Add pnp_rot for PCF8574 footprint. |
| edg/parts/IoExpander_Pca9554.py | Add pnp_rot for PCA9554 footprint. |
| edg/parts/Inamp_Ina826.py | Add pnp_rot for INA826 footprint. |
| edg/parts/Ina219.py | Add pnp_rot for INA219 footprint. |
| edg/parts/GateDriver_Ucc27282.py | Add pnp_rot for UCC27282 footprint. |
| edg/parts/GateDriver_Ncp3420.py | Add pnp_rot for NCP3420 footprint. |
| edg/parts/GateDriver_Ir2301.py | Add pnp_rot for IR2301 footprint. |
| edg/parts/Fusb302b.py | Add pnp_rot for FUSB302B footprint. |
| edg/parts/EnvironmentalSensor_Ti.py | Add pnp_rot for TMP1075 footprint. |
| edg/parts/EnvironmentalSensor_Bme680.py | Add pnp_rot for BME680 footprint. |
| edg/parts/Distance_Vl53l0x.py | Add pnp_rot for VL53L0X footprint. |
| edg/parts/DacSpi_Mcp4901.py | Add pnp_rot for MCP4921 footprint. |
| edg/parts/DacI2c_Mcp4728.py | Add pnp_rot for MCP4728 footprint. |
| edg/parts/CurrentSense_Ad8418.py | Add pnp_rot for AD8418 footprint. |
| edg/parts/Connector_Rf.py | Add pnp_offset for U.FL connector footprint. |
| edg/parts/Comparator_Lmv331.py | Add pnp_rot for LMV331 footprint. |
| edg/parts/CanTransceiver_Sn65hvd230.py | Add pnp_rot for SN65HVD230 footprint. |
| edg/parts/BuckConverter_TexasInstruments.py | Add pnp_rot for TPS561201 and TPS54202H footprints. |
| edg/parts/BuckConverter_Ap3418.py | Add pnp_rot for AP3418 footprint. |
| edg/parts/BoostConverter_Torex.py | Add pnp_rot for XC9142 footprint. |
| edg/parts/BoostConverter_TexasInstruments.py | Add pnp_rot for TPS61040 and LM2733 footprints. |
| edg/parts/BoostConverter_DiodesInc.py | Add pnp_rot for AP3012 footprint. |
| edg/parts/BoostConverter_AnalogDevices.py | Add pnp_rot for LTC3429 footprint. |
| edg/parts/Bldc_Drv8313.py | Add pnp_rot for DRV8313 footprint. |
| edg/parts/BatteryProtector_S8261A.py | Add pnp_rot for S-8261 footprint. |
| edg/parts/BatteryCharger_Mcp73831.py | Add pnp_rot for MCP73831 footprint. |
| edg/parts/AnalogSwitch_Nlas4157.py | Add pnp_rot for NLAS4157 footprint. |
| edg/parts/AnalogSwitch_Dg468.py | Add pnp_rot for DG468 footprint. |
| edg/parts/AnalogSwitch_7400.py | Add pnp_rot for SN74LVC1G3157 footprint. |
| edg/parts/AdcSpi_Mcp3201.py | Add pnp_rot for MCP3201 footprint. |
| edg/electronics_model/test_bom.py | Add unit tests for BoM output including PNP columns. |
| edg/electronics_model/CircuitBlock.py | Extend FootprintBlock with PNP params and document behavior. |
| edg/electronics_model/BomBackend.py | Emit PNP rotation/offset columns in generated BoM; include in aggregation key. |
| edg/abstract_parts/PassiveConnector.py | Allow connector families to provide PNP rotation/offset to footprint(). |
| .gitignore | Ignore examples/*/gerbers output directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4
to
+5
| from typing import Type, List, Dict | ||
|
|
There was a problem hiding this comment.
override is imported from typing, but this repo supports Python >=3.9; typing.override only exists in newer Python versions. Import override from typing_extensions (as done in other tests like test_netlist.py) to avoid ImportError on supported runtimes.
Comment on lines
+33
to
+37
| @staticmethod | ||
| def generate_bom(design: Type[Block], refinements: Refinements = Refinements()) -> List[Dict[str, str]]: | ||
| compiled = ScalaCompiler.compile(design, refinements) | ||
| compiled.append_values(RefdesRefinementPass().run(compiled)) | ||
| boms = GenerateBom().run(compiled) |
There was a problem hiding this comment.
generate_bom is annotated to return Netlist, but it actually returns GenerateBom().run(...) (a List[Tuple[LocalPath, str]]). With mypy strict=true, this return-type mismatch will fail type checking. Update the return annotation (and drop the unused Netlist import if it becomes unnecessary).
Comment on lines
82
to
+90
| with open(f"{args.file_path_prefix}.csv", "r", newline="") as bom_in: | ||
| csv_in = csv.reader(bom_in) | ||
|
|
||
| rows = list(csv_in) | ||
| refdes_list_index = rows[0].index("Designator") | ||
| lcsc_index = rows[0].index("JLCPCB Part #") | ||
| pnp_rot_index = rows[0].index("PNP Rotation Offset") | ||
| pnp_offset_x_index = rows[0].index("PNP Offset X") | ||
| pnp_offset_y_index = rows[0].index("PNP Offset Y") |
There was a problem hiding this comment.
The script now unconditionally looks up the new BoM columns (PNP Rotation Offset, PNP Offset X, PNP Offset Y) via rows[0].index(...). If --merge-boms is used with input BoMs that don't have these headers (or if someone runs the script on an older BoM), this will raise ValueError. Consider making these columns optional (eg, check header presence and treat missing as unspecified) or emitting a clearer error message that instructs regenerating the BoM with a compatible EDG version.
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.
This refactors the rotation and offset for all non-parts-table parts so that pnp data for new parts can be added without modifying the single pnp postprocessing script. This outputs it as a BoM table column, which the postprocessing script reads.
Also adds a unit test for BoM generation.
Handling parts-table-parts may be refactored in the future, especially sharing common data between parts like SOT-xx between BJT and FETs.