Release 4.2.11 - #1006
Merged
Merged
Conversation
The return type selector in the variables editor was gated on editor.type === 'plc-textual', so functions written in FBD, LD, or SFC had no way to select their return type even though the store action, POU creation defaults, and the ST transpiler already fully support return types for graphical functions. Extend the condition to include 'plc-graphical'. Add a component test covering the selector's visibility for FBD/LD/ST functions and its absence for programs and function blocks. Fixes #696 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5TQ1y3yBEDx73tYTHoUBj
Address review feedback: the return type SelectTrigger reused the
class filter's id ("class-filter"), producing duplicate ids when both
render, and the label's htmlFor pointed at nothing. Use a dedicated
"return-type" id wired to the label, assert the association in a test,
and add an SFC function case alongside FBD/LD/ST.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5TQ1y3yBEDx73tYTHoUBj
…ding Trends interpolated between samples with curve: 'smooth', so an instantaneous reset rendered as a descending ramp implying a gradual decrease that never happens. Debug values are sampled once per scan, so hold each value until the next sample and jump vertically -- for every type, not just BOOL. Two problems surfaced while validating that: - The poller only commits to the store when a value differs, so the sampling effect (keyed on the value Maps) never re-ran while a variable held steady. No samples meant no renderTrigger, and renderSeries' `now` froze with it, so the x-window stopped sliding too. Sample on a fixed 100ms clock instead, reading the latest maps through a ref. - ApexCharts' ctx.update() clears and rebuilds the entire SVG on every update, series or options alike. Tweening that rebuild replays partial states through rAF, which reads as flicker. Redraw atomically and repaint on a 200ms interval decoupled from the sample rate. Fixes #590 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QwVC4DQxNTKewgeDvmwWY3
Drop comments that restated the line below them. What survives marks the three traps: why chart animations stay off, why repaint is slower than sampling, and why sampling runs on a clock instead of on value change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QwVC4DQxNTKewgeDvmwWY3
…epline fix(debugger): trend sample-and-hold rendering and sliding window (DOPE-330)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SST13A31igS6DxRUjFfx9k
…eturn-type-graphical fix(pou): show return type selector for graphical function POUs
…-interval fix(debug): expose HTTP fallback poll interval as a platform capability
Forcing a TIME variable did nothing at all — no value written, no
error shown. `getVariableTypeInfo` (variable-types.ts) had no `time`
case, so it returned null and all three force UIs bailed out on that
null before building a buffer, silently closing the modal. The same
gate blocked DT / DATE / TOD / LTIME / WSTRING and every enum-typed
variable.
TIME encoding:
New `parseDurationLiteral` (utils/iec-duration.ts) — the inverse of
`formatTimeValue`. Accepts T# / TIME# / LT# / LTIME# or no prefix at
all (the watch panel renders durations prefix-less, so what it shows
can be typed straight back in), a sign on either side of the prefix,
`_` digit separators, unit chains (`1h30m`), overflow units (`90s`),
and a fraction on the smallest unit (`1.5s`, round half away from
zero). Result is range-checked against int64.
`encodeByWireFormat` encodes `duration-ns-i64` as int64 LE
nanoseconds — the shape strucpp already reads back. No runtime or
firmware change: 8-byte forces (LINT/ULINT/LREAL) already work, and
the endianness swap layer is type-generic.
Single force encoder:
The watch panel and the ladder / FBD variable nodes each carried
their own copy of a getVariableTypeInfo + parse/buffer dispatch.
All three now call `encodeForceValue`, which was already the
canonical encoder (driven by strucpp's iec-types registry) but had
no live caller. Errors surface as a toast instead of a silent modal
close. STRING encoding moved into the encoder so the unified path
keeps working; the panel also forwards `enumValues`, so forcing an
enum by member name works there for the first time.
DATE / TOD / DT / WSTRING still lack an encoder (they need calendar
literal parsing / UTF-16 framing) but now say so out loud.
Removed utils/variable-types.ts: every export was dead once the three
components shared one encoder.
Behaviour notes:
- STRING input is trimmed, so leading/trailing spaces need IEC
quoting (`' hi '`); quotes are unwrapped.
- Forcing BOOL `0` / `FALSE` from the modal now renders forced-low
(blue) instead of green, matching the Force False menu action, and
the modal accepts the TRUE / FALSE keywords.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Btq4UkctubeNQvh2cYAUv
…e-variable fix(debugger): support forcing TIME values (DOPE-331, #634)
…r into fix/issue-745
fix: OPC UA ARRAY of user-defined datatype does not work (issue #745)
…fails (DOPE-495) The graphical flow write-back validates with zod before persisting into `pou.body.value`. On failure it returned silently, so the save flow went on to serialize the stale pre-edit body, mark every file saved, clear every `updated` flag and show "Changes saved!". The user's edit survived only in memory and died with the app. `runWriteBack` now reports failure and logs the zod issues; `flushFlowWriteBacks` returns the POUs whose body is still stale. The save paths handle those per-POU: the flow keeps `updated`, the file stays dirty, its undo baseline is not reset, and a failure toast names it — while every other POU still saves. Single-file save aborts before writing rather than overwriting disk with the stale body. Flush also had a hole: it swept only POUs with a live debounce timer, so a timer that had already fired and failed left nothing pending and the save reported success anyway. It now writes back every `updated` flow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Btq4UkctubeNQvh2cYAUv
…5-flow-writeback-silent-skip
fix: Modbus/TCP: incorrect behavior when the connection is lost (issue #691)
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>
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
…un/stop Brings the connection work that the VPP licensing branch was carrying as an implicit dependency onto its own footing, with no licensing in it. 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 and serial is always a candidate, since the always-on debugger keeps the serial protocol compiled into every baremetal firmware. 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, the status poll -- now borrows that one client; none of them can open a connection. That is what fixes run/stop over Modbus TCP: the command path used to recognise only an RTU client as reusable, so a tcp stop opened a transient SECOND socket, which an Arduino Modbus TCP server (one client at a time) never answers. Run/stop (FC 0x4b PLC_SET_STATE) rides the same link. Reads come from the status frame (FC 0x46), which already carries the run/stop state and the mode-switch position, so a switch flipped by hand shows up within one poll interval with no second timer and no extra traffic. A RUN request is refused -- not queued -- while the switch reads STOP, and `refusedBySwitch` says so. Session structure: two channel slots (control + debug), lazily opened, so a debug session shares the link rather than replacing it. Runtime v3/v4 come into the same session; the runtime-v4 WebSocket keeps its own client, being a different protocol to a different target. Also here: serial-port descriptors carried as data rather than a display string, a connection-lost warning that names the endpoint and the right advice per transport, and an upload that releases the link only when it is the serial one holding that port -- so a Modbus TCP link, and the debug session on it, survives an upload. Two fixes that are prerequisites rather than features: validate:arch now resolves SRC_ROOT via fileURLToPath (URL.pathname produced C:\C:\... on win32 and the check could not scan the tree at all), and it learned to read multi-line imports, which it never could -- so it had been reporting success while missing real violations of its own rules. No licensing: no license blob, no on-device license storage, no activation, no FC 0x48-0x4C licensing set, no isLicensable/licenseStore capabilities. The device screen confirms a held link and nothing more. `classifyDeviceLink` (backend/editor/hardware/device-probe.ts) keeps only the board-id read that says whether an OpenPLC firmware answered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Formatting only, no behaviour change. Also drops three imports left unused by the licensing removal (Popover/Copy in board.tsx, DebugBoardIdResult in the WebSocket transport) and normalises two files to LF. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n a miss
Two defects, one symptom: "No Firmware Detected" on a board that is running
perfectly well.
FIRST, a real misalignment. `DEBUG_BAUD` was read from the `serial` screen
section alone, falling back to 115200. A VPP published today has no `serial`
section -- it configures the baud on the RTU section -- so every project built
against a published package compiled a firmware listening at 115200 while the
editor dialled the RTU's rate. The port opened, nothing decoded, and the editor
concluded there was no firmware.
`resolveDebugBaud` states the rule the firmware actually follows: an explicit
`serial` section wins; otherwise the RTU's baud, but ONLY when the RTU is on the
default port, because there the firmware brings that one port up at
MBSERIAL_BAUD and the debugger shares it (MBSERIAL_SHARES_DEBUG_SERIAL);
otherwise the firmware default. With the RTU on a SECOND UART the debugger keeps
the default port to itself and nothing in the project states its speed -- which
is what the second half of this commit is for.
SECOND, defence in depth. A wrong baud is the one misconfiguration that presents
as healthy silence, and the advice it produced -- reflash the device -- is
exactly the wrong move on a device in the field. Connect now tries the
configured rate first and then sweeps FALLBACK_BAUD_RATES, so a board whose rate
nobody remembers is reachable instead of looking dead.
The sweep is deliberately not free-form:
- Guesses are appended AFTER every endpoint the project declared. A configured
Modbus TCP address is a better next try than a rate nobody asked for.
- Guesses carry `speculative`, and verification gives them
SPECULATIVE_BOARD_ID_PROBE (2 attempts) instead of the patient budget. Two
rather than one because opening the port asserts DTR and resets AVR/ESP8266
boards, so the first read can land mid-boot and reject a correct rate.
- The patient budget stays with the last DECLARED endpoint (`patient`), not
the last candidate overall. Without that the sweep would quietly take ~32s
of patience away from the configured endpoint -- the one case that needs it,
a board still booting right after a flash.
- Each guess names its rate in the trace, so the console says which rate is
being tried rather than listing the same port five times.
- The debug channel of an already-established session opts out
(`probeBaudRates: false`): there the rate is settled, and reopening the port
at other rates would be wrong, not merely wasteful.
Cost, stated plainly: a full sweep is four extra port opens, each resetting the
board. It runs only after everything configured has already failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the screen's rate when Modbus is off
Two defects the first field test exposed, both mine.
THE SWEEP NEVER RAN. `descriptor` is an IDENTIFIER: it is matched against the OS
serial-port list and against the port an upload asks to borrow. Writing
"COM5 @ 115200 baud" into it meant every swept candidate matched no port and was
dismissed as absent in 1ms:
COM5 @ 115200 baud: serial port is not enumerated, skipping
open: rtu COM5 @ 115200 baud rejected in 2ms — not available
The rate now travels beside the endpoint as `baudRate`, and
`describeLinkCandidate` composes the caption where a caption is wanted (trace,
attempt list). The same slip would have broken the upload handoff, which
compares the held descriptor against the port arduino-cli wants.
The trace also named the rate on guesses only, so the log showed four rates being
tried and never said which one the CONFIGURED attempt used — the reader could not
tell that the 32s patient probe had already spent itself on 9600. Every serial
candidate now reports its rate.
DEBUG_BAUD IGNORED A DISABLED RTU. The editor dials
`screens.modbus_rtu.rtu_baud_rate` whether or not the RTU is enabled: a debug
spec's `params` are read independently of its `enabledWhen`. `resolveDebugBaud`
required `enabled === true`, so a project with Modbus off and 9600 on the screen
compiled a firmware at 115200 while the editor dialled 9600 — the exact shape of
the reported failure. The rate is now taken whenever it exists, except when an
ENABLED RTU owns a second UART: only there does it genuinely describe a
different port.
Regression tests, both of which the previous commit passed:
- two candidates on one port, differing only in baud, are both opened rather
than skipped as missing ports;
- a failed open reports each attempt's rate;
- DEBUG_BAUD follows the screen rate with the RTU disabled, and only falls back
to 115200 for an enabled RTU on a second UART.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The parity test hardcoded the editor's `resources/sources/Baremetal` path, so on openplc-web it died at import with ENOENT and took the whole suite file with it. The sync gate MAPS that directory onto `src/assets/firmware/Baremetal` rather than mirroring the path (MAPPED_SURFACES in compare-surfaces.py), and the test never knew. Unnoticed until now because neither repo runs a test job in CI. Resolves whichever layout the checkout has, and throws when neither exists rather than skipping: a parity test that vanishes when it cannot find its header is worse than no test at all, because the suite goes green while the struct layout it is supposed to pin drifts unwatched. Verified by renaming the header -- it fails loudly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Someone merged development into this branch in parallel, resolving the same three
connect-row conflicts. board.tsx came out identical on both sides. The other two
differed, and git's three-way merge kept this side, which is the one to keep:
- the button test: `getByRole('button').textContent === 'Disconnect'` is an exact
match, where the accessible-name query would let a stray label through;
- index.tsx: the comment explaining that `text-cp-sm` is correct now that cn()
knows the cp-* scale. Without it the next person reads the class as the bug
that dfef0a3 already tried to "fix" by resizing the button, and undoes
a45b99c.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Go-to-definition on a user type now lands in that type's own code view
with the cursor on the declaration — a struct field target lands on its
field line — instead of merely opening the form tab (DOPE-537).
The buffer gets a real identity (`inmemory://dtview/<name>.dt`) so the
LSP can recognise it, and `resolveStLspContext` remaps it onto the
aggregate datatypes document. Both frames open with a `TYPE` line, so a
single shift derived from the type's span covers completion, hover,
signature help, definition, references and formatting at once. The span
map is computed from the serializer rather than tracked in the offset
registry: there is no registration lifecycle to keep in sync and no
window where a datatype edit and the stored offset disagree.
Semantic tokens and diagnostics need more than a shift, because the
model's text and the document the answers come from are two different
strings that only agree while the buffer is committed:
- The token window holds the entry's own lines and is rebased onto the
view's frame via `outputStartLine`. Widening the window instead
would drag in the previous entry's last line, whose columns overrun
the 4-character `TYPE` line and make Monaco reject the whole batch.
- While the buffer diverges from the store the window is empty. No
colours beats colours describing the previous text.
- A store change re-drives both, since the model's text is untouched
by it and Monaco would otherwise never re-query.
- The diagnostics mirror caches each publish together with the spans
it was computed against, and replays it when a model mounts later.
Replaying through freshly computed spans puts markers on the wrong
line, or drops them, once the store has moved.
The go-to-definition cursor is deliberately keyed on its own identity
and not on the current display: including the display would re-fire the
forced switch when the user toggles back to the table and pin the tab in
code mode.
Refs DOPE-537
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY
…37-datatype-lsp-goto-def
…parsers `parsePouUri`, `parsePouVarsUri` and `parseDtViewUri` decoded their name segment with a bare `decodeURIComponent`, which raises `URIError` on input like `%ZZ`. All three run inside `resolveStLspContext`, on every model URI the providers see, so one malformed URI would take hover, completion and definition down for that model rather than simply not matching. Not reachable today — these URIs are only minted by the matching builders, which encode the name — but the guard belongs in all three rather than in whichever one was touched last. Refs DOPE-537 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY
A `.dt` view whose name has no entry in the aggregate document fell back to `span?.start ?? 1`, which is the first entry's mapping — so hover, completion and go-to-definition answered for an unrelated type. An unparseable `.dt` file reaches this path with a live, visible buffer. Pass the view's own un-indexed URI through instead: strucpp guards every handler on the document being known, so each provider answers nothing. Move the frame arithmetic into `dtview-context.ts` so it can be tested without a worker, and drop the duplicated shift in the diagnostics fan-out. Gate the store subscription on `isDataTypeFilesEnabled()` and on a `.dt` model actually being mounted. `refreshSemanticTokens()` re-tokenises every model in the ST language, so with the flag off a datatype table edit was triggering a worker round trip per open ST editor. Convert the goto-definition cursor at the call site, next to the POU conversions, rather than half inside the routing helper. Reported by review on #657 / #998. The frame-line aliasing (DOPE-554) and the formatting end-clip (DOPE-555) are tracked separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY
…-lsp-goto-def feat(data-types): wire the .dt code view into the ST language server (DOPE-537)
…nsing feature(RTOP-193): VPP licensing over the baremetal connection
Signature verification ran at import and at project open, and neither says anything about the package as it exists when a build starts: userData/packages/<id>/ is plain user-writable disk and the compiler reads it fresh every compile. The window was "project open -> click Compile", entirely user-controlled. What that window is worth: hal.source is C++ linked into the firmware, hal.pluginEntry is C the runtime compiles ON a live PLC, hal.licenseStore is the on-device licence backend, and because capabilities.isLicensable is a manifest field, editing the installed manifest switches the whole licensing flow off - no licence FCs on connect, no activation call, weak license_* defaults linked in. Add PackageManagerModule.verifyBoardPackageIntegrity(boardName): resolves the VPP behind the board, re-runs verifyPackageSignature, reports the package id and reason on failure. No-op for built-in hals.json boards and when REQUIRE_SIGNATURE is false. Called from compileProgram (before any package file is read), compileForDebugger, and again from handleVendorPluginPackaging - that step runs minutes later in wall-clock terms and is what copies vendor code into the PLC bundle, so it re-checks rather than trusting compile entry. There the gate sits outside the catch-all and throws, because packageVppPlugin turns a throw into the errors[] the pipeline bails on; a logged error would upload a bundle with no vendor I/O. Refuses the compile rather than de-listing the package: tearing a directory out from under a build in flight is a worse failure than stopping and saying why. The project-open sweep keeps ownership of removal. This shortens the window to sub-second, it does not close it - the gate hashes the directory and the pipeline reads it again. Verifying the bytes that actually enter the build is DOPE-558, which touches the shared verify-package-signature.ts and so needs a mirror PR on openplc-web. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Build output is captured from a pipe and pushed to the console verbatim, so
the two terminal control sequences arduino-cli uses were both mishandled.
**Carriage returns.** A download progress bar redraws by rewriting one line
with `\r`. Each redraw arrived as its own chunk and became its own timestamped
entry, so one core install produced hundreds of near-identical lines that
pushed the real output out of view:
[09:12:34]: ...54.94 MiB / 93.67 MiB [=====>-----] 58.65%
[09:12:35]: ...54.94 MiB / 93.67 MiB [=====>-----] 58.65%
[09:12:35]: ...57.68 MiB / 93.67 MiB [======>----] 61.58%
A chunk is now collapsed to the frame a terminal would leave on screen, and
the entry is marked `transient` while the line is still open. The next redraw
overwrites it; a trailing newline commits it and the following download starts
a fresh line. One live-updating line, as in a terminal.
**SGR colour.** arduino-cli colours its compile summary table (bright green
headers, yellow platform id, grey paths). The editor suppressed this with
`output.no_color` in `arduino-cli.yaml` — inherited from the 2022 Python
editor, which added `--no-color` because the raw `ESC[92m` bytes were printed
literally. The console parses SGR now, so the suppression is gone.
Colour is split off once, at the console slice: `message` always holds clean
text and `segments` carries the styling only when there was any. Search,
level filters and copy-to-clipboard keep working on `message` untouched — no
consumer besides the renderer learns that colour exists, and uncoloured logs
(the overwhelming majority) allocate nothing extra.
Dead code removed rather than left behind:
- `output.no_color` is dropped from `ARDUINO_DATA`, and existing configs are
migrated. The config was written once with `{ flag: 'wx' }` and skipped on
EEXIST forever after, so every install that ever ran an older build would
have kept colour off and made the new renderer unreachable. Reconciliation
is narrow and non-destructive: add missing board-manager URLs, drop
`no_color`, prune the `output` map only if it is left empty, and never touch
anything else. Uses the `yaml` Document API so user comments, ordering and
custom indexes survive; an unparseable config is left alone.
- `ArduinoCliConfigSchema` / `ArduinoCliConfig` deleted — a zod schema that
described the config's `no_color` shape and was imported by nothing.
Not a terminal emulator: cursor addressing, scroll regions and erase-in-line
are stripped rather than interpreted, because build output never uses them.
Tests: 29 new across the parser, the CR state machine, the slice's overwrite
rule and the config migration (including the upgrade-with-no_color path).
Verified against real captured arduino-cli bytes: 24 CR frames collapse, the
summary table maps to green/plain/green/plain/grey, and neither an escape nor
a carriage return survives into stored text. Full suite 6354 passing.
Paired with openplc-web (shared-core parity).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collapsing the carriage-return redraws put each download on a single log
entry, but a wide frame then wrapped into three visual lines, which loses
most of the benefit:
[11:50:32]: esp32:esp-rv32@2601 339.62 MiB / 562.71 MiB [==========
================================>------------------------
------] 60.35% 00m20s
Two causes, both addressed:
- **The bar is sized to a width that is not ours.** arduino-cli draws the
`[===>---]` bar against a width it guesses from its own environment —
roughly 150 glyphs — and there is no flag or env var to tell it otherwise
(COLUMNS, TERM and a PTY all make no difference; when it cannot guess it
simply omits the bar). The console panel is resizable and the side bar
moves, so no fixed width would hold either. `stripProgressBar` drops the
bar and keeps the numbers, which is exactly the compact form arduino-cli
itself emits when it cannot size one:
esp32:esp-x32@2601 44.48 MiB / 311.65 MiB 14.27% 00m26s
Nothing is lost — the bar is a redundant rendering of the percentage
printed beside it. Applied only to carriage-return redraws, so ordinary
bracketed output (`[MANUAL_OVERRIDE / body line 7]`, `array[0]`) is
untouched.
- **Progress lines wrapped like prose.** They now render `whitespace-pre`
with `overflow-x-auto`, so a frame that is still too wide scrolls within
its own line instead of breaking across several, and the scroll stays on
that one line rather than shifting the whole console. Ordinary output
keeps wrapping, which is what long compiler diagnostics want.
Tests: 8 new, built from the exact frame that wrapped (200 chars -> under
80) and covering the bracketed text that must not be touched.
Paired with openplc-web (shared-core parity).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This reverts commit 2dfbb55.
…grity-gate-at-build fix(package-manager): gate the build on VPP package integrity (DOPE-539)
Addresses the review on #1001. - **Format Check.** `prettier --write` on `user-service/index.ts`; the CI log named exactly that file and it was the one statement flagged. - **`boardManagerUrl` is now validated before it can reach the subprocess.** It rides through `PackageManifestSchema` on `.passthrough()` today, so it is typed in `types.ts` but entirely unchecked at runtime — and it becomes an `--additional-urls` argument to arduino-cli, which downloads a board package full of toolchain executables that later builds run. Signing a VPP vouches for the manifest, not for what the URL serves: arduino-cli's package checksums live INSIDE the index it fetches, so a plaintext index can be intercepted and the package that lands replaced. Requiring https closes that. Only the scheme is constrained — arduino-cli reads compressed indexes (.json.gz, .zip, .bz2) too, so pinning the path suffix would refuse valid vendors for no security gain. This follows the decision already recorded in this file for the version floors: a field a gate reads should not reach it as `unknown`. Strict where the artefact enters, tolerant where we only read what is already on disk — the same split the floors use, and for the same reason. A package installed before this constraint existed has its URL dropped with a warning rather than its whole manifest rejected, which would make every board it provides vanish from the board lookup on an upgrade the user never asked for. Dropping the field leaves it exactly as capable as it was before vendor indexes existed. openplc-packages carries the matching `"pattern": "^https://"`, so a package refused here cannot be built there either. - **Dropped an unnecessary `as unknown as PackageManifest`** in `resolve-board-selection.test.ts`. Verified genuinely redundant: the file typechecks without it. The remaining casts CodeRabbit flagged in `handle-core-installation.test.ts` are pre-existing jest scaffolding and that file's established fixture pattern; diverging from it there would make the file less consistent, not more. Tests: 13 new covering the accepted/refused URL shapes, the compressed-index case, and the drop-not-reject behaviour on the installed-read path. All 12 packages in openplc-packages still validate — every boardManagerUrl in the repo is already https. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Array.isArray(value.devices)` already makes `.map` available, and annotating the callback parameter as `unknown` is what preserves the runtime element checking — the `as unknown[]` cast added nothing. tsc and eslint are both clean without it. Raised by CodeRabbit on #1001. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(compile): install vendor cores from the VPP's board manager URL
Resolves the expected conflict in `#checkIfArduinoCliConfigExists`, where #1001 (now on development) and this branch both rewrote the same method. Took this branch's version: `reconcileArduinoCliConfig` is a superset of the regex #1001 introduced — it backfills missing board-manager URLs *and* retires the obsolete `output.no_color`, using the `yaml` Document API rather than anchored regexes. That also settles the review finding on #1001 that the regex could not see `additional_urls: []`, the shape `arduino-cli config init` writes, along with the `existing.includes(url)` whole-file substring match. The doc comment is merged rather than replaced, so the board-manager-URL rationale from #1001 survives alongside the no_color one. Everything else auto-merged. #1001's boardManagerUrl schema validation and pipeline plumbing are intact; so is this branch's console work. Editor suite: 6386 passing, typecheck / lint / prettier clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses seven of the eight CodeRabbit threads on #1002. Each was reproduced before being fixed. **Windows CRLF was classified as a redraw — losing log lines.** Splitting on `\n` leaves the `\r` of a CRLF terminator on every Windows line, which `logCompilerEvent` counted as a carriage-return redraw. Reproduced two ways: progress line, then a CRLF line in one chunk -> the CRLF line OVERWRITES the live progress line "Windows line\r" and "\n" in separate chunks, then a redraw -> "Windows line" is destroyed by the redraw arduino-cli always writes a progress CR at the START of a frame, so position separates the two cases: a trailing `\r` is a line terminator, anything else is a redraw. That fixes both without carrying state between chunks, which is what the review had assumed would be needed. **Escape sequences leaked past a documented contract.** The module promises stored messages carry no escapes, but the tokenizer matched only CSI, so OSC payloads (terminal hyperlinks carry a URL) and bare ESC bytes reached search, copy and the DOM. Verified: `stripAnsi` on an OSC hyperlink returned the ESC bytes intact. The pattern now covers CSI, OSC (BEL- or ST-terminated), other two-byte escapes, and a stray ESC. **A scalar parent aborted the whole config migration.** `board_manager: 5` made yaml's nested `getIn`/`setIn` throw ("Expected YAML collection at board_manager"). The throw is caught upstream, so the app survived — but the user silently kept `no_color` and a monochrome console with no visible cause. Parents are now fetched and checked before use; a scalar `board_manager` is left alone while the `no_color` retirement still proceeds. `NO_COLOR_PATH` became unused and is deleted rather than left behind. **The config is written atomically.** It is rewritten on every start, is owned by the user, and must stay parseable for arduino-cli; a crash mid-write left it truncated. Now written to a sibling temp file and renamed over the original. **Type assertions removed** from `arduino-cli-config.ts` and both test files (`requireUpdated()` instead of `as string`, a type guard in `urlsOf`, and the log callback typed at its declaration). Not doing #5 (search matches spanning a colour boundary) — reasoning in the thread. 12 new tests, all built from the reproductions above. Editor suite: 6398 passing; typecheck, lint and prettier clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tput feat(console): render arduino-cli colour and collapse progress redraws
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
thiagoralves
approved these changes
Aug 11, 2026
`main` carried 27 commits that never came back down to `development`: the merge commits GitHub created for every past release PR, plus two CI hotfixes committed directly on `main`. That left `development` BEHIND `main`, and `main`'s protection has "require branches to be up to date" with admin enforcement — so release PR #1006 could not merge. The only file with real content to bring back is `.github/workflows/release.yml`: the `windows-2022` pin for both Windows release jobs (35d8b90, 6c86e0b) was a direct hotfix on `main` and never reached `development`, which still had both jobs on `windows-latest`. Merging #1006 without this back-merge would have reverted the pin and broken the v4.2.11 Windows ARM64 build on VS 2026's node-gyp maxBuffer bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Carried into the release PR itself so #1006 promotes the correct version to `main`, instead of needing a second PR after the fact. `APP_VERSION` is the single source of truth the About modal renders, and is byte-compared against openplc-web by the mirror gate — the identical edit lands there in Autonomy-Logic/openplc-web#667. `package.json.version` is what electron-builder stamps on the desktop binary and what the `v4.2.11` tag must match, so it is kept equal. Shipping only the package.json half is what left About showing 4.2.6 on the 4.2.7 and 4.2.8 releases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Why this is blocked, and the order to unblock itBlocker:
#1007 fixes that (back-merge) and also carries the 4.2.11 version bump, so this PR promotes the correct version instead of needing a follow-up PR. Note the back-merge is not just bookkeeping — it restores the
|
…to-development chore: back-merge main + bump version to 4.2.11 (unblocks release PR #1006)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promote
development→mainfor the 4.2.11 release.Highlights since 4.2.10:
.dtfiles): form/code toggle, ST serializer/parser, persistence with legacy migration, undo/redo and dirty tracking, rename reference propagation with an impact modal, and LSP wiring into the code view (DOPE-530, 532-537). Ships behind a feature flag, still off (see DOPE-542).Version bump (
APP_VERSIONto 4.2.11) and thev4.2.11tag come as a follow-up once this is reviewed.🤖 Generated with Claude Code