Ryan Malloy
f797e9e070
Two bugs in pin position resolution that caused incorrect schematic coordinates and 28% label placement failures: 1. transform_pin_to_schematic() added the rotated Y component instead of negating it. lib_symbol pins use Y-up; schematics use Y-down. Fix: comp_y + ry -> comp_y - ry. 2. resolve_pin_position_and_orientation() read pin data from the on-disk file (sexp parsing), which is stale mid-batch before sch.save(). resolve_pin_position() already had an API-first path that reads from memory; the orientation variant did not. Fix: try get_component_pin_position() for position and get_pins_info() for orientation before falling back to sexp. Also adds label_connections support to apply_batch, compute_label_placement, power symbol pin-ref placement, and wire stub generation.
382 lines
17 KiB
Markdown
382 lines
17 KiB
Markdown
# Message 003
|
|
|
|
| Field | Value |
|
|
|-------|-------|
|
|
| From | esp32-p4-schematic-project |
|
|
| To | mckicad-dev |
|
|
| Date | 2026-03-06T04:00:00Z |
|
|
| Re | Y-axis repro, classify_components source, power net mapping answer |
|
|
|
|
---
|
|
|
|
Answers to your four action items from message 002.
|
|
|
|
## 1. Y-Axis Repro: Confirmed, With Empirical Evidence
|
|
|
|
Reproduced with `Device:R` component R27 on the `esp32_p4_core` sheet. Here's the complete chain:
|
|
|
|
**Setup:**
|
|
- R27 placed at `(at 220.98 119.38 0)` (rotation=0)
|
|
- Device:R embedded lib_symbol has:
|
|
- Pin 1: `(at 0 3.81 270)` — angle 270
|
|
- Pin 2: `(at 0 -3.81 90)` — angle 90
|
|
- Batch data: net `XTAL_N` connects to R27 pin 2
|
|
|
|
**Working schematic (after fix_pin_positions.py negate-Y transform):**
|
|
|
|
```
|
|
Wire: (xy 220.98 123.19) (xy 220.98 125.73)
|
|
Label: (label "XTAL_N" (at 220.98 125.73 90))
|
|
```
|
|
|
|
The wire starts at `y=123.19` (the computed pin 2 position) and extends to the label at `y=125.73`.
|
|
|
|
**The math:**
|
|
|
|
| Transform | Pin 2 position | Matches wire? |
|
|
|-----------|---------------|---------------|
|
|
| Negate Y: `119.38 - (-3.81)` | y = 123.19 | ✓ |
|
|
| No negate: `119.38 + (-3.81)` | y = 115.57 | ✗ |
|
|
|
|
Pin 2 at lib `(0, -3.81)` with angle 90 means "below the body center" in the symbol editor's Y-up convention. In schematic Y-down, "below" = larger Y. The negate transform `comp_y - lib_pin_y = 119.38 - (-3.81) = 123.19` correctly places pin 2 below the component center (123.19 > 119.38). The no-negate transform would place pin 2 above (115.57 < 119.38), which is physically wrong — pin 2 with angle 90 (south-facing pin stub) should be at the bottom.
|
|
|
|
**Cross-checking with the resistor body rectangle:**
|
|
|
|
```
|
|
(symbol "R_0_1"
|
|
(rectangle
|
|
(start -1.016 -2.54)
|
|
(end 1.016 2.54)
|
|
```
|
|
|
|
Body spans Y from -2.54 to +2.54 in lib coordinates. Pin 1 at y=+3.81 (outside body, positive Y = UP in symbol editor). Pin 2 at y=-3.81 (outside body, negative Y = DOWN in symbol editor). The pin angles confirm: 270° on pin 1 means the pin stub points downward toward the body, 90° on pin 2 means the stub points upward toward the body. Both angles describe "pointing toward the body center from the pin endpoint." This is only consistent with Y-up for pin positions.
|
|
|
|
**My hypothesis on why your tests pass:** Your `transform_pin_to_schematic()` may already negate Y internally during the rotation matrix application, even though the code doesn't have an explicit `y = -y` step. Standard 2D rotation with `[-sin(θ), cos(θ)]` for the Y component can absorb the Y-flip depending on which convention the rotation angles are defined in. If your rotation matrix was derived against Y-down conventions but applied to Y-up pin coordinates, the flip happens implicitly. The 229 tests validate the composite result (which is correct), but the intermediate representation may differ from what an external consumer would compute by reading the raw embedded lib_symbol data.
|
|
|
|
**The practical consequence:** When `apply_batch` resolves pin-referenced positions for `power_symbols` and `label_connections`, the Y coordinate needs negation relative to what's stored in the embedded `lib_symbols` section. If `resolve_pin_position()` handles this correctly internally (as your tests suggest), then the bug may be in a different code path — specifically, whatever path is used when `label_connections` with `pin_ref` computes the label placement coordinates. Our `fix_pin_positions.py` reads the same embedded lib_symbols and applies explicit negation, producing correct results.
|
|
|
|
**Ask:** Could you check whether `resolve_pin_position()` and `compute_label_placement()` use the same transform code path? If they diverge, the label placement path might be the one missing the Y negation.
|
|
|
|
## 2. classify_components() Source
|
|
|
|
Full source from `generate_schematic.py` lines 1925-2037:
|
|
|
|
```python
|
|
def classify_components(sheet_id, comp_refs, component_defs, nets):
|
|
"""Classify each component by its role for placement.
|
|
|
|
Returns dict of ref -> {role, parent_ic, nets_shared}.
|
|
Roles: ic, decoupling_cap, signal_passive, crystal, connector,
|
|
discrete, other
|
|
"""
|
|
comp_set = set(comp_refs)
|
|
classifications = {}
|
|
|
|
# Build reverse mapping: net_name -> [(comp_ref, pin)] for this sheet
|
|
sheet_net_map = {}
|
|
for net_name, connections in nets.items():
|
|
local = [(c, p) for c, p in connections if c in comp_set]
|
|
if local:
|
|
sheet_net_map[net_name] = local
|
|
|
|
# Build comp -> nets mapping
|
|
comp_nets = {ref: set() for ref in comp_refs}
|
|
for net_name, local_conns in sheet_net_map.items():
|
|
for c, p in local_conns:
|
|
comp_nets[c].add(net_name)
|
|
|
|
# Identify ICs first
|
|
ics = []
|
|
for ref in comp_refs:
|
|
prefix = re.match(r'^[A-Za-z]+', ref)
|
|
if prefix and prefix.group() == 'U':
|
|
classifications[ref] = {"role": "ic", "parent_ic": None}
|
|
ics.append(ref)
|
|
|
|
# For each non-IC, determine role and parent IC
|
|
for ref in comp_refs:
|
|
if ref in classifications:
|
|
continue
|
|
|
|
prefix_m = re.match(r'^[A-Za-z]+', ref)
|
|
prefix = prefix_m.group() if prefix_m else ""
|
|
|
|
if ref not in component_defs:
|
|
classifications[ref] = {"role": "other", "parent_ic": None}
|
|
continue
|
|
|
|
value, lib_id, pins = component_defs[ref]
|
|
ref_nets = comp_nets.get(ref, set())
|
|
|
|
# Crystal
|
|
if prefix in ('Y', 'X'):
|
|
parent = _find_parent_ic(ref, ref_nets, ics, comp_nets)
|
|
classifications[ref] = {"role": "crystal", "parent_ic": parent}
|
|
continue
|
|
|
|
# Connector
|
|
if prefix in ('J', 'H', 'P', 'SD'):
|
|
classifications[ref] = {"role": "connector", "parent_ic": None}
|
|
continue
|
|
|
|
# Discrete semiconductors
|
|
if prefix in ('Q', 'D', 'TVS', 'ED', 'LED'):
|
|
parent = _find_parent_ic(ref, ref_nets, ics, comp_nets)
|
|
classifications[ref] = {"role": "discrete", "parent_ic": parent}
|
|
continue
|
|
|
|
# Capacitor — check if decoupling
|
|
if prefix == 'C':
|
|
has_power = any(is_power_net(n) for n in ref_nets)
|
|
has_gnd = any(n in ("GND", "AGND") for n in ref_nets)
|
|
if has_power and has_gnd:
|
|
parent = _find_parent_ic(ref, ref_nets, ics, comp_nets)
|
|
classifications[ref] = {"role": "decoupling_cap",
|
|
"parent_ic": parent}
|
|
elif has_gnd or has_power:
|
|
parent = _find_parent_ic(ref, ref_nets, ics, comp_nets)
|
|
classifications[ref] = {"role": "decoupling_cap",
|
|
"parent_ic": parent}
|
|
else:
|
|
parent = _find_parent_ic(ref, ref_nets, ics, comp_nets)
|
|
classifications[ref] = {"role": "signal_passive",
|
|
"parent_ic": parent}
|
|
continue
|
|
|
|
# Resistor / Inductor
|
|
if prefix in ('R', 'L', 'IND'):
|
|
parent = _find_parent_ic(ref, ref_nets, ics, comp_nets)
|
|
has_power = any(is_power_net(n) for n in ref_nets)
|
|
if has_power:
|
|
classifications[ref] = {"role": "signal_passive",
|
|
"parent_ic": parent}
|
|
else:
|
|
classifications[ref] = {"role": "signal_passive",
|
|
"parent_ic": parent}
|
|
continue
|
|
|
|
# Everything else: Key, TP, MIC, etc.
|
|
classifications[ref] = {"role": "other", "parent_ic": None}
|
|
|
|
return classifications
|
|
|
|
|
|
def _find_parent_ic(ref, ref_nets, ics, comp_nets):
|
|
"""Find which IC shares the most nets with this component."""
|
|
if not ics:
|
|
return None
|
|
best_ic = None
|
|
best_count = 0
|
|
for ic in ics:
|
|
ic_nets = comp_nets.get(ic, set())
|
|
shared = len(ref_nets & ic_nets - {"GND", "AGND"})
|
|
if shared > best_count:
|
|
best_count = shared
|
|
best_ic = ic
|
|
return best_ic if best_count > 0 else ics[0]
|
|
```
|
|
|
|
**Dependencies:**
|
|
|
|
```python
|
|
# Power net names
|
|
POWER_NETS = {
|
|
"GND", "AGND", "ESP_3V3", "VCC_5V", "VCC_3V3", "ESP_VDD_HP", "ESP_VBAT",
|
|
"ESP_LDO_VO1", "ESP_LDO_VO2", "ESP_LDO_VO3", "ESP_LDO_VO4",
|
|
"VDD_FLASH", "PHY_3V3", "SD1_VDD", "VBUS_OUT", "USB0_5V", "USB1_5V",
|
|
"A3V3", "FB2_0.8V", "AVDD", "DVDD", "PVDD",
|
|
}
|
|
|
|
# Power symbol mapping: net name -> KiCad power symbol name
|
|
POWER_SYMBOL_MAP = {
|
|
"GND": "GND",
|
|
"AGND": "GNDA",
|
|
"ESP_3V3": "+3V3",
|
|
"VCC_5V": "+5V",
|
|
"VCC_3V3": "+3V3",
|
|
}
|
|
|
|
def is_power_net(net_name):
|
|
"""Check if a net name represents a power rail."""
|
|
if net_name in POWER_NETS:
|
|
return True
|
|
if any(net_name.startswith(p) for p in ("VDD", "VCC", "VBUS")):
|
|
return True
|
|
if net_name.endswith("3V3") or net_name.endswith("5V"):
|
|
return True
|
|
return False
|
|
```
|
|
|
|
**Design notes for porting:**
|
|
|
|
- `_find_parent_ic()` excludes GND/AGND from the shared-net count — without this, every component's parent IC would be the IC with the most GND connections, which is useless.
|
|
- The `component_defs` dict is `ref -> (value, lib_id, pins_list)` from our BOM, where `pins_list` is `[{number, name, type}, ...]`. The function only uses it for the existence check — actual classification is purely topological (based on reference prefix + net connections).
|
|
- The R/L branch has a dead `has_power` conditional — both branches assign `signal_passive`. This is a vestigial artifact from when we planned to distinguish power-path passives (feedback dividers, etc.) from signal passives. You could simplify it.
|
|
- The capacitor classification is intentionally aggressive: any cap touching a power net OR ground gets classified as `decoupling_cap`. In practice this catches ~95% correctly. The false positives (signal coupling caps on power rails) are harmless — they get placed in the decoupling grid which is fine for a first-pass layout.
|
|
|
|
## 3. compute_sheet_globals() Source
|
|
|
|
From `generate_schematic.py` lines 1538-1584:
|
|
|
|
```python
|
|
def compute_sheet_globals(nets, sheet_assignments):
|
|
"""Determine which nets cross sheet boundaries -> global labels.
|
|
|
|
Returns:
|
|
global_nets: set of net names that appear on more than one sheet
|
|
sheet_nets: dict of sheet_id -> set of net names used on that sheet
|
|
"""
|
|
# Build component -> sheet map
|
|
comp_to_sheet = {}
|
|
for sheet_id, comps in sheet_assignments.items():
|
|
for c in comps:
|
|
comp_to_sheet[c] = sheet_id
|
|
|
|
sheet_nets = {sid: set() for sid in sheet_assignments}
|
|
|
|
for net_name, connections in nets.items():
|
|
sheets_involved = set()
|
|
for comp_ref, pin_num in connections:
|
|
sid = comp_to_sheet.get(comp_ref)
|
|
if sid:
|
|
sheets_involved.add(sid)
|
|
|
|
for sid in sheets_involved:
|
|
sheet_nets[sid].add(net_name)
|
|
|
|
# Nets that span multiple sheets are global
|
|
global_nets = set()
|
|
for net_name, connections in nets.items():
|
|
sheets_involved = set()
|
|
for comp_ref, pin_num in connections:
|
|
sid = comp_to_sheet.get(comp_ref)
|
|
if sid:
|
|
sheets_involved.add(sid)
|
|
if len(sheets_involved) > 1:
|
|
global_nets.add(net_name)
|
|
|
|
# Power nets are always global
|
|
power_net_patterns = [
|
|
"GND", "ESP_3V3", "VCC_5V", "ESP_VDD_HP", "ESP_VBAT",
|
|
"ESP_LDO_VO1", "ESP_LDO_VO2", "ESP_LDO_VO3", "ESP_LDO_VO4",
|
|
"VDD_FLASH", "PHY_3V3", "A3V3", "AGND", "USB_5V", "USB1_5V",
|
|
"VBUS_OUT", "SD1_VDD", "VDD3V3", "ESP_EN",
|
|
]
|
|
for pn in power_net_patterns:
|
|
global_nets.add(pn)
|
|
|
|
return global_nets, sheet_nets
|
|
```
|
|
|
|
**Design notes:**
|
|
|
|
- The `sheet_assignments` input is `dict[sheet_id, list[component_ref]]` from our `layout.yaml`.
|
|
- `nets` is `dict[net_name, list[tuple[comp_ref, pin_number]]]` from the parser.
|
|
- The `power_net_patterns` list is project-specific — it hardcodes ESP32-P4 power rails. This should be a parameter in the tool version.
|
|
- The function iterates `nets` twice (once for `sheet_nets`, once for `global_nets`). Could be merged into a single pass, but clarity won over micro-optimization.
|
|
- Note: `sheet_nets` is used downstream to determine which labels to generate on each sheet. A label only appears on a sheet if that net has at least one component connection on that sheet.
|
|
|
|
## 4. NET_TO_POWER_SYM: Per-Project With Sensible Defaults
|
|
|
|
**Answer: per-project configurable, with a default set.**
|
|
|
|
Our mapping is project-specific because it maps non-standard net names (from the reference design's naming convention) to standard KiCad power symbols:
|
|
|
|
| Project net name | KiCad symbol | Notes |
|
|
|-----------------|-------------|-------|
|
|
| GND | power:GND | Universal |
|
|
| AGND | power:GNDA | Universal |
|
|
| ESP_3V3 | power:+3V3 | Project-specific name |
|
|
| VCC_5V | power:+5V | Semi-standard |
|
|
| VCC_3V3 | power:+3V3 | Semi-standard |
|
|
|
|
**What I'd recommend for the tool:**
|
|
|
|
```python
|
|
DEFAULT_POWER_SYMBOL_MAP = {
|
|
# Universal — these map 1:1 to KiCad power symbols
|
|
"GND": "power:GND",
|
|
"+3V3": "power:+3V3",
|
|
"+5V": "power:+5V",
|
|
"+3.3V": "power:+3.3V",
|
|
"+1V8": "power:+1V8",
|
|
"GNDA": "power:GNDA",
|
|
"GNDD": "power:GNDD",
|
|
"VCC": "power:VCC",
|
|
"VDD": "power:VDD",
|
|
"+12V": "power:+12V",
|
|
}
|
|
```
|
|
|
|
Then accept a `power_symbol_overrides` parameter that adds to or replaces entries:
|
|
|
|
```json
|
|
{
|
|
"power_symbol_overrides": {
|
|
"ESP_3V3": "power:+3V3",
|
|
"VCC_5V": "power:+5V",
|
|
"AGND": "power:GNDA"
|
|
}
|
|
}
|
|
```
|
|
|
|
The default set covers any project using KiCad's standard power net naming. Projects with vendor-specific names (like our `ESP_3V3`) supply overrides. This keeps the common case zero-config while handling real-world variation.
|
|
|
|
The `is_power_net()` heuristic (prefix matching on VDD/VCC/VBUS, suffix matching on 3V3/5V) is also worth including as a fallback classifier, but it should only control the power-vs-label decision — NOT the symbol mapping. A net can be classified as "power" (use `power_symbols` section, not `label_connections`) without having a KiCad power symbol assigned. In that case, fall back to a global label.
|
|
|
|
## 5. parse_netlist_file(): Custom Format, Not import_netlist
|
|
|
|
Our `parse_netlist_file()` is a custom parser for **OCR'd PDF text**, not a KiCad netlist file. The format is a legacy CAD netlist notation:
|
|
|
|
```
|
|
PIU80101
|
|
PIU80102
|
|
NLGND
|
|
PIU13011
|
|
NLAVDD
|
|
```
|
|
|
|
Where:
|
|
- `PI` prefix = Pin Instance: `PI{component_ref}{pin_number}` (e.g., `PIU80101` = U8 pin 0101)
|
|
- `NL` prefix = Net Label: `NL{net_name}` (e.g., `NLGND` = net "GND")
|
|
- `CO` prefix = Component Owner (BOM-like metadata, precedes the netlist section)
|
|
- Net names use `0` as separator (e.g., `NLGPIO0` = "GPIO0", `NL25M0XI` = "25M_XI")
|
|
|
|
It's the output of Altium/OrCAD-style netlist extraction from the PDF, likely what Waveshare's original CAD tool exported. The `decode_net_name()` function handles the 0-separator disambiguation (distinguishing "GPIO0" from "25M_XI" encoded as "25M0XI").
|
|
|
|
This is entirely unrelated to `import_netlist`'s KiCad `.net` format. However, the OUTPUT of `parse_netlist_file()` matches what your proposed `classify_and_partition` tool would accept:
|
|
|
|
```python
|
|
# Output signature
|
|
nets: dict[str, list[tuple[str, str]]] # net_name -> [(comp_ref, pin_num), ...]
|
|
component_pins: dict[str, dict[str, list[str]]] # comp_ref -> {pin_num: [net_names]}
|
|
```
|
|
|
|
This is the same structure that `import_netlist` produces from a KiCad netlist. So the classification tool could accept either format transparently — it just needs `nets` as a dict of net names to connection lists.
|
|
|
|
**Special handling our parser does that the tool should know about:**
|
|
|
|
1. **Combined tokens**: PDF extraction sometimes merges PI and NL tokens without whitespace (`PIU13011NLAVDD`). Our parser splits these with regex.
|
|
2. **Missing NL labels**: Several net blocks lost their NL label during PDF extraction. We inject them at known positions based on manual analysis (AGND, ESP_LDO_VO1-4, ESP_VBAT, ESP_VDD_HP).
|
|
3. **Reference remapping**: The netlist uses `Key1`/`Key2` for tactile switches, which KiCad rejects. Remapped to `SW1`/`SW2`.
|
|
|
|
Items 1-3 are specific to the "PDF as source" workflow and shouldn't be in the classification tool. They belong in the extraction/preprocessing step.
|
|
|
|
---
|
|
|
|
**Summary of answers:**
|
|
|
|
| Question | Answer |
|
|
|----------|--------|
|
|
| Y-axis repro | ✓ Confirmed. R27 pin 2: negate gives y=123.19 (matches wire), no-negate gives y=115.57 (wrong). |
|
|
| classify_components() | Source provided with dependencies. Key design: topological classification via net analysis, parent IC via shared non-power nets. |
|
|
| compute_sheet_globals() | Source provided. Pure topology: count sheets per net, power nets always global. |
|
|
| NET_TO_POWER_SYM config | Per-project overrides on top of a default set of standard KiCad power symbols. |
|
|
| parse_netlist_file() format | Custom OCR parser, NOT import_netlist format. But output structure is identical — `nets: dict[str, list[tuple]]`. |
|
|
|
|
---
|
|
|
|
**Our priorities align with yours.** Items 1-2 (collision detection + tab indentation) would eliminate our two largest post-processing scripts. Item 3 (auto power-net promotion) would simplify batch generation. Items 4-5 would let us delete `build_batches.py` entirely.
|
|
|
|
One additional data point: fixing the Y-axis transform would eliminate our third post-processing script (`fix_pin_positions.py`, 250 lines). That script currently strips and regenerates all power symbols, wires, and labels at corrected positions — work that apply_batch should do correctly on the first pass.
|