From 2218b854ce490609dd89178043a0e017590ca8dc Mon Sep 17 00:00:00 2001 From: Raphael Maenle Date: Fri, 26 Jun 2026 22:01:20 +0200 Subject: [PATCH] fix(daemon)+test: accurate duplicate-ARRIVED message; broaden E90/E157 + names coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The duplicate-ARRIVED fix from the previous commit returned INVALID_OBJECT with the message "no substrate 'X'" — a lie, since the substrate exists. Rewrite ReportSubstrate so ARRIVED has its own ack mapping: a duplicate is CANNOT_DO_NOW with "substrate 'X' already exists" (a state conflict, not a missing object), and we never silently re-create over live FSM state. Coverage gaps closed: - C++: ARRIVED records carrier_id/slot (now asserted); module NOT_EXECUTING reset transition; duplicate-ARRIVED expects CANNOT_DO_NOW. - Interop: @eq.command now drives the real host S2F41 path (was @eq.on, so the headline decorator had zero wire coverage); @eq.command NameError on unknown name; eq.names var/alarm + dir() + typo-suggestion; replaced the two `check(..., True)` tautologies with full E90 journey + AT_DESTINATION and real error paths (ghost wafer raises, illegal module jump raises). All 8 daemon test cases (248 assertions) and 24 pyclient interop checks pass. Co-Authored-By: Claude Opus 4.8 --- include/secsgem/daemon/equipment_service.hpp | 28 ++++++-- interop/pyclient_interop.py | 68 +++++++++++++++----- tests/test_daemon_service.cpp | 42 ++++++++++-- 3 files changed, 110 insertions(+), 28 deletions(-) diff --git a/include/secsgem/daemon/equipment_service.hpp b/include/secsgem/daemon/equipment_service.hpp index 83c55ab..95ba8a4 100644 --- a/include/secsgem/daemon/equipment_service.hpp +++ b/include/secsgem/daemon/equipment_service.hpp @@ -533,13 +533,29 @@ class EquipmentService final : public pb::Equipment::Service { const std::string cid = req->carrier_id(); const auto slot = static_cast(req->slot()); const auto m = req->milestone(); - auto outcome = rt_.read_sync([this, sid, cid, slot, m]() -> std::optional { - auto& subs = rt_.model().substrates; - if (m == pb::SubstrateReport::ARRIVED) { - if (subs.create(sid, cid, slot) == gem::SubstrateStore::CreateResult::Denied_AlreadyExists) - return std::nullopt; // → INVALID_OBJECT: duplicate substrate ID - return true; + + // ARRIVED creates the substrate. A duplicate is a real conflict (re-creating + // would silently wipe the wafer's FSM state + history), so reject it with an + // accurate message rather than the generic "no such object" path. + if (m == pb::SubstrateReport::ARRIVED) { + auto created = rt_.read_sync([this, sid, cid, slot]() { + return rt_.model().substrates.create(sid, cid, slot) == + gem::SubstrateStore::CreateResult::Created; + }); + if (!created) { + resp->set_code(pb::Ack::CANNOT_DO_NOW); + resp->set_message("engine io thread did not answer (not running?)"); + } else if (!*created) { + resp->set_code(pb::Ack::CANNOT_DO_NOW); + resp->set_message("substrate '" + sid + "' already exists"); + } else { + resp->set_code(pb::Ack::ACCEPT); } + return grpc::Status::OK; + } + + auto outcome = rt_.read_sync([this, sid, m]() -> std::optional { + auto& subs = rt_.model().substrates; if (!subs.has(sid)) return std::nullopt; switch (m) { case pb::SubstrateReport::AT_WORK: diff --git a/interop/pyclient_interop.py b/interop/pyclient_interop.py index a0ee5c2..8bf8922 100644 --- a/interop/pyclient_interop.py +++ b/interop/pyclient_interop.py @@ -8,8 +8,11 @@ host actually receives on the wire: eq.set / eq["..."] -> S1F3-visible values, GetVariables round-trip eq.fire -> host receives S6F11 with the configured report eq.alarm / eq.clear -> host receives S5F1 set/clear - @eq.on + eq.listen -> host's S2F41 gets HCACK=4, handler runs, - completion event reaches the host + @eq.command + eq.listen -> host's S2F41 gets HCACK=4, the name-bound + handler runs, completion event reaches the host + eq.names.* -> Describe-backed, typo-safe name lookup + eq.report_substrate -> E90 wafer journey (+ unknown-wafer error path) + eq.report_module -> E157 module walk/reset (+ illegal-jump error path) eq.control_state / request_control_state / eq.health Exits 0 on success. @@ -50,10 +53,12 @@ def run(grpc_addr: str, hsms_host: str, hsms_port: int) -> int: started = threading.Event() - @eq.on("START") - def _start(cmd): # noqa: ANN001 + # @eq.command binds by function name, validated against the live equipment + # (Describe) at decoration time — this is the path the host's S2F41 drives. + @eq.command + def START(cmd): # noqa: ANN001 started.set() - eq.fire("ProcessStarted") # the host's real completion signal + eq.fire(eq.names.event.ProcessStarted) # typo-safe completion signal eq.listen(background=True) @@ -140,31 +145,60 @@ def run(grpc_addr: str, hsms_host: str, hsms_port: int) -> int: body = host.settings.streams_functions.decode(rsp).get() check("host S2F41 -> HCACK=4", isinstance(body, dict) and int(body.get("HCACK") or -1) == 4) - check("@eq.on('START') handler ran", started.wait(timeout=10)) + check("@eq.command('START') handler ran", started.wait(timeout=10)) check("handler's eq.fire reached the host (completion signal)", ceid300.wait(timeout=10)) - # ---- Describe-backed names + @eq.command binding ---- + # ---- Describe-backed names (every category, dir(), typo hint) ---- check("eq.names.event has ProcessStarted", "ProcessStarted" in eq.names.event) check("eq.names.command has START", "START" in eq.names.command) + check("eq.names.var has ChamberPressure", + "ChamberPressure" in eq.names.var) + check("eq.names.alarm has chiller_temp_high", + "chiller_temp_high" in eq.names.alarm) + check("dir(eq.names.event) lists names for autocomplete", + "ProcessStarted" in dir(eq.names.event)) try: - _ = eq.names.event.NoSuchEvent - check("typo on eq.names raises", False) - except AttributeError: - check("typo on eq.names raises", True) - eq.fire(eq.names.event.ProcessStarted) - check("eq.fire(eq.names.event.*) accepted", True) + _ = eq.names.event.ProcessStated # one-letter typo + check("typo on eq.names raises with suggestion", False) + except AttributeError as e: + check("typo on eq.names raises with suggestion", + "ProcessStarted" in str(e)) # close-match hint present - # ---- E90/E157 material tracking ---- - eq.report_substrate("WFR-PY-1", "ARRIVED") + # ---- @eq.command rejects an unknown command name at decoration time ---- + try: + @eq.command + def NOT_A_REAL_COMMAND(cmd): # noqa: ANN001 + pass + check("@eq.command on unknown name raises NameError", False) + except NameError: + check("@eq.command on unknown name raises NameError", True) + + # ---- E90 substrate tracking: full journey, then the error path ---- + eq.report_substrate("WFR-PY-1", "ARRIVED", carrier_id="FOUP-PY", slot=4) eq.report_substrate("WFR-PY-1", "AT_WORK") eq.report_substrate("WFR-PY-1", "PROCESSING") eq.report_substrate("WFR-PY-1", "PROCESSED") - check("report_substrate journey accepted", True) + eq.report_substrate("WFR-PY-1", "AT_DESTINATION") + check("report_substrate full journey accepted (no raise)", True) + try: + eq.report_substrate("WFR-GHOST", "AT_WORK") # never ARRIVED + check("report_substrate on unknown wafer raises", False) + except SecsGemError as e: + check("report_substrate on unknown wafer raises", "WFR-GHOST" in str(e)) + + # ---- E157 module tracking: walk and reset ---- eq.report_module("MOD-PY-1", "GENERAL_EXECUTING") eq.report_module("MOD-PY-1", "STEP_EXECUTING") - check("report_module accepted", True) + eq.report_module("MOD-PY-1", "STEP_COMPLETED") + eq.report_module("MOD-PY-1", "NOT_EXECUTING") + check("report_module walk + reset accepted (no raise)", True) + try: + eq.report_module("MOD-PY-2", "STEP_EXECUTING") # illegal from idle + check("report_module illegal jump raises", False) + except SecsGemError: + check("report_module illegal jump raises", True) # ---- operator offline via the client ---- eq.request_control_state("HOST_OFFLINE") diff --git a/tests/test_daemon_service.cpp b/tests/test_daemon_service.cpp index 5621cdc..d5d17cf 100644 --- a/tests/test_daemon_service.cpp +++ b/tests/test_daemon_service.cpp @@ -759,8 +759,30 @@ TEST_CASE("ReportSubstrate (E90) and ReportModule (E157) drive the FSMs") { return ack.code(); }; + auto sub_with = [&](const std::string& sid, pb::SubstrateReport::Milestone m, + const std::string& cid, uint32_t slot) { + grpc::ClientContext ctx; + pb::SubstrateReport req; + pb::Ack ack; + req.set_substrate_id(sid); + req.set_milestone(m); + req.set_carrier_id(cid); + req.set_slot(slot); + REQUIRE(stub->ReportSubstrate(&ctx, req, &ack).ok()); + return ack.code(); + }; + // A wafer's journey: arrive -> picked up -> process -> done -> deposited. - CHECK(sub("WFR-1", pb::SubstrateReport::ARRIVED) == pb::Ack::ACCEPT); + // ARRIVED also records where the wafer came from (carrier + slot). + CHECK(sub_with("WFR-1", pb::SubstrateReport::ARRIVED, "FOUP-7", 3) == pb::Ack::ACCEPT); + auto origin = rt.read_sync([&rt]() { + const auto* s = rt.model().substrates.get("WFR-1"); + return s ? std::make_pair(s->carrierid, s->slot) + : std::make_pair(std::string{}, uint8_t{0}); + }); + REQUIRE(origin.has_value()); + CHECK(origin->first == "FOUP-7"); + CHECK(static_cast(origin->second) == 3); CHECK(sub("WFR-1", pb::SubstrateReport::AT_WORK) == pb::Ack::ACCEPT); CHECK(sub("WFR-1", pb::SubstrateReport::PROCESSING) == pb::Ack::ACCEPT); CHECK(sub("WFR-1", pb::SubstrateReport::PROCESSED) == pb::Ack::ACCEPT); @@ -772,10 +794,11 @@ TEST_CASE("ReportSubstrate (E90) and ReportModule (E157) drive the FSMs") { REQUIRE(loc.has_value()); CHECK(*loc == gem::SubstrateState::AtDestination); - // Reporting on a substrate that never ARRIVED is rejected. + // Reporting on a substrate that never ARRIVED is rejected (no such object). CHECK(sub("WFR-GHOST", pb::SubstrateReport::AT_WORK) == pb::Ack::INVALID_OBJECT); - // Duplicate ARRIVED (same substrate ID already in the store) is rejected. - CHECK(sub("WFR-1", pb::SubstrateReport::ARRIVED) == pb::Ack::INVALID_OBJECT); + // Duplicate ARRIVED is a conflict, not a missing object: CANNOT_DO_NOW so we + // never silently wipe the existing wafer's state/history. + CHECK(sub("WFR-1", pb::SubstrateReport::ARRIVED) == pb::Ack::CANNOT_DO_NOW); auto mod = [&](const std::string& mid, pb::ModuleReport::State st) { grpc::ClientContext ctx; @@ -787,7 +810,7 @@ TEST_CASE("ReportSubstrate (E90) and ReportModule (E157) drive the FSMs") { return ack.code(); }; - // Module: auto-created, then walked General -> Step -> StepCompleted. + // Module: auto-created, then walked General -> Step -> StepCompleted -> Reset. CHECK(mod("MOD-1", pb::ModuleReport::GENERAL_EXECUTING) == pb::Ack::ACCEPT); CHECK(mod("MOD-1", pb::ModuleReport::STEP_EXECUTING) == pb::Ack::ACCEPT); CHECK(mod("MOD-1", pb::ModuleReport::STEP_COMPLETED) == pb::Ack::ACCEPT); @@ -798,6 +821,15 @@ TEST_CASE("ReportSubstrate (E90) and ReportModule (E157) drive the FSMs") { REQUIRE(mstate.has_value()); CHECK(*mstate == gem::ModuleState::StepCompleted); + // NOT_EXECUTING resets the module back to idle (re-usable for the next wafer). + CHECK(mod("MOD-1", pb::ModuleReport::NOT_EXECUTING) == pb::Ack::ACCEPT); + auto reset = rt.read_sync([&rt]() { + const auto* m = rt.model().modules.get("MOD-1"); + return m ? m->fsm->state() : gem::ModuleState::NoState; + }); + REQUIRE(reset.has_value()); + CHECK(*reset == gem::ModuleState::NotExecuting); + // An illegal jump (StepExecuting straight from a fresh NotExecuting module) // is rejected by the E157 table. CHECK(mod("MOD-2", pb::ModuleReport::STEP_EXECUTING) == pb::Ack::CANNOT_DO_NOW);