fix: get_symbol_info returns wrong data for every small symbol
Parser used a 5000-char heuristic slice per symbol; any symbol block shorter than 5000 chars bled into the next one, and last-write-wins in the properties dict ensured the neighbor's data clobbered the target. Reported as Simulation_SPICE:OPAMP returning PJFET data. Switch to parenthesis-depth tracking to find the true end of each (symbol ...) block. Also surface Sim.Pins so agents can read opamp pin numbering without inferring it from schematic placement. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Eugene Mikhantyev
@@ -33,6 +33,7 @@ class SymbolInfo:
|
||||
stock: str = "" # Stock (from JLCPCB libs)
|
||||
price: str = "" # Price (from JLCPCB libs)
|
||||
lib_class: str = "" # Basic/Preferred/Extended
|
||||
sim_pins: str = "" # Sim.Pins pin mapping (e.g. "1=in+ 2=in- 3=vcc 4=vee 5=out")
|
||||
|
||||
|
||||
class SymbolLibraryManager:
|
||||
@@ -268,11 +269,28 @@ class SymbolLibraryManager:
|
||||
# Find the start position of this symbol
|
||||
start_pos = match.start()
|
||||
|
||||
# Extract properties from this symbol block
|
||||
# We need to find the matching closing paren - use a simple heuristic
|
||||
# Look for the next 2000 characters for properties
|
||||
block_end = min(start_pos + 5000, len(content))
|
||||
symbol_block = content[start_pos:block_end]
|
||||
# Walk forward tracking parenthesis depth to find the true end of the block
|
||||
depth = 0
|
||||
i = start_pos
|
||||
end_pos = start_pos
|
||||
while i < len(content):
|
||||
ch = content[i]
|
||||
if ch == "(":
|
||||
depth += 1
|
||||
elif ch == ")":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
end_pos = i + 1
|
||||
break
|
||||
i += 1
|
||||
|
||||
if end_pos == start_pos:
|
||||
logger.warning(
|
||||
f"Malformed symbol block for '{symbol_name}' in {library_path}; skipping"
|
||||
)
|
||||
continue
|
||||
|
||||
symbol_block = content[start_pos:end_pos]
|
||||
|
||||
# Extract properties
|
||||
properties = self._extract_properties(symbol_block)
|
||||
@@ -292,6 +310,7 @@ class SymbolLibraryManager:
|
||||
stock=properties.get("Stock", ""),
|
||||
price=properties.get("Price", ""),
|
||||
lib_class=properties.get("Class", ""),
|
||||
sim_pins=properties.get("Sim.Pins", ""),
|
||||
)
|
||||
|
||||
symbols.append(symbol_info)
|
||||
|
||||
@@ -158,6 +158,7 @@ Returns symbol references that can be used directly in schematics.`,
|
||||
info.category ? `Category: ${info.category}` : "",
|
||||
info.lib_class ? `Class: ${info.lib_class}` : "",
|
||||
info.datasheet ? `Datasheet: ${info.datasheet}` : "",
|
||||
info.sim_pins ? `Sim.Pins: ${info.sim_pins}` : "",
|
||||
]
|
||||
.filter((line) => line)
|
||||
.join("\n");
|
||||
|
||||
421
tests/fixtures/Simulation_SPICE_minimal.kicad_sym
vendored
Normal file
421
tests/fixtures/Simulation_SPICE_minimal.kicad_sym
vendored
Normal file
@@ -0,0 +1,421 @@
|
||||
(kicad_symbol_lib
|
||||
(version 20241209)
|
||||
(generator "kicad_symbol_editor")
|
||||
(generator_version "9.0")
|
||||
(symbol "OPAMP"
|
||||
(pin_names
|
||||
(offset 0.254)
|
||||
)
|
||||
(exclude_from_sim no)
|
||||
(in_bom yes)
|
||||
(on_board yes)
|
||||
(property "Reference" "U"
|
||||
(at 3.81 3.175 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
)
|
||||
(property "Value" "${SIM.PARAMS}"
|
||||
(at 3.81 -3.175 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
)
|
||||
(property "Footprint" ""
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Datasheet" "https://ngspice.sourceforge.io/docs/ngspice-html-manual/manual.xhtml#sec__SUBCKT_Subcircuits"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Description" "Operational amplifier, single"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Sim.Pins" "1=in+ 2=in- 3=vcc 4=vee 5=out"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Sim.Device" "SUBCKT"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Sim.Library" "${KICAD9_SYMBOL_DIR}/Simulation_SPICE.sp"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Sim.Name" "kicad_builtin_opamp"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "ki_keywords" "simulation"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(symbol "OPAMP_0_1"
|
||||
(polyline
|
||||
(pts
|
||||
(xy 5.08 0) (xy -5.08 5.08) (xy -5.08 -5.08) (xy 5.08 0)
|
||||
)
|
||||
(stroke
|
||||
(width 0.254)
|
||||
(type default)
|
||||
)
|
||||
(fill
|
||||
(type background)
|
||||
)
|
||||
)
|
||||
)
|
||||
(symbol "OPAMP_1_1"
|
||||
(pin input line
|
||||
(at -7.62 2.54 0)
|
||||
(length 2.54)
|
||||
(name "+"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "1"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(pin input line
|
||||
(at -7.62 -2.54 0)
|
||||
(length 2.54)
|
||||
(name "-"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "2"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(pin power_in line
|
||||
(at -2.54 7.62 270)
|
||||
(length 3.81)
|
||||
(name "V+"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "3"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(pin power_in line
|
||||
(at -2.54 -7.62 90)
|
||||
(length 3.81)
|
||||
(name "V-"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "4"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(pin output line
|
||||
(at 7.62 0 180)
|
||||
(length 2.54)
|
||||
(name "~"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "5"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(embedded_fonts no)
|
||||
)
|
||||
(symbol "PJFET"
|
||||
(pin_numbers
|
||||
(hide yes)
|
||||
)
|
||||
(pin_names
|
||||
(offset 0)
|
||||
)
|
||||
(exclude_from_sim no)
|
||||
(in_bom yes)
|
||||
(on_board yes)
|
||||
(property "Reference" "Q"
|
||||
(at 5.08 1.27 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
)
|
||||
(property "Value" "PJFET"
|
||||
(at 5.08 -1.27 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(justify left)
|
||||
)
|
||||
)
|
||||
(property "Footprint" ""
|
||||
(at 5.08 2.54 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Datasheet" "https://ngspice.sourceforge.io/docs/ngspice-html-manual/manual.xhtml#cha_JFETs"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Description" "P-JFET transistor, for simulation only"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Sim.Device" "PJFET"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Sim.Type" "SHICHMANHODGES"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "Sim.Pins" "1=D 2=G 3=S"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(property "ki_keywords" "transistor PJFET P-JFET"
|
||||
(at 0 0 0)
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
(hide yes)
|
||||
)
|
||||
)
|
||||
(symbol "PJFET_0_1"
|
||||
(polyline
|
||||
(pts
|
||||
(xy -1.143 0) (xy -0.127 0.381) (xy -0.127 -0.381) (xy -1.143 0)
|
||||
)
|
||||
(stroke
|
||||
(width 0)
|
||||
(type default)
|
||||
)
|
||||
(fill
|
||||
(type outline)
|
||||
)
|
||||
)
|
||||
(polyline
|
||||
(pts
|
||||
(xy 0.254 1.905) (xy 0.254 -1.905) (xy 0.254 -1.905)
|
||||
)
|
||||
(stroke
|
||||
(width 0.254)
|
||||
(type default)
|
||||
)
|
||||
(fill
|
||||
(type none)
|
||||
)
|
||||
)
|
||||
(circle
|
||||
(center 1.27 0)
|
||||
(radius 2.8194)
|
||||
(stroke
|
||||
(width 0.254)
|
||||
(type default)
|
||||
)
|
||||
(fill
|
||||
(type none)
|
||||
)
|
||||
)
|
||||
(polyline
|
||||
(pts
|
||||
(xy 2.54 2.54) (xy 2.54 1.397) (xy 0.254 1.397)
|
||||
)
|
||||
(stroke
|
||||
(width 0)
|
||||
(type default)
|
||||
)
|
||||
(fill
|
||||
(type none)
|
||||
)
|
||||
)
|
||||
(polyline
|
||||
(pts
|
||||
(xy 2.54 -2.54) (xy 2.54 -1.27) (xy 0.254 -1.27)
|
||||
)
|
||||
(stroke
|
||||
(width 0)
|
||||
(type default)
|
||||
)
|
||||
(fill
|
||||
(type none)
|
||||
)
|
||||
)
|
||||
)
|
||||
(symbol "PJFET_1_1"
|
||||
(pin input line
|
||||
(at -5.08 0 0)
|
||||
(length 5.334)
|
||||
(name "G"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "2"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(pin passive line
|
||||
(at 2.54 5.08 270)
|
||||
(length 2.54)
|
||||
(name "D"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "1"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(pin passive line
|
||||
(at 2.54 -5.08 90)
|
||||
(length 2.54)
|
||||
(name "S"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
(number "3"
|
||||
(effects
|
||||
(font
|
||||
(size 1.27 1.27)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
(embedded_fonts no)
|
||||
)
|
||||
)
|
||||
117
tests/test_symbol_library.py
Normal file
117
tests/test_symbol_library.py
Normal file
@@ -0,0 +1,117 @@
|
||||
"""
|
||||
Regression tests for symbol library parsing.
|
||||
|
||||
Covers:
|
||||
- The 5000-char heuristic bug where PJFET properties bled into the OPAMP block.
|
||||
- The sim_pins field exposed from Sim.Pins properties.
|
||||
"""
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent / "python"))
|
||||
|
||||
from commands.library_symbol import SymbolLibraryCommands, SymbolLibraryManager
|
||||
|
||||
FIXTURE = Path(__file__).parent / "fixtures" / "Simulation_SPICE_minimal.kicad_sym"
|
||||
SPICE_LIB = Path("/usr/share/kicad/symbols/Simulation_SPICE.kicad_sym")
|
||||
|
||||
|
||||
def _manager_for_fixture() -> SymbolLibraryManager:
|
||||
manager = SymbolLibraryManager.__new__(SymbolLibraryManager)
|
||||
manager.project_path = None
|
||||
manager.libraries = {"Simulation_SPICE": str(FIXTURE)}
|
||||
manager.symbol_cache = {}
|
||||
return manager
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestSymbolLibraryManagerParsing:
|
||||
"""Unit tests using the minimal fixture file (no disk I/O to system libs)."""
|
||||
|
||||
def test_both_symbols_present(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
names = [s.name for s in symbols]
|
||||
assert "OPAMP" in names
|
||||
assert "PJFET" in names
|
||||
assert len(symbols) == 2
|
||||
|
||||
def test_opamp_description_is_not_pjfet(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
opamp = next(s for s in symbols if s.name == "OPAMP")
|
||||
assert "JFET" not in opamp.description
|
||||
|
||||
def test_opamp_description_correct(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
opamp = next(s for s in symbols if s.name == "OPAMP")
|
||||
assert opamp.description == "Operational amplifier, single"
|
||||
|
||||
def test_opamp_value_correct(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
opamp = next(s for s in symbols if s.name == "OPAMP")
|
||||
assert opamp.value == "${SIM.PARAMS}"
|
||||
|
||||
def test_opamp_sim_pins_exposed(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
opamp = next(s for s in symbols if s.name == "OPAMP")
|
||||
assert opamp.sim_pins == "1=in+ 2=in- 3=vcc 4=vee 5=out"
|
||||
|
||||
def test_pjfet_description_correct(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
pjfet = next(s for s in symbols if s.name == "PJFET")
|
||||
assert pjfet.description == "P-JFET transistor, for simulation only"
|
||||
|
||||
def test_pjfet_value_correct(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
pjfet = next(s for s in symbols if s.name == "PJFET")
|
||||
assert pjfet.value == "PJFET"
|
||||
|
||||
def test_pjfet_sim_pins_exposed(self):
|
||||
manager = _manager_for_fixture()
|
||||
symbols = manager.list_symbols("Simulation_SPICE")
|
||||
pjfet = next(s for s in symbols if s.name == "PJFET")
|
||||
assert pjfet.sim_pins == "1=D 2=G 3=S"
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
class TestGetSymbolInfoHandler:
|
||||
"""Integration tests against the real system Simulation_SPICE library."""
|
||||
|
||||
def test_opamp_via_commands_handler(self):
|
||||
if not SPICE_LIB.exists():
|
||||
pytest.skip(f"System library not found: {SPICE_LIB}")
|
||||
manager = SymbolLibraryManager.__new__(SymbolLibraryManager)
|
||||
manager.project_path = None
|
||||
manager.libraries = {"Simulation_SPICE": str(SPICE_LIB)}
|
||||
manager.symbol_cache = {}
|
||||
commands = SymbolLibraryCommands(library_manager=manager)
|
||||
result = commands.get_symbol_info({"symbol": "Simulation_SPICE:OPAMP"})
|
||||
assert result["success"] is True
|
||||
info = result["symbol_info"]
|
||||
assert info["description"] == "Operational amplifier, single"
|
||||
assert "JFET" not in info["description"]
|
||||
assert info["value"] == "${SIM.PARAMS}"
|
||||
assert info["sim_pins"] == "1=in+ 2=in- 3=vcc 4=vee 5=out"
|
||||
|
||||
def test_pjfet_via_commands_handler(self):
|
||||
if not SPICE_LIB.exists():
|
||||
pytest.skip(f"System library not found: {SPICE_LIB}")
|
||||
manager = SymbolLibraryManager.__new__(SymbolLibraryManager)
|
||||
manager.project_path = None
|
||||
manager.libraries = {"Simulation_SPICE": str(SPICE_LIB)}
|
||||
manager.symbol_cache = {}
|
||||
commands = SymbolLibraryCommands(library_manager=manager)
|
||||
result = commands.get_symbol_info({"symbol": "Simulation_SPICE:PJFET"})
|
||||
assert result["success"] is True
|
||||
info = result["symbol_info"]
|
||||
assert info["description"] == "P-JFET transistor, for simulation only"
|
||||
assert info["value"] == "PJFET"
|
||||
Reference in New Issue
Block a user