fix: change snap_to_grid default grid from 2.54mm to 1.27mm (50 mil)
Half of all valid KiCAD schematic pin positions are on the 50-mil (1.27mm) grid but not the 100-mil (2.54mm) grid — e.g. 26.67mm = 21 × 1.27mm. Snapping to 2.54mm displaced those coordinates by 1.27mm, moving labels off their pins and increasing floating-label count. KiCAD source confirms: DEFAULT_CONNECTION_GRID_MILS = 50 and the ERC off-grid check uses exact integer modulo against this value, so any displacement breaks connectivity unconditionally. Also update the kicad-source absolute path in CLAUDE.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
60
CLAUDE.md
Normal file
60
CLAUDE.md
Normal file
@@ -0,0 +1,60 @@
|
|||||||
|
# KiCAD MCP Server
|
||||||
|
|
||||||
|
## Related Source
|
||||||
|
|
||||||
|
- KiCad source code is located at `../kicad-source/` (absolute: `/home/eugene/Projects/kicad-source/`)
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
### When to Write Tests
|
||||||
|
|
||||||
|
Write tests for every non-trivial change to Python handler or business logic code:
|
||||||
|
|
||||||
|
- **New MCP tools** — add tests for schema validation, handler dispatch, parameter validation, and the core logic path (happy path + key error cases).
|
||||||
|
- **Changes to existing tools** — add tests covering the changed behaviour; update any tests that no longer reflect reality.
|
||||||
|
- **Bug fixes** — add a regression test that would have caught the bug before adding the fix.
|
||||||
|
- **Refactors that delete or rename public methods** — add a test asserting the old name no longer exists (see `TestConnectionManagerOrphanedMethodsRemoved` for the pattern).
|
||||||
|
|
||||||
|
You do **not** need tests for TypeScript/TS-layer glue code that only forwards calls to Python (the TS test runner is not yet configured).
|
||||||
|
|
||||||
|
### Test Levels
|
||||||
|
|
||||||
|
| Level | Use for | Marker |
|
||||||
|
| ----------- | -------------------------------------------------------------------------------------------------- | -------------------------- |
|
||||||
|
| Unit | Schema shape, parameter validation, pure logic, mock-heavy handler dispatch | `@pytest.mark.unit` |
|
||||||
|
| Integration | Real file I/O against a `.kicad_sch` / `.kicad_pcb` copy; WireManager, JunctionManager round-trips | `@pytest.mark.integration` |
|
||||||
|
|
||||||
|
Keep unit tests free of file I/O. Keep integration tests free of business-logic assertions that belong in unit tests.
|
||||||
|
|
||||||
|
### Where to Put Tests
|
||||||
|
|
||||||
|
```
|
||||||
|
tests/
|
||||||
|
test_<feature_name>.py # all Python tests go here
|
||||||
|
```
|
||||||
|
|
||||||
|
Group related test classes inside a single file (e.g. `TestSchemas`, `TestHandlerDispatch`, `TestHandleAddSchematicWireRouting` all in `test_wire_junction_changes.py`). Name classes `Test<Area>` and methods `test_<what_is_verified>`.
|
||||||
|
|
||||||
|
Use `python/templates/empty.kicad_sch` as the base fixture for integration tests — copy it to a `tempfile` directory, run the handler, then parse the result with `sexpdata`.
|
||||||
|
|
||||||
|
### Running Tests
|
||||||
|
|
||||||
|
Always use the `.venv` virtualenv for Python commands:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
npm run test:py # pytest tests/ -v
|
||||||
|
.venv/bin/pytest tests/ -v # all Python tests
|
||||||
|
.venv/bin/pytest -m unit # unit tests only
|
||||||
|
.venv/bin/pytest -m integration # integration tests only
|
||||||
|
.venv/bin/pytest --cov=python # with coverage report
|
||||||
|
.venv/bin/mypy python/ # type checking
|
||||||
|
```
|
||||||
|
|
||||||
|
## Git Workflow
|
||||||
|
|
||||||
|
- **Never open a pull request automatically.** Commit and push when asked, but always wait for explicit instructions before running `gh pr create` or any equivalent command.
|
||||||
|
|
||||||
|
## Python Code Style
|
||||||
|
|
||||||
|
- **Never use `assert` in production code** — raise a specific exception (`ValueError`, `RuntimeError`, etc.) instead. `assert` is stripped in optimised builds and gives poor error messages.
|
||||||
|
- **Do not introduce logic-breaking workarounds to satisfy the type checker** (e.g. `x or ""` when `""` is not a valid substitute for `None`). Fix the types or narrow with a proper guard (`if x is None: raise ...`).
|
||||||
@@ -4,9 +4,18 @@ Snap-to-grid tool for KiCAD schematics.
|
|||||||
Snaps wire endpoints, junction positions, net labels, and optionally component
|
Snaps wire endpoints, junction positions, net labels, and optionally component
|
||||||
positions to the nearest grid point. Modifies the schematic file in place.
|
positions to the nearest grid point. Modifies the schematic file in place.
|
||||||
|
|
||||||
The standard KiCAD eeschema grid is 2.54 mm (0.1 inch). Off-grid coordinates
|
The standard KiCAD schematic grid is 50 mil (1.27 mm). Component pins are
|
||||||
cause wires that appear visually connected to fail ERC connectivity checks
|
placed at multiples of 1.27 mm relative to the symbol origin, so absolute pin
|
||||||
because KiCAD uses exact integer (IU) matching internally.
|
coordinates end up as odd multiples of 1.27 mm (e.g. 26.67 mm = 21 × 1.27 mm).
|
||||||
|
These are valid on-grid positions that must not be moved.
|
||||||
|
|
||||||
|
The coarser 2.54 mm (100-mil) grid is a common mistake: exactly half of all
|
||||||
|
valid 1.27 mm positions are not multiples of 2.54 mm and would be displaced by
|
||||||
|
1.27 mm — moving labels or wire endpoints off their pins and breaking
|
||||||
|
connectivity.
|
||||||
|
|
||||||
|
Off-grid coordinates cause wires that appear visually connected to fail ERC
|
||||||
|
connectivity checks because KiCAD uses exact integer (IU) matching internally.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
@@ -18,7 +27,7 @@ from sexpdata import Symbol
|
|||||||
|
|
||||||
logger = logging.getLogger("kicad_interface")
|
logger = logging.getLogger("kicad_interface")
|
||||||
|
|
||||||
_DEFAULT_GRID_MM: float = 2.54
|
_DEFAULT_GRID_MM: float = 1.27
|
||||||
|
|
||||||
# Element type names exposed in the public API
|
# Element type names exposed in the public API
|
||||||
_VALID_ELEMENTS = frozenset({"wires", "junctions", "labels", "components"})
|
_VALID_ELEMENTS = frozenset({"wires", "junctions", "labels", "components"})
|
||||||
@@ -91,7 +100,10 @@ def snap_to_grid(
|
|||||||
|
|
||||||
Args:
|
Args:
|
||||||
schematic_path: Path to the ``.kicad_sch`` file.
|
schematic_path: Path to the ``.kicad_sch`` file.
|
||||||
grid_size: Grid spacing in mm (default 2.54 mm = 0.1 inch).
|
grid_size: Grid spacing in mm (default 1.27 mm = 50 mil).
|
||||||
|
Do NOT use 2.54 mm — half of all valid KiCAD pin
|
||||||
|
positions fall between 2.54 mm grid lines and would
|
||||||
|
be displaced 1.27 mm, breaking connectivity.
|
||||||
elements: List of element types to snap. Valid values:
|
elements: List of element types to snap. Valid values:
|
||||||
``"wires"``, ``"junctions"``, ``"labels"``,
|
``"wires"``, ``"junctions"``, ``"labels"``,
|
||||||
``"components"``. Defaults to
|
``"components"``. Defaults to
|
||||||
|
|||||||
@@ -3121,7 +3121,7 @@ class KiCADInterface:
|
|||||||
if not schematic_path:
|
if not schematic_path:
|
||||||
return {"success": False, "message": "schematicPath is required"}
|
return {"success": False, "message": "schematicPath is required"}
|
||||||
|
|
||||||
grid_size = float(params.get("gridSize", 2.54))
|
grid_size = float(params.get("gridSize", 1.27))
|
||||||
elements = params.get("elements") # None → defaults inside snap_to_grid
|
elements = params.get("elements") # None → defaults inside snap_to_grid
|
||||||
|
|
||||||
result = snap_to_grid(Path(schematic_path), grid_size=grid_size, elements=elements)
|
result = snap_to_grid(Path(schematic_path), grid_size=grid_size, elements=elements)
|
||||||
|
|||||||
@@ -1902,12 +1902,12 @@ SCHEMATIC_TOOLS = [
|
|||||||
"gridSize": {
|
"gridSize": {
|
||||||
"type": "number",
|
"type": "number",
|
||||||
"description": (
|
"description": (
|
||||||
"Grid spacing in mm. "
|
"Grid spacing in mm (default: 1.27 — standard KiCAD schematic grid). "
|
||||||
"Standard KiCAD schematic grid is 2.54 mm (0.1 inch). "
|
"Do NOT use 2.54: half of all valid KiCAD pin positions are at odd "
|
||||||
"Use 1.27 mm for high-density layouts. "
|
"multiples of 1.27 mm and would be displaced 1.27 mm, breaking "
|
||||||
"Defaults to 2.54."
|
"connectivity."
|
||||||
),
|
),
|
||||||
"default": 2.54,
|
"default": 1.27,
|
||||||
},
|
},
|
||||||
"elements": {
|
"elements": {
|
||||||
"type": "array",
|
"type": "array",
|
||||||
|
|||||||
@@ -203,7 +203,7 @@ class TestSnapDefaults:
|
|||||||
path = _make_temp_schematic(extra)
|
path = _make_temp_schematic(extra)
|
||||||
result = snap_to_grid(path) # defaults: grid=2.54, elements=None
|
result = snap_to_grid(path) # defaults: grid=2.54, elements=None
|
||||||
assert result["snapped"] >= 3
|
assert result["snapped"] >= 3
|
||||||
assert result["grid_size"] == pytest.approx(2.54)
|
assert result["grid_size"] == pytest.approx(1.27)
|
||||||
|
|
||||||
def test_idempotent(self):
|
def test_idempotent(self):
|
||||||
path = _make_temp_schematic(_wire_sexp(2.51, 5.03, 7.56, 5.03))
|
path = _make_temp_schematic(_wire_sexp(2.51, 5.03, 7.56, 5.03))
|
||||||
@@ -213,6 +213,23 @@ class TestSnapDefaults:
|
|||||||
content_after_second = path.read_text(encoding="utf-8")
|
content_after_second = path.read_text(encoding="utf-8")
|
||||||
assert content_after_first == content_after_second
|
assert content_after_first == content_after_second
|
||||||
|
|
||||||
|
def test_default_grid_is_1_27mm(self):
|
||||||
|
# Regression: default was 2.54 mm, which displaces valid KiCAD pin
|
||||||
|
# coordinates that fall on the 50-mil (1.27 mm) grid but not on the
|
||||||
|
# 100-mil (2.54 mm) grid — e.g. 26.67 mm = 21 × 1.27 mm.
|
||||||
|
# With the correct 1.27 mm default those coordinates must be left
|
||||||
|
# untouched (snapped == 0, already_on_grid >= 1).
|
||||||
|
# 26.67 / 2.54 == 10.5 → would snap to 25.40 mm (off by 1.27 mm).
|
||||||
|
# 26.67 / 1.27 == 21.0 → already on grid, no move.
|
||||||
|
path = _make_temp_schematic(_wire_sexp(335.28, 26.67, 350.52, 26.67))
|
||||||
|
result = snap_to_grid(path) # default grid
|
||||||
|
assert result["grid_size"] == pytest.approx(1.27)
|
||||||
|
assert result["snapped"] == 0, (
|
||||||
|
"Wire at valid 50-mil pin coordinates was displaced by default snap — "
|
||||||
|
"default grid must be 1.27 mm, not 2.54 mm"
|
||||||
|
)
|
||||||
|
assert result["already_on_grid"] >= 1
|
||||||
|
|
||||||
def test_custom_grid(self):
|
def test_custom_grid(self):
|
||||||
# 1.27 mm grid — wire at 1.25 should snap to 1.27
|
# 1.27 mm grid — wire at 1.25 should snap to 1.27
|
||||||
path = _make_temp_schematic(_wire_sexp(1.25, 1.25, 2.51, 2.51))
|
path = _make_temp_schematic(_wire_sexp(1.25, 1.25, 2.51, 2.51))
|
||||||
|
|||||||
Reference in New Issue
Block a user