feat(editor): one device connection per target, with run/stop and mode-switch support - #986
thiagoralves wants to merge 84 commits into
Conversation
Expose the debug function codes over serial without enabling full Modbus RTU/TCP. A new DEBUGGER_ENABLED gate brings up the serial port and the debug FCs without allocating any operation buffers (coils/holding/input regs), saving SRAM on small AVR boards. When Modbus is enabled behaviour is unchanged. Firmware: - MB_SERIAL_ACTIVE gate (MBSERIAL || DEBUGGER_ENABLED) guards serial RX/framing - debug-only setup path (Serial/115200/slave 1 defaults, overridable via DEBUG_IFACE/DEBUG_BAUD/DEBUG_SLAVE); no init_mbregs()/mapEmptyBuffers() - process_mbpacket() gates operation FCs under MODBUS_ENABLED -> operation requests return ILLEGAL_FUNCTION in debug-only builds - new FCs 0x46 status, 0x47 version, 0x48 board-id (ArduinoUniqueID, with a compilable fallback); RTU framing + CRC-bypass wired for all three - OPENPLC_RUNTIME_VERSION in openplc_version.h Editor/shared: - ArduinoUniqueID added to GLOBAL_LIBRARIES - mirrored FC enums (editor + simulator) - modbus-pdu build/parse helpers for the 3 new FCs (100% covered) - ModbusRtuClient getStatus/getVersion/getBoardId (100% covered) - generate-defines emits the //Debugger block for baremetal targets when full Modbus is off - simulator debug E2E cases for 0x46/0x47/0x48 (gated by CHRIS_DEMO_HEX) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…roup Unify the Modbus slave configuration entry point across targets (Bugs/Features item 4). Arduino boards used a separate VPP "Modbus" vendor-screen tab, while runtime v4 configures Modbus via the "+" → Servers flow. Now Arduino targets reach the same config under Servers. UI-only re-homing — the backing store stays vendorScreenData.modbus_rtu/tcp and the firmware build pipeline is untouched, so defines.h is byte-identical and no project migration is needed. - utils/vpp/modbus-screen: findModbusScreenName helper (100% covered) - explorer: render a fixed "Modbus" node under Servers for Arduino targets and drop it from the generic vendor-screen list - create-element: the "+" → Servers card opens the Modbus config (singleton) for Arduino instead of the runtime-v4-only message - project-tree: forceExpandable prop so the Servers branch expands to show externally supplied children (the Modbus node isn't a project.data.servers entry) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tions Phase 2 (serial-network-modbus split) foundation: on project load, seed the new `serial` section's baud_rate from the legacy `modbus_rtu.rtu_baud_rate`, and lift the network fields out of `modbus_tcp` into a dedicated `network` section (stripping them from modbus_tcp). Idempotent — an already-migrated project is a no-op — and preserves the vendorScreenData === vendorScreenDataByBoard[deviceBoard] invariant across every board bucket. No on-disk format change beyond the new sections; existing projects open with their baud/network config preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 2 (serial-network-modbus split): let a VPP screen `select` field source its options from per-board context via `optionsRef` (e.g. "board.serialPorts"), so a shared screen adapts to each board — the Modbus RTU serial-port picker now lists only the UARTs the board actually exposes instead of a static Serial/ Serial1/2/3 list. - BoardInfo + PackageManifest device gain serialPorts/defaultSerial; the hardware module forwards them from the manifest onto the board info. - utils/vpp/field-options: resolveFieldOptions helper (optionsRef wins when it resolves to a non-empty array, else falls back to static options). 100% covered. - form-layout: select uses resolveFieldOptions with the current board as context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 2: emit an always-on serial debugger and read Modbus config from the new sections. - generate-defines: the Debugger block is now unconditional for baremetal Arduino targets, emitting DEBUG_IFACE (default serial) and DEBUG_BAUD (from the Serial section). The old "only when Modbus is off" gate is gone. - modbus-defines: RTU reads serial_port (legacy rtu_interface fallback) and takes its baud from the Serial section on the default port or its own baud on a secondary port; emits MBSERIAL_SHARES_DEBUG_SERIAL when it runs on the default port so the firmware begins the port once. TCP reads network config from the Network section (legacy modbus_tcp.* fallback). New optional defaultSerial arg. - compiler-module: feed the serial/network sections into vppModbusState. Backward-tolerant: pre-migration projects (legacy modbus_rtu/modbus_tcp shape) still generate correct defines. 55 tests pass, 100% stmts/lines/functions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 2: with the always-on debugger now emitted unconditionally (DEBUGGER_ENABLED + DEBUG_IFACE/DEBUG_BAUD), a Modbus-TCP-only build (MODBUS_ENABLED without MBSERIAL) previously left the default serial uninitialised, so the debugger had no port. setup() now brings up DEBUG_IFACE @ DEBUG_BAUD on mb_serialport in that case. Single-serial model note: the debugger and Modbus RTU share one mb_serialport; when MBSERIAL_SHARES_DEBUG_SERIAL is set the RTU port IS the debugger's default serial (single begin). Running the debugger on the default serial while RTU uses a different UART simultaneously needs a second serial handler — documented follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 2: the always-on debugger keeps serial debug compiled into every baremetal firmware even with Modbus disabled, so resolveDebugConnection now falls back to the serial (rtu) channel instead of surfacing "Modbus Required" when no channel's enabledWhen matches. A TCP-only Modbus build leaves the tcp channel eligible, so it never hits the fallback and correctly debugs over TCP. Errors only when there is no serial channel at all. The serial channel's baud is sourced from the Serial section (screens.serial. baud_rate) via the VPP debug spec (NodeMCU pilot). 31 tests pass, 100% stmts/lines/functions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…er keeps the default Phase 2 dual-serial: support the debugger on the default (USB) serial AND Modbus RTU on a distinct UART simultaneously. - modbus-defines: emit MBSERIAL_ON_SECONDARY when the RTU serial_port differs from the board's default serial (and MBSERIAL_SHARES_DEBUG_SERIAL when it's the same port). Tested, 100% stmts/lines/functions. - ModbusSlave.cpp: factor the serial servicing into handle_serial_port(port, txpin, slaveid, buf, len, last) with a parametrised mb_rtu_drop_front. Under MBSERIAL_ON_SECONDARY, two contexts each own an RX buffer (debug + rtu) and mb_frame is transient process/TX scratch; otherwise the single-port path is unchanged (buf IS mb_frame, no copy) — zero RAM cost on single-UART boards. - Baremetal.ino: bring up both serials in setup() under the dual-serial macro. - ModbusSlave.h: DEBUG_* defaults now apply whenever DEBUGGER_ENABLED (the dual path needs DEBUG_SLAVE even with MBSERIAL defined). RAM cost (dual builds only): 2*MAX_MB_FRAME + 12 bytes (268 B on 32U4, 524 B on 256-frame boards). Firmware verified via arduino-cli build (multi-UART board). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Break the monolithic ModbusSlave.{cpp,h} into 10 cohesive modbus_*
translation units, each owning one concern and its own build gate.
Behavior-preserving; validated by arduino-cli builds (RTU single-serial,
debug-only/TCP, dual-serial).
- modbus_config.h build gates (defines.h + MB_SERIAL_ACTIVE/DEBUG_* derived)
- modbus_types.h enums, MBinfo, frame-size/status constants
- modbus_frame.* shared seam (mb_frame/mb_frame_len/modbus, exceptionResponse)
- modbus_crc.* CRC-16 + tables (defined once; fixes latent per-TU flash dup)
- modbus_registers.* register store + operation FCs (#ifdef MODBUS_ENABLED)
- modbus_debug.* debugger FCs 0x41-0x48 (home for future licensing FCs)
- modbus_pdu.* process_mbpacket + mb_pdu_request_len/mb_pdu_skips_crc
- modbus_serial.* RTU/debugger serial transport (single + dual-serial)
- modbus_tcp.* Modbus TCP transport (Ethernet/WiFi/ETH)
- ModbusSlave.* umbrella header + mbtask() facade (Baremetal.ino unchanged)
Key decoupling: the serial transport no longer knows the function-code set.
It asks modbus_pdu for per-FC frame shape (mb_pdu_request_len) and CRC policy
(mb_pdu_skips_crc), so adding a function code touches only modbus_pdu +
its handler, never the transports.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018esifhUpuyPmJB29BneUqr
Reference for the modularized ModbusSlave layer: layer diagram, per-module responsibility table (with build gates and dependencies), request lifecycle (RTU/TCP/dual-serial), the two invariants (transports don't know the FC set; mb_frame is the one seam), a "how to add a function code" guide, and the known single-serial + TCP shared-buffer constraint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018esifhUpuyPmJB29BneUqr
Introduce the canonical license blob and its storage layer, shared across
targets. The license is public signed data (integrity, not secrecy), so
writing bytes is safe in open-source firmware; only ECDSA verify stays in
the closed license-core (out of scope here).
- license_blob.h: packed little-endian lic_blob_t (106 B, 38 B signed
payload), _Static_assert on sizes, magic 'OPLC', CRC-32/ISO-HDLC bitwise.
- license_store.h: license_store_{write,read,erase} + lic_store_status_t
{OK,EMPTY,CORRUPT,IO_ERROR,TOO_LARGE} + lic_status_to_mb mapping.
- license_store_avr.cpp: EEPROM backend (len@0 + blob@2, update, TOO_LARGE
guard, virgin 0xFFFF -> EMPTY, magic/crc validation), no dynamic alloc.
- license_store_esp32.cpp: NVS backend via Preferences (namespace oplc-lic,
key blob).
- license_store.cpp: compile-time backend selection (#error off AVR/ESP32).
Golden CRC vector 0xCBF43926 and struct sizes verified host-side.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
…-on channel
Expose the license storage layer through two new debug function codes on the
always-on Modbus-debugger channel, so the editor can write/read a license
asynchronously without reflashing. Reuses the existing 0x48 GET_BOARD_ID for
the hardware anchor.
- modbus_types.h: MB_FC_DEBUG_WRITE_LICENSE=0x49, MB_FC_DEBUG_READ_LICENSE=0x4A;
MB_DEBUG_LIC_EMPTY=0x83, MB_DEBUG_LIC_CORRUPT=0x84.
- modbus_debug.{h,cpp}: debugWriteLicense/debugReadLicense delegating to
license_store_*. Wire len is big-endian; blob content is little-endian
(dual-endianness documented on both sides). READ writes blob at mb_frame[5]
(slave id@0), len BE @3..4, mb_frame_len=5+len (no overlap).
- modbus_pdu.cpp: wire both FCs into the 3 switches (process_mbpacket dispatch,
mb_pdu_request_len, mb_pdu_skips_crc).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
…age golden Byte-identical little-endian mirror of the C lic_blob_t, so the editor can build/parse license blobs that the firmware accepts verbatim. - license-blob.ts: serialize/deserialize (explicit DataView LE), crc32IsoHdlc (poly 0xEDB88320, init/xorout 0xFFFFFFFF), LIC_BLOB_SIZE/LIC_PAYLOAD_SIZE. - license-golden.json: deterministic golden fixture (106 B expected bytes + expectedCrc32 0xC0948DA7) shared with the C host test. - license-blob.test.ts: crc vector 0xCBF43926, serialize==fixture, deserialize==input, round-trip stability (12 tests). Cross-language parity proven: C struct memcmp==0 against this fixture. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
End-to-end editor path for the 0x49/0x4A license function codes, following the existing getBoardId/getVariablesList pattern. - modbus-client.ts / simulator/types.ts: DEBUG_WRITE_LICENSE=0x49, DEBUG_READ_LICENSE=0x4a, LIC_EMPTY=0x83, LIC_CORRUPT=0x84 (byte-identical). - modbus-pdu.ts / types.ts: build/parse WriteLicense & ReadLicense (len BE; READ SUCCESS reads len@2, blob@4; LIC_EMPTY/CORRUPT -> success with flag). - editor + simulator ModbusRtuClient and ModbusTcpClient: writeLicense/ readLicense (framed offsets +6, same reconnection machinery). - port/adapter/IPC/bridge: debugger:write-license & debugger:read-license channels; adapter rehydrates number[] -> Uint8Array. tsc --noEmit clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
…d-trip - modbus-pdu.test.ts: parser integrity for every status (SUCCESS, LIC_EMPTY 0x83, LIC_CORRUPT 0x84, TOO_LARGE/OUT_OF_BOUNDS 0x81, OUT_OF_MEMORY 0x82), truncated frames; write states. - simulator/modbus-rtu-client.test.ts + debugger-adapter.test.ts: unit coverage for writeLicense/readLicense. - license-roundtrip-e2e.test.ts: skip-guarded (LICENSE_STORE_HEX) avr8js round-trip 0x49 write -> 0x4A read -> byte match, magic-byte endianness sentinel. Skips cleanly when no .hex is present. jest: 142 passed, 1 skipped, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
Add a unified editor-side device-id (anchor) acquisition that dispatches on
connectionType: runtime (Linux v4, websocket) fetches it over the runtime
webserver HTTP API; arduino-cli targets (ESP32/AVR/simulator) read it via the
always-on debugger FC 0x48 (GET_BOARD_ID).
- getBoardId() on the editor ModbusRtuClient and ModbusTcpClient, reusing the
existing buildGetBoardIdRequest/parseGetBoardIdResponse (no inline parsing).
- device:get-anchor IPC handler branching to makeRuntimeApiRequest (runtime)
or ensureDebuggerModbusClient().getBoardId() (arduino-cli); unified result
{ success, source, anchorHex, anchor }.
- port/adapter/bridge for getDeviceAnchor.
- Runtime endpoint /api/device-id is provisional (TODO D56, openplc-runtime).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
arduino-cli compiles every .cpp in the sketch folder as its own translation unit, so license_store.cpp including license_store_esp32.cpp (or _avr.cpp) defined write/read/erase twice -> "multiple definition" link error on the ESP32 Generic build. Each backend already self-gates its whole body on its ARDUINO_ARCH_* macro, so the build system links exactly one. license_store.cpp now carries only the negative #error guard for architectures without a backend and includes nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
The ESP8266 has neither real EEPROM nor NVS/Preferences (ESP32-only), so it gets its own backend over the core's emulated EEPROM (a RAM-mirrored flash sector). - license_store_esp8266.cpp: same on-flash layout as AVR (blobLen LE @0, blob @2), self-gated on ARDUINO_ARCH_ESP8266. Differences vs AVR: EEPROM.begin(size) before access and EEPROM.commit() to persist (commit()==false -> IO_ERROR). EMPTY via virgin 0xFFFF; magic/crc validation identical. - license_store.cpp: extend the no-backend #error guard to accept ESP8266. - license_store.h: doc note lists the three backends. Still out of MVP, but ESP8266 (NodeMCU/D1 mini) is a real board target in the repo. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
…k backend Prepares the license store to move into platform VPPs (D59/D60): the editor keeps the contract (blob format, interface, FCs) plus a weak default backend so firmware always links even when no VPP provides a real backend. - LIC_STORE_UNSUPPORTED (+ MB_DEBUG_LIC_UNSUPPORTED 0x85) and its lic_status_to_mb mapping. - license_store_weak.cpp: __attribute__((weak)) write/read/erase returning UNSUPPORTED; a VPP-provided strong backend overrides them at link time. - TS: ModbusDebugResponse.LIC_UNSUPPORTED and parser classification (unsupported -> device state, not a transport error). Non-breaking: existing per-arch backends stay; the weak default only wins when they are absent (a later step moves them to the VPPs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
Couples the injection of the VPP-provided license backend with the removal of
the editor's built-in backends (D59/D60), so exactly one strong implementation
links. Boards whose VPP declares hal.licenseStore get its source injected into
the Baremetal sketch (recompiled against the editor's license_blob.h) plus
-DVPP_HAS_LICENSE_STORE; boards without it fall back to license_store_weak.cpp
and report LIC_STORE_UNSUPPORTED.
- compiler: resolve device.hal.licenseStore (string|array) and inject the
source(s) into the sketch, mirroring the HAL source path (containment guards,
distinctive basenames so they never collide with the HAL esp*.cpp).
- generate-defines: emit VPP_HAS_LICENSE_STORE when the device declares it.
- remove license_store_{esp32,esp8266,avr}.cpp and license_store.cpp from the
Baremetal skeleton; the contract (license_blob.h, license_store.h, FCs) and
the weak default stay.
Transitional: AVR and any board whose VPP does not yet ship the backend report
UNSUPPORTED until its VPP is updated (ESP32/ESP8266 via com.openplc.espressif).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
…nsing UX Derives a board-level `licenseStore` capability from the selected VPP device's hal.licenseStore (D60), mirroring how `vppIo` is resolved. It is the gate a future licensing UI checks to offer the anchor->activate->write flow only on boards whose VPP ships the storage backend; boards without it (weak default -> UNSUPPORTED) do not surface the flow. - TargetCapabilities.licenseStore (+ false in all presets). - resolve(): licenseStore=true when the board declares hal.licenseStore. - exposed through useCapabilities() like the other capabilities. No new UI yet; this only surfaces the capability. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
The Baremetal sketch compiles every .cpp as C++, where _Static_assert (a C11
keyword) is rejected by the xtensa/avr gcc C++ frontend ("expected constructor,
destructor, or type conversion before '(' token") - which broke the ESP8266
build. Use static_assert under C++ and keep _Static_assert for the C11 host
golden test, via a small LIC_STATIC_ASSERT macro.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
After a successful arduino-cli serial upload, run a one-shot best-effort probe (D61): open a transient RTU connection with retries/backoff (the device reboots after flashing), read the hardware id (FC 0x48) and, when the board declares the licenseStore capability, the stored license (FC 0x4A); persist the result in the device slice (deviceProbeInfo) and close the serial. The transient client never touches the debugger session state; the debugger still opens the serial on demand. - main: device:probe-storage IPC (connect-with-retries -> getBoardId -> [readLicense] -> disconnect), bridge probeDeviceStorage. - device slice: deviceProbeInfo + setDeviceProbeInfo. - workspace-activity-bar: fire the probe post-upload for directUsbUpload boards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
Trace every step of the device:probe-storage routine: connect attempts/backoff, board id, license status, and serial open/close via the main-process logger; the renderer prints the full response object (DevTools) plus a readable summary in the editor console. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
Opening the serial port toggles DTR/RTS and auto-resets ESP8266/AVR boards, so right after a flash connect() returns while the firmware is still booting and the first FC 0x48 exchange gets no reply (observed: hasId=false, license status 0x00). Retry the board-id read (6x, 500ms) as a firmware-readiness probe before reading the license; skip the license read when no id ever comes. Lower the transient client timeout to 2s so the retries stay cheap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
…baud The post-flash probe hard-coded baudRate 115200/slaveId 1, but the debugger's RTU baud/slaveId/port come from the board debug spec (e.g. espressif's rtu_baud_rate, default "115200"), so a board configured for another baud never answered (6/6 id reads failed while a manual debug-session anchor read worked). Resolve the same DebugConnectionConfig the debugger uses (resolveDebugConnection, non-interactive) and pass its RTU params to the probe, coercing baud/slaveId to numbers; skip the probe (best-effort, no dialogs) when no direct RTU config resolves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
Remove the [device-probe] logger lines (main) and the console.* plus editor-console addLog lines (renderer) added while debugging. The probe is now silent to the user and still persists deviceProbeInfo internally. Behavior (connect + id-read retries, license guard, resolved RTU params) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
…flow Add `isLicensable` to TargetCapabilities (types/presets/resolve), defaulting to false everywhere. Gates the post-flash license activation routine (PLA-01): only boards whose VPP manifest declares `capabilities.isLicensable: true` run the derive -> activate -> write flow. Flows verbatim from the manifest capability block via the existing board-loader spread, exactly like `vppIo` — no hardware-module change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
New main-side module `backend/editor/license/device-identity.ts` (PLA-05):
deriveDeviceId(anchor) = sha256("openplc-dev-v1|" || anchor)[:16] hex
deriveVppId(packageId) = sha256(packageId)[:8] hex
Uses node:crypto (main-only), so it lives under backend/editor rather
than the byte-identical backend/shared surface that must not depend on
node:crypto. Golden-vector tests pin the deterministic outputs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
New main-side `backend/editor/license/license-activation-client.ts`
(PLA-06). `checkDeviceActivation({ deviceId, vppId, packageId })`:
- OPLC_LICENSE_MOCK=licensed -> { licensed: true, license: <golden
106-byte blob> } (exercises the FC 0x49 write path).
- OPLC_LICENSE_MOCK=demo -> { licensed: false }.
- absent -> real edge client: POST {base}/vpp-licenses/activate,
unwrapping the { statusCode, data } envelope, reusing
getEdgeApiBaseUrl (default https://api.autonomylogic.com). Sends a
Bearer token when OPENPLC_EDGE_TOKEN is set. Best-effort: any failure
(route missing -> 404, network, bad JSON) resolves to
{ licensed: false, error } instead of throwing.
The golden blob is built from the shared serializeLicenseBlob so the
mock stays byte-valid. TODO(D49/D51): drop the toggle once the edge
vpp-licenses module exists.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PWS1nDF7s7sWcw9ddK9V3K
Zero callers anywhere in src (confirmed by grep across the whole tree), despite its own docstring inviting one: 'Compiler module should call this instead of reading hals.json directly.' It built a bare BoardInfoResolver -- not VerifiedBoardInfoResolver -- so wiring it up as invited would silently bypass the signature gate #39 added to every real compile/upload path. Deleted rather than pointed at the verified resolver: nothing calls it, so there's no behavior to preserve, and a live but currently-pointless verified version would still carry the same standing invitation to misuse, just with one fewer footgun. Removed the now-unused imports it alone required (pathResolve, pathSep, BoardInfoResolver, BoardBuildInfo) -- PackageManagerModule and assertPathContained stay, both used elsewhere in this file. Verified via grep, not assumption, that BoardBuildInfo (the return type) and getBoardBuildInfo (the method name) have no other reference in src or in any test file.
The baremetal runtime had no notion of RUN vs STOP -- loop() called scheduler() forever from boot. This adds the state machine every Arduino target gets, plus Modbus FC 0x49 to drive it over the existing debugger transport (0x46-0x48 stay reserved for the planned debug streaming codes). State is derived every cycle from the mode switch and a software latch. hardwareStateSwitch() has a weak default returning RUN, so a HAL that does not override it behaves exactly as before: boots RUNNING and runs. A rising edge to RUN clears the software latch, so a physical flip to RUN always wins -- otherwise a software-stopped device would sit dead in the RUN position with no local way to recover. While stopped the loop keeps cycling: inputs are still refreshed so the debugger sees live field data, outputs are re-zeroed every cycle (so a Modbus client cannot energise an output while stopped -- the write lands in the image and is cleared before the HAL sees it), updateOutputBuffers() is still called so a HAL can drive a status LED from it, and IEC time is frozen so timers resume rather than jumping. Entering STOP is a cold stop: the program is re-initialised so the next start begins at cycle 1 with IEC initial values. No dynamic allocation is involved -- g_config has static storage and placement new constructs into it. The destructor call is paired with it because Configuration derives from ConfigurationInstance, which has a defaulted virtual destructor: non-trivial, but owning nothing. runtime_discover_tasks() is deliberately not re-run, as it new[]-allocates and its tables stay valid. Two consequences to know about: debugger forces are cleared by the re-init (force state lives inside each IECVar, matching v4, which unloads the program outright), and a program using the explicit IEC NEW operator must DELETE before stopping or it leaks across restarts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Start/Stop now works for arduino-cli targets, gated by the new plcStateControl capability. The PDU codec lives in the shared modbus-pdu.ts so all four transports (editor TCP + RTU, the shared simulator RTU client, and the WebSocket transport) agree on the bytes, and the IPC handler reuses an already-open debug client rather than opening a second one that would fight it for the serial port. Two protection layers, per the design: the editor never sends a start to a device whose switch reads STOP (pre-check), and it still handles the device's refusal in case the switch moved in between. Firmware predating FC 0x49 answers the Modbus exception form, which surfaces as an informational "rebuild and upload" rather than an error, so devices in the field don't look broken after an editor upgrade. Runtime v4 gets the same treatment over REST via the additive switchPosition status field and START:ERROR_SWITCH_STOP. Tests: FC 0x49 codec unit tests, plus a gated end-to-end suite that boots real firmware in avr8js and exercises query/stop/restart, output de-energisation, program re-init, and -- with a HAL that overrides the switch -- the boot gate, the RUN refusal and the rising-edge auto-start. Known gap: nothing polls a baremetal target's state yet, so the button shows Play even when the device is running. That needs a persistent connection (transient clients can't poll: RTU connect() waits 2.5s for the Arduino bootloader) and lands on top of feat/always-on-debugger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings the persistent device CONNECT (D72), then uses it to close the gap this branch shipped with: nothing polled a baremetal target's state, so the Start/Stop button showed Play even while the device was running. --- Polling, built on what already existed ------------------------------- No new timer, no new connection, no extra traffic. The main process already polls the held device link every 2.5s to keep it honest, calling getBoardId (0x48) purely to prove the firmware answers. That call becomes getStatus (0x46), which proves exactly the same thing and also carries the run/stop state and mode-switch position. The tick pushes them to the renderer, a small hook mirrors them into the store, and `plcStatus` is the SAME field the Runtime v4 poll writes -- so the button, its tooltip and the debugger's "PLC is stopped" prompt all work unchanged for both target types. switchPosition lands next to plcStatus, which lets the start pre-check become a store lookup instead of its own round trip over a serial port the poll is already using. A null position means "unknown / no switch" and must not block a start, or a board without a switch would be un-startable. --- Conflict resolutions (12 files) -------------------------------------- Function-code collision: that branch already uses 0x49 for DEBUG_WRITE_LICENSE, 0x4a for DEBUG_READ_LICENSE and 0x83-0x85 for the LIC_* statuses -- all of which this branch had taken. Run/stop moves to FC 0x4b and its refusal status to 0x86. FIRMWARE ALREADY FLASHED FROM THIS BRANCH MUST BE RE-FLASHED; editor and firmware move together, and older firmware still degrades to "rebuild and upload" via the Modbus exception form. Consolidated the state READ onto FC 0x46 rather than keep two function codes reporting the same thing. debugGetStatus already returned a `running` byte, hardcoded to 1 with the comment "PLC scan is always running on baremetal" -- which this branch makes false -- so it now reports runtime_get_plc_state() and appends the switch position. The QUERY sub-command is gone, with getPlcState on the port, adapter, preload bridge and transports: a second path to the same bytes, and dead once polling reads 0x46. Placements follow that branch's Modbus split: FC in modbus_types.h, length and dispatch in modbus_pdu.cpp, handler beside debugGetStatus in modbus_debug.cpp. Deliberately NOT in mb_pdu_skips_crc -- the debug FCs are exempt because their payloads are arbitrary bytes that make framing awkward, which does not apply to a fixed 2-byte PDU that changes machine state. getStatus/setPlcState are optional on LicenseCapableTransport because run/stop is baremetal-only: the RTU/TCP clients implement them; the runtime-v4 WebSocket transport (which implements that interface only for licensing) does not, as v4 drives run/stop over REST. Verified: both firmwares build, the 14-case avr8js hardware-in-the-loop suite passes against the merged sources, the P1AM-100 target compiles, and the full editor suite is green (5797 passing). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The picker showed only the path (`/dev/cu.usbmodem11101`) instead of `/dev/cu.usbmodem11101 (Arduino MKR)`. serialPortDisplay was written on the premise that `name` holds a bare manufacturer string, so it used the address as the label and pushed `name` to a hover title. But `mergeSerialPortList` — the editor's enumerator — has always returned `name: "<address> (<descriptor>)"`, already composed. Taking the address alone therefore discarded the board name, and the title ended up carrying the string that should have been visible. Compose instead of choosing, so both invariants hold: the path always leads (a NodeMCU must never read "wch.cn" instead of "COM5") and the descriptor survives (it is what tells two identical-looking /dev/cu.usbmodem* nodes apart). Pre-composed names are detected rather than assumed, so a bare manufacturer still gets wrapped as `COM5 (wch.cn)` and an already-composed one is not double-wrapped into `COM5 (COM5 (wch.cn))`. The existing tests passed because every fixture used the bare-manufacturer shape, which the real producer never emits. Both shapes are covered now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y string
Fixes the communication-port label properly rather than by sniffing shapes.
`CommunicationPort` was `{ name, address }` where `name` was a pre-composed
`"<address> (<descriptor>)"` built by mergeSerialPortList. That conflated a
fact with a presentation decision, and it is what let the board name get
dropped: serialPortDisplay could not tell an already-composed name from a
bare manufacturer, so it took the address alone and pushed the composed
string into a hover title. My first fix detected both shapes, which worked
but left the ambiguity in place.
The port now reports facts -- `address`, `boardName?`, `manufacturer?` -- and
serialPortDisplay is the single place that decides how they read: the path
always leads (`COM5` on Windows, `/dev/cu.usbmodem*` on macOS,
`/dev/ttyUSB0` on Linux), with the descriptor in parentheses, preferring
arduino-cli's identified board name and falling back to the OS manufacturer
string. Every platform goes through that one path, so there is no second
labelling rule to drift out of sync -- which is how macOS and Windows came to
disagree in the first place.
Knock-on cleanups: the merge no longer picks a descriptor winner (that
precedence moved to the renderer, where it is testable), and the two preload
signatures that still advertised `{ name, address }` are corrected -- they
compiled only because excess properties pass for non-literals, which is
exactly the drift that hid this.
Tests: the label suite now asserts the platform expectations directly
(12 cases incl. Windows COM numbering and the precedence rule); the producer
suite asserts it reports both descriptors instead of composing. The old label
tests passed because every fixture used the bare-manufacturer shape the real
producer never emits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Serial debugging failed with "Resource temporarily unavailable / Cannot lock port" whenever the device was connected, and worked only after disconnecting. The debug path opened a SECOND handle on a port the Connect flow already holds, and the OS will not grant it twice. Debugging a baremetal target now requires being connected and runs over that connection, mirroring Runtime v4 (which already requires a runtime connection). The debug client is the held device client, adopted rather than opened, in all three RTU paths: connect, verifyMd5, and the lazy reconnect. That also fixes the Start/Stop button going stale during a session. The button is fed by the device link's liveness tick; the previous attempt at this handed the port over to the debugger, which tore that link down and silenced the state exactly while the user was watching. Sharing the client means the link is never torn down, so no second poll or extra traffic is needed to keep the button live. Ownership is explicit, because a shared client has two ways to go wrong: debugger disconnect drops its reference WITHOUT closing a borrowed client (closing it would kill the user's device connection), it never calls connect() on one (that would re-open the port and re-wait the 2.5s Arduino bootloader delay), and device teardown clears the borrowed reference so a debug read reports needsReconnect instead of talking to a closed port. The gate is declarative: a new `deviceConnected` precondition alongside the existing `runtimeConnected` / `jwtToken`, so a package states the requirement in its debug spec and gets the standard "Connection Required" dialog. The main process refuses independently, so a spec that omits it still cannot lock the port. This was pre-existing on feat/always-on-debugger, not from the merge: it released the port only in verifyMd5, but the renderer connects first, so the lock was already taken by the time the handoff ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Connect reported "Select a communication port for this device first" with a port already selected. Self-inflicted, in the previous commit. The debugger-requires-connection gate was expressed as a `deviceConnected` debug-spec precondition. But the Connect flow resolves the SAME spec to derive the port, baud rate and slave id it needs in order to OPEN the connection -- with nothing connected yet, because that is what Connect is for. The precondition therefore failed, resolveDebugConnection returned `error` instead of `config`, and use-device-connect surfaced its generic "select a port" message. Connect needed a connection to establish a connection. A debugger-only requirement cannot be a spec precondition while the spec has two consumers, so the precondition mechanism is removed entirely rather than left as an unusable knob: the enum value, the capability field, the resolver branch, the context population, and the manifest entry are all gone. The gate now sits in the debugger entry point, where only the debugger reaches it, and the main process still refuses independently -- so a session cannot lock the port even if the renderer check is bypassed. Tests pin both halves: a baremetal spec resolves to an rtu config while disconnected, a genuinely missing port still produces its `required` message, and a spec carrying any precondition demonstrably breaks Connect -- which is the trap this commit exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two corrections to the previous commit's gate, both about scope. The gate keyed off the board (`directUsbUpload`), which also caught Modbus TCP. Only SERIAL debugging is coupled to the device connection, and for a physical reason: it shares the one client holding the port open, because the OS will not grant a second handle. Modbus TCP opens its own socket to an address it already knows and has no such coupling, so requiring a connection there blocked a perfectly good debug path. The gate now keys off the RESOLVED transport instead, which is the thing the requirement is actually about. The coupling also has to hold in the other direction: a serial session whose link drops (unplug, reset, liveness failure, or the user pressing Disconnect) has no transport left, so it now stops. Leaving it "active" showed a frozen variable table over a dead port and left the debugger unable to reconnect. TCP sessions are deliberately untouched. Also fails the build when a sketch target resolves without a HAL. Nothing else defines hardwareInit / updateInput/OutputBuffers, so the result was a wall of undefined-reference errors whose actual cause — the board resolved without a HAL, e.g. a VPP that failed to load — appeared only as a warning far earlier in the log. Scoped to targets that build a sketch: Runtime v3 legitimately has no HAL, since its on-device MatIEC compiles the ST itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rning A pulled cable used to leave the editor reporting "connected" while every request timed out, and the link was then torn down for good on the second silent poll with no attempt to get it back and nothing said to the user. Connection manager. The liveness poll doubles as the recovery loop, so there is still exactly one timer: two silent polls close the dead handle (a stale open fd is what makes a reopen fail with "cannot lock port") and report 'connecting', each later tick makes one reopen attempt from the remembered connect params, and a port that answers restores 'connected' with nothing for the user to click. After ~30s of failures the link is torn down and reported with reason 'lost', which warns through the same dialog Runtime v4 uses (parameterized rather than cloned). Reopening deliberately skips probeAndRecover: the license recover from the original Connect still stands, and re-running it would hammer the backend for as long as a cable is out. The counting is extracted as SerialLinkPolicy so the thresholds — the part a user feels as "it recovered by itself" vs "it gave up too early" — are unit tested without a cable to pull. The I/O keeps owning the client. Status now reaches the store from workspace level (useSerialConnectionMonitor) instead of a device-screen-local subscription. The link outlives the screen that opened it, so navigating to a POU used to leave the store advertising a link that was already dead. Connect resolves the SERIAL channel explicitly, via a shared resolveSerialLink. Auto-selection returned a tcp config in a Modbus-TCP-only project, so Connect refused with "select a communication port" while one was plainly selected, and the post-upload reconnect silently skipped, leaving the held link closed. Both serial flows now name the channel; naming it is right on its own terms, since the always-on debugger keeps the serial protocol compiled in even with Modbus RTU off. Failures also report the resolver's actual reason instead of blaming the port for everything. Run/stop commands go over the held serial link whatever the debug transport is (websocket excluded — v4 uses REST). Keying it off the debug transport sent Stop to the board's IP in a Modbus-TCP-enabled project, which times out on a board whose ethernet shield is not connected, while the open cable sat unused. A stop during recovery says so instead of opening a transient client that would contend with the reopen. Also: a serial debug session now ends the moment the link leaves 'connected' (recovery included — its client is already closed), and two cross-layer imports from the run/stop work are declared in KNOWN_EXCEPTIONS, so validate:arch is green again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reported defect: with a live Modbus TCP debug session, Stop failed with "Request timeout". The command path recognised only an RTU client as reusable (`connectionType === 'rtu' || 'simulator'`), so a tcp stop skipped the open TCP client and opened a transient SECOND socket — which an Arduino Modbus TCP server, serving one client at a time, never answers. The connection was fine; the command went somewhere else. That was a symptom. Three places opened clients for the same device (the debug session, two lazy-reconnect paths, one transient per run/stop), each with its own idea of which transport counted as reusable, and Modbus clients were constructed at nine sites. This replaces all of it with one owner. DeviceLinkManager owns THE connection: it takes the ordered candidates resolved from the board's debug spec, tries them in order, and keeps the first that both opens and verifies. Modbus TCP is preferred when the project enables it (with the spec's DHCP address prompt, which Connect could not surface before) and serial is always a candidate, since the always-on debugger keeps the serial protocol compiled into every baremetal firmware. If neither answers, the attempt fails and reports what was tried. Verification per candidate is what makes preferring TCP safe: a socket that opens but answers nothing falls through to the cable instead of stranding the user. Every command — debug reads and writes, run/stop, md5, licensing, the status poll — now borrows that one client. None of them can open a connection. The debugger no longer owns a Modbus client at all; only the runtime-v4 WebSocket, a different protocol to a different target, keeps its own. Failure handling is transport-aware and fast (the previous 30s window is gone): a serial port that disappears from the OS fails immediately, since there is nothing to retry against, while an unresponsive device gets two silent polls and two reopen attempts. Recovery retries the whole candidate list, so a link that drops comes back on either transport. The floor of two silent polls is not timidity — reopening a serial port asserts DTR, which resets an AVR board, so a single dropped frame must not restart the user's program. Structure, all of it now single-source: DeviceModbusTransport (both clients implement it, so transport is a detail, not a branch), buildDeviceModbusTransport (the one constructor; the license factory delegates to it), resolveDeviceLink- Candidates (replaces resolveSerialLink), and services/device-link-resolution (the resolve → ask → resolve loop, shared by Connect and the debugger instead of existing only in the activity bar). Also fixed along the way: an upload now releases the connection only when it is the serial one holding that port, so a Modbus TCP link — and the debug session on it — survives an upload; the store's `serialConnection` became `deviceConnection`, because a field named for one transport while holding either is what made callers branch wrongly to begin with; and validate:arch learned to read multi-line imports, which it never could, so it had been reporting success while missing real violations of its own rules. Deleted: debuggerModbusClient, debuggerBorrowedDeviceClient, the stored RTU session params, debuggerReconnecting, adoptHeldSerialClientForDebug, ensureDebuggerModbusClient, the duplicated reconnect blocks, the per-command teardowns, and SerialLinkPolicy. Net 315 lines lighter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The firmware silently dropped every debug and control request that arrived over
Modbus TCP:
mb_frame_len = mb_mbap[4] << 8 | mb_mbap[5];
if (mb_frame_len < 6 || mb_frame_len > MAX_MB_FRAME) return;
6 is the minimum length of a standard DATA request ([unit][fc][addr:2][qty:2]).
Every debug/control request carries no payload at all, so its MBAP length is 2 —
below the floor, discarded before process_mbpacket() ever saw it. That is 0x41
debug-info, 0x44 get-list, 0x45 md5, 0x46 status, 0x47 version, 0x48 board id and
0x4b run/stop: the entire always-on debugger, the run/stop control and the
connect probe. Standard reads and writes were unaffected, which is exactly why
the board looked healthy over TCP while nothing the editor needed worked.
Measured against a real P1AM-100 at 192.168.2.20 before the change:
FC 0x03 read holding regs -> reply in 0.01s
FC 0x01 read coils -> reply in 0.03s
FC 0x41 / 0x45 / 0x46 / 0x47 / 0x48 -> no reply (timeout)
FC 0x46 padded to length 6 -> 00 46 7e 01 ... 01 <- same FC, answers
The padded request proves the floor was the only obstacle: process_mbpacket()
dispatches on mb_frame[1] and never looks at the length. Response sizing was
already correct — every handler sets mb_frame_len, and the TCP path re-reads it
after dispatch.
The floor is now 2 ([unit][fc]) in both TCP branches. Per-FC shape is validated
in process_mbpacket(); over TCP the MBAP length is authoritative, there being no
CRC to check.
This is why "the only connection mechanism that works is serial": Connect could
never verify a TCP candidate, so it always fell through to the cable, and a debug
session started over TCP would never have received an answer either.
Verified by compiling the P1AM target through the editor's own pipeline with the
project's Modbus TCP settings, so the #ifdef MBTCP blocks are actually built:
35,864 -> 49,396 bytes of program storage. Requires a rebuild and upload to take
effect on a device.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Confirmed on hardware: with the firmware fix flashed, Modbus TCP now verifies in
51ms and Connect takes the TCP path. But the log from the failing run showed the
other half of the problem — ruling out ONE unreachable candidate cost 32.5s:
192.168.2.20: classified as "no-firmware"
192.168.2.20: opened but did NOT answer the debug protocol (waited 32540ms)
That is `readBoardIdWithRetries(attempts: 6, backoffMs: 500)` against a 5s request
timeout. The patience is deliberate and correct for a board that was just flashed
and is still booting — but not while alternatives are waiting, where it delays the
connection that would have worked. So the retry budget is now explicit
(PATIENT_BOARD_ID_PROBE / QUICK_BOARD_ID_PROBE) and the manager tells `verify`
whether alternatives remain: quick while they do, patient for the last resort.
A stale address costs ~10s instead of ~33s, and a lone serial candidate keeps the
full post-flash tolerance.
Also corrects the firmware comment, which overstated what the length floor broke.
Measured per function code on the unpatched board:
0x44 get-list len 4+3n passed 0x41 debug-info len 2 dropped
0x45 md5 len 6 passed 0x46 status len 2 dropped
0x47 version len 2 dropped
0x48 board id len 2 dropped
0x4b run/stop len 3 dropped
A debug SESSION only uses 0x44 and 0x45, which is why debugging over Modbus TCP
always worked while Connect and run/stop over TCP never could. My earlier claim
that the debug protocol never worked over TCP was wrong.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from testing on real hardware. 1. The DHCP prompt could never appear. `debugger-ip-input` is one of the modals app-layout mounts explicitly, and it was missing from that list — so opening it rendered nothing and the promise it returns never settled. Connect simply hung, which is exactly what the log showed: resolution returned "prompt" and the trace stopped there. Now mounted. (The debugger's own DHCP path was equally dead; no one had reached it.) 2. Serial is now tried FIRST, Modbus TCP second. Preferring TCP was wrong: serial is the direct, local, physically unambiguous path — if a cable is attached, that is the device the user is looking at, with no address to be stale and nothing to ask them. TCP is the remote fallback. 3. A DHCP address is asked for LAST, and only if it is needed. Resolution now runs in two passes: everything that needs no input (serial, TCP on a static address), and then — only if all of that failed — the channels that need a question. So a user with a cable attached is never interrupted by a dialog about an address they do not need to know, which is what item 2 alone would still have done. The automatic post-upload reconnect defers prompts permanently: it must never pop a dialog behind the user's back. Also unifies the Connect button. Runtime v4 and baremetal had two hand-written copies that had drifted: different colour when connected, different disabled rules, and the baremetal one never showed the green "● Connected" confirmation at all, so a connected device looked identical to a disconnected one apart from the label. One `DeviceConnectButton` now serves both; the connection mechanics stay with each caller. Its disabled rule also got smarter than either copy: a missing serial port only blocks Connect when serial is the ONLY way in, since with Modbus TCP configured the network path is still available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reported bug: connected over serial, with Modbus TCP + DHCP configured in the
project, clicking Stop asked for the device's IP address. `handlePlcControl`
re-resolved the board's debug spec purely to obtain a transport label to pass
down, and that resolution hit the DHCP prompt. Main then ignored the label and
sent the command over the held serial connection — so the address the user typed
was discarded.
Commands now carry a payload and nothing else:
setPlcState('STOPPED') // was setPlcState(config, 'STOPPED')
Which medium delivers it is the connection manager's business. The type change is
the regression guard: with no config parameter, the bug is unrepresentable.
Also removes two related cases of deciding things by transport:
- Debug-session start no longer resolves a spec for a connected target. It asks
the TARGET whether its session rides the device connection, so a baremetal board
reached over Modbus TCP is treated as the connected device it is. Runtime v3/v4
and the simulator still describe their own channel (phase 3 gives them sessions).
- `isRuntimeTarget` was computed as `connectionType === 'websocket' || 'tcp'`,
which misclassified a baremetal board on TCP as a runtime and would have offered
it the "PLC is stopped, start it?" dialog. It now asks `isOpenPLCRuntimeTarget`.
- The debug-drop handler keyed off the session's transport being 'rtu', so a
session over Modbus TCP would have survived its own connection dropping. It now
tracks whether the session rides the device connection at all.
`deviceConnection` gains a read-only `transport` mirror, because the debug poll
must size its frame budget to the medium. That reads the manager's decision; it
does not make one.
Deletes `getDeviceAnchor` (port, adapter, preload, main handler, IPC channel,
tests): dead through all four layers, and one of the four transport-carrying APIs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ase 2) DeviceLinkManager becomes DeviceSessionManager and holds a session with CONTROL and DEBUG slots, because that is the shape real targets have: a baremetal board answers both over one Modbus connection, a Runtime v3/v4 is controlled over REST but debugged over something else, and the simulator answers both over its in-process virtual serial port. When one medium serves both roles the slots hold the SAME channel, so nothing opens twice and releasing the debug role cannot close the connection run/stop is using. When they differ (phase 3) the debug channel opens on request and closes when the last holder releases it — holders tracked as a set of reasons, because a license check must not close a channel a live debug session is reading through. A debug channel that will not open is reported to whoever asked and leaves the control connection untouched. The simulator now has a real session, created by the emulator's own lifecycle: starting it opens the session, stopping it closes the session first and then the emulator. Stop still stops the emulator entirely, not just the program it runs. That inverts an ownership that was backwards — `useDebugSession.stopSession()` used to call `simulator.stop()`, so a debug session owned the thing on the other end of its connection. It now ends only itself, and an emulator that stops takes its debug session down through the same handler a pulled cable goes through (deleting the bespoke onStopped wiring). Splits one overloaded question into the two it actually was: whether a debug session RIDES a manager-held session (baremetal and simulator) and whether the user must have pressed Connect first (baremetal only — the simulator's session is created by pressing Start). Verified against the real avr8js emulator, headlessly: the session opens 20ms after loadAndRun, control and debug share one channel, a debug read works over it, FC 0x4b run/stop takes effect inside the emulator (running true -> false -> true), releasing the debug channel leaves the connection answering, and close leaves the emulator stoppable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
v3 and v4 have the same shape — controlled over REST, debugged over something
else — so they get sessions with the two slots phase 2 introduced: control over
REST (recorded, not opened: REST is connectionless, there is no socket to hold,
poll or recover) and a debug channel opened only when a debug session asks for it.
v3's is Modbus TCP on the runtime's address, v4's is the debug WebSocket. Logging
in establishes the session; logging out closes it.
With that, the transport parameters leave the IPC surface entirely:
debuggerPort.connect() // was connect(config)
debuggerPort.verifyMd5(md5) // was verifyMd5(md5, config)
Nothing resolves a channel per command or per debug session any more. Every
target's session is established before a debug session can start — a device by
Connect, a runtime by logging in, the simulator by pressing Start — so the only
question left at the debug button is whether that session exists. Which medium it
uses is the manager's to know, and the debug gate now says "connect first" for a
runtime as well, in the runtime's own words.
The WebSocket stops being a special case: it satisfies the same `DeviceDebugChannel`
contract the Modbus clients do, so main holds no `debuggerWebSocketClient`, and
verifyMd5 / getVariablesList / setVariable / debugger-connect each collapse from
two branches into one. Stopping the debugger releases the debug channel instead of
disconnecting a client — which closes a channel of its own once nothing holds it,
and never closes one shared with control.
Deleted as dead: `ensureDebuggerWebSocket`, `ensureDeviceLinkFor`,
`resolveDebugConfigWithUx` and the ref/callback pair that fed it.
Re-verified against the real avr8js emulator after the change: session opens,
control and debug share one channel, FC 0x4b run/stop takes effect inside the
emulator, releasing debug leaves the connection answering, close stops it cleanly.
Runtime v3 is implemented but UNTESTED on hardware — I have no v3 to reach. Its
debug channel is the same ModbusTcpClient a baremetal TCP connection now exercises,
so the untested part is the wiring, not the transport.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`handlePlcControl` had two copies of the same logic — one sending Modbus FC 0x4b,
one POSTing to a runtime — each with its own switch pre-check, refusal handling,
error reporting and optimistic state update, kept in step by hand. There is now
one path: `setPlcState('RUNNING' | 'STOPPED')`, routed by the session's control
channel. "Start the PLC" is the same request whether it travels down a cable or
over HTTP.
Main maps the REST reply into the same `PlcControlResult` the Modbus path returns,
including `ERROR_SWITCH_STOP` -> `refusedBySwitch` — the runtime's way of saying
the mode switch refused a start, which is what FC 0x4b status 0x86 means. So the
caller handles one result type, and the switch warning is written once.
The build flow's "stop the PLC first" and the debugger's "PLC is stopped, start
it?" now take that same path, which removes the last target-specific control calls
from the renderer.
Start/Stop gating keys off the session: a device connection for a baremetal target,
a runtime login for v3/v4. It used to key off `directUsbUpload`, which said nothing
about whether anything was connected — so on a baremetal board the button always
looked ready and failed with "connect first" AFTER the click, instead of explaining
itself before it.
`RuntimePort.startPlc/stopPlc` are deliberately left in place despite having no
renderer callers: the ports layer is shared with openplc-web, and I cannot see that
repo from here to know whether it uses them. Flagged rather than deleted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…se 5) Deletes `debuggerTargetIp` and `debuggerJwtToken` from main — the WebSocket's credentials now travel with the debug channel the session opens, so nothing reads them — and updates comments that still named `DeviceLinkManager` or described the session as a single link. Sweep is clean: no references remain to debuggerWebSocketClient, debuggerRtuPort, ensureDebuggerModbusClient, ensureDeviceLinkFor, resolveDebugConfigWithUx, getDeviceAnchor, activeDebugTransportRef or plcControlNeedsConnection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…state Brings in 13 commits: the VPP package signature verification in the compiler paths, `verified-board-info-resolver`, the debugger trend fixes (sample-and-hold drawing, sliding window), st-lsp project-sync work, POU signature serialization changes, removal of the dead `getBoardBuildInfo`, and the 4.2.10 version bump. One conflict, in `resources/sources/arduino/openplc.h`: both sides edited the block after the HAL declarations. Kept both — our run/stop HAL surface (`hardwareStateSwitch`, `runtime_get_plc_state`) and their corrected account of the license-core raw-I/O wrappers, which supersedes the shorter note we had written in the same place. Verified the resolved header compiles standalone as both C and C++, with the extern "C" block balanced and every declaration from each side present. Three files auto-merged where both branches had touched them, each keeping both changes: `validate.ts` (their `fileURLToPath` fix for SRC_ROOT plus our multi-line import extractor and its exception entries), `hardware-module.ts` (their `getBoardBuildInfo` removal plus our `isSerialPortPresent`), and `compiler-module.ts` (their signature verification plus our hard failure when a sketch target resolves without a HAL). Suite green at 5898 passing (their new suites included), 0 lint errors, validate:arch passes, typecheck clean apart from the four pre-existing reactflow errors in the graphical editor. No dependency changes — package.json moved only the version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eligible Runtime v4 targets had no session at all. `resolveDeviceLinkCandidates` decided eligibility from a serial-then-Modbus-TCP order written into the resolver, so a `websocket` channel was never a candidate: resolution returned an error, no runtime session was ever opened, and every command afterwards answered "not connected" — on an SLM-RP4 the user had connected to and uploaded a running program to. Both the debugger and Stop failed that way. Eligibility and order now come from the target's `debuggerTransports`, which the capability matrix already declares and already gets right: `['modbus-serial', 'modbus-tcp']` for an Arduino board (serial first, exactly the preference that was hardcoded), `['websocket']` for a Runtime v4, `['modbus-tcp']` for a v3, `['modbus-serial']` for the simulator. One resolver serves every target because the target says which media it speaks, rather than the resolver assuming. A channel a target cannot speak is not a candidate however the spec describes it. Serial keeps its exemption from `enabledWhen` — "Modbus RTU disabled" does not mean a board is unreachable over serial, since the always-on debugger is compiled in either way. The service entry points now take the board rather than a bare spec, because both halves matter: the spec says how a channel is built, the capabilities say which channels exist. That also removes the second resolution path I had started to add. Two message fixes, both prompted by how bad this read on hardware: - "Connect to the device first: the debugger and run/stop share the device connection" is now just "not connected to the target". The old text explained a reason that was written for a baremetal board and is false for a v4 (whose debug channel is its own WebSocket and shares nothing) — and it appeared on a target the user HAD connected to, so the explanation was not merely irrelevant but wrong. The caller already names the failed action, so the reason is all this has to supply. - The debugger's failure dialog is now "Can't Start Debugger" / "Can't start the debugger — <reason>." Runtime sessions also no longer fail silently: the hook logs why when no address is recorded, no debug channel can be described, or the session will not open. That silence is what let this reach hardware. Verified by resolving the INSTALLED SLM-RP4 manifest with the runtime-v4 preset's transports: it yields its WebSocket channel with both the address and the JWT populated. Suite green at 5906 passing, 0 lint errors, validate:arch clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… a dead param Three cleanups from the architecture review, all consequences of one rule: a component sends a payload and the connection manager owns the medium. 1. The manager now publishes the DEBUG channel's medium alongside the control one. These are two facts: the control medium decides what "the connection dropped" means, the debug medium decides the poll's frame budget (a WebSocket swallows 500 variables per round trip, Modbus TCP 60, RTU 19). A Runtime v4 session is controlled over REST — no medium there at all — so publishing only the control medium left the poller with nothing and it silently fell back to TCP-sized batches: 60 variables instead of 500, eight times the round trips. Functionally invisible, which is why it needed finding rather than reporting. 2. `connectAndStart` no longer takes a `DebugConnectionConfig`. Nothing passed one, and a dead parameter of exactly the type removed everywhere else is how the old design creeps back. Its target-IP display now comes from the session too. 3. Run/stop and debug-start gate on ONE question — does the manager hold a session — instead of asking the target's kind first and then which of two statuses to read. Every session publishes its status: a device connection, a runtime login, a running emulator. Asking "did the user log in" for a runtime is a different question, and the two diverged in practice: logged in, session never opened, every command refused. Build & Upload is deliberately NOT gated. Uploading is how a blank board stops being blank, so it cannot require a connection; `handleBuild` consults the connection only to hand the serial port over to arduino-cli. Verified unchanged: its only gate remains `isCompiling`. Suite green at 5907, 0 lint errors, validate:arch clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pload Uploading with the hardware mode switch in STOP ended with: Compilation completed successfully (exit code: 0). Failed to start PLC: START:ERROR_SWITCH_STOP Failed to upload to runtime. Stopping compilation process. All three lines after the first are wrong. The program did reach the device and compiled there; the runtime simply declined to start it, which is what a mode switch in STOP is for. Leaving the switch there while uploading is a normal thing to do, so the message should not send anyone looking for a problem. startPlcAfterBuild now recognises ERROR_SWITCH_STOP as its own outcome and explains it as a warning: "Program uploaded. The PLC was not started because the mode switch is in STOP -- move it to RUN to start." It is not retried either; nothing changes until a human moves the switch, so the 5 s BUSY loop had no business spinning on it. deployRuntimeProgram maps that to UPLOADED_NOT_STARTED, and both platform adapters treat it as a successful upload. Whether an outcome means "the program reached the device" now lives in one shared predicate, deployReachedDevice(), rather than being re-derived as `outcome === 'STARTED'` in each adapter -- the editor had it in two places and web in a third, which is how they would drift. Not addressed here, but noticed: START_TIMEOUT still maps to a failed upload, so a runtime that stays BUSY past the deadline reports "Failed to upload to runtime" after logging a warning that says otherwise. Same shape of bug, different trigger.
TRANSITIONING means a start or stop is already underway: the runtime answers COMMAND:BUSY to everything except PING and STATUS, and the state it will settle on is not decided yet — so the icon is drawn from a state that is about to change and a click cannot do what it appears to. Folded into the existing plcControlBlocked / plcControlBlockedReason pair rather than added as a second mechanism, so the tooltip explains this the same way it explains a missing connection: 'PLC is changing state...'. handlePlcControl carries the same guard. Status arrives by poll, so a render can be up to one interval stale; the guard closes the window where the button still looks live and covers callers that are not the click.
|
Important Review skippedToo many files! This PR contains 123 files, which is 23 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (31)
📒 Files selected for processing (123)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Superseded by #996. This branch had Everything else on it is already in Branch tip preserved here for reference: |
Run/stop state control for the editor: one device connection per target, run/stop over whatever medium that connection uses, and a hardware mode switch that is authoritative over the editor.
What this adds
debuggerTransports), not a hardcoded serial-then-TCP list. That is what makes a Runtime v4 target's WebSocket a candidate at all — without it no session was ever opened and every command answered "not connected" on a target the user had connected to.refusedBySwitchfrom both transports (Modbus status0x86, RESTERROR_SWITCH_STOP) and the simulator, and surfaced with the switch label the VPP declares.Two fixes from hardware testing
Failed to start PLC/Failed to upload to runtime/Stopping compilation processafter a successful compile. The program had reached the device; only the start was declined. Now a warning — "Program uploaded. The PLC was not started because the mode switch is in STOP" — and the pipeline no longer bails. Whether an outcome means "the program reached the device" is now one shared predicate (deployReachedDevice) instead ofoutcome === 'STARTED're-derived in three places.COMMAND:BUSYto everything but PING and STATUS, and the icon is drawn from a state about to change. Folded into the existingplcControlBlockedpair so the tooltip explains it the same way it explains a missing connection.Paired with
dlclosereload fix, the state interlock)Verification
Hardware (SLM-RP4): 150 rapid switch flips produced 19 transitions, 19 landings, 3 reconciliations, 0 wedges. Upload-with-switch-in-STOP and the transitioning block were both exercised on the device.
Test suites and
tscare at parity withdevelopment; the 4 remaining editor type errors are pre-existing ReactFlow typings in the graphical editor, untouched here.🤖 Generated with Claude Code