Repository navigation
Release v0.6.8 - #239
Merged
Merged
Release v0.6.8#239
Conversation
`handle_read` copies a value into a caller's buffer. That is the right shape for a Modbus frame, but wrong for a protocol stack that can serve memory it does not own: OPC-UA hands open62541 a `UA_Variant` and, marked `UA_VARIANT_DATA_NODELETE`, it will neither copy nor free it. Today the baremetal server allocates from a ~19 KB arena for every value of every read, and multiplies that by `maxNodesPerRead`. So `TypeOps` gains a sixth op and `handle_ptr(arr, elem, &len)` exposes it: the same value `handle_read` would produce, addressed where it already lives, with its length in bytes. STRING hands back the payload rather than the `[len][payload]` wire form, and WSTRING the code-unit buffer, so `len` is bytes there, not characters. The op reads through a new `IECVar::read_ptr()` rather than `raw_ptr()`. They agree the instant a force is applied, because `force()` writes through to `value_` — but a located variable is driven by the program straight into `value_` every scan, and only `read_ptr()` still resolves to the forced value afterwards. `raw_ptr()` would have leaked the program's value past an active force, silently and only for located variables. It mirrors `IECStringVar::c_str()`, which already resolves the force this way. `read_wstring` splits each code unit explicitly so the WIRE format is little-endian on any host. A pointer cannot do that — the caller gets host order — so `ptr_wstring` carries a `static_assert` on `__BYTE_ORDER__` instead of silently serving byte-swapped text on a big-endian target. Not exported through the `strucpp_debug_*` C ABI. The pointer is into live PLC storage, safe only for a caller inside the scan's own thread of control — true of the baremetal super-loop, false of the Runtime v4 `.so`, where the scan has its own thread. Linux callers keep `strucpp_debug_read()`. The test compiles real generated code and checks the properties that matter: the pointer agrees with `handle_read`, follows a later write with no second call, resolves a force the same way, keeps resolving it after the program writes the located storage directly, and returns null rather than crashing for an out-of-range leaf. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
Eight items from the review on #238. **The endianness assert took the header down with it (blocker).** It sat at namespace scope and unconditional, so it fired when `debug_dispatch.hpp` was INCLUDED, not when `ptr_wstring` was used: a big-endian build of firmware that only ever calls handle_read/handle_write stopped compiling, and MSVC (which defines no `__BYTE_ORDER__`) failed regardless of endianness. Reproduced both. It is now a compile-time selection — on a target that cannot be served, the op returns nullptr with *len = 0, which is already handle_ptr's contract for "no pointer available", and routes the caller to the copying path the old assertion text told them to use anyway. **A parameterised string was registered wrong rather than not at all.** Every string op casts the leaf to `IECStringVar<254>`, but codegen emits the DECLARED size, and `length_` sits at a different offset for every `n`. Measured on `STRING(20) := 'hello'`: sizeof 50 vs 518, length read from 468 bytes past the end. It read 0 there — but with non-zero neighbouring memory it reads as the cap, and the read path then serves that many bytes of adjacent memory. `Entry` has nowhere to carry a per-leaf width, so the generator now SKIPS such a leaf with a reason instead of registering it. Nothing authored in the editor can reach this (its type picker is a closed list from the IEC registry, which has no length parameter) — it is hand-written ST through the CLI. **Skipped leaves were silent.** `skipped` was collected and dropped, so a variable was simply absent from the debugger and from OPC-UA with nothing in the build to say why. It is now a warning per leaf, like `incomplete` already was. **The pointer ops no longer truncate at DEBUG_STRING_CAP.** That cap is the Modbus debug FRAME budget — one length byte and a fixed window. A pointer has neither, and OPC-UA carries its own length, so clamping there silently shortened a 200-character string to 126 on a transport that never asked for it. Verified: 200 chars now arrive whole, and an empty string still returns a non-null pointer with length 0. **ptr_wstring had no test.** Added the byte-vs-code-unit arithmetic checked against what read_wstring puts on the wire, empty STRING and WSTRING (a valid value, not a miss), and a forced STRING through handle_ptr. Confirmed they fail when the `* 2` is removed. Also: the lifetime contract now says a pointer dies when the force state changes (read_ptr returns a different object after unforce), `docs/RUNTIME.md` documents `read_ptr()` beside `raw_ptr()`, the type_ops comment no longer claims to be flash-resident on AVR and carries the measured cost of the new column, and the table's column alignment is restored. 2399 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
S8 from the review, now measured on target rather than argued.
The reviewer asked whether the new `ptr` column moved the smallest supported
target's footprint, and could not measure it — nor could I at first, because the
Arduino AVR toolchain ships no C++ standard headers. It does build: the editor
carries the shim at `resources/sources/avr-libstdcpp/include`, and its simulator
compiles for `arduino:avr:mega` (an ATmega2560) through arduino-cli. Same sketch,
same runtime tree, only calling handle_read/handle_write:
with the ptr column 425 B SRAM 5128 B flash
without it (baseline) 383 B SRAM 4654 B flash
So yes: +42 bytes of SRAM and ~474 of flash, on every AVR firmware, for an op
only the baremetal OPC-UA server calls. `type_ops` carries no
STRUCPP_DEBUG_FLASH, so on a Harvard target it is .rodata copied into SRAM at
startup — "inline constexpr" does not make it flash-resident.
The fix is not to move `type_ops` to PROGMEM (that would route every row read
through pgm_read_ptr in the hot path of handle_read/handle_write/handle_set,
which is a change that deserves its own evidence). It is to stop the pointer op
being a column at all: `ptr_ops[]` is a table of its own, read only by
handle_ptr. Nothing references it unless handle_ptr is called, so
-ffunction-sections/-fdata-sections + --gc-sections drop both the table and the
ptr_impl<T> instantiations.
Measured after the split, same sketch:
never calls handle_ptr 383 B SRAM 4728 B flash
calls handle_ptr 425 B SRAM 5196 B flash
The feature now costs what it costs, and only to firmware that uses it. The
consumers are unaffected — the baremetal glue calls handle_ptr and never touches
the table.
2399 tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
…nter-accessor feat(debug): address a leaf's value in place instead of copying it out
Keep the committed version in step with the release tag. The release workflow injects the version from the tag at build time, so this is for consistency rather than function. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p
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
2 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.
Release v0.6.8 — patch bump over v0.6.7.
Merges
developmentintomainto cut the release. New since v0.6.7:feat(debug): address a leaf's value in place instead of copying it out — the live-pointer accessor (type_ops[].ptr) that OpenPLC's baremetal OPC-UA zero-copy read depends on (feat(debug): address a leaf's value in place instead of copying it out #238).fix(debug): review fixes on the live pointer accessor.perf(debug): split the pointer op into its own table so AVR firmware isn't charged for it when unused.Tagging
v0.6.8on the merge commit kicks the build-and-release workflow (publishesstrucpp-0.6.8.tgz+ platform binaries). The OpenPLC editor/web strucpp pin is bumped tov0.6.8in a follow-up on their RTOP-285 branch.🤖 Generated with Claude Code
https://claude.ai/code/session_01NegApcNt2SzRk3FDAs5T9p