From ef71f95a642ee6370836dba8e463162aadc27000 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 8 Oct 2026 20:53:58 +0200 Subject: [PATCH 1/5] feat(estop): add latching switch mode and persist active state Latching mode clears the E-Stop when the switch is disengaged instead of requiring the hold-to-clear press; a software triggered or restored E-Stop needs the switch cycled. The active state is persisted on every change and restored on boot, and jsonconfig import never changes it. Switch type is configured over serial (estop latching), as the portal does not expose E-Stop settings. --- .../host_test/main/config_test_helpers.cpp | 4 +- .../config/host_test/main/test_corrupt.cpp | 10 +-- .../config/host_test/main/test_defaults.cpp | 8 +- .../config/host_test/main/test_legacy.cpp | 9 +- .../config/host_test/main/test_roundtrip.cpp | 27 ++++++ components/config/include/config/Config.h | 4 + .../config/include/config/EStopConfig.h | 4 +- components/config/src/Config.cpp | 42 +++++++++ components/config/src/EStopConfig.cpp | 38 +++++++- .../main/test_estop_state_machine.cpp | 89 +++++++++++++++++++ .../include/estop/EStopManager.h | 3 +- .../device_control/src/EStopManager.cpp | 83 +++++++++++++++-- .../src/estop/EStopStateMachine.h | 54 ++++++++--- .../src/command_handlers/estop.cpp | 31 +++++++ frontend/src/lib/mappers/ConfigMapper.test.ts | 2 +- frontend/src/lib/mappers/ConfigMapper.ts | 6 ++ scripts/gen_env_header.py | 1 + test/host_support/include/openshock_board.h | 1 + 18 files changed, 383 insertions(+), 33 deletions(-) diff --git a/components/config/host_test/main/config_test_helpers.cpp b/components/config/host_test/main/config_test_helpers.cpp index fc29eab2..ef03003b 100644 --- a/components/config/host_test/main/config_test_helpers.cpp +++ b/components/config/host_test/main/config_test_helpers.cpp @@ -24,7 +24,7 @@ namespace ConfigTest { config.backend = Config::BackendConfig("api.example.org", "tok_0123456789abcdef"); config.serialInput = Config::SerialInputConfig(false); config.otaUpdate = Config::OtaUpdateConfig(false, "fw.example.org", OtaUpdateChannel::Develop, true, true, 120, false, true, 4242, OtaUpdateStep::Validating); - config.estop = Config::EStopConfig(true, GPIO_NUM_13); + config.estop = Config::EStopConfig(true, GPIO_NUM_13, true, true); return config; } @@ -130,5 +130,7 @@ namespace ConfigTest { TEST_ASSERT_EQUAL_MESSAGE(expected.estop.enabled, actual.estop.enabled, "estop.enabled"); TEST_ASSERT_EQUAL_INT_MESSAGE(expected.estop.gpioPin, actual.estop.gpioPin, "estop.gpioPin"); + TEST_ASSERT_EQUAL_MESSAGE(expected.estop.latching, actual.estop.latching, "estop.latching"); + TEST_ASSERT_EQUAL_MESSAGE(expected.estop.active, actual.estop.active, "estop.active"); } } // namespace ConfigTest diff --git a/components/config/host_test/main/test_corrupt.cpp b/components/config/host_test/main/test_corrupt.cpp index 961edcd6..84593969 100644 --- a/components/config/host_test/main/test_corrupt.cpp +++ b/components/config/host_test/main/test_corrupt.cpp @@ -250,10 +250,10 @@ TEST_CASE("An unknown OTA channel in JSON falls back to the default, not the run TEST_ASSERT_EQUAL_UINT8(static_cast(OtaUpdateStep::None), static_cast(loaded.otaUpdate.updateStep)); } -TEST_CASE("Schema fields the firmware does not model are dropped on save", "[config][corrupt]") +TEST_CASE("E-Stop active and latching survive a save", "[config][corrupt]") { - // estop.active / estop.latching (and wifi AP/LAN fields) exist in the schema but nothing in the firmware reads - // them, so they are intentionally not round-tripped. This pins that behaviour. + // estop.active is restored on boot so a reboot can't clear an active E-Stop; dropping it on an unrelated save + // would silently release it. namespace Fbs = Serialization::Configuration; flatbuffers::FlatBufferBuilder builder; auto estop = Fbs::CreateEStopConfig(builder, false, -1, /*active=*/true, /*latching=*/true); @@ -265,8 +265,8 @@ TEST_CASE("Schema fields the firmware does not model are dropped on save", "[con const std::vector stored = StoredFile(); const auto* root = Fbs::GetHubConfig(stored.data()); TEST_ASSERT_NOT_NULL(root->estop()); - TEST_ASSERT_FALSE(root->estop()->active()); - TEST_ASSERT_FALSE(root->estop()->latching()); + TEST_ASSERT_TRUE(root->estop()->active()); + TEST_ASSERT_TRUE(root->estop()->latching()); } TEST_CASE("RemoveWiFiCredentials reports unknown IDs", "[config]") diff --git a/components/config/host_test/main/test_defaults.cpp b/components/config/host_test/main/test_defaults.cpp index d8c06156..4833ae24 100644 --- a/components/config/host_test/main/test_defaults.cpp +++ b/components/config/host_test/main/test_defaults.cpp @@ -171,10 +171,14 @@ TEST_CASE("jsonconfig set with an empty object resets to defaults", "[config][de { TEST_ASSERT_TRUE(InitFrom(Serialize(MakeSampleConfig()))); TEST_ASSERT_TRUE(Config::SaveFromJSON("{}")); - AssertConfigEqual(Config::RootConfig(), Snapshot()); + + // Except estop.active: an active E-Stop is never released by a JSON import (the sample's is active). + Config::RootConfig expected; + expected.estop.active = true; + AssertConfigEqual(expected, Snapshot()); TEST_ASSERT_TRUE(InitFrom(StoredFile())); - AssertConfigEqual(Config::RootConfig(), Snapshot()); + AssertConfigEqual(expected, Snapshot()); } TEST_CASE("jsonconfig set with partial sections fills in defaults", "[config][defaults][json]") diff --git a/components/config/host_test/main/test_legacy.cpp b/components/config/host_test/main/test_legacy.cpp index 52f35727..67b02704 100644 --- a/components/config/host_test/main/test_legacy.cpp +++ b/components/config/host_test/main/test_legacy.cpp @@ -235,7 +235,14 @@ TEST_CASE("jsonconfig set accepts JSON exported by 1.5.2 firmware", "[config][le TEST_CASE("jsonconfig get emits the same document develop did", "[config][legacy][json]") { TEST_ASSERT_TRUE(InitFrom(Legacy::kDevelopConfig, sizeof(Legacy::kDevelopConfig))); - TEST_ASSERT_EQUAL_STRING(kDevelopJson, Config::GetAsJSON(true).c_str()); + + // Same document plus the E-Stop's latching/active keys, which develop did not emit yet. + std::string expected = kDevelopJson; + const std::string oldEStop = R"("estop":{"enabled":true,"gpioPin":13})"; + const size_t pos = expected.find(oldEStop); + TEST_ASSERT_NOT_EQUAL(std::string::npos, pos); + expected.replace(pos, oldEStop.size(), R"("estop":{"enabled":true,"gpioPin":13,"latching":false,"active":false})"); + TEST_ASSERT_EQUAL_STRING(expected.c_str(), Config::GetAsJSON(true).c_str()); } TEST_CASE("Fields from a newer schema are tolerated on load", "[config][legacy]") diff --git a/components/config/host_test/main/test_roundtrip.cpp b/components/config/host_test/main/test_roundtrip.cpp index 1e115e69..f7c5c949 100644 --- a/components/config/host_test/main/test_roundtrip.cpp +++ b/components/config/host_test/main/test_roundtrip.cpp @@ -250,6 +250,8 @@ TEST_CASE("jsonconfig get/set round trip keeps every section", "[config][roundtr Config::RootConfig sample = MakeSampleConfig(); // Strings that need JSON escaping are covered separately below. sample.wifi.credentialsList[2] = Config::WiFiCredentials(7, "Office", "s3cret!", WIFI_AUTH_WPA3_PSK); + // estop.active is exported but never imported (see "jsonconfig never changes estop.active"). + sample.estop.active = false; TEST_ASSERT_TRUE(InitFrom(Serialize(sample))); const std::string json = Config::GetAsJSON(true); TEST_ASSERT_FALSE(json.empty()); @@ -266,6 +268,31 @@ TEST_CASE("jsonconfig get/set round trip keeps every section", "[config][roundtr TEST_ASSERT_EQUAL_STRING(json.c_str(), Config::GetAsJSON(true).c_str()); } +TEST_CASE("jsonconfig never changes estop.active", "[config][roundtrip][json]") +{ + // An active E-Stop is restored on boot; importing a config must not be a way to release it. + Config::RootConfig sample = MakeSampleConfig(); + TEST_ASSERT_TRUE(sample.estop.active); + TEST_ASSERT_TRUE(InitFrom(Serialize(sample))); + + TEST_ASSERT_TRUE(Config::SaveFromJSON(R"({"estop":{"enabled":true,"gpioPin":13,"latching":false,"active":false}})")); + bool active = false; + TEST_ASSERT_TRUE(Config::GetEStopActive(active)); + TEST_ASSERT_TRUE(active); + TEST_ASSERT_TRUE(InitFrom(StoredFile())); + TEST_ASSERT_TRUE(Config::GetEStopActive(active)); + TEST_ASSERT_TRUE(active); + + TEST_ASSERT_TRUE(Config::SaveFromJSON(R"({"rf":{"txPin":13}})")); // estop section missing + TEST_ASSERT_TRUE(Config::GetEStopActive(active)); + TEST_ASSERT_TRUE(active); + + InitDefault(); + TEST_ASSERT_TRUE(Config::SaveFromJSON(R"({"estop":{"active":true}})")); + TEST_ASSERT_TRUE(Config::GetEStopActive(active)); + TEST_ASSERT_FALSE(active); +} + TEST_CASE("jsonconfig round trip keeps strings that need escaping", "[config][roundtrip][json]") { Config::RootConfig sample = MakeSampleConfig(); diff --git a/components/config/include/config/Config.h b/components/config/include/config/Config.h index 4b09616f..b82e2d55 100644 --- a/components/config/include/config/Config.h +++ b/components/config/include/config/Config.h @@ -101,4 +101,8 @@ namespace OpenShock::Config { bool SetEStopEnabled(bool enabled); bool GetEStopGpioPin(gpio_num_t& out); bool SetEStopGpioPin(gpio_num_t gpioPin); + bool GetEStopLatching(bool& out); + bool SetEStopLatching(bool latching); + bool GetEStopActive(bool& out); + bool SetEStopActive(bool active); } // namespace OpenShock::Config diff --git a/components/config/include/config/EStopConfig.h b/components/config/include/config/EStopConfig.h index 7bec4f1c..b7181cda 100644 --- a/components/config/include/config/EStopConfig.h +++ b/components/config/include/config/EStopConfig.h @@ -5,13 +5,15 @@ #include namespace OpenShock::Config { - // The schema's `active` and `latching` fields are not modelled: nothing reads them, so they are dropped on save. struct EStopConfig : public ConfigBase { EStopConfig(); EStopConfig(bool enabled, gpio_num_t gpioPin); + EStopConfig(bool enabled, gpio_num_t gpioPin, bool latching, bool active); bool enabled; gpio_num_t gpioPin; + bool latching; // Latching switch (clears when disengaged) instead of a momentary button (hold to clear) + bool active; // E-Stop was active when last saved; restored on boot so a reboot can't clear it void ToDefault() override; diff --git a/components/config/src/Config.cpp b/components/config/src/Config.cpp index 71f46aed..664e4260 100644 --- a/components/config/src/Config.cpp +++ b/components/config/src/Config.cpp @@ -844,3 +844,45 @@ bool Config::SetEStopGpioPin(gpio_num_t gpioPin) return true; }); } + +bool Config::GetEStopLatching(bool& out) +{ + CONFIG_LOCK_READ(false); + + out = _configData.estop.latching; + + return true; +} + +bool Config::SetEStopLatching(bool latching) +{ + CONFIG_LOCK_WRITE(false); + + return mutateAndSave([&](Config::RootConfig& root) { + root.estop.latching = latching; + return true; + }); +} + +bool Config::GetEStopActive(bool& out) +{ + CONFIG_LOCK_READ(false); + + out = _configData.estop.active; + + return true; +} + +bool Config::SetEStopActive(bool active) +{ + CONFIG_LOCK_WRITE(false); + + if (_configData.estop.active == active) { + return true; + } + + return mutateAndSave([&](Config::RootConfig& root) { + root.estop.active = active; + return true; + }); +} diff --git a/components/config/src/EStopConfig.cpp b/components/config/src/EStopConfig.cpp index e6866be0..aff2752f 100644 --- a/components/config/src/EStopConfig.cpp +++ b/components/config/src/EStopConfig.cpp @@ -12,19 +12,30 @@ using namespace OpenShock::Config; EStopConfig::EStopConfig() : enabled(OpenShock::IsValidInputPin(OPENSHOCK_ESTOP_PIN)) , gpioPin(static_cast(OPENSHOCK_ESTOP_PIN)) + , latching(OPENSHOCK_ESTOP_LATCHING) + , active(false) { } EStopConfig::EStopConfig(bool enabled, gpio_num_t gpioPin) + : EStopConfig(enabled, gpioPin, OPENSHOCK_ESTOP_LATCHING, false) +{ +} + +EStopConfig::EStopConfig(bool enabled, gpio_num_t gpioPin, bool latching, bool active) : enabled(enabled) , gpioPin(gpioPin) + , latching(latching) + , active(active) { } void EStopConfig::ToDefault() { - enabled = OpenShock::IsValidInputPin(OPENSHOCK_ESTOP_PIN); - gpioPin = static_cast(OPENSHOCK_ESTOP_PIN); + enabled = OpenShock::IsValidInputPin(OPENSHOCK_ESTOP_PIN); + gpioPin = static_cast(OPENSHOCK_ESTOP_PIN); + latching = OPENSHOCK_ESTOP_LATCHING; + active = false; } bool EStopConfig::FromFlatbuffers(const Serialization::Configuration::EStopConfig* config) @@ -39,10 +50,14 @@ bool EStopConfig::FromFlatbuffers(const Serialization::Configuration::EStopConfi if (!Internal::Utils::FromIntGpioNum(gpioPin, config->gpio_pin())) { OS_LOGW(TAG, "Invalid E-Stop GPIO pin %d, using default", config->gpio_pin()); ToDefault(); + latching = config->latching(); + active = config->active(); // Never drop an active E-Stop, even when the rest of the section is unusable return true; } - enabled = config->enabled(); + enabled = config->enabled(); + latching = config->latching(); + active = config->active(); Normalize(); return true; @@ -50,13 +65,17 @@ bool EStopConfig::FromFlatbuffers(const Serialization::Configuration::EStopConfi flatbuffers::Offset EStopConfig::ToFlatbuffers(flatbuffers::FlatBufferBuilder& builder, bool withSensitiveData) const { - return Serialization::Configuration::CreateEStopConfig(builder, enabled, gpioPin); + return Serialization::Configuration::CreateEStopConfig(builder, enabled, gpioPin, active, latching); } +// `active` is owned by EStopManager (persisted on every state change, restored on boot). JSON import never touches it, +// so a jsonconfig can't release an active E-Stop across a reboot. bool EStopConfig::FromJSON(JSON::JsonView json) { if (!json.valid()) { + bool wasActive = active; ToDefault(); // Set to default if config is null + active = wasActive; return true; } @@ -77,6 +96,15 @@ bool EStopConfig::FromJSON(JSON::JsonView json) } else { enabled = OpenShock::IsValidInputPin(gpioPin); } + + if (JSON::JsonView latchingJson = json["latching"]; latchingJson.valid()) { + if (!latchingJson.tryGetBool(latching)) { + OS_LOGE(TAG, "Failed to parse latching"); + return false; + } + } else { + latching = OPENSHOCK_ESTOP_LATCHING; + } Normalize(); return true; @@ -96,5 +124,7 @@ void EStopConfig::ToJSON(json_gen_str_t* gen, const char* name, bool withSensiti JSON::objBegin(gen, name); json_gen_obj_set_bool(gen, "enabled", enabled); json_gen_obj_set_int(gen, "gpioPin", gpioPin); + json_gen_obj_set_bool(gen, "latching", latching); + json_gen_obj_set_bool(gen, "active", active); JSON::objEnd(gen, name); } diff --git a/components/device_control/host_test/main/test_estop_state_machine.cpp b/components/device_control/host_test/main/test_estop_state_machine.cpp index acf0def4..4d140065 100644 --- a/components/device_control/host_test/main/test_estop_state_machine.cpp +++ b/components/device_control/host_test/main/test_estop_state_machine.cpp @@ -16,6 +16,11 @@ namespace { const int64_t kSampleMs = 5; // EStopManager samples at 200 Hz struct Rig { + explicit Rig(bool latching = false) + : machine(latching) + { + } + EStopStateMachine machine; int64_t now = 1'000'000; @@ -335,3 +340,87 @@ TEST_CASE("EStop is e-stopped in every state except Idle", "[device_control][est TEST_ASSERT_TRUE(sawState[static_cast(EStopState::Active)]); TEST_ASSERT_TRUE(sawState[static_cast(EStopState::ActiveClearing)]); } + +// --------------------------------------------------------------------------- +// Latching mode: the switch stays engaged mechanically, so disengaging it clears the E-Stop. +// --------------------------------------------------------------------------- + +TEST_CASE("EStop latching: engaging activates, staying engaged never clears", "[device_control][estop][latching]") +{ + Rig rig(true); + TEST_ASSERT_TRUE(rig.machine.latching()); + + rig.sample(kPressed); + assertState(EStopState::Active, rig); + + rig.hold(kPressed, EStopStateMachine::kHoldToClearTime * 2); // no hold-to-clear in latching mode + assertState(EStopState::Active, rig); +} + +TEST_CASE("EStop latching: disengaging clears after the debounce", "[device_control][estop][latching]") +{ + Rig rig(true); + rig.sample(kPressed); + + rig.feed(kReleased, EStopStateMachine::kCheckCount - 1); + assertState(EStopState::Active, rig); + + rig.sample(kReleased); + assertState(EStopState::Idle, rig); +} + +TEST_CASE("EStop latching: rearm grace applies after clearing", "[device_control][estop][latching]") +{ + Rig rig(true); + rig.sample(kPressed); + rig.feed(kReleased, EStopStateMachine::kCheckCount); + assertState(EStopState::Idle, rig); + + rig.sample(kPressed); + assertState(EStopState::Idle, rig); // inside the grace window + + rig.hold(kPressed, EStopStateMachine::kRearmGraceTime); + assertState(EStopState::Active, rig); +} + +TEST_CASE("EStop latching: a software trigger needs the switch cycled to clear", "[device_control][estop][latching]") +{ + Rig rig(true); + rig.hold(kReleased, 100); + + rig.machine.Trigger(rig.now); + assertState(EStopState::Active, rig); + + // Switch already disengaged: staying disengaged must not clear it. + rig.hold(kReleased, 10'000); + assertState(EStopState::Active, rig); + + // Engage, then disengage. + rig.sample(kPressed); + assertState(EStopState::Active, rig); + rig.feed(kReleased, EStopStateMachine::kCheckCount); + assertState(EStopState::Idle, rig); +} + +TEST_CASE("EStop latching: a software trigger while engaged clears on disengage", "[device_control][estop][latching]") +{ + Rig rig(true); + rig.sample(kPressed); // engaged + + rig.machine.Trigger(rig.now); + rig.hold(kPressed, 100); // still engaged: seen engaged again, not yet disengaged + assertState(EStopState::Active, rig); + + rig.feed(kReleased, EStopStateMachine::kCheckCount); + assertState(EStopState::Idle, rig); +} + +TEST_CASE("EStop momentary is the default mode", "[device_control][estop]") +{ + Rig rig; + TEST_ASSERT_FALSE(rig.machine.latching()); + + rig.sample(kPressed); + rig.feed(kReleased, EStopStateMachine::kCheckCount * 4); + assertState(EStopState::Active, rig); // releasing alone never clears a momentary E-Stop +} diff --git a/components/device_control/include/estop/EStopManager.h b/components/device_control/include/estop/EStopManager.h index 16fc7688..5522337b 100644 --- a/components/device_control/include/estop/EStopManager.h +++ b/components/device_control/include/estop/EStopManager.h @@ -8,10 +8,11 @@ namespace OpenShock::EStopManager { [[nodiscard]] bool Init(); - // Both apply the change to the running E-Stop and then persist it to the config (like CommandHandler::SetRfTxPin), + // These apply the change to the running E-Stop and then persist it to the config (like CommandHandler::SetRfTxPin), // so callers don't have to keep the two in sync. They refuse while the E-Stop is active. bool SetEStopEnabled(bool enabled); bool SetEStopPin(gpio_num_t pin); + bool SetEStopLatching(bool latching); bool IsEStopped(); int64_t LastEStopped(); diff --git a/components/device_control/src/EStopManager.cpp b/components/device_control/src/EStopManager.cpp index f70d51b1..d46471ff 100644 --- a/components/device_control/src/EStopManager.cpp +++ b/components/device_control/src/EStopManager.cpp @@ -18,6 +18,7 @@ const char* const TAG = "EStopManager"; #include #include +#include #include #include @@ -32,6 +33,7 @@ static OpenShock::SimpleMutex s_estopMutex = {}; // Guarded via Mutex static ManagedTask s_estopTask(TAG, "EStop task"); static gpio_num_t s_estopPin = GPIO_NUM_NC; // Captured by value when the task starts +static bool s_estopLatching = false; // Captured by value when the task starts // Wrapped in atomics as they're read (or set via public methods) by Tasks potentially running on other cores. static std::atomic s_estopActivatedAt = 0; // When == 0, EStop not active. When != 0, EStop is active. @@ -58,12 +60,12 @@ static void estopmgr_publishState(EStopState state, EStopState& lastState, TickT static void estopmgr_latchWithoutTask(); // Samples the estop at a fixed rate and updates internal state + events -static void estopmgr_managerTask(gpio_num_t estopPin) +static void estopmgr_managerTask(gpio_num_t estopPin, bool latching) { - EStopStateMachine machine; + EStopStateMachine machine(latching); EStopState lastPublishedState = EStopState::Idle; - // Carry over an E-Stop latched while no task was running (software trigger); it must be cleared with the button. + // Carry over an E-Stop latched while no task was running (software trigger, or restored on boot); it must be cleared with the button. int64_t latchedAt = s_estopActivatedAt.load(std::memory_order_relaxed); if (latchedAt != 0) { machine.Trigger(latchedAt); @@ -163,9 +165,10 @@ static bool estopmgr_taskStart() s_estopPin = pin; } - // The pin is captured by value, so the task keeps sampling the pin it was started with even if s_estopPin changes. + // The pin and mode are captured by value, so the task keeps running with what it was started with even if the globals change. gpio_num_t pin = s_estopPin; - if (!s_estopTask.start(TAG, k_estopTaskStackSize, k_estopTaskPriority, 1, [pin] { estopmgr_managerTask(pin); })) { + bool latching = s_estopLatching; + if (!s_estopTask.start(TAG, k_estopTaskStackSize, k_estopTaskPriority, 1, [pin, latching] { estopmgr_managerTask(pin, latching); })) { OS_LOGE(TAG, "Failed to create EStop event handler task"); return false; } @@ -193,6 +196,21 @@ static bool estopmgr_taskStop() return true; } +// Persists the E-Stop active state so it survives a reboot. +// Runs on the event loop rather than the manager task so flash writes never stall sampling. +static void estopmgr_handleStateChange(void* event_handler_arg, esp_event_base_t event_base, int32_t event_id, void* event_data) +{ + (void)event_handler_arg; + (void)event_base; + (void)event_id; + + EStopState state = *static_cast(event_data); + + if (!OpenShock::Config::SetEStopActive(state != EStopState::Idle)) { + OS_LOGE(TAG, "Failed to persist EStop active state"); + } +} + bool EStopManager::Init() { if (s_estopInitialized) { @@ -206,11 +224,31 @@ bool EStopManager::Init() return false; } + OpenShock::ScopedLock lock__(&s_estopMutex); + + // Set regardless of enabled so a later SetEStopEnabled(true) uses the right mode. + s_estopLatching = cfg.latching; + + esp_err_t err = esp_event_handler_register(OPENSHOCK_EVENTS, OPENSHOCK_EVENT_ESTOP_STATE_CHANGED, estopmgr_handleStateChange, nullptr); + if (err != ESP_OK) { + OS_LOGE(TAG, "Failed to register EStop state change handler: %s", esp_err_to_name(err)); + return false; + } + if (!cfg.enabled) { + // A disabled E-Stop can't be active, drop any stale persisted state + if (cfg.active && !OpenShock::Config::SetEStopActive(false)) { + OS_LOGE(TAG, "Failed to clear persisted EStop active state"); + } return true; } - OpenShock::ScopedLock lock__(&s_estopMutex); + // Restore a persisted E-Stop first, so it stays latched even if the input can't be brought up below. + // The manager task carries it over and it must be cleared with the button. + if (cfg.active) { + OS_LOGW(TAG, "EStop was active before reboot, restoring active state"); + estopmgr_latchWithoutTask(); + } if (!estopmgr_configurePin(cfg.gpioPin)) { return false; @@ -299,6 +337,37 @@ bool EStopManager::SetEStopPin(gpio_num_t pin) return true; } +bool EStopManager::SetEStopLatching(bool latching) +{ + OpenShock::ScopedLock lock__(&s_estopMutex); + + if (s_estopLatching == latching) { + return true; + } + + // Restarting the task resets the activation, which would release an active EStop without clearing it. + if (EStopManager::IsEStopped()) { + OS_LOGW(TAG, "Refusing to change EStop switch type while EStop is active"); + return false; + } + + s_estopLatching = latching; + + // The task captures the mode on start, restart it to apply + if (s_estopTask.running()) { + if (!estopmgr_taskStop() || !estopmgr_taskStart()) { + return false; + } + } + + if (!Config::SetEStopLatching(latching)) { + OS_LOGE(TAG, "Failed to save EStop switch type to config"); + return false; + } + + return true; +} + bool EStopManager::IsEStopped() { return EStopManager::LastEStopped() != 0; @@ -315,7 +384,7 @@ int64_t EStopManager::LastEStopped() static void estopmgr_latchWithoutTask() { int64_t expected = 0; - if (!s_estopActivatedAt.compare_exchange_strong(expected, OpenShock::millis(), std::memory_order_relaxed)) { + if (!s_estopActivatedAt.compare_exchange_strong(expected, std::max(OpenShock::millis(), 1), std::memory_order_relaxed)) { // 0 means inactive return; // Already active } diff --git a/components/device_control/src/estop/EStopStateMachine.h b/components/device_control/src/estop/EStopStateMachine.h index cefe3b59..6fea4d15 100644 --- a/components/device_control/src/estop/EStopStateMachine.h +++ b/components/device_control/src/estop/EStopStateMachine.h @@ -8,6 +8,13 @@ namespace OpenShock { /// @brief The EStop debounce + hold-to-clear state machine, free of FreeRTOS/GPIO so it can be host tested. /// EStopManager's task feeds it one GPIO sample per tick (or a software trigger) and mirrors activatedAt() into /// the atomic that IsEStopped() reads. activatedAt() is non-zero in every state except Idle. + /// + /// Momentary mode (latching == false), for a momentary push button: + /// Idle -> press -> Active -> press and hold for kHoldToClearTime -> AwaitingRelease -> release -> Idle + /// + /// Latching mode (latching == true), for a switch that mechanically stays engaged (e.g. twist-to-release mushroom button): + /// Idle -> engage -> Active -> disengage -> Idle + /// A software triggered (or restored) E-Stop is cleared by cycling the switch: engage, then disengage. class EStopStateMachine { public: static constexpr int64_t kHoldToClearTime = 5000; @@ -17,6 +24,12 @@ namespace OpenShock { // Grace period after deactivation (prevents immediate re-trigger on release bounce/EMI) static constexpr int64_t kRearmGraceTime = 250; // tune as needed + explicit EStopStateMachine(bool latching = false) noexcept + : m_latching(latching) + { + } + + bool latching() const noexcept { return m_latching; } EStopState state() const noexcept { return m_state; } int64_t activatedAt() const noexcept { return m_activatedAt; } @@ -27,8 +40,9 @@ namespace OpenShock { m_activatedAt = now; } - m_state = EStopState::Active; - m_rearmBlocked = false; + m_state = EStopState::Active; + m_rearmBlocked = false; + m_switchEngaged = false; // Do not modify history/lastBtnState here; rely on physical button state // on subsequent samples. @@ -61,12 +75,22 @@ namespace OpenShock { } if (btnState) { - m_state = EStopState::Active; - m_activatedAt = now; + m_state = EStopState::Active; + m_activatedAt = now; + m_switchEngaged = true; } break; case EStopState::Active: + if (m_latching) { + if (btnState) { + m_switchEngaged = true; + } else if (m_switchEngaged) { // switch disengaged -> clear E-Stop + clear(now); + } + break; + } + // Once active, if the input gets pressed, start hold-to-clear timing. if (pressedEdge) { m_state = EStopState::ActiveClearing; @@ -85,12 +109,7 @@ namespace OpenShock { case EStopState::AwaitingRelease: if (!btnState) { // fully released -> clear E-Stop - m_state = EStopState::Idle; - m_activatedAt = 0; - - // Start grace period to prevent immediate re-trigger. - m_rearmBlocked = true; - m_rearmAt = now + kRearmGraceTime; + clear(now); } break; @@ -101,6 +120,18 @@ namespace OpenShock { } private: + void clear(int64_t now) noexcept + { + m_state = EStopState::Idle; + m_activatedAt = 0; + + // Start grace period to prevent immediate re-trigger. + m_rearmBlocked = true; + m_rearmAt = now + kRearmGraceTime; + } + + bool m_latching; + EStopState m_state = EStopState::Idle; int64_t m_activatedAt = 0; // When == 0, EStop not active. When != 0, EStop is active. @@ -114,5 +145,8 @@ namespace OpenShock { // Debounced button state: true == pressed, false == released bool m_lastBtnState = false; + + // Latching mode: whether the switch has been seen engaged since the E-Stop became active + bool m_switchEngaged = false; }; } // namespace OpenShock diff --git a/components/serial_console/src/command_handlers/estop.cpp b/components/serial_console/src/command_handlers/estop.cpp index db3d60fe..702265df 100644 --- a/components/serial_console/src/command_handlers/estop.cpp +++ b/components/serial_console/src/command_handlers/estop.cpp @@ -58,6 +58,33 @@ static void handleEStopPinCommand(std::string_view arg, bool isAutomated) SERPR_SUCCESS("Saved config"); } +static void handleEStopLatchingCommand(std::string_view arg, bool isAutomated) +{ + bool latching; + if (arg.empty()) { + if (!OpenShock::Config::GetEStopLatching(latching)) { + SERPR_ERROR("Failed to get EStop latching from config"); + return; + } + + // Get EStop latching + SERPR_RESPONSE("EStopLatching|%s", latching ? "true" : "false"); + return; + } + + if (!OpenShock::Convert::ToBool(arg, latching)) { + SERPR_ERROR("Invalid argument (must be a boolean)"); + return; + } + + if (!OpenShock::EStopManager::SetEStopLatching(latching)) { + SERPR_ERROR("Failed to set EStop latching (refused while the EStop is active)"); + return; + } + + SERPR_SUCCESS("Saved config"); +} + OpenShock::SerialCmds::CommandGroup OpenShock::SerialCmds::CommandHandlers::EStopHandler() { auto group = OpenShock::SerialCmds::CommandGroup("estop"sv); @@ -70,5 +97,9 @@ OpenShock::SerialCmds::CommandGroup OpenShock::SerialCmds::CommandHandlers::ESto auto& setPinCommand = group.addCommand("pin"sv, "Set the GPIO pin used for the E-Stop."sv, handleEStopPinCommand); setPinCommand.addArgument("pin"sv, "must be a number"sv, "4"sv); + group.addCommand("latching"sv, "Get whether the E-Stop is a latching switch."sv, handleEStopLatchingCommand); + auto& setLatchingCommand = group.addCommand("latching"sv, "Set whether the E-Stop is a latching switch (clears when released) or a momentary button (hold to clear)."sv, handleEStopLatchingCommand); + setLatchingCommand.addArgument("latching"sv, "must be a boolean"sv, "true"sv); + return group; } diff --git a/frontend/src/lib/mappers/ConfigMapper.test.ts b/frontend/src/lib/mappers/ConfigMapper.test.ts index 83eb4a4a..447a81bb 100644 --- a/frontend/src/lib/mappers/ConfigMapper.test.ts +++ b/frontend/src/lib/mappers/ConfigMapper.test.ts @@ -93,7 +93,7 @@ describe('mapConfig', () => { expect(config?.backend.domain).toBe('api.example.org'); expect(config?.otaUpdate.cdnDomain).toBe('fw.example.org'); expect(config?.otaUpdate.updateChannel).toBe(OtaUpdateChannel.Beta); - expect(config?.estop).toEqual({ enabled: true, gpioPin: 13 }); + expect(config?.estop).toEqual({ enabled: true, gpioPin: 13, latching: false, active: false }); }); it('accepts empty strings the hub can legitimately store', () => { diff --git a/frontend/src/lib/mappers/ConfigMapper.ts b/frontend/src/lib/mappers/ConfigMapper.ts index 3e38f3f1..0b4ce2b1 100644 --- a/frontend/src/lib/mappers/ConfigMapper.ts +++ b/frontend/src/lib/mappers/ConfigMapper.ts @@ -46,6 +46,8 @@ export interface OtaUpdateConfig { export interface EStopConfig { enabled: boolean; gpioPin: number; + latching: boolean; + active: boolean; } export interface Config { @@ -176,10 +178,14 @@ function mapEstopConfig(hubConfig: HubConfig): EStopConfig { const enabled = estop.enabled(); const gpioPin = estop.gpioPin(); + const latching = estop.latching(); + const active = estop.active(); return { enabled, gpioPin, + latching, + active, }; } diff --git a/scripts/gen_env_header.py b/scripts/gen_env_header.py index 30e22180..e3397377 100644 --- a/scripts/gen_env_header.py +++ b/scripts/gen_env_header.py @@ -59,6 +59,7 @@ BOARD_PINS: dict[str, tuple[str, int]] = { 'OPENSHOCK_RF_TX_GPIO': ('int', -1), 'OPENSHOCK_ESTOP_PIN': ('int', -1), + 'OPENSHOCK_ESTOP_LATCHING': ('bool', 0), 'OPENSHOCK_LED_GPIO': ('int', -1), 'OPENSHOCK_LED_WS2812B': ('int', -1), 'OPENSHOCK_LED_SWAP_RG_CHANNELS': ('bool', 0), diff --git a/test/host_support/include/openshock_board.h b/test/host_support/include/openshock_board.h index 0828624e..bb4b7235 100644 --- a/test/host_support/include/openshock_board.h +++ b/test/host_support/include/openshock_board.h @@ -4,6 +4,7 @@ // the real Chipset.h checks pins against come from this component's CMakeLists.txt. #pragma once +#define OPENSHOCK_ESTOP_LATCHING 0 #define OPENSHOCK_ESTOP_PIN -1 #define OPENSHOCK_FW_BOARD "HostTest" #define OPENSHOCK_FW_BOARD_HOSTTEST 1 From 95ee416bca210aba99d3cdcbc46615eac98c377d Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Tue, 3 Nov 2026 12:24:18 +0100 Subject: [PATCH 2/5] style(estop): put the latch comments on their own lines for clang-format --- components/device_control/src/EStopManager.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/components/device_control/src/EStopManager.cpp b/components/device_control/src/EStopManager.cpp index 937cd39e..242b793a 100644 --- a/components/device_control/src/EStopManager.cpp +++ b/components/device_control/src/EStopManager.cpp @@ -403,10 +403,11 @@ static void estopmgr_latchWithoutTask() { int64_t expected = 0; bool latched = s_estopActivatedAt.compare_exchange_strong(expected, std::max(OpenShock::millis(), 1)); // 0 means inactive - s_estopActive.store(true); // Active either way: just latched, or already was - s_externallyTriggered.store(false); // Taken over by the latch + // Active either way: just latched, or already was. The external trigger is taken over by the latch. + s_estopActive.store(true); + s_externallyTriggered.store(false); if (!latched) { - return; // Already active + return; // Already active } // Bounded: the latch itself is already in effect, the event only informs listeners From f423a4b575182bf985b3cf608b98c39faccf6aba Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 5 Nov 2026 14:08:35 +0100 Subject: [PATCH 3/5] fix(estop): keep the switch type when the E-Stop activates mid-change and always save it A press or software trigger landing while the task stops now keeps the old switch type, so the new one can't clear it without the old type's clearing. The switch type is saved even when the running one matches, so a retry after a failed save writes it. --- .../device_control/src/EStopManager.cpp | 47 ++++++++++++++----- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/components/device_control/src/EStopManager.cpp b/components/device_control/src/EStopManager.cpp index 4b79983a..4b432558 100644 --- a/components/device_control/src/EStopManager.cpp +++ b/components/device_control/src/EStopManager.cpp @@ -313,31 +313,40 @@ bool EStopManager::SetEStopPin(gpio_num_t pin) return true; } -bool EStopManager::SetEStopLatching(bool latching) +// Requires s_estopMutex. Restarts the task with the new switch type; refuses while the E-Stop is active. +static bool estopmgr_applyLatching(bool latching) { - OpenShock::ScopedLock lock__(&s_estopMutex); - - if (s_estopLatching == latching) { - return true; - } - - // Restarting the task resets the activation, which would release an active EStop without clearing it. + // An active E-Stop carried into the other switch type could be cleared without the way its own type clears it. if (EStopManager::IsEStopped()) { OS_LOGW(TAG, "Refusing to change EStop switch type while EStop is active"); return false; } - bool oldLatching = s_estopLatching; - bool wasRunning = s_estopTask.running(); + if (!s_estopTask.running()) { + s_estopLatching = latching; + return true; + } // The task captures the switch type when it starts, so restart it to apply the new one. - if (wasRunning && !estopmgr_taskStop()) { + if (!estopmgr_taskStop()) { return false; } - s_estopLatching = latching; + // A press or software trigger can activate the E-Stop while the task stops; keep the old switch type then. + if (EStopManager::IsEStopped()) { + OS_LOGW(TAG, "EStop activated while changing the switch type, keeping the old one"); + if (!estopmgr_taskStart()) { + // Task is gone and we can't bring it back, so the pin isn't sampled. The E-Stop stays latched until reboot. + OS_LOGE(TAG, "Failed to restart EStop task, latching EStop until reboot"); + estopmgr_latchWithoutTask(); + } + return false; + } - if (wasRunning && !estopmgr_taskStart()) { + bool oldLatching = s_estopLatching; + s_estopLatching = latching; + + if (!estopmgr_taskStart()) { // Task is gone and we can't bring it back, so the pin isn't sampled. Fail safe: latch until reboot, which restarts // the task with the switch type still in config. OS_LOGE(TAG, "Failed to restart EStop task, latching EStop until reboot"); @@ -346,6 +355,18 @@ bool EStopManager::SetEStopLatching(bool latching) return false; } + return true; +} + +bool EStopManager::SetEStopLatching(bool latching) +{ + OpenShock::ScopedLock lock__(&s_estopMutex); + + // Saved even when the running switch type already matches, so a retry after a failed save still writes it. + if (s_estopLatching != latching && !estopmgr_applyLatching(latching)) { + return false; + } + if (!Config::SetEStopLatching(latching)) { OS_LOGE(TAG, "Failed to save EStop switch type to config"); return false; From c22f8102315643f565c88a267a962f5250c2c94a Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 5 Nov 2026 14:08:35 +0100 Subject: [PATCH 4/5] style(scripts): ruff format test_stamp_board.py --- scripts/tests/test_stamp_board.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/tests/test_stamp_board.py b/scripts/tests/test_stamp_board.py index 484cc759..67263644 100644 --- a/scripts/tests/test_stamp_board.py +++ b/scripts/tests/test_stamp_board.py @@ -42,7 +42,9 @@ def test_fragment_defaults_to_no_pins(tmp_path): def test_fragment_sets_the_estop_latching_flag(tmp_path): - spec = boards.read_board_fragment(write_fragment(tmp_path, 'Twist', 'CONFIG_IDF_TARGET="esp32"\nOPENSHOCK_ESTOP_LATCHING=y\n')) + spec = boards.read_board_fragment( + write_fragment(tmp_path, 'Twist', 'CONFIG_IDF_TARGET="esp32"\nOPENSHOCK_ESTOP_LATCHING=y\n') + ) assert spec.estop_latching is True assert spec.flags == boards.FLAG_ESTOP_LATCHING From 0190b2c745534463b9efec88af2dc940ea756ab0 Mon Sep 17 00:00:00 2001 From: HeavenVR Date: Thu, 5 Nov 2026 14:59:23 +0100 Subject: [PATCH 5/5] ci: bump release-tool to v1.1.0 --- .github/workflows/check-changes.yml | 2 +- .github/workflows/ci-build.yml | 2 +- .github/workflows/release.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/check-changes.yml b/.github/workflows/check-changes.yml index 0930968d..d059f167 100644 --- a/.github/workflows/check-changes.yml +++ b/.github/workflows/check-changes.yml @@ -22,7 +22,7 @@ jobs: fetch-depth: 0 - name: Run change file check - uses: OpenShock/release-tool@50f3d4be9462cb69931d134fd4df2a4f5fcd1c5e # v1.0.1 + uses: OpenShock/release-tool@fa1f825330760c1a7d28a2b22c44c6f7607606a6 # v1.1.0 with: mode: check base-ref: ${{ github.event.pull_request.base.ref }} diff --git a/.github/workflows/ci-build.yml b/.github/workflows/ci-build.yml index d345f23f..9740a1b2 100644 --- a/.github/workflows/ci-build.yml +++ b/.github/workflows/ci-build.yml @@ -106,7 +106,7 @@ jobs: # The next version from .changes/; status mode works on every branch and creates no tag. - name: Compute next version id: relmeta - uses: OpenShock/release-tool@50f3d4be9462cb69931d134fd4df2a4f5fcd1c5e # v1.0.1 + uses: OpenShock/release-tool@fa1f825330760c1a7d28a2b22c44c6f7607606a6 # v1.1.0 with: mode: status diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e6f21634..df08587a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -51,7 +51,7 @@ jobs: - name: Run release tool id: meta - uses: OpenShock/release-tool@50f3d4be9462cb69931d134fd4df2a4f5fcd1c5e # v1.0.1 + uses: OpenShock/release-tool@fa1f825330760c1a7d28a2b22c44c6f7607606a6 # v1.1.0 with: mode: release output: release.json