Fix: IPC handlers now convert inch units to mm for position commands (#158)
The _ipc_move_component and _ipc_place_component handlers were ignoring the unit field from position parameters, always treating values as mm. When inches were specified, components would be placed at 1/25.4th of the intended position. Now reads the unit field and converts to mm before passing to the IPC backend. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -5003,10 +5003,16 @@ print("ok")
|
||||
position = params.get("position", {})
|
||||
x = position.get("x", 0) if isinstance(position, dict) else params.get("x", 0)
|
||||
y = position.get("y", 0) if isinstance(position, dict) else params.get("y", 0)
|
||||
unit = position.get("unit", "mm") if isinstance(position, dict) else "mm"
|
||||
rotation = params.get("rotation", 0)
|
||||
layer = params.get("layer", "F.Cu")
|
||||
value = params.get("value", "")
|
||||
|
||||
# Convert inches to mm since ipc_backend expects mm
|
||||
if unit == "inch":
|
||||
x = x * 25.4
|
||||
y = y * 25.4
|
||||
|
||||
success = self.ipc_board_api.place_component(
|
||||
reference=reference,
|
||||
footprint=footprint,
|
||||
@@ -5043,8 +5049,14 @@ print("ok")
|
||||
position = params.get("position", {})
|
||||
x = position.get("x", 0) if isinstance(position, dict) else params.get("x", 0)
|
||||
y = position.get("y", 0) if isinstance(position, dict) else params.get("y", 0)
|
||||
unit = position.get("unit", "mm") if isinstance(position, dict) else "mm"
|
||||
rotation = params.get("rotation")
|
||||
|
||||
# Convert inches to mm since ipc_backend.move_component expects mm
|
||||
if unit == "inch":
|
||||
x = x * 25.4
|
||||
y = y * 25.4
|
||||
|
||||
success = self.ipc_board_api.move_component(
|
||||
reference=reference, x=x, y=y, rotation=rotation
|
||||
)
|
||||
|
||||
89
tests/test_ipc_position_inch_conversion.py
Normal file
89
tests/test_ipc_position_inch_conversion.py
Normal file
@@ -0,0 +1,89 @@
|
||||
"""Regression test for inch-unit conversion in IPC place/move handlers.
|
||||
|
||||
Bug: ``_ipc_place_component`` and ``_ipc_move_component`` ignored the
|
||||
``unit`` field in ``position`` and forwarded the values directly to the IPC
|
||||
backend, which expects mm. Calling with ``unit="inch"`` placed the part at
|
||||
1/25.4 of the intended position. The fix reads ``unit`` and converts inches
|
||||
→ mm before invoking ``self.ipc_board_api.*``.
|
||||
"""
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent / "python"))
|
||||
|
||||
|
||||
def _make_iface() -> Any:
|
||||
"""Build a stripped-down KiCADInterface with a mock ipc_board_api."""
|
||||
with patch("kicad_interface.USE_IPC_BACKEND", True):
|
||||
from kicad_interface import KiCADInterface
|
||||
|
||||
iface = KiCADInterface.__new__(KiCADInterface)
|
||||
|
||||
iface.use_ipc = True
|
||||
iface.board = None
|
||||
iface.ipc_board_api = MagicMock()
|
||||
iface.ipc_board_api.place_component.return_value = True
|
||||
iface.ipc_board_api.move_component.return_value = True
|
||||
return iface
|
||||
|
||||
|
||||
def test_place_component_converts_inches_to_mm():
|
||||
iface = _make_iface()
|
||||
iface._ipc_place_component(
|
||||
{
|
||||
"reference": "R1",
|
||||
"footprint": "Resistor_SMD:R_0805",
|
||||
"position": {"x": 1, "y": 2, "unit": "inch"},
|
||||
"rotation": 0,
|
||||
"layer": "F.Cu",
|
||||
"value": "220",
|
||||
}
|
||||
)
|
||||
|
||||
args, kwargs = iface.ipc_board_api.place_component.call_args
|
||||
assert kwargs["x"] == pytest.approx(25.4)
|
||||
assert kwargs["y"] == pytest.approx(50.8)
|
||||
|
||||
|
||||
def test_place_component_passes_mm_unchanged():
|
||||
iface = _make_iface()
|
||||
iface._ipc_place_component(
|
||||
{
|
||||
"reference": "R1",
|
||||
"footprint": "Resistor_SMD:R_0805",
|
||||
"position": {"x": 25.4, "y": 50.8, "unit": "mm"},
|
||||
"rotation": 0,
|
||||
"layer": "F.Cu",
|
||||
"value": "220",
|
||||
}
|
||||
)
|
||||
args, kwargs = iface.ipc_board_api.place_component.call_args
|
||||
assert kwargs["x"] == 25.4
|
||||
assert kwargs["y"] == 50.8
|
||||
|
||||
|
||||
def test_move_component_converts_inches_to_mm():
|
||||
iface = _make_iface()
|
||||
iface._ipc_move_component(
|
||||
{
|
||||
"reference": "R1",
|
||||
"position": {"x": 1, "y": 0.5, "unit": "inch"},
|
||||
}
|
||||
)
|
||||
args, kwargs = iface.ipc_board_api.move_component.call_args
|
||||
assert kwargs["x"] == pytest.approx(25.4)
|
||||
assert kwargs["y"] == pytest.approx(12.7)
|
||||
|
||||
|
||||
def test_default_unit_is_mm():
|
||||
"""Omitting unit defaults to mm — values pass through unchanged."""
|
||||
iface = _make_iface()
|
||||
iface._ipc_move_component({"reference": "R1", "position": {"x": 10, "y": 20}})
|
||||
args, kwargs = iface.ipc_board_api.move_component.call_args
|
||||
assert kwargs["x"] == 10
|
||||
assert kwargs["y"] == 20
|
||||
Reference in New Issue
Block a user