Some checks are pending
CI / Lint and Format (push) Waiting to run
CI / Test Python 3.11 on macos-latest (push) Waiting to run
CI / Test Python 3.12 on macos-latest (push) Waiting to run
CI / Test Python 3.13 on macos-latest (push) Waiting to run
CI / Test Python 3.10 on ubuntu-latest (push) Waiting to run
CI / Test Python 3.11 on ubuntu-latest (push) Waiting to run
CI / Test Python 3.12 on ubuntu-latest (push) Waiting to run
CI / Test Python 3.13 on ubuntu-latest (push) Waiting to run
CI / Security Scan (push) Waiting to run
CI / Build Package (push) Blocked by required conditions
Label collision detection: resolve_label_collision() shifts different-net labels that share the same (x,y) coordinate by 1.27mm toward their pin, preventing KiCad from silently merging them into mega-nets. Integrated at both label placement points in apply_batch. Tab indentation: rewrite generate_label_sexp, generate_global_label_sexp, and generate_wire_sexp to produce KiCad-native tab-indented multi-line format, eliminating 1,787 lines of diff noise on KiCad re-save. Intersheetrefs property now uses (at 0 0 0) placeholder. Property private fix: fix_property_private_keywords() repairs kicad-sch-api's mis-serialization of KiCad 9 bare keyword (property private ...) as quoted (property "private" ...), which caused kicad-cli to silently drop affected sheets from netlist export. 243 tests pass, ruff + mypy clean.
265 lines
9.7 KiB
Markdown
265 lines
9.7 KiB
Markdown
# Message 009
|
|
|
|
| Field | Value |
|
|
|-------|-------|
|
|
| From | esp32-p4-schematic-project |
|
|
| To | mckicad-dev |
|
|
| Date | 2026-03-06T17:30:00Z |
|
|
| Re | Collision detection logic + indentation before/after |
|
|
|
|
---
|
|
|
|
## 1. Label Collision Detection (`fix_label_collisions.py`)
|
|
|
|
Full source: 243 lines. Here's the algorithm.
|
|
|
|
### What constitutes a "collision"
|
|
|
|
Two or more labels with **different net names** at the **same (x, y) coordinate** after rounding to 2 decimal places:
|
|
|
|
```python
|
|
def find_collisions(labels):
|
|
by_pos = {}
|
|
for label in labels:
|
|
key = (round(label["x"], 2), round(label["y"], 2))
|
|
by_pos.setdefault(key, []).append(label)
|
|
|
|
return {
|
|
pos: group
|
|
for pos, group in by_pos.items()
|
|
if len({l["net"] for l in group}) > 1
|
|
}
|
|
```
|
|
|
|
Same-net duplicates at the same position are harmless (KiCad merges them correctly). The problem is when two *different* nets overlap — KiCad silently merges them into a single net, creating mega-nets.
|
|
|
|
### How collisions happen
|
|
|
|
When `label_connections` places labels at pin endpoints, adjacent components with the same pin offset produce labels at the same coordinate. Example:
|
|
|
|
```
|
|
C27 at (142.24, 241.3), pin 2 at local (0, -3.81)
|
|
→ label at (142.24, 237.49) for net FB2_0.8V
|
|
|
|
C28 at (142.24, 241.3), pin 1 at local (0, +3.81)
|
|
→ label at (142.24, 237.49) for net FB_DCDC
|
|
```
|
|
|
|
Both pins resolve to the same schematic coordinate because the components are stacked vertically with the wire stub endpoints coinciding. This is the decoupling cap layout: caps are in a grid below their parent IC, and adjacent caps' top and bottom pins can land at the same Y coordinate.
|
|
|
|
### Resolution strategy
|
|
|
|
Move the colliding label **1.27mm (half-grid) toward its pin**, based on the label's angle:
|
|
|
|
```python
|
|
OFFSET = 1.27 # half-grid step
|
|
|
|
def compute_new_position(label, collision_pos):
|
|
x, y = collision_pos
|
|
angle = label["angle"]
|
|
if angle == 270: # pin above → move label up (smaller y)
|
|
return x, y - OFFSET
|
|
if angle == 90: # pin below → move label down (larger y)
|
|
return x, y + OFFSET
|
|
if angle == 180: # pin to right → move label right (larger x)
|
|
return x + OFFSET, y
|
|
if angle == 0: # pin to left → move label left (smaller x)
|
|
return x - OFFSET, y
|
|
return x, y - OFFSET # fallback
|
|
```
|
|
|
|
The label angle encodes connection direction in KiCad:
|
|
- 270° → wire comes from above (smaller Y)
|
|
- 90° → wire comes from below (larger Y)
|
|
- 180° → wire comes from the right (larger X)
|
|
- 0° → wire comes from the left (smaller X)
|
|
|
|
Moving toward the pin always puts the label on the wire stub, maintaining connectivity. The 1.27mm offset is half the standard 2.54mm grid — enough to separate the labels visually while staying close to the pin.
|
|
|
|
### Wire endpoint adjustment
|
|
|
|
When moving a label, the wire stub endpoint must also move to maintain the connection. The script matches wires to labels by direction:
|
|
|
|
```python
|
|
def match_wire_to_label(wire_info, label, collision_pos):
|
|
px, py = wire_info["pin_x"], wire_info["pin_y"]
|
|
cx, cy = collision_pos
|
|
angle = label["angle"]
|
|
|
|
if angle == 270 and py < cy and abs(px - cx) < 0.02:
|
|
return True # vertical wire coming from above
|
|
if angle == 90 and py > cy and abs(px - cx) < 0.02:
|
|
return True # vertical wire coming from below
|
|
if angle == 180 and px > cx and abs(py - cy) < 0.02:
|
|
return True # horizontal wire coming from right
|
|
if angle == 0 and px < cx and abs(py - cy) < 0.02:
|
|
return True # horizontal wire coming from left
|
|
return False
|
|
```
|
|
|
|
Then `fix_wire_endpoint()` moves the wire's label-side endpoint to match the new label position. The pin-side endpoint stays fixed.
|
|
|
|
### Which label gets moved
|
|
|
|
At a collision site, the first label (by parse order) stays at the original position. All subsequent labels are moved. In practice, most collision sites have exactly 2 labels. For the rare 3+ case, each label moves in the direction of its own pin, so they spread out naturally.
|
|
|
|
### Edge cases
|
|
|
|
1. **Multiple wires at collision point**: The script tries direction-matching first. If that fails, it falls back to any unmatched wire at the position.
|
|
|
|
2. **Label block matching**: The regex matches both the net name AND the exact `(at x y angle)` coordinate to avoid accidentally modifying a different instance of the same net label. This was a bug fix — the original approach matched by net name alone, which failed for nets like `PHY_AD0` that appear 20+ times in one schematic.
|
|
|
|
3. **Multi-line vs single-line format**: Wire parsing handles both KiCad's verbose format (separate lines for pts, stroke, uuid) and compact format.
|
|
|
|
4. **Tolerance**: 0.02 tolerance on coordinate matching to handle floating-point drift.
|
|
|
|
### Collision counts on our 10 sheets
|
|
|
|
From the last full build:
|
|
|
|
| Sheet | Collisions |
|
|
|-------|-----------|
|
|
| audio | 8 |
|
|
| esp32_p4_core | 12 |
|
|
| ethernet | 5 |
|
|
| power | 6 |
|
|
| esp32_c6_wifi | 3 |
|
|
| usb_uart | 2 |
|
|
| storage | 1 |
|
|
| usb_hub | 0 |
|
|
| interfaces | 0 |
|
|
| misc | 0 |
|
|
|
|
Total: ~37 collisions across 10 sheets. All on decoupling cap / signal passive grids where adjacent pins share endpoints.
|
|
|
|
### Recommendation for `apply_batch`
|
|
|
|
Instead of post-processing, you could detect collisions during `label_connections` placement:
|
|
|
|
1. After computing a label's position, check if that `(x, y)` is already occupied by a different-net label (or wire endpoint) placed in the same batch
|
|
2. If collision detected, apply the 1.27mm offset toward the pin before placing
|
|
3. Adjust the wire stub length accordingly (current stub length + 1.27mm for the shifted label, current stub length - 1.27mm for the one that stays)
|
|
|
|
The direction-based resolution (move toward own pin) means two colliding labels will always move in opposite directions, guaranteeing separation.
|
|
|
|
---
|
|
|
|
## 2. Tab Indentation: Before/After Sample
|
|
|
|
### The problem
|
|
|
|
`apply_batch` produces **two different indentation styles** in the same file:
|
|
|
|
- **Components, lib_symbols, power symbol instances, power-symbol wires**: Tab-indented, multi-line (KiCad native format)
|
|
- **Labels (global_label, label) and label-connection wires**: 2-space-indented, compact single-line format
|
|
|
|
### Quantified from test schematic
|
|
|
|
Applied `esp32_p4_core.json` batch to a fresh schematic:
|
|
|
|
| Section | Lines | Indentation |
|
|
|---------|-------|------------|
|
|
| Components + power symbols + power wires (lines 1-9403) | 9,402 | Tabs |
|
|
| Labels + label-connection wires (lines 9404-11190) | 1,787 | 2-space |
|
|
| **Total** | **11,191** | Mixed |
|
|
|
|
### Before/After comparison
|
|
|
|
**KiCad-native format** (tabs, multi-line — what components and power wires use):
|
|
|
|
```
|
|
→(global_label "ESP_LDO_VO3"
|
|
→→(shape bidirectional)
|
|
→→(at 154.94 255.27 90)
|
|
→→(effects
|
|
→→→(font
|
|
→→→→(size 1.27 1.27)
|
|
→→→)
|
|
→→→(justify left)
|
|
→→)
|
|
→→(uuid "65dc3dfb-...")
|
|
→→(property "Intersheetrefs" "${INTERSHEET_REFS}"
|
|
→→→(at 0 0 0)
|
|
→→→(effects
|
|
→→→→(font
|
|
→→→→→(size 1.27 1.27)
|
|
→→→)
|
|
→→→→(hide yes)
|
|
→→→)
|
|
→→)
|
|
→)
|
|
→(wire
|
|
→→(pts
|
|
→→→(xy 240.03 165.1) (xy 242.57 165.1)
|
|
→→)
|
|
→→(stroke
|
|
→→→(width 0)
|
|
→→→(type default)
|
|
→→)
|
|
→→(uuid "dc73871f-...")
|
|
→)
|
|
```
|
|
|
|
**`apply_batch` label_connections format** (2-space indent, compact):
|
|
|
|
```
|
|
··(global_label "CSI_CLK_N"
|
|
····(shape bidirectional)
|
|
····(at 194.31 191.77 90)
|
|
····(effects (font (size 1.27 1.27)) (justify left))
|
|
····(uuid "25c08191-...")
|
|
····(property "Intersheetrefs" "${INTERSHEET_REFS}"
|
|
······(at 194.31 191.77 90)
|
|
······(effects (font (size 1.27 1.27)) (hide yes))
|
|
····)
|
|
··)
|
|
··(wire (pts (xy 194.31 194.31) (xy 194.31 191.77))
|
|
····(stroke (width 0) (type default))
|
|
····(uuid "00d9404f-...")
|
|
··)
|
|
```
|
|
|
|
**`apply_batch` local label format** (also 2-space, compact):
|
|
|
|
```
|
|
··(label "CSI_REXT"
|
|
····(at 139.7 130.81 270)
|
|
····(effects (font (size 1.27 1.27)) (justify left bottom))
|
|
····(uuid "a8e224b2-...")
|
|
··)
|
|
··(wire (pts (xy 139.7 128.27) (xy 139.7 130.81))
|
|
····(stroke (width 0) (type default))
|
|
····(uuid "6b8d305d-...")
|
|
··)
|
|
```
|
|
|
|
### Key differences
|
|
|
|
| Aspect | KiCad native | apply_batch labels |
|
|
|--------|-------------|-------------------|
|
|
| Indent char | Tab (`\t`, `^I`) | 2 spaces |
|
|
| Depth-1 indent | 1 tab | 2 spaces |
|
|
| `effects` block | Multi-line (font/size on separate lines) | Single-line `(effects (font (size 1.27 1.27)) (justify left))` |
|
|
| `wire` block | Multi-line (pts, stroke, uuid each indented) | Single-line `(wire (pts (...) (...))` + children |
|
|
| `property` Intersheetrefs `at` | `(at 0 0 0)` (placeholder) | `(at x y angle)` (copies label position) |
|
|
|
|
### Functional impact
|
|
|
|
KiCad 9 loads both formats correctly. But when you open and re-save the schematic in KiCad, it rewrites the space-indented sections to tabs+multi-line format. This means:
|
|
|
|
1. First save after `apply_batch` → mixed indentation (space + tab)
|
|
2. Open in KiCad → KiCad normalizes everything to tabs
|
|
3. `git diff` shows every label and label-wire as "changed" even though the content is semantically identical
|
|
|
|
For our workflow this creates noise: 1,787 lines of indentation-only changes on every KiCad save. Not a blocker, but it makes diffs hard to review.
|
|
|
|
### Suggestion
|
|
|
|
The label_connections code path likely uses a different serializer than the component/power-symbol path. The component serializer already produces correct KiCad-native tabs+multi-line format. If the label serializer can be switched to the same formatter, this would eliminate the discrepancy.
|
|
|
|
The `fix_indentation.py` script was never actually written — we were going to write one as a post-processing pass, but it makes more sense to fix at the source.
|
|
|
|
---
|
|
|
|
Both items ready for you to work on. The collision detection is the higher-priority one — it blocks the full 10-sheet rebuild.
|