fix(build): compile C++ libraries carried in a STruC++ upload

core/src/plc_app/plc_retain.cpp

@@ -59,7 +59,18 @@ std::atomic<bool>    g_active{false};
  */
 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;
+    const int rc = runtime_external_write(arr, elem, (uint8_t)DBGW_OP_WRITE, bytes, len);
+
+    /* Restoring happens once, at program load, before any task is released —
+     * so apply the write now instead of leaving it for the dispatcher's
+     * cycle-end drain. Left queued, scan 1 would run on the initial values and
+     * the restore would then overwrite what scan 1 wrote; and a program with
+     * more retained leaves than the queue holds would lose the rest. */
+    image_lock();
+    debug_write_journal_drain();
+    image_unlock();
+
+    return rc == 0 ? 0x7E : 0x82;
 }
 
 /**

core/src/plc_app/plc_retain.h

@@ -115,8 +115,12 @@ void plc_retain_init(void);
  * a machine starting from its defaults is recoverable, one starting from
  * plausible-looking garbage is not.
  *
+ * The values are applied before this returns (not left for the dispatcher's
+ * cycle-end drain), so scan 1 sees them.
+ *
  * Safe and cheap when nothing is retained or no driver claimed the store, and
- * idempotent: call once per program start, after plc_retain_init().
+ * idempotent: call once per program start, after plc_retain_init() and
+ * journal_init(), before any task is released.
  */
 void plc_retain_read(void);
 

core/src/plc_app/plc_retain_file_store.cpp

@@ -11,6 +11,7 @@
 #include "plc_retain.h"  // PLC_RETAIN_PROGRAM_ID_LEN — one definition for both sides
 
 #include <atomic>
+#include <chrono>
 #include <mutex>
 #include <string>
 #include <thread>
@@ -55,7 +56,15 @@ bool                 g_dirty = false;
 std::string          g_program_md5;
 
 std::string       g_path;
-int               g_flush_seconds = 5;
+int               g_flush_seconds = 10;
+
+/* When the last commit happened, and whether one has happened since start. A
+ * change after a quiet period is committed at once; changes that follow within
+ * g_flush_seconds are held and committed, latest values only, when that period
+ * ends. So a setpoint is on disk straight away, and a value that keeps changing
+ * costs at most one write per period. Touched by the flusher thread only. */
+std::chrono::steady_clock::time_point g_last_commit;
+bool                                  g_committed_once = false;
 std::atomic<bool> g_enabled{false};
 std::atomic<bool> g_running{false};
 std::thread       g_flusher;
@@ -77,7 +86,7 @@ void read_config(const char *config_path)
 {
     g_enabled.store(false);
     g_path.clear();
-    g_flush_seconds = 5;
+    g_flush_seconds = 10;
 
     FILE *f = fopen(config_path, "r");
     if (!f) return;
@@ -207,14 +216,17 @@ void discard_stored()
 
 void flush_loop()
 {
+    g_committed_once = false;
     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));
-        }
+        /* A tenth of a second: how long a change after a quiet period waits,
+         * and how promptly a stop is noticed. */
+        std::this_thread::sleep_for(std::chrono::milliseconds(100));
         if (!g_running.load()) break;
 
+        const auto now = std::chrono::steady_clock::now();
+        if (g_committed_once && now - g_last_commit < std::chrono::seconds(g_flush_seconds)) continue;
+
         std::vector<uint8_t> snapshot;
         std::string          snapshot_id;
         {
@@ -228,7 +240,12 @@ void flush_loop()
             snapshot_id = g_program_md5;
             g_dirty     = false;
         }
-        if (!snapshot.empty()) commit(snapshot.data(), (uint16_t)snapshot.size(), snapshot_id);
+        if (!snapshot.empty())
+        {
+            commit(snapshot.data(), (uint16_t)snapshot.size(), snapshot_id);
+            g_last_commit    = std::chrono::steady_clock::now();
+            g_committed_once = true;
+        }
     }
 }
 
@@ -242,7 +259,8 @@ bool plc_retain_file_store_start(const char *config_path)
 
     g_running.store(true);
     g_flusher = std::thread(flush_loop);
-    log_info("Retain: built-in file store enabled — %s, flushing every %ds",
+    log_info("Retain: built-in file store enabled — %s, a change is saved at once, "
+             "then at most every %ds while values keep changing",
              g_path.c_str(), g_flush_seconds);
     return true;
 }
@@ -322,11 +340,40 @@ int plc_retain_file_store_load(const char *program_md5, uint16_t md5_len, uint8_
 
     if (memcmp(stored_id, program_md5, PROGRAM_ID_LEN) != 0)
     {
-        fclose(f);
-        discard_stored();
-        log_info("Retain: stored values belong to a different program — storage cleared, "
-                 "retained variables start at their initial values");
-        return 0;
+        /* NOT discarded. Offered upward, and let the layout decide.
+         *
+         * PROGRAM_ID is the MD5 of program.st, so it changes on ANY edit —
+         * move a rung, rename a comment — and discarding here meant every
+         * retained value in a commissioned plant reset on every upload. That
+         * is the one guarantee the RETAIN qualifier exists to give:
+         * IEC 61131-3 §6.5.6.1 rule 1 defines a retained value as "the values
+         * the variables had when the resource or configuration was stopped",
+         * conditioned on the STARTING OPERATION being a warm restart and on
+         * nothing else. The words "download", "reload" and "online change"
+         * appear nowhere in Part 3.
+         *
+         * The check that belongs here already exists one layer up and was
+         * built for exactly this: ext_strucpp_retain_unpack() validates magic,
+         * format, crc, TRUNCATION and the LAYOUT HASH before a byte reaches a
+         * variable, and plc_retain.cpp names which of those failed. STruC++'s
+         * own debug-table-gen.ts says why it hashes the layout rather than the
+         * program: "a body edit leaves this unchanged and retained values
+         * survive, while adding, removing, retyping or reordering a retained
+         * variable changes it and the stored blob is refused. The project MD5
+         * would have discarded retained state on every unrelated edit."
+         *
+         * So this gate was not reinforcing that design, it was defeating it —
+         * and ModBee's ESP32 store (NodeUioRetain.cpp) keeps the identity for
+         * saves and never compares it on read, which is why retain has been
+         * seen surviving program changes on real hardware and not here.
+         *
+         * WHAT IS GIVEN UP, stated plainly for the review: two DIFFERENT
+         * projects sharing one retain.bin path whose layout hashes happen to
+         * collide would now read each other's values. That is a 32-bit
+         * collision on a path nobody takes, weighed against a certainty on the
+         * path everybody takes. */
+        log_info("Retain: stored values were written by a different program — "
+                 "keeping them; the layout check decides whether they fit");
     }
 
     const size_t n = fread(out, 1, cap, f);

core/src/plc_app/plc_retain_file_store.h

@@ -34,11 +34,14 @@
  *
  *     enabled=1
  *     path=/var/lib/openplc-runtime/retain.bin
- *     flush_seconds=5
+ *     flush_seconds=10
  *
- * `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.
+ * Nothing is written while nothing changes. A change after a quiet period is
+ * committed at once; changes that follow within `flush_seconds` are held and
+ * committed, latest values only, when that period ends. So a setpoint is kept
+ * straight away and `flush_seconds` bounds how hard a value that keeps changing
+ * works the storage — and how much of such a value a power cut can cost. It
+ * belongs to whoever installs the machine, which is why it is configuration.
  */
 
 #ifndef PLC_RETAIN_FILE_STORE_H

core/src/plc_app/plc_state_manager.cpp

@@ -443,18 +443,6 @@ 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,
-     * which driver will hold the bytes — and read() asks that driver for what it
-     * has for THIS program, which is also where a driver discards a previous
-     * program's values. 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, and read() lands before the first task is released, so a new
-     * program never runs a scan on the old one's state. */
-    plc_retain_init();
-    plc_retain_read();
-
     journal_buffer_ptrs_t journal_ptrs = {
         .bool_input   = bool_input,
         .bool_output  = bool_output,
@@ -482,6 +470,18 @@ void *plc_cycle_thread(void *arg)
         log_info("Journal buffer initialized");
     }
 
+    /* Retained variables. init() decides once whether retain can run here —
+     * does the .so export the entry points, does the program retain anything,
+     * which driver will hold the bytes — and read() asks that driver for what it
+     * has for THIS program, which is also where a driver discards a previous
+     * program's values. Both must follow the located-variable binding and
+     * journal_init() above: a retained variable may also be located, its image
+     * slot has to exist, and its restore goes through the image journal. Both
+     * are no-ops when retain is not in play. read() applies every value before
+     * the first task is released, so scan 1 already sees the retained state. */
+    plc_retain_init();
+    plc_retain_read();
+
     if (plugin_driver)
     {
         plugin_driver_start(plugin_driver);

scripts/Makefile.strucpp

@@ -5,11 +5,15 @@
 #
 #   - Per-file compilation rules let `make -j` saturate every core on
 #     the target platform (Pi 4 = 4×, x86 servers = 8–32×).
-#   - `wildcard $(GENERATED_DIR)/*.cpp` discovers however many .cpp
-#     files STruC++ split into. The codegen emits one TU per POU plus
-#     a shared configuration.cpp, so the file set varies per project
-#     — keeping the list out of this file means no Makefile churn
-#     when POUs are added or removed.
+#   - Sources are discovered by searching $(GENERATED_DIR), so the
+#     file set varies per project with no Makefile churn when POUs
+#     are added or removed. The codegen emits one TU per POU plus a
+#     shared configuration.cpp.
+#   - A project can also carry C++ libraries under
+#     $(GENERATED_DIR)/libraries/<name>/, each an ordinary library
+#     folder with its sources under src/. Every such src/ goes on the
+#     include path and its sources are compiled, so a block resolves
+#     `#include <Foo.h>` the same way it does on Arduino.
 #   - `ccache` is picked up automatically when present. Re-running a
 #     build where only one POU's body changed reuses every other .o
 #     from the cache, so incremental builds drop from minutes to a
@@ -79,9 +83,48 @@ CXX_BIN := g++
 # preserve. Verified on an SLM-RP4: with the flag the .so leaves
 # /proc/<pid>/maps on every stop and re-maps fresh on every start; without it,
 # it never leaves -- one build's image stays pinned for the life of the process.
+comma := ,
+
+# Libraries the editor materialised from enabled .stlib archives. Each is an
+# ordinary library folder, so `src/` is its include root; a folder without one
+# is taken as its own root.
+RESOURCE_LIB_DIRS := $(wildcard $(GENERATED_DIR)/libraries/*)
+RESOURCE_LIB_INC  := $(foreach d,$(RESOURCE_LIB_DIRS),$(if $(wildcard $d/src),$d/src,$d))
+
+# A LIBRARY MAY NAME THE DEFINES ITS OWN SOURCES NEED.
+#
+# `defines=A,B,C` in a library's library.properties becomes -DA -DB -DC. The
+# key is ours; arduino-cli ignores keys it does not know, so the same file
+# stays valid as an Arduino library descriptor.
+#
+# WHY IT IS NEEDED, with the case that found it. A vendored C library is very
+# often configured by a header it ships - wolfSSL is the example, and its own
+# documentation says "the WOLFSSL_USER_SETTINGS macro must be defined
+# project-wide", because its wolfcrypt/settings.h reads user_settings.h only
+# when that macro is set. Without it every source compiles with DEFAULT
+# features instead of the ones the library was vendored for. Nothing fails at
+# compile time: the objects are produced, the .so links (a shared object may
+# carry undefined symbols), and the failure arrives when the runtime dlopens
+# the program - `undefined symbol: wolfSSL_CTX_get_cert_store`, naming a
+# function whose absence has no obvious connection to a missing -D.
+#
+# This is the same gap as the .c discovery above, one layer along: the branch
+# taught the build where a vendored library's HEADERS are and then how to
+# compile its SOURCES, and a library that needs a define to configure those
+# sources still could not say so.
+#
+# Only from libraries/, and only as -D: a library cannot inject an include
+# path, a warning flag or an optimisation level into somebody else's build.
+RESOURCE_LIB_DEFS := $(strip $(foreach d,$(RESOURCE_LIB_DIRS),\
+    $(if $(wildcard $d/library.properties),\
+        $(addprefix -D,$(subst $(comma), ,\
+            $(patsubst defines=%,%,\
+                $(filter defines=%,$(shell tr -d '\r' < $d/library.properties))))),)))
+
 CXXFLAGS := -std=c++17 -O1 -pipe -fPIC -Wall -DSTRUCPP_THREADED -fno-gnu-unique -MMD -MP \
             -Wno-unknown-pragmas -Wno-deprecated-declarations \
-            -I $(GENERATED_DIR) -I $(RUNTIME_INC) -I $(PYTHON_INC)
+            -I $(GENERATED_DIR) -I $(RUNTIME_INC) -I $(PYTHON_INC) \
+            $(addprefix -I ,$(RESOURCE_LIB_INC)) $(RESOURCE_LIB_DEFS)
 
 # Python POU stubs (`{external …}` blocks emitted by the editor) call
 # `getpid()`, `create_shm_name()`, `python_block_loader()`. Those
@@ -91,6 +134,23 @@ CXXFLAGS := -std=c++17 -O1 -pipe -fPIC -Wall -DSTRUCPP_THREADED -fno-gnu-unique
 # any Python POU pay nothing; the header is declarations only.
 GENERATED_CXXFLAGS := $(CXXFLAGS) -include iec_python.h
 
+# And the same for a C source in a vendored library.
+#
+# Mirrored from CXXFLAGS rather than derived from it, deliberately. The C++
+# flags carry -std=c++17, -fno-gnu-unique, -DSTRUCPP_THREADED, the Python
+# include root and a force-include of a C++ header, none of which mean anything
+# to a C compiler - and subtracting them would break quietly the next time one
+# was added. What has to stay in step is the INCLUDE ROOTS, and those are
+# written out here so the next person editing either list can see both.
+#
+# No -std: the existing python_loader.c rule below does not set one either, and
+# a vendored C library that needs a particular dialect says so in its own
+# headers rather than having one imposed on it from here.
+GENERATED_CFLAGS := -O1 -pipe -fPIC -Wall -MMD -MP \
+                    -Wno-unknown-pragmas -Wno-deprecated-declarations \
+                    -I $(GENERATED_DIR) -I $(RUNTIME_INC) \
+                    $(addprefix -I ,$(RESOURCE_LIB_INC)) $(RESOURCE_LIB_DEFS)
+
 # ---------------------------------------------------------------------------
 # Retain capability probe.
 #
@@ -133,11 +193,37 @@ SHIM_HAS_RETAIN := $(strip $(if $(wildcard $(RUNTIME_INC)/iec_retain.hpp),\
 # the probe just measured, so they are already internally consistent.
 SHIM_CXXFLAGS := $(CXXFLAGS) $(if $(SHIM_HAS_RETAIN),-DSTRUCPP_SHIM_HAS_RETAIN,)
 
-# Discover every .cpp emitted into core/generated/. STruC++ splits
-# across configuration.cpp + one pou_<NAME>.cpp per POU, but this
-# Makefile doesn't care — wildcard adapts.
-GEN_CPP := $(wildcard $(GENERATED_DIR)/*.cpp)
-GEN_OBJ := $(patsubst $(GENERATED_DIR)/%.cpp,$(BUILD_DIR)/%.o,$(GEN_CPP))
+# Discover every source under core/generated/, at any depth: the top-level TUs
+# STruC++ emitted, plus whatever each resource library carries below its own
+# root. `sort` makes the order reproducible across filesystems and drops any
+# duplicate.
+#
+# BOTH LANGUAGES, and the .c half was missing. This search found only *.cpp and
+# the only pattern rule was for *.cpp, so every C source inside a vendored
+# resource library compiled to nothing - and a C library is what a library
+# vendors: TLS, compression, codecs and drivers are C almost without exception.
+# AND IT DID NOT FAIL LOUDLY, which is the worst part. A shared object is
+# allowed to carry undefined symbols, so the link SUCCEEDED and printed "Build
+# complete"; the editor reported a clean compile and the failure surfaced later
+# as `dlopen: undefined symbol` when the runtime tried to load the program. The
+# file had not failed to compile — it had never been offered to a compiler, and
+# nothing anywhere said so.
+#
+# This branch is where that gap came from. RESOURCE_LIB_DIRS and
+# RESOURCE_LIB_INC above taught the build where a vendored library's HEADERS
+# are; its sources were never added beside them. A vendored C library therefore
+# got its headers found and its sources ignored - the same feature, half
+# finished.
+GEN_CPP := $(sort $(shell find $(GENERATED_DIR) -name '*.cpp'))
+GEN_C   := $(sort $(shell find $(GENERATED_DIR) -name '*.c'))
+
+# One `sort` over the union as well as over each half: a library that shipped
+# foo.c and foo.cpp side by side would otherwise name $(BUILD_DIR)/foo.o twice.
+# Make still has to choose a rule for it, and chooses the .cpp one because it
+# is written first - deterministic, and a file layout to avoid rather than one
+# to support.
+GEN_OBJ := $(sort $(patsubst $(GENERATED_DIR)/%.cpp,$(BUILD_DIR)/%.o,$(GEN_CPP)) \
+                  $(patsubst $(GENERATED_DIR)/%.c,$(BUILD_DIR)/%.o,$(GEN_C)))
 
 SHIM_OBJ      := $(BUILD_DIR)/runtime_v4_entry.o
 PYTHON_OBJ    := $(if $(wildcard $(PYTHON_LOADER)),$(BUILD_DIR)/python_loader.o,)
@@ -174,10 +260,24 @@ $(LIBPLC): $(GEN_OBJ) $(SHIM_OBJ) $(EXTRA_OBJS) | $(BUILD_DIR)
 
 # Generated TUs from the editor's upload. Force-include iec_python.h
 # so unqualified Python loader symbols in {external} blocks resolve.
+# `$(@D)` because a nested source maps to a nested object: a library source at
+# libraries/foo/src/transport/bar.cpp builds to
+# $(BUILD_DIR)/libraries/foo/src/transport/bar.o.
 $(BUILD_DIR)/%.o: $(GENERATED_DIR)/%.cpp | $(BUILD_DIR)
 	@echo "[INFO] Compiling $<..."
+	@mkdir -p $(@D)
 	$(CXX) $(GENERATED_CXXFLAGS) -c $< -o $@
 
+# The same, for a C source a resource library brought with it. Not the C++
+# compiler with a different flag set: a C library compiled as C++ fails on
+# things that are legal C and not legal C++ - an implicit void* conversion, a
+# variable called `new`, a designated initialiser out of order - and it fails
+# deep inside somebody else's code where it reads as their bug.
+$(BUILD_DIR)/%.o: $(GENERATED_DIR)/%.c | $(BUILD_DIR)
+	@echo "[INFO] Compiling $<..."
+	@mkdir -p $(@D)
+	$(CC) $(GENERATED_CFLAGS) -c $< -o $@
+
 # Static runtime shim — lives in the runtime repo, not in the user
 # upload. Doesn't get the iec_python.h force-include.
 #