Add loop-closure scheduling and shared-step rebind

cmake/flags.cmake

@@ -1,4 +1,10 @@
-set(CMAKE_CXX_STANDARD 20)
+# Honor a standard supplied by the toolchain (e.g. a consumer's Conan
+# cppstd); default to 20 only when none was set, so the package_id's
+# cppstd never disagrees with the objects actually built. Piper's own
+# code requires >= 20.
+if(NOT CMAKE_CXX_STANDARD)
+    set(CMAKE_CXX_STANDARD 20)
+endif()
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 

conan/all/conanfile.py

@@ -50,6 +50,14 @@ def configure(self):
     def layout(self):
         cmake_layout(self)
 
+    def package_id(self):
+        # The packaged public headers contain zero standard-conditional code
+        # (no __cplusplus / feature-test-macro branches), so one binary serves
+        # any consumer cppstd at or above the validate() floor. Drop cppstd
+        # from the id so a single binary is produced instead of Conan's
+        # compatibility fallback silently serving a lower-std binary.
+        del self.info.settings.compiler.cppstd
+
     def validate(self):
         if self.settings.compiler.get_safe("cppstd"):
             check_min_cppstd(self, 20)

engine/include/piper/engine/step.h

@@ -249,15 +249,17 @@ namespace piper::engine
             output<T>(name) = value;
         }
 
-        // Engine calls this exactly once per Step instance, just
-        // before declare_io(). A second call -- or any user-code call
-        // -- throws std::logic_error.
+        // Engine binds the step to its per-engine IoBlock: once at
+        // build() (just before declare_io()) and again before every
+        // compute() call at tick time. The per-tick rebind is what lets
+        // ONE live step instance serve several engines -- e.g. a
+        // hardware-singleton device step shared by two pipelines: each
+        // engine wires its own IoBlock, and input()/output() resolve
+        // against whichever engine is currently ticking. Engines are
+        // single-threaded and tick sequentially, so this is race-free;
+        // the cost is one pointer assignment per step per stage.
         void init(IoBlock& block)
         {
-            if (io_ != nullptr)
-            {
-                throw std::logic_error("Step::init: already initialized");
-            }
             io_ = &block;
         }
 

engine/src/engine.cc

@@ -323,6 +323,10 @@ namespace piper::engine
                         make_build_diagnostic(BuildDiagnostic::Kind::UnknownStageOnPin,
                                   "stage '" + s + "' is not declared on the graph",
                                   node.id, attr_name));
+                    // An undeclared pin stage silently zeroes pin_active for
+                    // every stage, dropping the edge from all ordering graphs
+                    // and disarming cycle detection -- fail the build instead.
+                    has_error = true;
                     return;
                 }
                 active.insert(static_cast<uint16_t>(idx));
@@ -375,6 +379,61 @@ namespace piper::engine
         }
 
         // ---- Per-stage topo sort (Kahn) ----
+        // A link orders a stage only when BOTH of its pins are active
+        // there -- a pin with explicit stages is active on those, else it
+        // inherits its node's home stage. Filtering by node membership
+        // alone would drag e.g. a write-staged back-link between two
+        // read+write nodes into the read stage and report a false cycle
+        // (see tests/engine/cross_stage_link-t.cc).
+        std::map<piper::NodeId, std::map<std::string, std::vector<uint16_t>>> pin_stages;
+        for (auto const& node : graph.nodes())
+        {
+            auto const home = stage_index_of(node.stage);
+            for (auto const& attr : node.attrs)
+            {
+                if (attr.role != piper::AttributeSpec::Role::Input
+                    and attr.role != piper::AttributeSpec::Role::Output)
+                {
+                    continue;
+                }
+                std::vector<uint16_t> active_on;
+                if (attr.stages.empty())
+                {
+                    if (home < stage_data_.size())
+                    {
+                        active_on.push_back(static_cast<uint16_t>(home));
+                    }
+                }
+                else
+                {
+                    for (auto const& sname : attr.stages)
+                    {
+                        auto const idx = stage_index_of(sname);
+                        if (idx < stage_data_.size())
+                        {
+                            active_on.push_back(static_cast<uint16_t>(idx));
+                        }
+                    }
+                }
+                std::sort(active_on.begin(), active_on.end());
+                pin_stages[node.id][attr.name] = std::move(active_on);
+            }
+        }
+        auto pin_active = [&pin_stages](piper::NodeId id, std::string const& attr, uint16_t s)
+        {
+            auto nit = pin_stages.find(id);
+            if (nit == pin_stages.end())
+            {
+                return false;
+            }
+            auto ait = nit->second.find(attr);
+            if (ait == nit->second.end())
+            {
+                return false;
+            }
+            return std::binary_search(ait->second.begin(), ait->second.end(), s);
+        };
+
         // Use ordered containers so the resulting tick order is
         // deterministic across runs and platforms.
         for (uint16_t s = 0; s < stage_data_.size(); ++s)
@@ -402,6 +461,11 @@ namespace piper::engine
                 {
                     continue;
                 }
+                if (not pin_active(link.from.node, link.from.attr, s)
+                    or not pin_active(link.to.node, link.to.attr, s))
+                {
+                    continue;
+                }
                 succ[link.from.node].push_back(link.to.node);
                 ++in_degree[link.to.node];
             }
@@ -546,6 +610,10 @@ namespace piper::engine
             {
                 continue;
             }
+            // Rebind before compute: a step instance may be shared by
+            // several engines (hardware singletons); input()/output()
+            // must resolve against THIS engine's wiring.
+            bit->second.step->init(bit->second);
             bit->second.step->compute(stage);
         }
     }

tests/engine/CMakeLists.txt

@@ -1,5 +1,6 @@
 add_executable(piper_engine_test
     build_smoke-t.cc
+    cross_stage_link-t.cc
     cycle-t.cc
     declare_io_failure-t.cc
     external_io-t.cc
@@ -8,6 +9,9 @@ add_executable(piper_engine_test
     mode_gating-t.cc
     motor_control_smoke-t.cc
     multi_stage_tick-t.cc
+    optional_input-t.cc
+    pin_stage_typo_cycle-t.cc
+    shared_step-t.cc
     single_stage_tick-t.cc
 )
 target_link_libraries(piper_engine_test PRIVATE

tests/engine/cross_stage_link-t.cc

@@ -0,0 +1,171 @@
+#include <gtest/gtest.h>
+
+#include "piper/attribute.h"
+#include "piper/graph.h"
+#include "piper/node_type.h"
+#include "piper/stage.h"
+
+#include "piper/engine/builtin_steps.h"
+#include "piper/engine/engine.h"
+#include "piper/engine/registry.h"
+#include "piper/engine/step.h"
+
+using piper::AttributeSpec;
+using piper::Graph;
+using piper::NodeType;
+using piper::PinRef;
+using piper::Point;
+using piper::engine::Engine;
+using piper::engine::hash_name;
+using piper::engine::Step;
+using piper::engine::StepRegistry;
+
+namespace piper_engine_test
+{
+    // A device-style step: publishes a measurement in "read" and consumes a
+    // command in "write" (per-pin stages) -- the Bus pattern from
+    // docs/v2_format.md, one physical device spanning two stages.
+    class MotorStep final : public Step
+    {
+    public:
+        static constexpr auto READ  = hash_name("read");
+        static constexpr auto WRITE = hash_name("write");
+
+        void declare_io() override
+        {
+            declare_output<double>("position", position_);
+            declare_input<double>("torque_cmd");
+        }
+
+        void compute(piper::engine::Stage current) override
+        {
+            if (current.id == READ)
+            {
+                position_ = ++revs_;
+            }
+            else if (current.id == WRITE)
+            {
+                applied_ = input<double>("torque_cmd");
+            }
+        }
+
+        double applied() const { return applied_; }
+
+    private:
+        double position_{0.0};
+        double revs_{0.0};
+        double applied_{0.0};
+    };
+
+    // A transform-style step: forwards the measurement in "read", emits the
+    // command in "control". Its torque output feeds BACK into the motor --
+    // the bidirectional cross-stage pair.
+    class TransformStep final : public Step
+    {
+    public:
+        static constexpr auto READ    = hash_name("read");
+        static constexpr auto CONTROL = hash_name("control");
+
+        void declare_io() override
+        {
+            declare_input<double>("position_in");
+            declare_output<double>("torque_out", torque_);
+        }
+
+        void compute(piper::engine::Stage current) override
+        {
+            if (current.id == READ)
+            {
+                seen_at_read_ = input<double>("position_in");
+            }
+            else if (current.id == CONTROL)
+            {
+                torque_ = seen_at_read_ * 2.0;
+            }
+        }
+
+        double seen_at_read() const { return seen_at_read_; }
+
+    private:
+        double torque_{0.0};
+        double seen_at_read_{0.0};
+    };
+
+    NodeType make_motor_meta()
+    {
+        NodeType nt;
+        nt.type       = "test_motor";
+        nt.category   = "test";
+        nt.attributes = {
+            { "position",   "double", AttributeSpec::Role::Output, "" },
+            { "torque_cmd", "double", AttributeSpec::Role::Input,  "" },
+        };
+        return nt;
+    }
+
+    NodeType make_transform_meta()
+    {
+        NodeType nt;
+        nt.type       = "test_transform";
+        nt.category   = "test";
+        nt.attributes = {
+            { "position_in", "double", AttributeSpec::Role::Input,  "" },
+            { "torque_out",  "double", AttributeSpec::Role::Output, "" },
+        };
+        return nt;
+    }
+}
+
+// motor.position[read] -> transform.position_in, and
+// transform.torque_out[control] -> motor.torque_cmd[write].
+// Both nodes are active in "read", but the back-link's pins are not: it must
+// order nothing there. Filtering links by node membership alone drags the
+// back-link into "read" and reports a false cycle.
+TEST(EngineBuild, CrossStageBackLinkIsNotACycle)
+{
+    Graph g;
+    g.add_stage(piper::Stage{ "read",    0xFFFFFFFFu });
+    g.add_stage(piper::Stage{ "control", 0xFFFFFFFFu });
+    g.add_stage(piper::Stage{ "write",   0xFFFFFFFFu });
+
+    // Transform first: with the forward link filtered away too, NodeId
+    // tie-breaking would run it before the motor in "read" and it would see
+    // a stale position -- so this order also guards the forward edge.
+    auto transform_id = g.add_node(piper_engine_test::make_transform_meta(),
+                                   "transform", "read", Point{ 1.0f, 0.0f });
+    auto motor_id     = g.add_node(piper_engine_test::make_motor_meta(),
+                                   "motor", "read", Point{ 0.0f, 0.0f });
+
+    ASSERT_TRUE(g.set_attr_stages(motor_id, "torque_cmd",
+                                  std::vector<std::string>{ "write" }));
+    ASSERT_TRUE(g.set_attr_stages(transform_id, "torque_out",
+                                  std::vector<std::string>{ "control" }));
+
+    g.add_link(PinRef{ motor_id, "position" },       PinRef{ transform_id, "position_in" }, "double");
+    g.add_link(PinRef{ transform_id, "torque_out" }, PinRef{ motor_id, "torque_cmd" },      "double");
+
+    StepRegistry sr;
+    piper::engine::register_builtin_steps(sr);
+    sr.add("test_motor",     [] { return std::make_shared<piper_engine_test::MotorStep>(); });
+    sr.add("test_transform", [] { return std::make_shared<piper_engine_test::TransformStep>(); });
+
+    Engine e;
+    auto res = e.build(g, sr);
+    ASSERT_TRUE(res.ok) << (res.diagnostics.empty() ? "" : res.diagnostics.front().message);
+
+    auto* motor     = dynamic_cast<piper_engine_test::MotorStep*>(e.step_for(motor_id));
+    auto* transform = dynamic_cast<piper_engine_test::TransformStep*>(e.step_for(transform_id));
+    ASSERT_NE(motor,     nullptr);
+    ASSERT_NE(transform, nullptr);
+
+    for (int i = 1; i <= 3; ++i)
+    {
+        e.tick("read");
+        // Forward link still orders "read": the transform sees THIS tick's
+        // measurement, not last tick's.
+        EXPECT_EQ(transform->seen_at_read(), static_cast<double>(i));
+        e.tick("control");
+        e.tick("write");
+        EXPECT_EQ(motor->applied(), 2.0 * i);
+    }
+}