From 83a3cc3a4562c6d8ae58f5b6918870888086c7a6 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Mon, 24 Aug 2026 18:29:05 -0400 Subject: [PATCH 1/4] =?UTF-8?q?feat(retain):=20runtime=20v4=20retain=20?= =?UTF-8?q?=E2=80=94=20.so=20marshalling,=20plugin=20store,=20cold=20reset?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 4 of NODE-94. The daemon half: retained values now round-trip through a plugin-supplied store, with the same contract baremetal already has. The WALK lives inside the .so. `runtime_v4_entry.cpp` exports `strucpp_retain_pack` / `_unpack` / `_blob_size` / `_layout_hash`, because the debug tables and `handle_read` are there and the runtime is built once but loads many .so files — it cannot reach `strucpp::retain` by mangled name, and re-implementing the blob format on this side would put two copies of one wire format in two repos. `unpack` takes a write CALLBACK rather than using `handle_write` itself, and that is the important detail. A retained variable may also be located (`VAR RETAIN x AT %MW10`); poking such a leaf's IECVar is undone by the next copy-in from the process image, so the value would appear to restore and then silently revert on the first scan. The runtime passes a thunk that routes through `runtime_external_write`, which classifies the leaf and sends a located one through the image journal — the path OPC-UA writes already take. Reads need no such care, so pack uses `handle_read` directly. `plc_retain.{h,cpp}` owns the buffer and the call sites. `plc_retain_save()` runs once per scan from the dispatcher's quiescent window, where `g_tasks_running == 0` and no worker is inside a body — the same guarantee `copy_config_globals_out` relies on, and the only place the leaves can be read without racing the task threads. `plc_retain_init()` / `_load()` run in the start path after located variables are bound, because a retained variable may also be located and its image slot has to exist before anything writes through it. There is deliberately NO default backend. Retention hardware is a property of the device, not of the runtime, and a file-backed default here would look like support on every device and be wrong on most of them. With no plugin the calls are no-ops and retain degrades to NON_RETAIN — what the runtime did before. The plugin surface mirrors baremetal's `openplc_retain.h` name for name. Both `retain_save` and `retain_load` are required to be a store: one that can save and not load is worse than none, because it accepts values every scan and silently never returns them, which looks like working retention right up to the reboot that matters. The first plugin exporting both wins and any others are named and ignored — two stores would both appear to work and disagree on the next boot. `RETAIN:CLEAR` on the unix socket, called by the webserver on upload: CODESYS clears retained memory on download, and while the layout hash already refuses a genuinely different program, two programs that happen to share a retain layout would otherwise inherit each other's values silently. VERIFIED ON THE SLM-RP4, with a throwaway file-backed plugin flushing every 5 seconds so the asynchronous half of the contract was actually exercised: - init reported `18 bytes ... (layout 0590d5d2)`, matching the layout hash the editor put in debug-map.json for the same program; - the blob on disk decoded to exactly the designed header — magic 0x4F52, format 1, layout 0590d5d2, payload 4, valid crc; - the file's mtime advanced every 5 s while the runtime called `retain_save` at 50 Hz — roughly 250 API calls per write, which is the decoupling the per-cycle cadence exists to allow; - across a STOP/START, which unloads and reloads the .so and re-runs every declared initialiser, `boots` came back at its stored value and kept counting while `counter`, which is not retained, correctly reset. Found while deploying, NOT caused by this work: openplc-runtime's development branch requires `vpp_signature.json` (webserver/vpp_package_signature.py, which does not exist on main) and openplc-editor never emits it, so any VPP-target upload from a current editor to a current-development runtime fails. Tested against the non-VPP runtime-v4 target instead. Worth its own fix. Refs NODE-94 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012FNp61926UEggtmsQPj3A3 --- core/src/CMakeLists.txt | 1 + core/src/drivers/plugin_driver.c | 68 +++++++++ core/src/drivers/plugin_driver.h | 47 +++++++ core/src/plc_app/image_tables.cpp | 14 ++ core/src/plc_app/image_tables.h | 22 +++ core/src/plc_app/plc_retain.cpp | 164 ++++++++++++++++++++++ core/src/plc_app/plc_retain.h | 105 ++++++++++++++ core/src/plc_app/plc_state_manager.cpp | 16 +++ core/src/plc_app/unix_socket.c | 14 ++ core/strucpp_runtime/runtime_v4_entry.cpp | 60 ++++++++ webserver/app.py | 10 ++ webserver/runtimemanager.py | 23 +++ 12 files changed, 544 insertions(+) create mode 100644 core/src/plc_app/plc_retain.cpp create mode 100644 core/src/plc_app/plc_retain.h diff --git a/core/src/CMakeLists.txt b/core/src/CMakeLists.txt index 8093dab0..c6a3b3eb 100644 --- a/core/src/CMakeLists.txt +++ b/core/src/CMakeLists.txt @@ -44,6 +44,7 @@ add_executable(plc_main ${CMAKE_SOURCE_DIR}/core/src/plc_app/journal_buffer.c ${CMAKE_SOURCE_DIR}/core/src/plc_app/debug_write_journal.cpp ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_io_cycle.cpp + ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_retain.cpp ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_state_manager.cpp ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_switch.c ${CMAKE_SOURCE_DIR}/core/src/plc_app/plcapp_manager.c diff --git a/core/src/drivers/plugin_driver.c b/core/src/drivers/plugin_driver.c index a1e7e15f..103116aa 100644 --- a/core/src/drivers/plugin_driver.c +++ b/core/src/drivers/plugin_driver.c @@ -1446,6 +1446,14 @@ int native_plugin_get_symbols(plugin_instance_t *plugin) // get_stats is fully optional — plugins that don't publish statistics // simply don't export it. No warning. + // Retain store (NODE-94), fully optional. A plugin exporting both becomes + // a candidate for this device's retain store; see + // plugin_driver_find_retain_store. No warning when absent — most plugins + // have nothing to do with retention. + native_bundle->retain_save = (plugin_retain_save_func_t)dlsym(handle, "retain_save"); + native_bundle->retain_load = (plugin_retain_load_func_t)dlsym(handle, "retain_load"); + native_bundle->retain_clear = (plugin_retain_clear_func_t)dlsym(handle, "retain_clear"); + // Store the native bundle and handle in the plugin instance plugin->native_plugin = native_bundle; @@ -1473,6 +1481,66 @@ void python_plugin_cycle(plugin_instance_t *plugin) // Call cycle_start for all active native plugins that have registered the hook // This should be called at the beginning of each PLC scan cycle, before PLC logic execution // Plugins opt-in by implementing cycle_start(); opt-out by not implementing it (NULL pointer) +// --------------------------------------------------------------------------- +// Retain store +// --------------------------------------------------------------------------- + +static bool plugin_provides_retain_store(const plugin_instance_t *p) +{ + // BOTH halves required. A store that can save and not load is worse than + // none: it would accept values every scan and silently never give them + // back, which looks like working retention right up until the reboot that + // matters. + return p && p->native_plugin && p->native_plugin->retain_save && p->native_plugin->retain_load; +} + +plugin_instance_t *plugin_driver_find_retain_store(plugin_driver_t *driver) +{ + if (!driver) return NULL; + + plugin_instance_t *chosen = NULL; + for (int i = 0; i < driver->plugin_count; i++) + { + plugin_instance_t *p = &driver->plugins[i]; + if (p->degraded || !plugin_provides_retain_store(p)) continue; + + if (!chosen) + { + chosen = p; + continue; + } + // Two stores would both appear to work and disagree on the next boot, + // which is a worse failure than refusing the second. First wins, and + // the rest are named so the misconfiguration is visible. + log_warn("Retain: plugin '%s' also provides retain storage; ignoring it — " + "'%s' was found first", + p->config.name, chosen->config.name); + } + return chosen; +} + +int plugin_driver_retain_save(plugin_instance_t *store, const uint8_t *blob, uint16_t len) +{ + if (!plugin_provides_retain_store(store)) return -1; + return store->native_plugin->retain_save(blob, len); +} + +int plugin_driver_retain_load(plugin_instance_t *store, uint8_t *out, uint16_t cap, uint16_t *out_len) +{ + if (out_len) *out_len = 0; + if (!plugin_provides_retain_store(store)) return -1; + return store->native_plugin->retain_load(out, cap, out_len); +} + +int plugin_driver_retain_clear(plugin_instance_t *store) +{ + // Optional third hook: a plugin without it cannot be cold-reset, and + // reporting that as failure would make the editor's post-upload clear look + // broken on every such device. + if (!store || !store->native_plugin || !store->native_plugin->retain_clear) return 0; + return store->native_plugin->retain_clear(); +} + void plugin_driver_cycle_start(plugin_driver_t *driver) { if (!driver || driver->plugin_count == 0) diff --git a/core/src/drivers/plugin_driver.h b/core/src/drivers/plugin_driver.h index b834ee3f..83023957 100644 --- a/core/src/drivers/plugin_driver.h +++ b/core/src/drivers/plugin_driver.h @@ -33,6 +33,35 @@ typedef int (*plugin_execute_command_func_t)(const char *command_json, char *res // Return 0 on success; any other value means "skip me this cycle." typedef int (*plugin_get_stats_func_t)(char *out, size_t out_size); +/* ---- Optional: retain-variable storage (NODE-94) ------------------------- + * + * A plugin that owns retention hardware exports these two and becomes the + * device's retain store. Same names, same status meaning and the same contract + * text as baremetal's `openplc_retain.h`, so a vendor writes one shape twice + * rather than learning two interfaces for one job. + * + * The runtime MARSHALS and the plugin STORES: what arrives is an opaque blob, + * already validated on the way back in (magic, format, layout hash, crc32), so + * a backend needs no understanding of retained variables at all. + * + * `retain_save` is called ONCE PER SCAN CYCLE, unconditionally, from the + * dispatcher's quiescent window. The runtime does not diff and does not + * rate-limit — holding the bytes and flushing on a schedule the medium can + * sustain is the plugin's job, and the reason the call exists at that cadence + * is so a plugin that CAN write every cycle (FRAM, battery-backed SRAM) is free + * to. It MUST return promptly and MUST NOT block: this runs inside the scan, + * so time spent here is time the PLC is not scanning. + * + * Both must be exported for the plugin to be used as the store; a plugin + * exporting only one is ignored, since a store that can save and not load is + * worse than none. Return 0 on success, non-zero otherwise. + */ +typedef int (*plugin_retain_save_func_t)(const uint8_t *blob, uint16_t len); +typedef int (*plugin_retain_load_func_t)(uint8_t *out, uint16_t cap, uint16_t *out_len); +/* Optional third: discard the stored blob. A plugin without it simply cannot + * be cold-reset, and the editor's post-upload clear is a no-op for it. */ +typedef int (*plugin_retain_clear_func_t)(void); + typedef struct { void *handle; // Handle to the loaded shared library @@ -44,6 +73,10 @@ typedef struct plugin_cleanup_func_t cleanup; plugin_execute_command_func_t execute_command; plugin_get_stats_func_t get_stats; + /* Optional retain-store hooks; NULL unless the plugin exports them. */ + plugin_retain_save_func_t retain_save; + plugin_retain_load_func_t retain_load; + plugin_retain_clear_func_t retain_clear; } plugin_funct_bundle_t; // Plugin instance structure @@ -119,6 +152,20 @@ void plugin_driver_release_gil(void); void plugin_driver_cycle_start(plugin_driver_t *driver); void plugin_driver_cycle_end(plugin_driver_t *driver); +/* ---- Retain store ------------------------------------------------------- */ + +/* The plugin acting as this device's retain store, or NULL. + * + * The FIRST plugin that exports both retain_save and retain_load wins, and any + * others are logged and ignored. Two plugins writing the same retained values + * to different places would both appear to work and disagree on the next boot, + * which is a far worse failure than refusing the second. */ +plugin_instance_t *plugin_driver_find_retain_store(plugin_driver_t *driver); + +int plugin_driver_retain_save(plugin_instance_t *store, const uint8_t *blob, uint16_t len); +int plugin_driver_retain_load(plugin_instance_t *store, uint8_t *out, uint16_t cap, uint16_t *out_len); +int plugin_driver_retain_clear(plugin_instance_t *store); + // Route a command to a specific plugin by name (for async commands like scan) int plugin_driver_execute_command(plugin_driver_t *driver, const char *plugin_name, const char *command_json, char *response, size_t response_size); diff --git a/core/src/plc_app/image_tables.cpp b/core/src/plc_app/image_tables.cpp index 22663175..8136f99e 100644 --- a/core/src/plc_app/image_tables.cpp +++ b/core/src/plc_app/image_tables.cpp @@ -85,6 +85,12 @@ uint16_t (*ext_strucpp_debug_size) (uint8_t, uint16_t) = nullp uint8_t (*ext_strucpp_debug_set) (uint8_t, uint16_t, bool, const uint8_t *, uint16_t) = nullptr; uint16_t (*ext_strucpp_debug_read) (uint8_t, uint16_t, uint8_t *) = nullptr; +size_t (*ext_strucpp_retain_blob_size) (void) = nullptr; +uint32_t (*ext_strucpp_retain_layout_hash)(void) = nullptr; +size_t (*ext_strucpp_retain_pack) (uint8_t *, size_t) = nullptr; +uint8_t (*ext_strucpp_retain_unpack) (const uint8_t *, size_t, + uint8_t (*)(uint8_t, uint16_t, + const uint8_t *, uint16_t)) = nullptr; uint8_t (*ext_strucpp_debug_write) (uint8_t, uint16_t, const uint8_t *, uint16_t) = nullptr; int (*ext_strucpp_debug_locate) (uint8_t, uint16_t, uint8_t *, @@ -251,6 +257,14 @@ extern "C" int symbols_init(PluginManager *pm) *(void **)&ext_strucpp_debug_set = resolve(pm, "strucpp_debug_set", true); *(void **)&ext_strucpp_debug_read = resolve(pm, "strucpp_debug_read", true); *(void **)&ext_strucpp_debug_write = resolve(pm, "strucpp_debug_write", true); + + /* Optional: a program built by an older STruC++ has no retain exports, and + * the retain path then simply never runs. `required = false` so that is a + * quiet degradation rather than a failed load. */ + *(void **)&ext_strucpp_retain_blob_size = resolve(pm, "strucpp_retain_blob_size", false); + *(void **)&ext_strucpp_retain_layout_hash = resolve(pm, "strucpp_retain_layout_hash", false); + *(void **)&ext_strucpp_retain_pack = resolve(pm, "strucpp_retain_pack", false); + *(void **)&ext_strucpp_retain_unpack = resolve(pm, "strucpp_retain_unpack", false); /* Optional: present only on .so's built with strucpp_capabilities bit 2. * When NULL the debug-write drain routes every leaf as a global write. */ *(void **)&ext_strucpp_debug_locate = resolve(pm, "strucpp_debug_locate", false); diff --git a/core/src/plc_app/image_tables.h b/core/src/plc_app/image_tables.h index 6d2aa1f2..34d73e52 100644 --- a/core/src/plc_app/image_tables.h +++ b/core/src/plc_app/image_tables.h @@ -83,6 +83,28 @@ extern "C" const uint8_t *bytes, uint16_t len); + /* ---- Retain marshalling (NODE-94) -------------------------------------- + * + * The WALK lives inside the .so, not here: that is where the debug tables + * and handle_read/handle_write are, and re-implementing the blob format on + * this side would put two copies of one wire format in two repos. + * + * `unpack` takes a write CALLBACK because the runtime owns the write path. + * A retained variable may also be LOCATED (`VAR RETAIN x AT %MW10`), and + * poking such a leaf's IECVar is undone by the next copy-in from the + * process image — so the callback we hand over routes through + * runtime_external_write, which knows to send a located leaf through the + * image journal. + * + * Optional: a program built by an older STruC++ resolves these to NULL and + * the retain path simply never runs. */ + extern size_t (*ext_strucpp_retain_blob_size) (void); + extern uint32_t (*ext_strucpp_retain_layout_hash)(void); + extern size_t (*ext_strucpp_retain_pack) (uint8_t *out, size_t cap); + extern uint8_t (*ext_strucpp_retain_unpack) (const uint8_t *blob, size_t len, + uint8_t (*write_leaf)(uint8_t, uint16_t, + const uint8_t *, uint16_t)); + /* Located-variable classifier. Reports whether a debug (arr, elem) leaf is * a LOCATED variable and, if so, its image location (area / size / * byte_index / bit_index). Returns 1 + fills the out-params if located, 0 diff --git a/core/src/plc_app/plc_retain.cpp b/core/src/plc_app/plc_retain.cpp new file mode 100644 index 00000000..1013ff4d --- /dev/null +++ b/core/src/plc_app/plc_retain.cpp @@ -0,0 +1,164 @@ +/** + * @file plc_retain.cpp + * @brief Retain-variable persistence — the runtime's half (NODE-94). + * + * See plc_retain.h for the split: the .so marshals, a plugin stores, and this + * file owns the buffer and the call sites. + */ + +#include "plc_retain.h" + +#include +#include +#include + +extern "C" { +#include "../drivers/plugin_driver.h" +#include "utils/log.h" +} + +#include "debug_write_journal.h" +#include "image_tables.h" + +extern plugin_driver_t *plugin_driver; + +namespace { + +/** + * Cap on the blob this runtime will handle. + * + * Generous compared with baremetal's 512 bytes — there is no SRAM pressure + * here — but bounded on purpose: the buffer is read from the scan path, and an + * unbounded allocation driven by a program's declaration count is not + * something to discover on a running machine. A program needing more is + * refused at init with a message naming both numbers. + */ +constexpr size_t RETAIN_BUFFER_MAX = 64 * 1024; + +std::vector g_buffer; +std::atomic g_active{false}; + +/** + * Restore writes go through the runtime's external-write path, NOT straight to + * the IECVar. + * + * A retained variable may also be located (`VAR RETAIN x AT %MW10`). Poking + * such a leaf's storage directly is undone by the next copy-in from the process + * image, so the value would appear to restore and then silently revert on the + * first scan. `runtime_external_write` classifies the leaf and routes a located + * one through the image journal — the same path OPC-UA writes take. + * + * DBGW_OP_WRITE, never a force: restoring a retained value must not pin it. The + * program has to be able to move it on the very next scan, and an operator's + * force has to stay authoritative over whatever was stored. + */ +uint8_t retain_write_leaf(uint8_t arr, uint16_t elem, const uint8_t *bytes, uint16_t len) +{ + return runtime_external_write(arr, elem, (uint8_t)DBGW_OP_WRITE, bytes, len) == 0 ? 0x7E : 0x82; +} + +/** The first plugin that exports both hooks wins; see plc_retain_init(). */ +plugin_instance_t *g_store = nullptr; + +} // namespace + +void plc_retain_init(void) +{ + g_active.store(false); + g_store = nullptr; + g_buffer.clear(); + + if (!ext_strucpp_retain_blob_size || !ext_strucpp_retain_pack || !ext_strucpp_retain_unpack) + { + /* A program built by an older STruC++. Not an error — retain simply + * does not run, exactly as before these exports existed. */ + return; + } + + const size_t needed = ext_strucpp_retain_blob_size(); + if (needed == 0) return; /* the program retains nothing */ + + if (needed > RETAIN_BUFFER_MAX) + { + log_error("Retain: program needs %zu bytes, this runtime handles at most %zu — " + "retained variables will NOT be preserved", + needed, RETAIN_BUFFER_MAX); + return; + } + + g_store = plugin_driver_find_retain_store(plugin_driver); + if (!g_store) + { + /* Info, not a warning: a device with no retention plugin is a normal + * configuration, and the program still runs correctly — its retained + * variables just behave as NON_RETAIN. Said once so the operator can + * tell "not supported here" from "supported and broken". */ + log_info("Retain: %zu bytes of retained variables, but no plugin provides storage — " + "they will start at their initial values", + needed); + return; + } + + g_buffer.assign(needed, 0); + g_active.store(true); + log_info("Retain: %zu bytes across the program's retained variables (layout %08x)", + needed, ext_strucpp_retain_layout_hash ? ext_strucpp_retain_layout_hash() : 0u); +} + +void plc_retain_load(void) +{ + if (!g_active.load() || !g_store) return; + + uint16_t got = 0; + const int rc = plugin_driver_retain_load(g_store, g_buffer.data(), + (uint16_t)g_buffer.size(), &got); + if (rc != 0 || got == 0) + { + log_info("Retain: nothing stored yet — retained variables start at their initial values"); + return; + } + + const uint8_t res = ext_strucpp_retain_unpack(g_buffer.data(), got, retain_write_leaf); + if (res == 0) + { + log_info("Retain: restored %u bytes of retained variables", (unsigned)got); + return; + } + + /* Deliberately explicit about WHICH check failed. "Retain refused" sends + * someone hunting; "the stored layout is from a different program" tells + * them it was the upload, and a crc failure tells them it was the store. */ + static const char *why[] = {"ok", "no data", "bad magic", + "bad format", "crc mismatch", "layout is from a different program", + "truncated"}; + log_warn("Retain: stored values refused (%s) — retained variables start at their initial values", + res < (sizeof(why) / sizeof(why[0])) ? why[res] : "unknown"); +} + +void plc_retain_save(void) +{ + if (!g_active.load() || !g_store) return; + + const size_t n = ext_strucpp_retain_pack(g_buffer.data(), g_buffer.size()); + if (n == 0) return; + + /* Hand the bytes over and return. Whether this is committed to storage now, + * in five seconds, or on shutdown is the plugin's decision — it is the only + * layer that knows what its medium costs. */ + plugin_driver_retain_save(g_store, g_buffer.data(), (uint16_t)n); +} + +void plc_retain_clear(void) +{ + /* Not gated on g_active: a clear has to work even when this program has + * nothing retained, because what it is discarding belongs to the PREVIOUS + * program. That is the whole point of clearing on upload. */ + plugin_instance_t *store = g_store ? g_store : plugin_driver_find_retain_store(plugin_driver); + if (!store) return; + plugin_driver_retain_clear(store); +} + +bool plc_retain_active(void) +{ + return g_active.load(); +} diff --git a/core/src/plc_app/plc_retain.h b/core/src/plc_app/plc_retain.h new file mode 100644 index 00000000..dc801f8d --- /dev/null +++ b/core/src/plc_app/plc_retain.h @@ -0,0 +1,105 @@ +/** + * @file plc_retain.h + * @brief Retain-variable persistence — the runtime's half (NODE-94). + * + * The runtime MARSHALS and the platform STORES, exactly as on baremetal. The + * marshalling itself lives inside the loaded .so (STruC++'s `iec_retain.hpp`, + * reached through the `strucpp_retain_*` exports), because that is where the + * debug tables live and because one copy of a wire format is better than two. + * What this file owns is the buffer, the call sites, and the handover to a + * plugin. + * + * WHY A PLUGIN AND NOT A FILE HERE + * -------------------------------- + * There is no default backend. Retention hardware is a property of the device, + * not of the runtime: an SLM-RP4 has a data partition, another box has FRAM or + * battery-backed SRAM, a third has nothing at all. A file-backed default in the + * runtime would look like support on every device and be wrong on most of them. + * With no plugin the calls are no-ops and retain degrades to NON_RETAIN, which + * is what the runtime did before this file existed. + * + * The plugin surface mirrors baremetal's `openplc_retain.h` name for name, so a + * vendor writing retain support reads one contract and implements the same + * shape twice. + * + * CADENCE IS NOT OURS TO DECIDE + * ---------------------------- + * `plc_retain_save()` is called once per scan cycle, unconditionally. It does + * not diff, does not rate-limit and does not judge whether a value is worth + * keeping — the plugin holds the bytes and flushes on whatever schedule its + * medium can sustain. A driver over flash that wrote through on every call + * would consume its endurance budget in hours. + */ + +#ifndef PLC_RETAIN_H +#define PLC_RETAIN_H + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Decide once, after the program is loaded, whether retain can run. + * + * Checks that the .so exports the retain entry points (a program built by an + * older STruC++ does not), that the program retains anything at all, and that + * the blob fits the runtime's buffer. Logs what it found, once — including the + * layout hash, which is the thing to compare when a restore is unexpectedly + * refused. + * + * Call after symbols are resolved and `g_config` is constructed, before the + * first task is released. + */ +void plc_retain_init(void); + +/** + * @brief Restore retained values from the plugin's store. + * + * Safe and cheap when nothing is retained or no plugin provides storage. The + * blob is validated inside the .so (magic, format, layout hash, crc32) and a + * blob that fails leaves every variable at its declared initial value — a + * machine starting from its defaults is recoverable, one starting from + * plausible-looking garbage is not. + * + * Call once per program start, after plc_retain_init(). + */ +void plc_retain_load(void); + +/** + * @brief Hand the current retained values to the plugin. + * + * Called ONCE PER SCAN CYCLE from the dispatcher's quiescent window, where + * `g_tasks_running == 0` and no worker is inside a body — the same guarantee + * `image_tables_copy_config_globals_out()` relies on. Reading the leaves + * anywhere else would race the task threads. + */ +void plc_retain_save(void); + +/** + * @brief Discard the stored blob, so the next start uses the declared + * initialisers. + * + * Called on program upload (CODESYS clears retained memory on download, and a + * new program's values have no business surviving into it) and by the + * `RETAIN:CLEAR` socket command. + */ +void plc_retain_clear(void); + +/** + * @brief Whether retain is actually live: a program that retains something, + * exports the entry points, and a plugin willing to store the bytes. + * + * Reported to the editor so "retain is configured but this device cannot do + * it" is visible rather than silent. + */ +bool plc_retain_active(void); + +#ifdef __cplusplus +} +#endif + +#endif /* PLC_RETAIN_H */ diff --git a/core/src/plc_app/plc_state_manager.cpp b/core/src/plc_app/plc_state_manager.cpp index 6608d14f..cfe856ea 100644 --- a/core/src/plc_app/plc_state_manager.cpp +++ b/core/src/plc_app/plc_state_manager.cpp @@ -34,6 +34,7 @@ extern "C" { #include "debug_write_journal.h" #include "image_tables.h" #include "journal_buffer.h" +#include "plc_retain.h" #include "plc_state_manager.h" #include "plcapp_manager.h" #include "scan_cycle_manager.h" @@ -439,6 +440,15 @@ void *plc_cycle_thread(void *arg) image_tables_fill_null_pointers(); pthread_mutex_unlock(itm); + /* Retained variables. init() decides once whether retain can run here — + * does the .so export the entry points, does the program retain anything, + * is there a plugin willing to store it — and load() restores what was + * kept. Both must follow the located-variable binding above: a retained + * variable may also be located, and its image slot has to exist before + * anything writes through it. Both are no-ops when retain is not in play. */ + plc_retain_init(); + plc_retain_load(); + journal_buffer_ptrs_t journal_ptrs = { .bool_input = bool_input, .bool_output = bool_output, @@ -965,6 +975,12 @@ void *plc_cycle_thread(void *arg) * g_tasks_running == 0 so no worker is mid-scan, and we hold * image_lock. Cheap no-op when nothing is queued. */ debug_write_journal_drain(); + /* Retained values, once per scan. This window is the only place + * they can be read safely: g_tasks_running == 0, so no worker is + * inside a body mutating them — the same guarantee + * copy_config_globals_out relies on. The plugin decides whether + * these bytes are actually committed now; no-op with no store. */ + plc_retain_save(); image_unlock(); if (plugin_driver) plugin_driver_cycle_end(plugin_driver); cycle_end_pending = false; diff --git a/core/src/plc_app/unix_socket.c b/core/src/plc_app/unix_socket.c index 0244c278..0343f644 100644 --- a/core/src/plc_app/unix_socket.c +++ b/core/src/plc_app/unix_socket.c @@ -11,6 +11,7 @@ #include "../drivers/plugin_driver.h" #include "debug_handler.h" +#include "plc_retain.h" #include "plc_state_manager.h" #include "plc_switch.h" #include "scan_cycle_manager.h" @@ -309,6 +310,19 @@ void handle_unix_socket_commands(const char *command, char *response, size_t res { format_switch_response(response, response_size); } + else if (strcmp(command, "RETAIN:CLEAR") == 0) + { + /* Discard stored retained values, so the next start uses the declared + * initialisers. The webserver calls this on program upload — CODESYS + * clears retained memory on download, and a new program's values have + * no business surviving into it. + * + * Answers OK even with no retain plugin: "discard what is stored" is + * satisfied by a device that stores nothing, and failing there would + * make every upload to such a device look broken. */ + plc_retain_clear(); + strncpy(response, "RETAIN:OK\n", response_size); + } else if (strcmp(command, "START") == 0) { PLCState current_state = plc_get_state(); diff --git a/core/strucpp_runtime/runtime_v4_entry.cpp b/core/strucpp_runtime/runtime_v4_entry.cpp index b4683ae5..6d4ee089 100644 --- a/core/strucpp_runtime/runtime_v4_entry.cpp +++ b/core/strucpp_runtime/runtime_v4_entry.cpp @@ -28,6 +28,7 @@ #include "debug_dispatch.hpp" #include "iec_located.hpp" #include "iec_std_lib.hpp" // ConfigurationInstance + __CURRENT_TIME_NS +#include "iec_retain.hpp" // retain blob format + pack/unpack #include "generated.hpp" #include @@ -140,3 +141,62 @@ extern "C" void strucpp_set_current_time(int64_t ns) { // NOTE: the runtime no longer probes a "threaded ABI" capability symbol. It // compiles every .so itself with -DSTRUCPP_THREADED, so the threaded // process-image model is the only one; there is nothing to detect. + +// --------------------------------------------------------------------------- +// Retain marshalling. +// +// The WALK lives here, inside the .so, because that is where the debug tables +// and `handle_read` / `handle_write` are. The runtime is built once and loads +// many .so files, so it cannot reach `strucpp::retain` by mangled name — and +// re-implementing the blob format on its side would put two copies of a wire +// format in two repos, which is exactly the drift `iec_retain.hpp` exists to +// prevent. +// +// What the runtime DOES own is the write path, which is why unpack takes a +// callback instead of using `handle_write` directly: a retained variable may +// also be LOCATED (`VAR RETAIN x AT %MW10`), and poking such a leaf's IECVar +// is undone by the next copy-in from the process image. The runtime passes a +// thunk that routes through `runtime_external_write`, which knows to send a +// located leaf through the image journal. Reads need no such care — a read +// sees whatever the last copy-in left — so pack uses `handle_read` here. +// --------------------------------------------------------------------------- + +static uint16_t retain_read_leaf(uint8_t arr, uint16_t elem, uint8_t* dest) { + return strucpp::debug::handle_read(arr, elem, dest); +} + +static uint16_t retain_size_leaf(uint8_t arr, uint16_t elem) { + return strucpp::debug::handle_size(arr, elem); +} + +/** Bytes a full blob occupies for this program; 0 when nothing is retained. */ +extern "C" size_t strucpp_retain_blob_size(void) { + return strucpp::retain::blob_size(retain_size_leaf); +} + +/** Identity of the retain LAYOUT — reported so the runtime can log it. */ +extern "C" uint32_t strucpp_retain_layout_hash(void) { + return strucpp::debug::retain_layout_hash; +} + +/** Serialise every retained leaf. Returns bytes written, 0 on failure. */ +extern "C" size_t strucpp_retain_pack(uint8_t* out, size_t cap) { + return strucpp::retain::pack(out, cap, retain_read_leaf, retain_size_leaf); +} + +/** + * Restore every retained leaf, writing through the runtime's own callback. + * + * Returns `strucpp::retain::LoadResult` as a byte. Anything but 0 (Ok) means + * nothing was written and every variable keeps its declared initial value — + * the correct outcome for a corrupt or stale store, since a machine starting + * from its defaults is recoverable and one starting from plausible-looking + * garbage is not. + */ +extern "C" uint8_t strucpp_retain_unpack( + const uint8_t* blob, + size_t len, + uint8_t (*write_leaf)(uint8_t arr, uint16_t elem, const uint8_t* bytes, uint16_t n)) { + return static_cast( + strucpp::retain::unpack(blob, len, write_leaf, retain_size_leaf)); +} diff --git a/webserver/app.py b/webserver/app.py index 9c3d65c6..553b8a91 100644 --- a/webserver/app.py +++ b/webserver/app.py @@ -288,6 +288,16 @@ def handle_upload_file(data: dict) -> dict: safe_extract(zip_file, extract_dir, valid_files) + # A new program must not inherit the previous one's retained values. + # The layout hash already refuses a genuinely different set of retained + # variables, but two programs that happen to share a layout would + # otherwise silently inherit each other's state. CODESYS clears retained + # memory on download; this is the same rule. + try: + runtime_manager.clear_retained() + except Exception as e: # never fail an upload over this + logger.warning("Could not clear retained values before upload: %s", e) + # Verify the VPP package signature BEFORE anything from this upload is # copied into the runtime root and before any Makefile from it runs. # diff --git a/webserver/runtimemanager.py b/webserver/runtimemanager.py index 7a188c98..016f77d2 100644 --- a/webserver/runtimemanager.py +++ b/webserver/runtimemanager.py @@ -337,6 +337,29 @@ def stop_plc(self): logger.error("Failed to stop PLC runtime (unexpected): %s", e) return "STOP:ERROR\n" + def clear_retained(self): + """ + Discard the runtime's stored retained values. + + Called on program upload. CODESYS clears retained memory on download, + and a new program's retained values have no business surviving into it + — the layout hash would refuse a genuinely different program, but two + programs that happen to share a retain layout would otherwise inherit + each other's values silently. + + Failure is logged and swallowed: the upload itself is what matters, and + a runtime that is not currently reachable has nothing stored that this + program will read anyway. + """ + try: + return self.runtime_socket.send_and_receive("RETAIN:CLEAR\n") + except (OSError, socket.error) as e: + logger.warning("Could not clear retained values: %s", e) + return "RETAIN:ERROR\n" + except Exception as e: + logger.warning("Could not clear retained values (unexpected): %s", e) + return "RETAIN:ERROR\n" + def status_plc(self): """ Send STATUS command From 1378bc27485a509d476583af7f95a41b122c9d53 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Mon, 24 Aug 2026 20:23:00 -0400 Subject: [PATCH 2/4] fix(retain): a disabled plugin is not a retain store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Loading a plugin resolves its symbols; only starting it is gated on `enabled`. `plugin_driver_find_retain_store` checked neither, so a disabled plugin whose .so had loaded was still chosen as the store. The result was retain that reported itself working and was not. Init logged "Retain: N bytes across the program's retained variables (layout …)", the scan cycle handed the blob over every cycle, and the plugin — never started, its flush thread never running — kept none of it. The values were simply gone on the next boot, with nothing in the log suggesting anything was wrong. Found on hardware, and by the most ordinary route: uploading a project rewrote plugins.conf and disabled the storage plugin, and retain went on claiming to work. Anyone whose retain storage got switched off would have seen the same thing. With the check, the same configuration now logs "Retain: N bytes of retained variables, but no plugin provides storage — they will start at their initial values", which is the honest report and the one that says what to do about it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012FNp61926UEggtmsQPj3A3 --- core/src/drivers/plugin_driver.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/core/src/drivers/plugin_driver.c b/core/src/drivers/plugin_driver.c index 103116aa..c3156709 100644 --- a/core/src/drivers/plugin_driver.c +++ b/core/src/drivers/plugin_driver.c @@ -1487,11 +1487,24 @@ void python_plugin_cycle(plugin_instance_t *plugin) static bool plugin_provides_retain_store(const plugin_instance_t *p) { + if (!p) return false; + + // A DISABLED plugin is not a store, even though its symbols resolved. + // + // Loading resolves symbols for every plugin in plugins.conf; only starting + // is gated on `enabled`. Without this check a disabled plugin is still + // picked as the store, so retain reports itself active, hands it the blob + // every scan, and gets nothing back on the next boot — the values are + // simply gone, with a log line at start saying retain is configured and + // working. Found on hardware: an upload rewrote plugins.conf, disabled the + // storage plugin, and retain went on claiming to work. + if (!p->config.enabled) return false; + // BOTH halves required. A store that can save and not load is worse than // none: it would accept values every scan and silently never give them // back, which looks like working retention right up until the reboot that // matters. - return p && p->native_plugin && p->native_plugin->retain_save && p->native_plugin->retain_load; + return p->native_plugin && p->native_plugin->retain_save && p->native_plugin->retain_load; } plugin_instance_t *plugin_driver_find_retain_store(plugin_driver_t *driver) From c56f5176fa976e00198e90cf2b453a84c882cf55 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Mon, 24 Aug 2026 23:50:34 -0400 Subject: [PATCH 3/4] feat(retain): a built-in file store for runtime v4, off by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retention hardware is a property of the device, and a VPP that provides its own backend still wins outright. But "no backend" and "FRAM" are not the only two cases: a Linux box running runtime v4 has a filesystem, and a file on it is a perfectly good place to keep retained values. Making every vendor write that same file backend just to get retain at all is a poor trade — and during a review cycle it means retain cannot be tried on a device at all without first writing a driver for it. So the runtime ships one. It is DISABLED by default: writing to the data partition on a cadence the operator did not choose is not something to switch on for everyone. An SD-card-backed box has an endurance budget its owner may be counting on. Until someone turns it on, retain is a no-op exactly as before and every retained variable starts at its declared initial value. PRECEDENCE ---------- A VPP plugin ALWAYS wins over the built-in store. The vendor knows what the box actually has; a file on the data partition is the fallback, not the preference. Silently writing to disk on a device whose driver just implemented proper retention would be both slower and wrong. The two are reported separately, because they are different questions. `RETAIN:STATUS` on the control socket answers what is holding the bytes RIGHT NOW — none / plugin / file — so the editor can show "the settings are saved, and a driver is doing the work" rather than "enabled" over a file that will never grow. CONFIGURATION ------------- `./retain.conf` beside `plugins.conf`, read once per program load: enabled, path, flush_seconds. Flat key=value rather than JSON because the core parses it in C++ during startup, and a dependency-free parser for three keys beats pulling a JSON library into the PLC application. `flush_seconds` bounds how much retained state a power cut costs against how hard the storage is worked. That is a real trade and it belongs to whoever installs the machine, which is why it is configuration. Committed by write-and-rename with an fsync of the file AND of the directory. The file fsync commits the contents; the rename that publishes them is a directory operation and can still be lost to a power cut after the data is safely down — which looks exactly like retain silently skipping an interval. REST ---- `GET /api/retain-config` (any authenticated user) and `PUT /api/retain-config` (admin) — the same split the user endpoints use: seeing how a device is configured is not the same privilege as changing it. Settings the runtime could not honour are refused at the API, not discovered later as a store that never writes: a relative path, a directory that does not exist, a target that is itself a directory, an impossible flush period. The path check is explicitly NOT a privilege boundary and says so — the caller is an admin who can already upload a program the runtime compiles and executes. It is there for ordinary mistakes. Verified on an SLM-RP4. With the built-in store enabled and no plugin: "Retain: built-in file store enabled … flushing every 5s", then "restored 54 bytes", and a retained TON comes back already elapsed while its un-retained twin restarts its wait. With the synergy VPP plugin loaded, the same device reports "stored by synergy" and the API reports `enabled=true → backend=plugin (synergy)`. 60 pytest green (21 new). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012FNp61926UEggtmsQPj3A3 --- VERSION | 2 +- core/src/CMakeLists.txt | 1 + core/src/plc_app/plc_retain.cpp | 101 ++++++-- core/src/plc_app/plc_retain.h | 15 ++ core/src/plc_app/plc_retain_file_store.cpp | 267 +++++++++++++++++++++ core/src/plc_app/plc_retain_file_store.h | 78 ++++++ core/src/plc_app/unix_socket.c | 13 + tests/pytest/restapi/test_retain_config.py | 202 ++++++++++++++++ webserver/restapi.py | 132 ++++++++++ webserver/retain_config.py | 143 +++++++++++ webserver/runtimemanager.py | 33 +++ 11 files changed, 967 insertions(+), 20 deletions(-) create mode 100644 core/src/plc_app/plc_retain_file_store.cpp create mode 100644 core/src/plc_app/plc_retain_file_store.h create mode 100644 tests/pytest/restapi/test_retain_config.py create mode 100644 webserver/retain_config.py diff --git a/VERSION b/VERSION index 23ad5eb3..51be872e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v4.1.10 +v4.2.0 diff --git a/core/src/CMakeLists.txt b/core/src/CMakeLists.txt index c6a3b3eb..bdd89c56 100644 --- a/core/src/CMakeLists.txt +++ b/core/src/CMakeLists.txt @@ -45,6 +45,7 @@ add_executable(plc_main ${CMAKE_SOURCE_DIR}/core/src/plc_app/debug_write_journal.cpp ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_io_cycle.cpp ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_retain.cpp + ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_retain_file_store.cpp ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_state_manager.cpp ${CMAKE_SOURCE_DIR}/core/src/plc_app/plc_switch.c ${CMAKE_SOURCE_DIR}/core/src/plc_app/plcapp_manager.c diff --git a/core/src/plc_app/plc_retain.cpp b/core/src/plc_app/plc_retain.cpp index 1013ff4d..059b0fac 100644 --- a/core/src/plc_app/plc_retain.cpp +++ b/core/src/plc_app/plc_retain.cpp @@ -19,6 +19,7 @@ extern "C" { #include "debug_write_journal.h" #include "image_tables.h" +#include "plc_retain_file_store.h" extern plugin_driver_t *plugin_driver; @@ -57,7 +58,19 @@ uint8_t retain_write_leaf(uint8_t arr, uint16_t elem, const uint8_t *bytes, uint return runtime_external_write(arr, elem, (uint8_t)DBGW_OP_WRITE, bytes, len) == 0 ? 0x7E : 0x82; } -/** The first plugin that exports both hooks wins; see plc_retain_init(). */ +/** + * Where the bytes go. + * + * A VPP plugin ALWAYS wins over the runtime's own file store. The vendor knows + * what the box actually has — FRAM, battery-backed SRAM, an NVS partition — + * and a file on the data partition is the runtime's fallback, not its + * preference. Silently writing to disk on a device whose VPP just implemented + * proper retention would be both slower and wrong. + */ +enum class Backend { None, Plugin, BuiltinFile }; +Backend g_backend = Backend::None; + +/** Set only when g_backend == Plugin. */ plugin_instance_t *g_store = nullptr; } // namespace @@ -65,8 +78,13 @@ plugin_instance_t *g_store = nullptr; void plc_retain_init(void) { g_active.store(false); - g_store = nullptr; + g_store = nullptr; + g_backend = Backend::None; g_buffer.clear(); + /* Re-read on every program load, so an operator's change to the Persistent + * Storage settings takes effect on the next PLC start without needing the + * daemon restarted. */ + plc_retain_file_store_stop(); if (!ext_strucpp_retain_blob_size || !ext_strucpp_retain_pack || !ext_strucpp_retain_unpack) { @@ -87,31 +105,45 @@ void plc_retain_init(void) } g_store = plugin_driver_find_retain_store(plugin_driver); - if (!g_store) + if (g_store) + { + g_backend = Backend::Plugin; + } + else if (plc_retain_file_store_start("./retain.conf")) { - /* Info, not a warning: a device with no retention plugin is a normal - * configuration, and the program still runs correctly — its retained + g_backend = Backend::BuiltinFile; + } + else + { + /* Info, not a warning: a device with no retention configured is a + * normal state, and the program still runs correctly — its retained * variables just behave as NON_RETAIN. Said once so the operator can - * tell "not supported here" from "supported and broken". */ - log_info("Retain: %zu bytes of retained variables, but no plugin provides storage — " - "they will start at their initial values", + * tell "not switched on here" from "switched on and broken". */ + log_info("Retain: %zu bytes of retained variables, but no storage is configured — " + "they will start at their initial values. Enable the built-in store on the " + "device's Persistent Storage screen, or install a VPP that provides one.", needed); return; } g_buffer.assign(needed, 0); g_active.store(true); - log_info("Retain: %zu bytes across the program's retained variables (layout %08x)", - needed, ext_strucpp_retain_layout_hash ? ext_strucpp_retain_layout_hash() : 0u); + log_info("Retain: %zu bytes across the program's retained variables (layout %08x), stored by %s", + needed, ext_strucpp_retain_layout_hash ? ext_strucpp_retain_layout_hash() : 0u, + g_backend == Backend::Plugin ? g_store->config.name + : plc_retain_file_store_path()); } void plc_retain_load(void) { - if (!g_active.load() || !g_store) return; - - uint16_t got = 0; - const int rc = plugin_driver_retain_load(g_store, g_buffer.data(), - (uint16_t)g_buffer.size(), &got); + if (!g_active.load() || g_backend == Backend::None) return; + + uint16_t got = 0; + const int rc = g_backend == Backend::Plugin + ? plugin_driver_retain_load(g_store, g_buffer.data(), + (uint16_t)g_buffer.size(), &got) + : plc_retain_file_store_load(g_buffer.data(), + (uint16_t)g_buffer.size(), &got); if (rc != 0 || got == 0) { log_info("Retain: nothing stored yet — retained variables start at their initial values"); @@ -137,7 +169,7 @@ void plc_retain_load(void) void plc_retain_save(void) { - if (!g_active.load() || !g_store) return; + if (!g_active.load() || g_backend == Backend::None) return; const size_t n = ext_strucpp_retain_pack(g_buffer.data(), g_buffer.size()); if (n == 0) return; @@ -145,7 +177,10 @@ void plc_retain_save(void) /* Hand the bytes over and return. Whether this is committed to storage now, * in five seconds, or on shutdown is the plugin's decision — it is the only * layer that knows what its medium costs. */ - plugin_driver_retain_save(g_store, g_buffer.data(), (uint16_t)n); + if (g_backend == Backend::Plugin) + plugin_driver_retain_save(g_store, g_buffer.data(), (uint16_t)n); + else + plc_retain_file_store_save(g_buffer.data(), (uint16_t)n); } void plc_retain_clear(void) @@ -154,11 +189,39 @@ void plc_retain_clear(void) * nothing retained, because what it is discarding belongs to the PREVIOUS * program. That is the whole point of clearing on upload. */ plugin_instance_t *store = g_store ? g_store : plugin_driver_find_retain_store(plugin_driver); - if (!store) return; - plugin_driver_retain_clear(store); + if (store) + { + plugin_driver_retain_clear(store); + return; + } + /* No plugin: clear the built-in store. Deliberately NOT gated on whether it + * is currently enabled — what is being discarded belongs to the previous + * program, and may have been written while it was. */ + plc_retain_file_store_start("./retain.conf"); + plc_retain_file_store_clear(); } bool plc_retain_active(void) { return g_active.load(); } + +const char *plc_retain_backend(void) +{ + switch (g_backend) + { + case Backend::Plugin: return "plugin"; + case Backend::BuiltinFile: return "file"; + default: return "none"; + } +} + +const char *plc_retain_backend_detail(void) +{ + switch (g_backend) + { + case Backend::Plugin: return g_store ? g_store->config.name : ""; + case Backend::BuiltinFile: return plc_retain_file_store_path(); + default: return ""; + } +} diff --git a/core/src/plc_app/plc_retain.h b/core/src/plc_app/plc_retain.h index dc801f8d..9241f492 100644 --- a/core/src/plc_app/plc_retain.h +++ b/core/src/plc_app/plc_retain.h @@ -98,6 +98,21 @@ void plc_retain_clear(void); */ bool plc_retain_active(void); +/** + * @brief Which backend is holding the bytes, for the editor's Persistent + * Storage screen. + * + * One of "none", "plugin" or "file". The screen needs to distinguish them + * because a VPP plugin OVERRIDES the built-in file store: on such a device the + * file settings are still there and still saved, and are simply not what is + * being used. Showing "enabled" while a plugin quietly handles retention would + * be a lie the operator only discovers by looking for a file that never grows. + */ +const char *plc_retain_backend(void); + +/** @brief The store's target for display — a path, or a plugin name. */ +const char *plc_retain_backend_detail(void); + #ifdef __cplusplus } #endif diff --git a/core/src/plc_app/plc_retain_file_store.cpp b/core/src/plc_app/plc_retain_file_store.cpp new file mode 100644 index 00000000..1d0b2a8c --- /dev/null +++ b/core/src/plc_app/plc_retain_file_store.cpp @@ -0,0 +1,267 @@ +/** + * @file plc_retain_file_store.cpp + * @brief The runtime's own retain backend. See plc_retain_file_store.h. + */ + +#include "plc_retain_file_store.h" + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +extern "C" { +#include "utils/log.h" +} + +namespace { + +/* Matches plc_retain.cpp's RETAIN_BUFFER_MAX. A blob larger than the runtime + * will marshal cannot reach us, so this is a ceiling on what we will hold, not + * a limit anyone is expected to meet. */ +constexpr size_t RETAIN_MAX = 64 * 1024; + +std::mutex g_lock; +std::vector g_pending; +bool g_dirty = false; + +std::string g_path; +int g_flush_seconds = 5; +std::atomic g_enabled{false}; +std::atomic g_running{false}; +std::thread g_flusher; + +/** Trim ASCII whitespace from both ends, in place. */ +std::string trimmed(const std::string &s) +{ + const size_t b = s.find_first_not_of(" \t\r\n"); + if (b == std::string::npos) return ""; + const size_t e = s.find_last_not_of(" \t\r\n"); + return s.substr(b, e - b + 1); +} + +/** + * Parse `retain.conf`. A missing file is not an error — it means nobody has + * configured retention on this device, which is the default state. + */ +void read_config(const char *config_path) +{ + g_enabled.store(false); + g_path.clear(); + g_flush_seconds = 5; + + FILE *f = fopen(config_path, "r"); + if (!f) return; + + bool enabled = false; + char line[1024]; + while (fgets(line, sizeof(line), f)) + { + std::string s = trimmed(line); + if (s.empty() || s[0] == '#') continue; + const size_t eq = s.find('='); + if (eq == std::string::npos) continue; + const std::string key = trimmed(s.substr(0, eq)); + const std::string val = trimmed(s.substr(eq + 1)); + + if (key == "enabled") enabled = (val == "1" || val == "true"); + else if (key == "path") g_path = val; + else if (key == "flush_seconds") g_flush_seconds = atoi(val.c_str()); + } + fclose(f); + + if (g_flush_seconds < 1) g_flush_seconds = 1; + /* Enabled with no path is a misconfiguration, not a request to write + * somewhere arbitrary. Treat it as off and say so. */ + if (enabled && g_path.empty()) + { + log_warn("Retain: the built-in store is enabled but no path is set — leaving it off"); + enabled = false; + } + g_enabled.store(enabled); +} + +/** + * Publish the blob. + * + * Write-and-rename, so a power loss mid-write leaves the PREVIOUS good blob + * rather than a half-written one. The runtime's crc would catch a torn write + * and fall back to initial values anyway, but losing the previous values as + * well would be gratuitous. + */ +void commit(const uint8_t *buf, uint16_t len) +{ + const std::string tmp = g_path + ".tmp"; + + FILE *f = fopen(tmp.c_str(), "wb"); + if (!f) + { + /* Said once per failure rather than swallowed: a store that cannot + * write is indistinguishable from one that never had anything to + * write, and the difference matters after a power cut. */ + log_warn("Retain: cannot write %s — retained values will not be kept", tmp.c_str()); + return; + } + const bool wrote = fwrite(buf, 1, len, f) == len; + if (wrote) + { + fflush(f); + fsync(fileno(f)); + } + fclose(f); + if (!wrote) + { + remove(tmp.c_str()); + log_warn("Retain: short write to %s — keeping the previous stored values", tmp.c_str()); + return; + } + + if (rename(tmp.c_str(), g_path.c_str()) != 0) + { + remove(tmp.c_str()); + log_warn("Retain: cannot publish %s — keeping the previous stored values", g_path.c_str()); + return; + } + + /* fsync the DIRECTORY too. fsync on the file commits its contents; the + * rename that publishes them is a directory operation, and on ext4 it can + * still be lost to a power cut after the data is safely on disk. Without + * this the store can come back holding the previous blob even though the + * new one was written — the failure that looks like retain silently + * skipping an interval. */ + std::vector dircopy(g_path.begin(), g_path.end()); + dircopy.push_back('\0'); + const int dirfd = open(dirname(dircopy.data()), O_RDONLY | O_DIRECTORY); + if (dirfd >= 0) + { + fsync(dirfd); + close(dirfd); + } +} + +void flush_loop() +{ + while (g_running.load()) + { + for (int i = 0; i < g_flush_seconds && g_running.load(); i++) + { + std::this_thread::sleep_for(std::chrono::seconds(1)); + } + if (!g_running.load()) break; + + std::vector snapshot; + { + /* Copy under the lock, write outside it: the scan thread calls + * save() every cycle and must never wait on a disk write. */ + std::lock_guard guard(g_lock); + if (!g_dirty) continue; + snapshot = g_pending; + g_dirty = false; + } + if (!snapshot.empty()) commit(snapshot.data(), (uint16_t)snapshot.size()); + } +} + +} // namespace + +bool plc_retain_file_store_start(const char *config_path) +{ + plc_retain_file_store_stop(); + read_config(config_path ? config_path : "./retain.conf"); + if (!g_enabled.load()) return false; + + g_running.store(true); + g_flusher = std::thread(flush_loop); + log_info("Retain: built-in file store enabled — %s, flushing every %ds", + g_path.c_str(), g_flush_seconds); + return true; +} + +void plc_retain_file_store_stop(void) +{ + if (g_running.exchange(false)) + { + if (g_flusher.joinable()) g_flusher.join(); + + /* Final flush: a clean stop should not discard the last interval. */ + std::lock_guard guard(g_lock); + if (g_dirty && !g_pending.empty()) + { + commit(g_pending.data(), (uint16_t)g_pending.size()); + g_dirty = false; + } + } + g_enabled.store(false); +} + +bool plc_retain_file_store_active(void) +{ + return g_enabled.load(); +} + +const char *plc_retain_file_store_path(void) +{ + return g_path.c_str(); +} + +int plc_retain_file_store_save(const uint8_t *blob, uint16_t len) +{ + if (!g_enabled.load() || !blob || len == 0 || len > RETAIN_MAX) return -1; + + std::lock_guard guard(g_lock); + /* Only mark dirty on an actual change. The runtime deliberately does not + * diff — it cannot know what a write costs here — so doing it at this layer + * is how a slow medium avoids rewriting an unchanged blob every interval. */ + if (g_pending.size() != len || memcmp(g_pending.data(), blob, len) != 0) + { + g_pending.assign(blob, blob + len); + g_dirty = true; + } + return 0; +} + +int plc_retain_file_store_load(uint8_t *out, uint16_t cap, uint16_t *out_len) +{ + if (out_len) *out_len = 0; + if (!g_enabled.load() || !out || cap == 0) return -1; + + FILE *f = fopen(g_path.c_str(), "rb"); + if (!f) return -1; /* nothing stored — a first boot, or freshly cleared */ + const size_t n = fread(out, 1, cap, f); + fclose(f); + if (n == 0) return -1; + + if (out_len) *out_len = (uint16_t)n; + + /* Prime the in-memory copy so the first flush after start does not rewrite + * a byte-identical file. */ + std::lock_guard guard(g_lock); + g_pending.assign(out, out + n); + g_dirty = false; + return 0; +} + +int plc_retain_file_store_clear(void) +{ + { + std::lock_guard guard(g_lock); + g_pending.clear(); + g_dirty = false; + } + /* Not gated on `enabled`: a clear has to remove what a PREVIOUS + * configuration stored, which is the whole point of clearing on upload. */ + if (!g_path.empty()) + { + remove(g_path.c_str()); + remove((g_path + ".tmp").c_str()); + } + return 0; +} diff --git a/core/src/plc_app/plc_retain_file_store.h b/core/src/plc_app/plc_retain_file_store.h new file mode 100644 index 00000000..7c877477 --- /dev/null +++ b/core/src/plc_app/plc_retain_file_store.h @@ -0,0 +1,78 @@ +/** + * @file plc_retain_file_store.h + * @brief The runtime's own retain backend: a file on the data partition. + * + * WHY THE RUNTIME SHIPS ONE AT ALL + * -------------------------------- + * Retention hardware is a property of the device, so the storage decision + * belongs to the platform — that has not changed, and a VPP that provides its + * own backend still wins outright (see plc_retain_init). But "the platform has + * no backend" and "the platform has FRAM" are not the only two cases: a Linux + * box running runtime v4 has a filesystem, and a file on it is a perfectly good + * place to keep retained values. Making every vendor write that same file + * backend to get retain at all is a poor trade. + * + * So this is the default, and it is DISABLED by default. An operator turns it + * on for a given device; until then retain is a no-op exactly as it was, and + * every retained variable starts at its declared initial value. + * + * DISABLED BY DEFAULT IS DELIBERATE + * --------------------------------- + * Writing to the data partition on a cadence the operator did not choose is not + * something to switch on for everyone. An SD-card-backed box has an endurance + * budget its owner may be counting on; a read-mostly image may not want the + * partition written at all. Opt-in keeps the decision where it belongs. + * + * CONFIGURATION + * ------------- + * Read once, at program load, from a small key=value file the webserver owns + * (`./retain.conf` beside `plugins.conf`): + * + * enabled=1 + * path=/var/lib/openplc-runtime/retain.bin + * flush_seconds=5 + * + * `flush_seconds` bounds how much retained state a power cut costs, against how + * hard the storage is worked. It is a real trade and it belongs to whoever + * installs the machine, which is why it is configuration and not a constant. + */ + +#ifndef PLC_RETAIN_FILE_STORE_H +#define PLC_RETAIN_FILE_STORE_H + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Read `retain.conf` and start the flush thread if enabled. + * + * Safe to call repeatedly; a second call re-reads the file so an operator's + * change takes effect on the next PLC start without a daemon restart. + * Returns true when the store is enabled and usable. + */ +bool plc_retain_file_store_start(const char *config_path); + +/** @brief Final flush, then stop the flush thread. */ +void plc_retain_file_store_stop(void); + +/** @brief Whether the store is enabled and willing to hold bytes. */ +bool plc_retain_file_store_active(void); + +/** @brief The configured path, for logging. Empty when disabled. */ +const char *plc_retain_file_store_path(void); + +/* The three hooks, shaped exactly like the plugin ones so plc_retain.cpp can + * route to either without caring which it got. 0 on success. */ +int plc_retain_file_store_save(const uint8_t *blob, uint16_t len); +int plc_retain_file_store_load(uint8_t *out, uint16_t cap, uint16_t *out_len); +int plc_retain_file_store_clear(void); + +#ifdef __cplusplus +} +#endif + +#endif /* PLC_RETAIN_FILE_STORE_H */ diff --git a/core/src/plc_app/unix_socket.c b/core/src/plc_app/unix_socket.c index 0343f644..b905cddb 100644 --- a/core/src/plc_app/unix_socket.c +++ b/core/src/plc_app/unix_socket.c @@ -323,6 +323,19 @@ void handle_unix_socket_commands(const char *command, char *response, size_t res plc_retain_clear(); strncpy(response, "RETAIN:OK\n", response_size); } + else if (strcmp(command, "RETAIN:STATUS") == 0) + { + /* What is ACTUALLY holding the retained bytes right now, which is not + * the same question as what the settings say. A VPP plugin overrides + * the built-in file store, and the Persistent Storage screen has to be + * able to say so — otherwise it reports the file store as enabled + * while a plugin quietly does the work, and the operator finds out by + * wondering why the file never grows. */ + snprintf(response, response_size, "RETAIN:STATUS %s %s %s\n", + plc_retain_active() ? "active" : "inactive", + plc_retain_backend(), + plc_retain_backend_detail()); + } else if (strcmp(command, "START") == 0) { PLCState current_state = plc_get_state(); diff --git a/tests/pytest/restapi/test_retain_config.py b/tests/pytest/restapi/test_retain_config.py new file mode 100644 index 00000000..3b5716c7 --- /dev/null +++ b/tests/pytest/restapi/test_retain_config.py @@ -0,0 +1,202 @@ +"""Behavioural tests for the persistent-storage (RETAIN) settings endpoints. + +These settings decide whether a device keeps its retained variables at all, so +the things worth pinning down are: that it is OFF until somebody turns it on, +that only an admin can turn it on, and that a setting the runtime could not +honour is refused at the API rather than discovered as a store that silently +never writes. +""" + +import os + +import pytest +from conftest import create_user, token_for + +from webserver import retain_config + + +@pytest.fixture(autouse=True) +def isolated_conf(tmp_path, monkeypatch): + """Point retain.conf at a temp file — never the developer's own runtime.""" + conf = tmp_path / "retain.conf" + monkeypatch.setattr(retain_config, "RETAIN_CONF_PATH", conf) + from webserver import restapi + + monkeypatch.setitem(restapi.app_restapi.config, "RUNTIME_MANAGER", None) + return conf + + +@pytest.fixture() +def admin_headers(client): + create_user(client, "admin", "admin-pass") + return {"Authorization": f"Bearer {token_for(client, 'admin', 'admin-pass')}"} + + +@pytest.fixture() +def user_headers(client, admin_headers): + create_user(client, "bob", "bob-pass", token=admin_headers["Authorization"].split()[1], role="user") + return {"Authorization": f"Bearer {token_for(client, 'bob', 'bob-pass')}"} + + +# --- defaults ------------------------------------------------------------- + + +def test_retention_is_off_until_somebody_turns_it_on(client, admin_headers): + # The whole point of the default: a device does not start writing to its + # data partition on a cadence nobody chose. + body = client.get("/api/retain-config", headers=admin_headers).get_json() + assert body["enabled"] is False + assert body["path"] == retain_config.DEFAULT_RETAIN_PATH + assert body["flushSeconds"] == retain_config.DEFAULT_FLUSH_SECONDS + + +def test_get_reports_the_bounds_the_ui_should_enforce(client, admin_headers): + body = client.get("/api/retain-config", headers=admin_headers).get_json() + assert body["minFlushSeconds"] == retain_config.MIN_FLUSH_SECONDS + assert body["maxFlushSeconds"] == retain_config.MAX_FLUSH_SECONDS + assert body["defaultPath"] == retain_config.DEFAULT_RETAIN_PATH + + +def test_get_requires_authentication(client): + assert client.get("/api/retain-config").status_code == 401 + + +# --- who may change it ---------------------------------------------------- + + +def test_a_plain_user_may_read_but_not_change(client, user_headers): + assert client.get("/api/retain-config", headers=user_headers).status_code == 200 + resp = client.put("/api/retain-config", json={"enabled": True}, headers=user_headers) + assert resp.status_code == 403 + + +def test_put_requires_authentication(client): + assert client.put("/api/retain-config", json={"enabled": True}).status_code == 401 + + +# --- saving --------------------------------------------------------------- + + +def test_admin_enables_it_and_it_persists(client, admin_headers, tmp_path): + target = str(tmp_path / "retain.bin") + resp = client.put( + "/api/retain-config", + json={"enabled": True, "path": target, "flushSeconds": 30}, + headers=admin_headers, + ) + assert resp.status_code == 200 + + body = client.get("/api/retain-config", headers=admin_headers).get_json() + assert body["enabled"] is True + assert body["path"] == target + assert body["flushSeconds"] == 30 + + +def test_the_file_is_written_in_the_form_the_core_parses(client, admin_headers, tmp_path, isolated_conf): + target = str(tmp_path / "retain.bin") + client.put( + "/api/retain-config", + json={"enabled": True, "path": target, "flushSeconds": 7}, + headers=admin_headers, + ) + text = isolated_conf.read_text() + # Flat key=value, because the core parses this in C++ at startup. + assert "enabled=1" in text + assert f"path={target}" in text + assert "flush_seconds=7" in text + + +def test_a_partial_update_leaves_the_other_settings_alone(client, admin_headers, tmp_path): + target = str(tmp_path / "retain.bin") + client.put( + "/api/retain-config", + json={"enabled": True, "path": target, "flushSeconds": 30}, + headers=admin_headers, + ) + client.put("/api/retain-config", json={"enabled": False}, headers=admin_headers) + body = client.get("/api/retain-config", headers=admin_headers).get_json() + assert body["enabled"] is False + assert body["path"] == target + assert body["flushSeconds"] == 30 + + +# --- settings the runtime could not honour -------------------------------- + + +def test_a_relative_path_is_refused(client, admin_headers): + resp = client.put( + "/api/retain-config", json={"enabled": True, "path": "retain.bin"}, headers=admin_headers + ) + assert resp.status_code == 400 + assert "absolute" in resp.get_json()["msg"] + + +def test_a_missing_directory_is_refused(client, admin_headers): + resp = client.put( + "/api/retain-config", + json={"enabled": True, "path": "/definitely/not/here/retain.bin"}, + headers=admin_headers, + ) + assert resp.status_code == 400 + assert "does not exist" in resp.get_json()["msg"] + + +def test_a_directory_is_refused_as_a_target(client, admin_headers, tmp_path): + resp = client.put( + "/api/retain-config", json={"enabled": True, "path": str(tmp_path)}, headers=admin_headers + ) + assert resp.status_code == 400 + + +def test_an_unnormalised_path_is_refused(client, admin_headers, tmp_path): + # Not a security boundary — an admin here can already run code — but a + # traversal makes the stored location unobvious, which is worth refusing. + resp = client.put( + "/api/retain-config", + json={"enabled": True, "path": f"{tmp_path}/../retain.bin"}, + headers=admin_headers, + ) + assert resp.status_code == 400 + + +@pytest.mark.parametrize("seconds", [0, -1, retain_config.MAX_FLUSH_SECONDS + 1, "soon", None]) +def test_an_impossible_flush_period_is_refused(client, admin_headers, tmp_path, seconds): + resp = client.put( + "/api/retain-config", + json={"enabled": True, "path": str(tmp_path / "r.bin"), "flushSeconds": seconds}, + headers=admin_headers, + ) + assert resp.status_code == 400 + + +def test_a_non_boolean_enabled_is_refused(client, admin_headers): + resp = client.put("/api/retain-config", json={"enabled": "yes"}, headers=admin_headers) + assert resp.status_code == 400 + + +def test_enabling_without_ever_setting_a_path_uses_the_default(client, admin_headers, monkeypatch, tmp_path): + # The default has to be usable as-is, or "enable" fails for no good reason + # the first time anyone presses it. + monkeypatch.setattr(retain_config, "DEFAULT_RETAIN_PATH", str(tmp_path / "retain.bin")) + resp = client.put("/api/retain-config", json={"enabled": True}, headers=admin_headers) + assert resp.status_code == 200, resp.get_json() + + +# --- reading a file written by hand --------------------------------------- + + +def test_comments_and_blank_lines_are_ignored(client, admin_headers, isolated_conf, tmp_path): + isolated_conf.write_text( + "# hand-edited\n\nenabled=true\n" + f"path={tmp_path / 'x.bin'}\n" + "flush_seconds=12\n" + ) + body = client.get("/api/retain-config", headers=admin_headers).get_json() + assert body["enabled"] is True + assert body["flushSeconds"] == 12 + + +def test_a_corrupt_flush_value_falls_back_to_the_default(client, admin_headers, isolated_conf): + isolated_conf.write_text("enabled=0\nflush_seconds=banana\n") + body = client.get("/api/retain-config", headers=admin_headers).get_json() + assert body["flushSeconds"] == retain_config.DEFAULT_FLUSH_SECONDS diff --git a/webserver/restapi.py b/webserver/restapi.py index 4cee074c..32d0304c 100644 --- a/webserver/restapi.py +++ b/webserver/restapi.py @@ -17,6 +17,15 @@ import webserver.config from webserver.logger import get_logger +from webserver.retain_config import ( + DEFAULT_FLUSH_SECONDS, + DEFAULT_RETAIN_PATH, + MAX_FLUSH_SECONDS, + MIN_FLUSH_SECONDS, + RetainConfigError, + read_retain_config, + write_retain_config, +) from webserver.version import MIN_EDITOR_VERSION, RUNTIME_VERSION logger, buffer = get_logger("logger", use_buffer=True) @@ -676,6 +685,129 @@ def delete_user(user_id): # login endpoint +# Persistent storage (RETAIN) settings for the runtime's BUILT-IN file store. +# +# Read by any authenticated user, written by an admin — the same split the user +# endpoints use, and for the same reason: seeing how a device is configured is +# not the same privilege as changing it. +# +# The settings describe the built-in store only. A VPP plugin that provides its +# own retain backend OVERRIDES it, which is why the GET also reports what is +# actually holding the bytes right now. +@restapi_bp.route("/retain-config", methods=["GET"]) +@jwt_required() +def get_retain_config(): + """ + Return the persistent-storage settings and the live backend. + --- + tags: + - Runtime + security: + - BearerAuth: [] + responses: + 200: + description: Persistent storage settings + schema: + type: object + properties: + enabled: + type: boolean + path: + type: string + flushSeconds: + type: integer + defaultPath: + type: string + minFlushSeconds: + type: integer + maxFlushSeconds: + type: integer + backend: + type: string + description: What is holding retained bytes now — none, plugin, file or unknown + backendDetail: + type: string + active: + type: boolean + 401: + description: Authentication required + """ + cfg = read_retain_config() + manager = app_restapi.config.get("RUNTIME_MANAGER") + status = manager.retain_status() if manager else {"active": False, "backend": "unknown", "detail": ""} + cfg.update( + { + "defaultPath": DEFAULT_RETAIN_PATH, + "defaultFlushSeconds": DEFAULT_FLUSH_SECONDS, + "minFlushSeconds": MIN_FLUSH_SECONDS, + "maxFlushSeconds": MAX_FLUSH_SECONDS, + "backend": status.get("backend", "unknown"), + "backendDetail": status.get("detail", ""), + "active": status.get("active", False), + } + ) + return jsonify(cfg), 200 + + +@restapi_bp.route("/retain-config", methods=["PUT"]) +@jwt_required() +def put_retain_config(): + """ + Update the persistent-storage settings. + + Takes effect when the PLC next starts: the core reads the file once per + program load, and re-reading it mid-scan would mean changing where a + running program's retained values go, which is not a thing to do quietly. + --- + tags: + - Runtime + security: + - BearerAuth: [] + parameters: + - in: body + name: body + schema: + type: object + properties: + enabled: + type: boolean + path: + type: string + flushSeconds: + type: integer + responses: + 200: + description: Settings saved + 400: + description: A setting the runtime could not honour + 401: + description: Authentication required + 403: + description: Admin privileges required + """ + if not (current_user and current_user.is_admin()): + return jsonify({"msg": "Admin privileges required"}), 403 + + data = request.get_json() or {} + current = read_retain_config() + + enabled = data.get("enabled", current["enabled"]) + if not isinstance(enabled, bool): + return jsonify({"msg": "enabled must be true or false"}), 400 + + path = data.get("path", current["path"]) + flush_seconds = data.get("flushSeconds", current["flushSeconds"]) + + try: + saved = write_retain_config(enabled, path, flush_seconds) + except RetainConfigError as e: + return jsonify({"msg": str(e)}), 400 + except OSError as e: + return jsonify({"msg": f"Could not save the settings: {e}"}), 400 + + return jsonify(saved), 200 + + @restapi_bp.route("/login", methods=["POST"]) def login(): """ diff --git a/webserver/retain_config.py b/webserver/retain_config.py new file mode 100644 index 00000000..8bd25a55 --- /dev/null +++ b/webserver/retain_config.py @@ -0,0 +1,143 @@ +"""Persistent-storage (retain) settings for the built-in file store. + +The runtime core reads ``retain.conf`` once per program load; this module is +the only thing that writes it. Two separate processes touch retention, so the +split matters: the webserver owns the SETTINGS and the core owns the BYTES. + +The file is deliberately a flat ``key=value`` list rather than JSON. The core +parses it in C++ during startup, before anything else is available, and a +dependency-free parser for three keys is a better trade there than pulling a +JSON library into the PLC application. + +A missing file means "nobody has configured retention on this device", which +is the default state and not an error. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +from webserver.config import PERSISTENT_DATA_DIR + +# The runtime's working directory (systemd `WorkingDirectory=$OPENPLC_DIR`), so +# retain.conf lands beside plugins.conf where the core looks for it. +RUNTIME_ROOT = Path(os.path.abspath(os.path.dirname(__file__))).parent +RETAIN_CONF_PATH = RUNTIME_ROOT / "retain.conf" + +DEFAULT_RETAIN_PATH = str(PERSISTENT_DATA_DIR / "retain.bin") +DEFAULT_FLUSH_SECONDS = 5 + +# Bounds on the flush period. The floor is not arbitrary: the runtime hands the +# blob over every scan cycle, and a sub-second flush would write through at +# something close to scan rate, which is exactly what the buffering exists to +# avoid. The ceiling keeps "enabled" from meaning "saved once an hour", which +# would look like retention and behave like none. +MIN_FLUSH_SECONDS = 1 +MAX_FLUSH_SECONDS = 3600 + + +class RetainConfigError(ValueError): + """Raised for a setting the runtime would not be able to honour.""" + + +def read_retain_config() -> dict: + """Current settings, with defaults filled in for anything unset.""" + cfg = { + "enabled": False, + "path": DEFAULT_RETAIN_PATH, + "flushSeconds": DEFAULT_FLUSH_SECONDS, + } + try: + with open(RETAIN_CONF_PATH, "r", encoding="utf-8") as handle: + for raw in handle: + line = raw.strip() + if not line or line.startswith("#") or "=" not in line: + continue + key, _, value = line.partition("=") + key, value = key.strip(), value.strip() + if key == "enabled": + cfg["enabled"] = value in ("1", "true", "True") + elif key == "path" and value: + cfg["path"] = value + elif key == "flush_seconds": + try: + cfg["flushSeconds"] = int(value) + except ValueError: + pass + except FileNotFoundError: + pass + return cfg + + +def validate_retain_path(path: str) -> str: + """Normalise and sanity-check a store path. + + NOT a privilege boundary. The caller is an authenticated admin of this + runtime, who can already upload a program the runtime compiles and executes + — so an arbitrary path is not an escalation, and pretending otherwise with + a denylist would give false assurance. What these checks are for is + ordinary mistakes: a relative path (which would resolve against whatever + directory the core happened to start in), a traversal that makes the stored + location unobvious, and a directory that does not exist, where the store + would fail on every flush with nothing but a log line to show for it. + """ + candidate = (path or "").strip() + if not candidate: + raise RetainConfigError("A storage path is required when retention is enabled.") + if not candidate.startswith("/"): + raise RetainConfigError("The storage path must be absolute.") + + normalised = os.path.normpath(candidate) + if normalised != candidate.rstrip("/") and normalised != candidate: + raise RetainConfigError( + f"The storage path must be given in normalised form (did you mean {normalised}?)." + ) + if os.path.isdir(normalised): + raise RetainConfigError("The storage path names a directory, not a file.") + + parent = os.path.dirname(normalised) + if not os.path.isdir(parent): + raise RetainConfigError( + f"The directory {parent} does not exist, so nothing could be written there." + ) + return normalised + + +def validate_flush_seconds(value) -> int: + try: + seconds = int(value) + except (TypeError, ValueError): + raise RetainConfigError("The flush period must be a whole number of seconds.") + if seconds < MIN_FLUSH_SECONDS or seconds > MAX_FLUSH_SECONDS: + raise RetainConfigError( + f"The flush period must be between {MIN_FLUSH_SECONDS} and " + f"{MAX_FLUSH_SECONDS} seconds." + ) + return seconds + + +def write_retain_config(enabled: bool, path: str, flush_seconds: int) -> dict: + """Persist the settings the core will read on the next program load.""" + resolved_path = validate_retain_path(path) + seconds = validate_flush_seconds(flush_seconds) + + body = ( + "# Persistent storage for RETAIN variables.\n" + "# Written by the OpenPLC runtime's REST API; read by the PLC\n" + "# application at program load. Edits here are overwritten.\n" + f"enabled={'1' if enabled else '0'}\n" + f"path={resolved_path}\n" + f"flush_seconds={seconds}\n" + ) + + # Write-and-rename, for the same reason the store itself does: a torn + # retain.conf read at the next start would silently disable retention. + tmp = RETAIN_CONF_PATH.with_suffix(".conf.tmp") + with open(tmp, "w", encoding="utf-8") as handle: + handle.write(body) + handle.flush() + os.fsync(handle.fileno()) + os.replace(tmp, RETAIN_CONF_PATH) + + return {"enabled": bool(enabled), "path": resolved_path, "flushSeconds": seconds} diff --git a/webserver/runtimemanager.py b/webserver/runtimemanager.py index 016f77d2..7d6cd8d0 100644 --- a/webserver/runtimemanager.py +++ b/webserver/runtimemanager.py @@ -360,6 +360,39 @@ def clear_retained(self): logger.warning("Could not clear retained values (unexpected): %s", e) return "RETAIN:ERROR\n" + def retain_status(self): + """Ask the core which backend is actually holding the retained bytes. + + Not the same question as what ``retain.conf`` says. A VPP plugin + overrides the built-in file store, so a device can have the file store + switched on in settings while a plugin does the work — and the + Persistent Storage screen has to be able to say so rather than report + a file that will never grow. + + Returns ``{"active", "backend", "detail"}``; ``backend`` is one of + ``none`` / ``plugin`` / ``file``. A runtime that is not reachable, or + one too old to answer, reports ``unknown`` rather than guessing. + """ + unknown = {"active": False, "backend": "unknown", "detail": ""} + try: + reply = self.runtime_socket.send_and_receive("RETAIN:STATUS\n") + except (OSError, socket.error) as e: + logger.warning("Could not read retain status: %s", e) + return unknown + except Exception as e: + logger.warning("Could not read retain status (unexpected): %s", e) + return unknown + + parts = (reply or "").strip().split() + # "RETAIN:STATUS [detail]" + if len(parts) < 3 or parts[0] != "RETAIN:STATUS": + return unknown + return { + "active": parts[1] == "active", + "backend": parts[2], + "detail": parts[3] if len(parts) > 3 else "", + } + def status_plc(self): """ Send STATUS command From 588c7bcabc8572be593383c6542092bc04ae29bc Mon Sep 17 00:00:00 2001 From: Daniel Coutinho <60111446+dcoutinho1328@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:07:35 -0300 Subject: [PATCH 4/4] docs(plugin): document the native plugin retain-store contract NODE-94 adds an optional retain_save/retain_load/retain_clear plugin hook and the matching plugin_driver_retain_* API, but core/src/drivers/README.md was not updated to cover them. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ACXXWVDG9HEZwAmtRRzBsp --- core/src/drivers/README.md | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/core/src/drivers/README.md b/core/src/drivers/README.md index df69a407..27581806 100644 --- a/core/src/drivers/README.md +++ b/core/src/drivers/README.md @@ -94,6 +94,14 @@ void cleanup(void); // Called when plugin is being unloaded // Per-cycle hooks (called during each PLC scan cycle, synchronized with PLC execution) void cycle_start(void); // Called at start of each scan cycle, before PLC logic void cycle_end(void); // Called at end of each scan cycle, after PLC logic + +// Optional: retain-variable storage. A plugin exporting both retain_save and +// retain_load becomes the device's retain store (first one found wins). +// retain_clear is optional on top of that; without it the plugin simply +// cannot be cold-reset. +int retain_save(const uint8_t *blob, uint16_t len); // Persist the blob. Must not block. +int retain_load(uint8_t *out, uint16_t cap, uint16_t *out_len); // Restore the blob. +int retain_clear(void); // Discard the stored blob. ``` **Important: Native Plugin Args Lifetime** @@ -581,6 +589,18 @@ void plugin_driver_cycle_start(plugin_driver_t *driver); // Plugins opt-in by implementing cycle_end(); opt-out by not implementing it void plugin_driver_cycle_end(plugin_driver_t *driver); +// Find the plugin acting as this device's retain store (first plugin that +// exports both retain_save and retain_load), or NULL if none does +plugin_instance_t *plugin_driver_find_retain_store(plugin_driver_t *driver); + +// Save/load/clear the retain blob through the chosen store plugin +// Return 0 on success; retain_save/retain_load return -1 if `store` isn't a +// valid store, retain_clear is a no-op (returns 0) if the plugin has no +// retain_clear export +int plugin_driver_retain_save(plugin_instance_t *store, const uint8_t *blob, uint16_t len); +int plugin_driver_retain_load(plugin_instance_t *store, uint8_t *out, uint16_t cap, uint16_t *out_len); +int plugin_driver_retain_clear(plugin_instance_t *store); + // Destroy the plugin driver and free resources (calls 'cleanup' on plugins) void plugin_driver_destroy(plugin_driver_t *driver); ``` @@ -683,7 +703,17 @@ void plugin_driver_destroy(plugin_driver_t *driver); } ``` -4. **Memory Management (Python):** +4. **Native Plugin Retain Store:** + + A native plugin that owns retention hardware (FRAM, battery-backed SRAM, a data partition, etc.) can become the device's retain store by exporting `retain_save()` and `retain_load()`. The runtime marshals retained variables into an opaque blob and hands it to the store; the plugin only needs to persist and return bytes, with no knowledge of what they mean. + + * `retain_save(blob, len)`: Called once per scan cycle, unconditionally, from the scan's quiescent window. Must return promptly and must not block — this runs inside the scan cycle. + * `retain_load(out, cap, out_len)`: Called once at program start to restore the last saved blob. + * `retain_clear(void)`: Optional. Discards the stored blob so the next start uses the program's declared initial values. Called on program upload and by the runtime's `RETAIN:CLEAR` command. A plugin without it simply cannot be cold-reset. + + **Both `retain_save` and `retain_load` are required** to be selected as the store; a plugin exporting only one is ignored. If more than one plugin exports both, the first one found wins and the rest are logged and ignored. + +5. **Memory Management (Python):** * Python's garbage collector handles memory. However, explicitly close files, sockets, or release other external resources in `cleanup()`. ## Dependencies