From 80a0c9da8df2d6ca0a799196ecb26d3eda68b8c6 Mon Sep 17 00:00:00 2001 From: Edward Viaene Date: Fri, 9 Oct 2026 13:47:08 -0500 Subject: [PATCH] Name the caller's own fix in pod refusals; name eFuse rails consistently The firmware spells some fixes as raw protocol commands ("set it with la_voltage (mv 1800 or 3300)", "release it with {"cmd":"gpio","la":4,"mode":"off"}"). The SDK now rewrites them into SDK calls (BenchPod(conn, la_voltage=3.3), the benchpod_la_voltage fixture, bp.set_la_voltage, bp.release_gpio(4), bp.disable_i2c_sensor(), bp.disable_gps(), closing the UART/SPI session, bp.set_pull(7, False), ...) and the MCP server into tool names (set_la_voltage, gpio_release(la=[4]), uart_close, disable_i2c_sensor, disable_gps, set_pull, ...). Error classes, attributes and the leading words of each message are unchanged; firmware_message keeps the pod's own text and unknown hints pass through. The MCP rewrites from firmware_message itself, so it reads the same with any supported SDK release. MCP instructions, wiring reference and tool/result descriptions now say "internal 5 V (efuse 1)" and "external (efuse 2)". Co-Authored-By: Claude Opus 5.5 --- packages/embeddedci-mcp/CHANGELOG.md | 8 +++ packages/embeddedci-mcp/README.md | 13 ++-- .../src/embeddedci_mcp/guide.py | 8 +-- .../src/embeddedci_mcp/models.py | 13 ++-- .../src/embeddedci_mcp/server.py | 68 ++++++++++++++++-- .../embeddedci-mcp/tests/test_gpio_tools.py | 6 +- .../embeddedci-mcp/tests/test_refusals.py | 60 +++++++++++++++- .../embeddedci-mcp/tests/tools_surface.json | 12 ++-- packages/embeddedci/CHANGELOG.md | 11 +++ packages/embeddedci/README.md | 19 +++-- .../src/embeddedci/benchpod/errors.py | 64 +++++++++++++++-- packages/embeddedci/tests/test_gpio.py | 3 +- .../embeddedci/tests/test_refusal_errors.py | 71 +++++++++++++++++++ 13 files changed, 315 insertions(+), 41 deletions(-) diff --git a/packages/embeddedci-mcp/CHANGELOG.md b/packages/embeddedci-mcp/CHANGELOG.md index ab7e09c..d636ae1 100644 --- a/packages/embeddedci-mcp/CHANGELOG.md +++ b/packages/embeddedci-mcp/CHANGELOG.md @@ -8,6 +8,14 @@ or several, the error lists the pods it heard (name and address) and what to pass instead: `host[:port]`, `usb`, or `embeddedci:` (from `cloud_list_devices`). - The `connect` description, the `status` hint and the server instructions say so. +- Pod refusals name the tool that fixes them instead of a raw protocol command: "la voltage not + set; call set_la_voltage first with the DUT's I/O voltage (1.8 or 3.3)", and a pin conflict + says `release it with gpio_release(la=[4])`, `uart_close`, `disable_i2c_sensor` or + `disable_gps` (a pull conflict `set_pull(las=[7], enabled=false)`). The error kind and the + leading words stay the same; hints it does not know pass through unchanged. +- Target power is named the same way everywhere: "internal 5 V (efuse 1)" and "external + (efuse 2)", in the server instructions, the wiring reference and the tool and result + descriptions. ## 2.9.0 diff --git a/packages/embeddedci-mcp/README.md b/packages/embeddedci-mcp/README.md index 47b54bb..188916a 100644 --- a/packages/embeddedci-mcp/README.md +++ b/packages/embeddedci-mcp/README.md @@ -176,7 +176,7 @@ description, ranges and enums, so an agent sees more than this table. | --- | --- | --- | | `power_on` | Switch the target's power rail on (optionally scheduled pod-side). | `efuse`, `delay` | | `power_off` | Switch the target's power rail off. | `efuse`, `delay` | -| `power_status` | Both eFuse rails: on/off, tripped, bus voltage and current, and the pod's own draw. | none | +| `power_status` | Both target-power rails, internal 5 V (efuse 1) and external (efuse 2): on/off, tripped, bus voltage and current, and the pod's own draw. | none | | `reset_target` | Drive the DUT's reset line from the pod's reset pin. | `action=pulse`, `pulse=0.1` | ### Power profiles @@ -324,8 +324,8 @@ resistors and analog paths) and `benchpod://help` (the server instructions). flows, units, error contract — so the agent knows to `connect` and `set_la_voltage` before anything else. - **Typed, structured results.** Every tool has an input schema with enums and ranges (paths, - sources, LA channels 1-14, eFuse 1/2, …) and an output schema; results come back as structured - content. Units are volts, seconds and hertz. + sources, LA channels 1-14, internal 5 V (efuse 1) or external (efuse 2), …) and an output + schema; results come back as structured content. Units are volts, seconds and hertz. - **Errors.** A tool that cannot do what was asked fails with an MCP tool error whose message names the cause, e.g. `FirmwareError: la voltage not set` or `NotConnectedError: …`. A completed operation with a negative outcome is a normal result: `flash` returns `ok: false` with its logs, @@ -339,6 +339,11 @@ resistors and analog paths) and `benchpod://help` (the server instructions). `UnsupportedFeatureError` means the pod cannot do it at all: its firmware or gateware lacks the feature, or it is the digital-only board without the analog front end; the hint says which. `status` also warns when a cloud job holds the pod or the LAN policy is locked. + Refusals whose fix the firmware spells as a raw protocol command name the tool instead: + `FirmwareError: uart_proxy_start: la voltage not set; call set_la_voltage first with the DUT's + I/O voltage (1.8 or 3.3)` and `PinConflictError: uart_proxy_start: pin conflict: LA4 is in use + by gpio; release it with gpio_release(la=[4])`. The other pin owners point to `uart_close`, + `disable_i2c_sensor` and `disable_gps`; a pull conflict to `set_pull(las=[7], enabled=false)`. - **Capabilities.** `connect` and `status` report what the pod can do as flags, so an agent can check before it calls a tool: for example `la_pins`, `capture_trigger`, `power_profile`, `nrst_pin`, `can`, `calibrate`, `current_out`, `pod_current` and `analog` (false on the @@ -401,7 +406,7 @@ With a wiring profile stored for the bench, the same run needs no pin numbers: ``` connect() -wiring() # SWCLK on LA11, DUT TX on LA5, rail eFuse 1, … +wiring() # SWCLK on LA11, DUT TX on LA5, rail internal 5 V (efuse 1), … flash(file="build/app.elf") power_cycle_and_capture(delay=1.0, duration=5.0, until_regex="APP_OK") measure_power(duration=2.0, points=100) # what the firmware draws once it is up diff --git a/packages/embeddedci-mcp/src/embeddedci_mcp/guide.py b/packages/embeddedci-mcp/src/embeddedci_mcp/guide.py index 90750e1..e11414b 100644 --- a/packages/embeddedci-mcp/src/embeddedci_mcp/guide.py +++ b/packages/embeddedci-mcp/src/embeddedci_mcp/guide.py @@ -14,9 +14,9 @@ Cloud pods: cloud_list_devices lists the pods registered on embeddedci.com (name, online) without connecting to any; connect to one with its `connection` ('embeddedci:'). Tools named cloud_* talk to embeddedci.com, not to a pod. Cloud access (cloud_* and connect to 'embeddedci:') authenticates with BENCHPOD_API_KEY, else the user's `benchpod login` session; if neither exists, ask the user to run `benchpod login`. -Wiring: the pod has 14 generic LA channels (LA1-LA14) and nothing is fixed. Call `wiring` first: it is the bench's profile — which DUT signal is on which channel, the target-power rail, the UART baud, the SWD target. Every channel, baud, rail or SWD argument you omit comes from it, and channel arguments also accept its names ("READY", "uart_rx"). Confirm it with the user if it looks wrong; `set_wiring` replaces it (save=true stores it on embeddedci.com). Target power is eFuse 1 (internal 5 V) or 2 (external). The DUT's reset line goes to the pod's own reset pin: flash(nreset=true), reset_target. +Wiring: the pod has 14 generic LA channels (LA1-LA14) and nothing is fixed. Call `wiring` first: it is the bench's profile: which DUT signal is on which channel, the target-power rail, the UART baud, the SWD target. Every channel, baud, rail or SWD argument you omit comes from it, and channel arguments also accept its names ("READY", "uart_rx"). Confirm it with the user if it looks wrong; `set_wiring` replaces it (save=true stores it on embeddedci.com). Target power is internal 5 V (efuse 1) or external (efuse 2). The DUT's reset line goes to the pod's own reset pin: flash(nreset=true), reset_target. -GPIO: each LA channel has one owner at a time. gpio_mode claims channels (input/output/open_drain), gpio_write drives them, gpio_read reads any channel's live level, gpio_wait polls for one, gpio_pulse emits FPGA-timed pulses, gpio_release frees them. Release a GPIO channel before uart_open, flash or enable_i2c_sensor uses it — otherwise the pod refuses with "pin conflict: LA4 is in use by gpio" and says how to free it; la_pins shows every channel's owner. Captures observe all 14 channels whatever owns them. +GPIO: each LA channel has one owner at a time. gpio_mode claims channels (input/output/open_drain), gpio_write drives them, gpio_read reads any channel's live level, gpio_wait polls for one, gpio_pulse emits FPGA-timed pulses, gpio_release frees them. Release a GPIO channel before uart_open, flash or enable_i2c_sensor uses it, otherwise the pod refuses with "pin conflict: LA4 is in use by gpio; release it with gpio_release(la=[4])": the error names the tool that frees the channel (gpio_release, uart_close, disable_i2c_sensor, disable_gps); la_pins shows every channel's owner. Captures observe all 14 channels whatever owns them. Typical flows: - Flash and boot: flash(file) then power_cycle_and_capture(until_regex) — with a wiring profile that is all; otherwise pass swclk, swdio, target, nreset and rx, tx. @@ -34,7 +34,7 @@ - Power profiles: measure_power(duration) reports the DUT's average/minimum/peak current, voltage, energy and charge over a window, with an optional trace (points). To profile across other tool calls, bracket them with power_profile_start and power_profile_stop. - CAN: can_open (mode 'internal' self-tests a lone pod), can_write, can_read, can_respond, can_close. -Units are volts, amps, seconds and hertz. A tool that cannot do what was asked fails with an error that names the cause (for example "FirmwareError: la voltage not set" or "NotConnectedError: ..."). A completed operation with a negative outcome is a normal result: flash returns ok=false with its logs, a UART capture returns matched=false. Policy refusals have their own names: PodLockedError (the pod's LAN policy keeps the command for the cloud or USB), PodLeasedError (a cloud job holds the pod; wait and retry), PermissionDeniedError (the credential needs an owner/admin role or access to the device). UnsupportedFeatureError means this pod cannot do it at all (its firmware or gateware lacks the feature, or it is the digital-only board without the analog front end): check the capabilities in status instead of retrying. +Units are volts, amps, seconds and hertz. A tool that cannot do what was asked fails with an error that names the cause (for example "FirmwareError: la voltage not set; call set_la_voltage first ..." or "NotConnectedError: ..."). A completed operation with a negative outcome is a normal result: flash returns ok=false with its logs, a UART capture returns matched=false. Policy refusals have their own names: PodLockedError (the pod's LAN policy keeps the command for the cloud or USB), PodLeasedError (a cloud job holds the pod; wait and retry), PermissionDeniedError (the credential needs an owner/admin role or access to the device). UnsupportedFeatureError means this pod cannot do it at all (its firmware or gateware lacks the feature, or it is the digital-only board without the analog front end): check the capabilities in status instead of retrying. flash runs OpenOCD on the machine hosting this server and reads `file` from that machine's filesystem. Over the cloud the device is shared: connect takes an exclusive lease, released by disconnect or after the server's idle timeout (the next call reconnects). """ @@ -54,7 +54,7 @@ UART: DUT TX -> pod LA5 rx=5 UART: DUT RX <- pod LA4 tx=4 I2C SDA / SCL LA2 / LA1 sda=2, scl=1 (engage their pull-ups) - Target power eFuse 1 (internal 5 V) efuse=1 (2 = external supply) + Target power internal 5 V (efuse 1) efuse=1 (external: efuse=2) Bias resistors (3V3-referenced, so unavailable while the LA bank is at 1.8 V): LA1, LA2 4.7k pull-up LA3, LA4 2.2k pull-up LA5, LA6 10k pull-up diff --git a/packages/embeddedci-mcp/src/embeddedci_mcp/models.py b/packages/embeddedci-mcp/src/embeddedci_mcp/models.py index c037197..97b7797 100644 --- a/packages/embeddedci-mcp/src/embeddedci_mcp/models.py +++ b/packages/embeddedci-mcp/src/embeddedci_mcp/models.py @@ -137,7 +137,8 @@ class WiringResult(BaseModel): source: str = Field(description="Where the profile came from: defaults, file, server or dict.") version: int = 1 la_voltage: float = Field(description="LA I/O-bank voltage the profile asks for, in volts.") - efuse: int = Field(description="Target-power rail the tools default to.") + efuse: int = Field(description=( + "Target-power rail the tools default to: internal 5 V (efuse 1) or external (efuse 2).")) uart_baud: int i2c_address: int = Field(description="Emulated-sensor address as a 7-bit integer.") swd_nreset: bool @@ -209,7 +210,7 @@ class GpioReleaseResult(BaseModel): # -- power ----------------------------------------------------------------------- class PowerResult(BaseModel): - efuse: int + efuse: int = Field(description="The rail used: internal 5 V (efuse 1) or external (efuse 2).") on: bool delay: Optional[float] = None @@ -231,8 +232,8 @@ class PodPowerResult(BaseModel): class PowerStatusResult(BaseModel): - internal: RailResult - external: RailResult + internal: RailResult = Field(description="The internal 5 V rail (efuse 1).") + external: RailResult = Field(description="The external rail (efuse 2).") pod: Optional[PodPowerResult] = Field(None, description="The pod's own draw; null on boards that don't measure it.") @@ -245,7 +246,7 @@ class PowerProfileResult(BaseModel): """A target-power rail profiled over time. Currents are amps, voltages volts, energy joules, charge coulombs, durations seconds.""" - efuse: int + efuse: int = Field(description="The rail used: internal 5 V (efuse 1) or external (efuse 2).") rate_hz: float = Field(description="Samples per second actually delivered (measured). The pod reads one sensor register per firmware pass, so this lands below what was asked for — roughly 350-450 Hz. Every sample carries its own timestamp, so the trace is exact regardless.") adc_rate_hz: float = Field(default=0.0, description="Conversion rate the current sensor was configured for — the ceiling, not what arrived.") n: int = Field(description="Raw samples the statistics cover.") @@ -268,7 +269,7 @@ class PowerProfileResult(BaseModel): class PowerProfileStartResult(BaseModel): running: bool = True - efuse: int + efuse: int = Field(description="The rail used: internal 5 V (efuse 1) or external (efuse 2).") rate_hz: float = Field(description="Sample rate asked for; the achieved rate is in the stop result.") max_duration: float = Field(description="Seconds after which the pod stops sampling by itself.") diff --git a/packages/embeddedci-mcp/src/embeddedci_mcp/server.py b/packages/embeddedci-mcp/src/embeddedci_mcp/server.py index 2048b11..7ccbf3a 100644 --- a/packages/embeddedci-mcp/src/embeddedci_mcp/server.py +++ b/packages/embeddedci-mcp/src/embeddedci_mcp/server.py @@ -74,9 +74,9 @@ "LA channel 1-14, or a name from the wiring profile — a signal ('READY') or a role " "('uart_rx', 'i2c_sda', 'swd_swclk'). Call `wiring` to see the names."))] BiasPin = Annotated[int, Field(ge=1, le=8)] -EfuseRail = Annotated[Literal[1, 2], Field(description="Target-power eFuse: 1 = internal 5 V, 2 = external supply.")] +EfuseRail = Annotated[Literal[1, 2], Field(description="Target-power rail: internal 5 V (efuse 1) or external (efuse 2).")] WiredEfuse = Annotated[Optional[Literal[1, 2]], Field(description=( - "Target-power eFuse: 1 = internal 5 V, 2 = external supply. Omit for the wiring profile's rail."))] + "Target-power rail: internal 5 V (efuse 1) or external (efuse 2). Omit for the wiring profile's rail."))] BaudRate = Annotated[int, Field(ge=300, le=4_000_000)] WiredBaud = Annotated[Optional[int], Field(ge=300, le=4_000_000, description=( "UART baud rate; omit for the wiring profile's uart_baud."))] @@ -137,8 +137,62 @@ def isa(name: str) -> bool: return "" +#: The firmware's "la voltage not set; set it with la_voltage (mv 1800 or 3300) first". +_LA_VOLTAGE_UNSET = "la voltage not set" +_LA_VOLTAGE_MCP = ("la voltage not set; call set_la_voltage first with the DUT's I/O voltage " + "(1.8 or 3.3)") + +#: Fixes the firmware spells as raw protocol commands (la_pins.c release_hint, the pull and +#: gpio-output checks, command_handler_dap.c, command_handler_dac.c), named as this server's +#: tools. Whole phrases first; anything not listed passes through unchanged. Kept here rather +#: than taken from the SDK so the text is the same with every supported embeddedci release. +_MCP_FIXES = ( + (re.compile(r'release it with \{"cmd":"gpio","la":(\d+),"mode":"off"\}'), + r"release it with gpio_release(la=[\1])"), + (re.compile(r"stop the uart proxy first"), + "close the UART session first with uart_close"), + (re.compile(r"end the SWD session first"), + "wait for the running flash (its SWD session) to finish"), + (re.compile(r'stop the sensor emulation first \(\{"cmd":"sensor_stop"\}\)'), + "stop the sensor emulation first with disable_i2c_sensor"), + (re.compile(r'stop the SPI session first \(\{"cmd":"spi_stop"\}\)'), + 'stop the SPI session first: another client holds it (the command tool can send ' + '{"cmd":"spi_stop"})'), + (re.compile(r'stop the GPS receiver first \(\{"cmd":"gps_stop"\}\)'), + "stop the GPS emulation first with disable_gps"), + (re.compile(r'\(\{"cmd":"spi_stop"\}\)'), + '(another client holds the SPI session; the command tool can send {"cmd":"spi_stop"})'), + (re.compile(r'\{"cmd":"la","la":(\d+),"pullup":"off"\}'), r"set_pull(las=[\1], enabled=false)"), + (re.compile(r'\{"cmd":"gpio","la":(\d+),"mode":"output"\}'), r'gpio_mode(la=[\1], mode="output")'), + (re.compile(r'\{"cmd":"fpga_image","image":0\}'), 'fpga_image(image="loop")'), + (re.compile(r'\{"cmd":"dac_loop_input"\}'), "loop_input"), +) + + +def _mcp_refusal_text(message: str) -> str: + """A pod refusal with its protocol-level fix named as this server's tool, or unchanged.""" + if message.startswith(_LA_VOLTAGE_UNSET): + return _LA_VOLTAGE_MCP + for pattern, repl in _MCP_FIXES: + message = pattern.sub(repl, message) + return message + + +def _error_text(exc: BenchPodError) -> str: + """``str(exc)``, with a firmware refusal's fix rewritten from the pod's own text (the SDK's + message names SDK calls; an agent here needs the tool names).""" + original = getattr(exc, "firmware_message", None) + if not isinstance(exc, _errors.FirmwareError) or not isinstance(original, str): + return str(exc) + text = _mcp_refusal_text(original) + if text == original: + return str(exc) + cmd = getattr(exc, "cmd", None) + return f"{cmd}: {text}" if cmd else text + + def _tool_error(exc: BenchPodError) -> ToolError: - msg = f"{type(exc).__name__}: {exc}" + msg = f"{type(exc).__name__}: {_error_text(exc)}" hint = _refusal_hint(exc) return ToolError(f"{msg}. {hint}" if hint else msg) @@ -449,7 +503,8 @@ def op() -> m.PowerProfileResult: @mcp.tool(annotations=_ann("Power status", read_only=True)) async def power_status() -> m.PowerStatusResult: - """Both target-power rails: eFuse on/off and tripped state, bus voltage and current draw.""" + """Both target-power rails, internal 5 V (efuse 1) and external (efuse 2): on/off and tripped + state, bus voltage and current draw.""" def op() -> m.PowerStatusResult: pod = SESSION.require() ts, ps = pod.target_status(), pod.power_status() @@ -625,7 +680,8 @@ async def flash( "The target's reset line is wired to the pod's reset pin: enables connect-under-reset. " "Omit for the profile's swd_nreset."))] = None, load_address: Annotated[str, Field(description="Load address for raw .bin images, e.g. 0x08000000.")] = "", - target_power: Annotated[Optional[Literal[1, 2]], Field(description="Power this eFuse on before flashing.")] = None, + target_power: Annotated[Optional[Literal[1, 2]], Field(description=( + "Power this rail on before flashing: internal 5 V (efuse 1) or external (efuse 2)."))] = None, verify: bool = True, reset: bool = True, connect_under_reset: Optional[bool] = None, @@ -1020,7 +1076,7 @@ async def gpio_mode( A channel stays GPIO — across disconnects — until gpio_release, and while it is GPIO nothing else can use it: release it before uart_open, flash or enable_i2c_sensor on that channel. A channel already owned by another function is refused with a PinConflictError naming the - owner, and an engaged bias resistor that would fight the mode with a PullConflictError. + owner and the tool that frees it, and an engaged bias resistor that would fight the mode with a PullConflictError. """ def op() -> m.GpioPinsResult: pod = SESSION.require() diff --git a/packages/embeddedci-mcp/tests/test_gpio_tools.py b/packages/embeddedci-mcp/tests/test_gpio_tools.py index ebac346..0e852d9 100644 --- a/packages/embeddedci-mcp/tests/test_gpio_tools.py +++ b/packages/embeddedci-mcp/tests/test_gpio_tools.py @@ -110,14 +110,16 @@ def test_gpio_pulse_runs_a_step_train(connected): def test_pin_conflict_names_the_owner_and_how_to_free_it(connected): call("uart_open", rx=5, tx=4) - with pytest.raises(ToolError, match="PinConflictError: gpio: pin conflict: LA5 is in use by uart_rx"): + with pytest.raises(ToolError, match="PinConflictError: gpio: pin conflict: LA5 is in use by uart_rx; " + "close the UART session first with uart_close"): call("gpio_mode", la=[5]) call("uart_close") def test_a_gpio_channel_blocks_the_uart_until_it_is_released(connected): call("gpio_mode", la=[5], mode="output") - with pytest.raises(ToolError, match=r"PinConflictError:.*LA5 is in use by gpio"): + with pytest.raises(ToolError, match=r"PinConflictError:.*LA5 is in use by gpio; " + r"release it with gpio_release\(la=\[5\]\)$"): call("uart_open", rx=5, tx=4) call("gpio_release", la=[5]) assert call("uart_open", rx=5, tx=4)["open"] is True diff --git a/packages/embeddedci-mcp/tests/test_refusals.py b/packages/embeddedci-mcp/tests/test_refusals.py index d937481..51569cf 100644 --- a/packages/embeddedci-mcp/tests/test_refusals.py +++ b/packages/embeddedci-mcp/tests/test_refusals.py @@ -73,12 +73,68 @@ def test_digital_board_says_it_has_no_analog(connected): def test_other_firmware_errors_have_no_hint(connected): def command(req): - raise FirmwareError("la voltage not set", cmd=req["cmd"]) + raise FirmwareError("unknown target", cmd=req["cmd"]) connected.command = command with pytest.raises(ToolError) as info: call("gpio_pulse", la=3, width=0.001, count=5) - assert str(info.value).endswith("FirmwareError: la: la voltage not set") + assert str(info.value).endswith("FirmwareError: la: unknown target") + + +# -- the firmware's protocol-level fixes, named as tools --------------------------------------- +# Texts: benchpod-firmware stm32h563/src/command_handler.c (require_la_voltage) and la_pins.c. + +def test_la_voltage_not_set_names_set_la_voltage(connected): + _refuse(connected, "la", "la voltage not set; set it with la_voltage (mv 1800 or 3300) first") + with pytest.raises(ToolError) as info: + call("gpio_pulse", la=3, width=0.001, count=5) + assert str(info.value).endswith("FirmwareError: la: la voltage not set; call set_la_voltage " + "first with the DUT's I/O voltage (1.8 or 3.3)") + + +@pytest.mark.parametrize("function, hint, fix", [ + ("gpio", 'release it with {"cmd":"gpio","la":3,"mode":"off"}', "release it with gpio_release(la=[3])"), + ("uart_rx", "stop the uart proxy first", "close the UART session first with uart_close"), + ("swd_dio", "end the SWD session first", "wait for the running flash (its SWD session) to finish"), + ("i2c_scl", 'stop the sensor emulation first ({"cmd":"sensor_stop"})', + "stop the sensor emulation first with disable_i2c_sensor"), + ("spi_cs", 'stop the SPI session first ({"cmd":"spi_stop"})', + 'stop the SPI session first: another client holds it (the command tool can send {"cmd":"spi_stop"})'), + ("gps_tx", 'stop the GPS receiver first ({"cmd":"gps_stop"})', "stop the GPS emulation first with disable_gps"), + ("step", "wait for the step train to finish", "wait for the step train to finish"), + ("can_rx", 'stop it with {"cmd":"can_stop"}', 'stop it with {"cmd":"can_stop"}'), # unknown: as is +]) +def test_pin_conflict_names_the_tool(connected, function, hint, fix): + _refuse(connected, "la", f"pin conflict: LA3 is in use by {function}; {hint}") + with pytest.raises(ToolError) as info: + call("gpio_pulse", la=3, width=0.001, count=5) + assert str(info.value).endswith( + f"PinConflictError: la: pin conflict: LA3 is in use by {function}; {fix}") + + +def test_pull_conflict_names_set_pull(connected): + _refuse(connected, "la", "pull conflict: LA7 has its 10k pull-down engaged, which gpio open_drain " + "can't work with (a released line would read low); disable it with " + '{"cmd":"la","la":7,"pullup":"off"} or use another channel') + with pytest.raises(ToolError) as info: + call("gpio_pulse", la=3, width=0.001, count=5) + assert "PullConflictError: la: pull conflict: LA7" in str(info.value) + assert str(info.value).endswith("disable it with set_pull(las=[7], enabled=false) or use another channel") + + +def test_efuse_rails_are_named_the_same_everywhere(): + import anyio + + from embeddedci_mcp import guide, server + + tools = anyio.run(server.mcp.list_tools) + texts = [guide.INSTRUCTIONS, guide.WIRING] + texts += [f"{t.description} {t.inputSchema} {t.outputSchema}" for t in tools] + joined = "\n".join(texts) + assert "internal 5 V (efuse 1)" in guide.INSTRUCTIONS and "external (efuse 2)" in guide.INSTRUCTIONS + assert "internal 5 V (efuse 1)" in guide.WIRING + for old in ("eFuse 1", "eFuse 2", "eFuse: 1", "= internal 5 V", "= external supply"): + assert old not in joined, old def test_status_warns_about_a_cloud_lease_and_a_locked_lan(connected): diff --git a/packages/embeddedci-mcp/tests/tools_surface.json b/packages/embeddedci-mcp/tests/tools_surface.json index fc1e164..73c0461 100644 --- a/packages/embeddedci-mcp/tests/tools_surface.json +++ b/packages/embeddedci-mcp/tests/tools_surface.json @@ -1809,7 +1809,7 @@ } ], "default": null, - "description": "Power this eFuse on before flashing.", + "description": "Power this rail on before flashing: internal 5 V (efuse 1) or external (efuse 2).", "title": "Target Power" }, "timeout": { @@ -2644,7 +2644,7 @@ } ], "default": null, - "description": "Target-power eFuse: 1 = internal 5 V, 2 = external supply. Omit for the wiring profile's rail.", + "description": "Target-power rail: internal 5 V (efuse 1) or external (efuse 2). Omit for the wiring profile's rail.", "title": "Efuse" }, "points": { @@ -2726,7 +2726,7 @@ } ], "default": null, - "description": "Target-power eFuse: 1 = internal 5 V, 2 = external supply. Omit for the wiring profile's rail.", + "description": "Target-power rail: internal 5 V (efuse 1) or external (efuse 2). Omit for the wiring profile's rail.", "title": "Efuse" }, "off_settle": { @@ -2837,7 +2837,7 @@ } ], "default": null, - "description": "Target-power eFuse: 1 = internal 5 V, 2 = external supply. Omit for the wiring profile's rail.", + "description": "Target-power rail: internal 5 V (efuse 1) or external (efuse 2). Omit for the wiring profile's rail.", "title": "Efuse" } }, @@ -2882,7 +2882,7 @@ } ], "default": null, - "description": "Target-power eFuse: 1 = internal 5 V, 2 = external supply. Omit for the wiring profile's rail.", + "description": "Target-power rail: internal 5 V (efuse 1) or external (efuse 2). Omit for the wiring profile's rail.", "title": "Efuse" } }, @@ -2914,7 +2914,7 @@ } ], "default": null, - "description": "Target-power eFuse: 1 = internal 5 V, 2 = external supply. Omit for the wiring profile's rail.", + "description": "Target-power rail: internal 5 V (efuse 1) or external (efuse 2). Omit for the wiring profile's rail.", "title": "Efuse" }, "max_duration": { diff --git a/packages/embeddedci/CHANGELOG.md b/packages/embeddedci/CHANGELOG.md index 1992b90..6b04738 100644 --- a/packages/embeddedci/CHANGELOG.md +++ b/packages/embeddedci/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog — `embeddedci` +## Unreleased + +- Friendlier pod refusals: a `FirmwareError` whose fix the firmware spells as a raw protocol + command now names the SDK call instead. "la voltage not set" says to set the board's I/O voltage + with `BenchPod(conn, la_voltage=3.3)`, the `benchpod_la_voltage` fixture or + `bp.set_la_voltage(3.3)`; a `PinConflictError` says `release it with bp.release_gpio(4)`, + `bp.disable_i2c_sensor()`, `bp.disable_gps()` or to close the UART or SPI session; a + `PullConflictError` says `bp.set_pull(7, False)`. The error classes, their attributes and the + leading words of each message are unchanged, and `firmware_message` still holds the pod's own + text. Hints the SDK does not know pass through as before. + ## 2.9.0 - More emulated I2C sensors: `Sensor.BME280` (temperature, pressure, humidity), `Sensor.SHT4X` diff --git a/packages/embeddedci/README.md b/packages/embeddedci/README.md index 85a93dc..0b49043 100644 --- a/packages/embeddedci/README.md +++ b/packages/embeddedci/README.md @@ -156,7 +156,11 @@ with BenchPod("192.168.1.213", la_voltage=3.3) as bp: **LA I/O-bank voltage** right after connecting — match it to the DUT's I/O voltage. The pod refuses every LA-bank operation (flashing, UART, LA capture, pull resistors, I2C-sensor emulation) until one is selected. You can also call `bp.set_la_voltage(3.3)` later; `bp.get_la_voltage()` returns a -`LaVoltage` whose `voltage` is `None` until one is chosen. 1.8 V needs a rev3 pod. +`LaVoltage` whose `voltage` is `None` until one is chosen. 1.8 V needs a rev3 pod. Until then a +refused command raises `FirmwareError` with the message `la voltage not set; set the board's I/O +voltage first: BenchPod(conn, la_voltage=3.3) (1.8 for a 1.8 V target), the benchpod_la_voltage +fixture in conftest.py, or bp.set_la_voltage(3.3)` (prefixed with the command, as every +`FirmwareError` is). Other constructor arguments: `timeout` (seconds, default 30), `api_key`, `api_base`, `cloud_token`, `cloud_audience`, `cloud_user_token`, `lease`, `lease_wait`, `lease_ttl` (see [Cloud](#cloud-embeddedcidevice-name)) @@ -689,18 +693,23 @@ bp.release_gpio(9, 10) # back to high-Z; no arguments rel **One function per channel.** Each channel has exactly one owner at a time: `none` (the default — high-Z and watched by captures), `gpio`, or a peripheral that claimed it — `uart_rx`/`uart_tx` (a UART session), -`swd_clk`/`swd_dio` (flashing), `i2c_sda`/`i2c_scl` (sensor emulation), `step`/`step_dir` (a pulse train). +`swd_clk`/`swd_dio` (flashing), `i2c_sda`/`i2c_scl` (sensor emulation), `step`/`step_dir` (a pulse train), +`spi_*` (an SPI session) and `gps_tx` (GPS emulation). The pod refuses a second function with `PinConflictError` (`.la`, `.function`) instead of letting one -silently override the other, and the message says how to free the channel: +silently override the other, and the message names the SDK call that frees the channel: ```python bp.gpio(4) try: bp.open_uart(rx=3, tx=4) -except PinConflictError as exc: # pin conflict: LA4 is in use by gpio; release it with ... +except PinConflictError as exc: # pin conflict: LA4 is in use by gpio; release it with bp.release_gpio(4) bp.release_gpio(exc.la) ``` +The other owners point to `bp.disable_i2c_sensor()`, `bp.disable_gps()`, closing the session from +`bp.open_uart()` or `bp.open_spi()`, or waiting for a flash or step train to finish. The pod's own +text, with its raw protocol command, stays in `exc.firmware_message`. + `bp.la_pins()` lists every channel's `LaPinState` (`function`, `gpio` mode, `level`, `pull`, `pull_on`), and `bp.configure_gpio(channels, mode)` is the same group claim as `gpio_pins()` returning those states. Captures observe every channel whatever its function. GPIO channels stay GPIO — also across @@ -1362,7 +1371,7 @@ Invalid arguments raise `ValueError` before anything is sent. Everything else ra | `TransportError` | the pod or tunnel could not be reached / talked to | | | `TransportTimeout` (a `TransportError` and a `TimeoutError`) | the pod or tunnel did not answer in time | | | `ConnectionClosedError` (a `TransportError`) | the connection ended or reset before the reply | | -| `FirmwareError` | the pod replied with an error (e.g. "la voltage not set") | `firmware_message`, `cmd` | +| `FirmwareError` | the pod replied with an error (e.g. "la voltage not set"); a fix the firmware spells as a raw protocol command is rewritten into the SDK call in the message, and `firmware_message` keeps the pod's own text | `firmware_message`, `cmd` | | `FlashError` | OpenOCD failed, is missing, or lacks the TCP backend | | | `TargetUnreachableError` (a `FlashError`) | the probe worked but no target answered on SWD | | | `DeviceBusyError` | a cloud device lease was not granted within `lease_wait` | | diff --git a/packages/embeddedci/src/embeddedci/benchpod/errors.py b/packages/embeddedci/src/embeddedci/benchpod/errors.py index 4136cb1..62f9ffd 100644 --- a/packages/embeddedci/src/embeddedci/benchpod/errors.py +++ b/packages/embeddedci/src/embeddedci/benchpod/errors.py @@ -45,15 +45,22 @@ class CloudAuthError(BenchPodError): class FirmwareError(BenchPodError): - """The pod accepted the request but replied ``{"status":"error"}``.""" + """The pod accepted the request but replied ``{"status":"error"}``. + + :attr:`firmware_message` is the pod's own text, unchanged. The exception's message is that + text with any fix the firmware spells as a raw protocol command (``{"cmd":"gpio",...}``) + rewritten into the SDK call that does it (``bp.release_gpio(4)``); the leading words the + firmware uses (``la voltage not set``, ``pin conflict: LA4 is in use by gpio``) stay as they are. + """ def __init__(self, message: str, *, cmd: Optional[str] = None) -> None: self.firmware_message = message self.cmd = cmd + text = _sdk_refusal_text(message) if cmd: - super().__init__(f"{cmd}: {message}") + super().__init__(f"{cmd}: {text}") else: - super().__init__(message) + super().__init__(text) class UnsupportedFeatureError(FirmwareError): @@ -129,8 +136,9 @@ class PinConflictError(FirmwareError): """The pod refused because an LA channel is already used by another function. ``la`` is the channel and ``function`` its owner (``gpio``, ``uart_tx``, ``i2c_sda``, ``swd_clk``, - ``step``, …). The message says how to free it — e.g. release a GPIO pin with - :meth:`BenchPod.release_gpio` before starting a UART session on it. + ``step``, …). The message names the SDK call that frees it (``release it with + bp.release_gpio(4)``, ``bp.disable_i2c_sensor()``, ``bp.disable_gps()``, close the UART or SPI + session); :attr:`firmware_message` keeps the pod's own text. """ def __init__(self, message: str, *, cmd: Optional[str] = None, la: int = 0, @@ -221,6 +229,52 @@ def __init__(self, message: str, *, cmd: Optional[str] = None, _NO_ANALOG = "this BenchPod has no analog front end" +#: The firmware's "la voltage not set; set it with la_voltage (mv 1800 or 3300) first" +#: (command_handler.c require_la_voltage). Matched on the leading words only. +_LA_VOLTAGE_UNSET = "la voltage not set" +_LA_VOLTAGE_SDK = ("la voltage not set; set the board's I/O voltage first: " + "BenchPod(conn, la_voltage=3.3) (1.8 for a 1.8 V target), the " + "benchpod_la_voltage fixture in conftest.py, or bp.set_la_voltage(3.3)") + +#: Fixes the firmware spells as raw protocol commands (la_pins.c release_hint and the pull and +#: gpio-output checks, command_handler_dap.c, command_handler_dac.c), in the caller's SDK terms. +#: Whole phrases first, so a hint reads naturally; anything not listed passes through unchanged. +_SDK_FIXES = ( + (re.compile(r'release it with \{"cmd":"gpio","la":(\d+),"mode":"off"\}'), + r"release it with bp.release_gpio(\1)"), + (re.compile(r"stop the uart proxy first"), + "close the UART session from bp.open_uart() first (session.close(), or leave its with block)"), + (re.compile(r"end the SWD session first"), + "wait for the running flash (its SWD session) to finish"), + (re.compile(r'stop the sensor emulation first \(\{"cmd":"sensor_stop"\}\)'), + "stop the sensor emulation first with bp.disable_i2c_sensor()"), + (re.compile(r'stop the SPI session first \(\{"cmd":"spi_stop"\}\)'), + "close the SPI session from bp.open_spi() first (spi.close(), or leave its with block)"), + (re.compile(r'stop the GPS receiver first \(\{"cmd":"gps_stop"\}\)'), + "stop the GPS emulation first with bp.disable_gps()"), + (re.compile(r'\(\{"cmd":"spi_stop"\}\)'), + "(close the SPI session from bp.open_spi() first)"), + (re.compile(r'\{"cmd":"la","la":(\d+),"pullup":"off"\}'), r"bp.set_pull(\1, False)"), + (re.compile(r'\{"cmd":"gpio","la":(\d+),"mode":"output"\}'), r'bp.gpio(\1, "output")'), + (re.compile(r'\{"cmd":"fpga_image","image":0\}'), "bp.fpga_image(0)"), + (re.compile(r'\{"cmd":"dac_loop_input"\}'), "bp.loop_input()"), +) + + +def _sdk_refusal_text(message: str) -> str: + """``message`` (a pod refusal) with its protocol-level fix rewritten into the SDK call. + + ``"pin conflict: LA4 is in use by gpio; release it with {"cmd":"gpio","la":4,"mode":"off"}"`` + becomes ``"pin conflict: LA4 is in use by gpio; release it with bp.release_gpio(4)"``. Text + with nothing to rewrite comes back unchanged. + """ + if message.startswith(_LA_VOLTAGE_UNSET): + return _LA_VOLTAGE_SDK + for pattern, repl in _SDK_FIXES: + message = pattern.sub(repl, message) + return message + + def classify_firmware_error(exc: FirmwareError) -> FirmwareError: """The specific :class:`FirmwareError` subclass for a pod refusal, or ``exc`` itself.""" if type(exc) is not FirmwareError: diff --git a/packages/embeddedci/tests/test_gpio.py b/packages/embeddedci/tests/test_gpio.py index f583599..8b261c4 100644 --- a/packages/embeddedci/tests/test_gpio.py +++ b/packages/embeddedci/tests/test_gpio.py @@ -158,7 +158,8 @@ def test_pin_conflicts_are_their_own_error(): with pytest.raises(PinConflictError) as ei: bp.open_uart(rx=3, tx=4) assert ei.value.la == 4 and ei.value.function == "gpio" - assert 'release it with {"cmd":"gpio","la":4,"mode":"off"}' in str(ei.value) + assert str(ei.value).endswith("pin conflict: LA4 is in use by gpio; release it with bp.release_gpio(4)") + assert 'release it with {"cmd":"gpio","la":4,"mode":"off"}' in ei.value.firmware_message assert isinstance(ei.value, FirmwareError) diff --git a/packages/embeddedci/tests/test_refusal_errors.py b/packages/embeddedci/tests/test_refusal_errors.py index 6af0961..8e5f68c 100644 --- a/packages/embeddedci/tests/test_refusal_errors.py +++ b/packages/embeddedci/tests/test_refusal_errors.py @@ -16,9 +16,11 @@ DeviceBusyError, FirmwareError, PermissionDeniedError, + PinConflictError, PodBusyError, PodLeasedError, PodLockedError, + PullConflictError, TransportError, firmware_error, ) @@ -116,3 +118,72 @@ def test_cloud_command_409_is_leased(monkeypatch): assert err.holder == "web UI (ward)" and err.expires_at == until and err.status == 409 assert 85 <= err.left_s <= 90 assert isinstance(err, TransportError) and isinstance(err, DeviceBusyError) + + +# -- protocol-level fixes rewritten into SDK calls ---------------------------------------------- +# The firmware texts are benchpod-firmware stm32h563/src/la_pins.c (release_hint, the pull and +# gpio-output checks), command_handler.c (require_la_voltage), command_handler_dap.c and +# command_handler_dac.c. + +LA_VOLTAGE_FW = "la voltage not set; set it with la_voltage (mv 1800 or 3300) first" + + +def test_la_voltage_not_set_names_the_sdk_ways_to_set_it(): + err = firmware_error(LA_VOLTAGE_FW, cmd="uart_proxy_start") + assert type(err) is FirmwareError + assert str(err) == ("uart_proxy_start: la voltage not set; set the board's I/O voltage first: " + "BenchPod(conn, la_voltage=3.3) (1.8 for a 1.8 V target), the " + "benchpod_la_voltage fixture in conftest.py, or bp.set_la_voltage(3.3)") + assert err.firmware_message == LA_VOLTAGE_FW and err.cmd == "uart_proxy_start" + assert "la voltage not set" in str(err) # what callers match on + + +@pytest.mark.parametrize("function, hint, fix", [ + ("gpio", 'release it with {"cmd":"gpio","la":4,"mode":"off"}', "release it with bp.release_gpio(4)"), + ("uart_tx", "stop the uart proxy first", + "close the UART session from bp.open_uart() first (session.close(), or leave its with block)"), + ("swd_clk", "end the SWD session first", "wait for the running flash (its SWD session) to finish"), + ("i2c_sda", 'stop the sensor emulation first ({"cmd":"sensor_stop"})', + "stop the sensor emulation first with bp.disable_i2c_sensor()"), + ("spi_sck", 'stop the SPI session first ({"cmd":"spi_stop"})', + "close the SPI session from bp.open_spi() first (spi.close(), or leave its with block)"), + ("gps_tx", 'stop the GPS receiver first ({"cmd":"gps_stop"})', + "stop the GPS emulation first with bp.disable_gps()"), + ("step", "wait for the step train to finish", "wait for the step train to finish"), + ("can_rx", 'stop it with {"cmd":"can_stop"}', 'stop it with {"cmd":"can_stop"}'), # unknown: as is +]) +def test_pin_conflict_names_the_sdk_call(function, hint, fix): + fw = f"pin conflict: LA4 is in use by {function}; {hint}" + err = firmware_error(fw, cmd="gpio") + assert type(err) is PinConflictError and isinstance(err, FirmwareError) + assert err.la == 4 and err.function == function + assert str(err) == f"gpio: pin conflict: LA4 is in use by {function}; {fix}" + assert err.firmware_message == fw + + +def test_pull_conflict_names_set_pull(): + fw = ("pull conflict: LA7 has its 10k pull-down engaged, which uart_rx can't work with (the line " + 'idles high); disable it with {"cmd":"la","la":7,"pullup":"off"} or use another channel') + err = firmware_error(fw, cmd="uart_proxy_start") + assert type(err) is PullConflictError and err.la == 7 + assert str(err).endswith("disable it with bp.set_pull(7, False) or use another channel") + assert err.firmware_message == fw + + +@pytest.mark.parametrize("fw, fixed", [ + ('LA4 is not a gpio output (function gpio, gpio input); configure it with ' + '{"cmd":"gpio","la":4,"mode":"output"}', 'configure it with bp.gpio(4, "output")'), + ('swd busy: an SPI session is using the engine ({"cmd":"spi_stop"})', + "using the engine (close the SPI session from bp.open_spi() first)"), + ('control loop not in the running gateware image (switch with {"cmd":"fpga_image","image":0})', + "(switch with bp.fpga_image(0))"), +]) +def test_other_protocol_fixes_name_the_sdk_call(fw, fixed): + err = firmware_error(fw) + assert str(err).endswith(fixed) and err.firmware_message == fw + + +def test_unknown_text_passes_through(): + for text in ("unknown target", 'something odd {"cmd":"mystery"}'): + err = FirmwareError(text, cmd="x") + assert str(err) == f"x: {text}" and err.firmware_message == text