fix: correct pin location calculation and symbol reference dedup in kicad-skip
Three bugs fixed in the schematic component and pin locator pipeline:
1. component_schematic: remove redundant symbol.append() after clone()
kicad-skip's clone() already inserts the raw element into the schematic
tree. The subsequent NamedCollection.append() detects the reference as
already registered (from the elementRename triggered by setting
property.Reference.value) and renames it "R1_" with a trailing
underscore, causing all subsequent pin lookups to fail.
2. pin_locator: negate lib y coordinate before rotation
lib_symbols in .kicad_sch use library y-up convention; schematic
coordinates use y-down. get_pin_location now negates pin_rel_y before
applying rotation, matching KiCad's own transform order (same approach
as _transform_local_point in schematic_analysis.py).
3. pin_locator: add .rstrip("_") guard in all symbol reference lookups
Defensive guard against any residual cases where kicad-skip writes a
trailing underscore to the Reference property value.
Also fixes the self-test script to use template_with_symbols.kicad_sch
(which contains placed _TEMPLATE_* symbols) rather than the expanded
template (which only contains lib_symbols definitions and has no cloneable
instances).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
committed by
Eugene Mikhantyev
parent
f494f5fb37
commit
ab2500fb8d
6
package-lock.json
generated
6
package-lock.json
generated
@@ -848,9 +848,9 @@
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
"node_modules/brace-expansion": {
|
"node_modules/brace-expansion": {
|
||||||
"version": "1.1.14",
|
"version": "1.1.13",
|
||||||
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.14.tgz",
|
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.13.tgz",
|
||||||
"integrity": "sha512-MWPGfDxnyzKU7rNOW9SP/c50vi3xrmrua/+6hfPbCS2ABNWfx24vPidzvC7krjU/RTo235sV776ymlsMtGKj8g==",
|
"integrity": "sha512-9ZLprWS6EENmhEOpjCYW2c8VkmOvckIJZfkr7rBW6dObmfgJ/L1GpSYW5Hpo9lDz4D1+n0Ckz8rU7FwHDQiG/w==",
|
||||||
"dev": true,
|
"dev": true,
|
||||||
"license": "MIT",
|
"license": "MIT",
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
|
|||||||
@@ -255,8 +255,9 @@ class ComponentManager:
|
|||||||
# Generate new UUID
|
# Generate new UUID
|
||||||
new_symbol.uuid.value = str(uuid.uuid4())
|
new_symbol.uuid.value = str(uuid.uuid4())
|
||||||
|
|
||||||
# Append to schematic
|
# NOTE: clone() already inserts the raw element into the schematic tree.
|
||||||
schematic.symbol.append(new_symbol)
|
# Calling schematic.symbol.append() again causes NamedCollection to detect
|
||||||
|
# the reference as "taken" and rename it to "R1_" (trailing underscore).
|
||||||
logger.info(f"Successfully added component {reference} to schematic")
|
logger.info(f"Successfully added component {reference} to schematic")
|
||||||
|
|
||||||
return new_symbol
|
return new_symbol
|
||||||
|
|||||||
@@ -180,7 +180,7 @@ class PinLocator:
|
|||||||
self._schematic_cache[sch_key] = Schematic(sch_key)
|
self._schematic_cache[sch_key] = Schematic(sch_key)
|
||||||
sch = self._schematic_cache[sch_key]
|
sch = self._schematic_cache[sch_key]
|
||||||
for symbol in sch.symbol:
|
for symbol in sch.symbol:
|
||||||
if symbol.property.Reference.value == symbol_reference:
|
if symbol.property.Reference.value.rstrip("_") == symbol_reference:
|
||||||
return symbol.lib_id.value if hasattr(symbol, "lib_id") else None
|
return symbol.lib_id.value if hasattr(symbol, "lib_id") else None
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
@@ -203,7 +203,7 @@ class PinLocator:
|
|||||||
|
|
||||||
target_symbol = None
|
target_symbol = None
|
||||||
for symbol in sch.symbol:
|
for symbol in sch.symbol:
|
||||||
if symbol.property.Reference.value == symbol_reference:
|
if symbol.property.Reference.value.rstrip("_") == symbol_reference:
|
||||||
target_symbol = symbol
|
target_symbol = symbol
|
||||||
break
|
break
|
||||||
|
|
||||||
@@ -258,10 +258,11 @@ class PinLocator:
|
|||||||
self._schematic_cache[sch_key] = Schematic(sch_key)
|
self._schematic_cache[sch_key] = Schematic(sch_key)
|
||||||
sch = self._schematic_cache[sch_key]
|
sch = self._schematic_cache[sch_key]
|
||||||
|
|
||||||
# Find the symbol instance
|
# Find the symbol instance.
|
||||||
|
# skip may write references with a trailing "_" (e.g. "R1_") — strip it when comparing.
|
||||||
target_symbol = None
|
target_symbol = None
|
||||||
for symbol in sch.symbol:
|
for symbol in sch.symbol:
|
||||||
ref = symbol.property.Reference.value
|
ref = symbol.property.Reference.value.rstrip("_")
|
||||||
if ref == symbol_reference:
|
if ref == symbol_reference:
|
||||||
target_symbol = symbol
|
target_symbol = symbol
|
||||||
break
|
break
|
||||||
@@ -313,9 +314,11 @@ class PinLocator:
|
|||||||
|
|
||||||
pin_data = pins[pin_number]
|
pin_data = pins[pin_number]
|
||||||
|
|
||||||
# Get pin position relative to symbol origin
|
# Get pin position relative to symbol origin.
|
||||||
|
# lib_symbols uses library y-up convention; schematic uses y-down.
|
||||||
|
# Negate y here before rotation, matching KiCad's transform order.
|
||||||
pin_rel_x = pin_data["x"]
|
pin_rel_x = pin_data["x"]
|
||||||
pin_rel_y = pin_data["y"]
|
pin_rel_y = -pin_data["y"]
|
||||||
|
|
||||||
logger.debug(f"Pin {pin_number} relative position: ({pin_rel_x}, {pin_rel_y})")
|
logger.debug(f"Pin {pin_number} relative position: ({pin_rel_x}, {pin_rel_y})")
|
||||||
|
|
||||||
@@ -361,7 +364,7 @@ class PinLocator:
|
|||||||
# Find symbol
|
# Find symbol
|
||||||
target_symbol = None
|
target_symbol = None
|
||||||
for symbol in sch.symbol:
|
for symbol in sch.symbol:
|
||||||
if symbol.property.Reference.value == symbol_reference:
|
if symbol.property.Reference.value.rstrip("_") == symbol_reference:
|
||||||
target_symbol = symbol
|
target_symbol = symbol
|
||||||
break
|
break
|
||||||
|
|
||||||
@@ -412,9 +415,7 @@ if __name__ == "__main__":
|
|||||||
|
|
||||||
# Create test schematic with components (cross-platform temp directory)
|
# Create test schematic with components (cross-platform temp directory)
|
||||||
test_path = Path(tempfile.gettempdir()) / "test_pin_locator.kicad_sch"
|
test_path = Path(tempfile.gettempdir()) / "test_pin_locator.kicad_sch"
|
||||||
template_path = (
|
template_path = Path(__file__).parent.parent / "templates" / "template_with_symbols.kicad_sch"
|
||||||
Path(__file__).parent.parent / "templates" / "template_with_symbols_expanded.kicad_sch"
|
|
||||||
)
|
|
||||||
|
|
||||||
shutil.copy(template_path, test_path)
|
shutil.copy(template_path, test_path)
|
||||||
print(f"\n✓ Created test schematic: {test_path}")
|
print(f"\n✓ Created test schematic: {test_path}")
|
||||||
|
|||||||
Reference in New Issue
Block a user