fix(daemon)+fix(client): close four fool-proofing gaps

C++ (equipment_service.hpp):
- ReportSubstrate ARRIVED: check CreateResult and return INVALID_OBJECT
  when the substrate ID already exists, instead of silently doing nothing
- ReportSubstrate/ReportModule default switch branches: return false
  (→ CANNOT_DO_NOW) for unknown enum values instead of silently accepting

Python (_client.py):
- @eq.command: raise NameError (client-side name validation) instead of
  SecsGemError (which means "daemon declined a request") — wrong type
- Module docstring: update example to show @eq.command / eq.names API

Test (test_daemon_service.cpp):
- Add duplicate-ARRIVED check (expects INVALID_OBJECT)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-06-26 21:48:29 +02:00
parent a2ebbf7c65
commit d22bbc4ab2
3 changed files with 11 additions and 9 deletions
+5 -6
View File
@@ -10,11 +10,12 @@ equipment.yaml; values are plain Python.
eq = Equipment("localhost:50051") eq = Equipment("localhost:50051")
eq.set(ChamberPressure=2.5) eq.set(ChamberPressure=2.5)
eq.fire("ProcessStarted") eq.fire(eq.names.event.ProcessStarted) # typo-safe; plain strings also work
@eq.on("START") @eq.command # function name IS the RCMD name,
def start(cmd): def START(cmd): # validated against equipment at import
run_recipe(cmd.params.get("PPID")) run_recipe(cmd.params.get("PPID"))
eq.fire(eq.names.event.ProcessStarted)
eq.listen() # blocks, dispatching host commands to your handlers eq.listen() # blocks, dispatching host commands to your handlers
""" """
@@ -238,9 +239,7 @@ class Equipment:
if name not in self.names.command: if name not in self.names.command:
close = difflib.get_close_matches(name, dir(self.names.command), n=3) close = difflib.get_close_matches(name, dir(self.names.command), n=3)
hint = f" Did you mean {', '.join(close)}?" if close else "" hint = f" Did you mean {', '.join(close)}?" if close else ""
raise SecsGemError( raise NameError(f"no host command '{name}' to bind @eq.command to.{hint}")
pb.Ack.PARAMETER_INVALID,
f"no host command '{name}' to bind @eq.command to.{hint}")
self._handlers[name] = fn self._handlers[name] = fn
return fn return fn
+4 -3
View File
@@ -536,7 +536,8 @@ class EquipmentService final : public pb::Equipment::Service {
auto outcome = rt_.read_sync([this, sid, cid, slot, m]() -> std::optional<bool> { auto outcome = rt_.read_sync([this, sid, cid, slot, m]() -> std::optional<bool> {
auto& subs = rt_.model().substrates; auto& subs = rt_.model().substrates;
if (m == pb::SubstrateReport::ARRIVED) { if (m == pb::SubstrateReport::ARRIVED) {
subs.create(sid, cid, slot); // AtSource / NeedsProcessing if (subs.create(sid, cid, slot) == gem::SubstrateStore::CreateResult::Denied_AlreadyExists)
return std::nullopt; // → INVALID_OBJECT: duplicate substrate ID
return true; return true;
} }
if (!subs.has(sid)) return std::nullopt; if (!subs.has(sid)) return std::nullopt;
@@ -550,7 +551,7 @@ class EquipmentService final : public pb::Equipment::Service {
case pb::SubstrateReport::PROCESSED: case pb::SubstrateReport::PROCESSED:
return subs.fire_processing_event(sid, gem::SubstrateProcessingEvent::EndProcessing); return subs.fire_processing_event(sid, gem::SubstrateProcessingEvent::EndProcessing);
default: default:
return true; return false; // unknown milestone
} }
}); });
ack_from_outcome(outcome, resp, "substrate '" + sid + "'", ack_from_outcome(outcome, resp, "substrate '" + sid + "'",
@@ -575,7 +576,7 @@ class EquipmentService final : public pb::Equipment::Service {
case pb::ModuleReport::NOT_EXECUTING: case pb::ModuleReport::NOT_EXECUTING:
return mods.fire(mid, gem::ModuleEvent::Reset); return mods.fire(mid, gem::ModuleEvent::Reset);
default: default:
return true; return false; // unknown state
} }
}); });
// Module auto-creates, so "not found" can't happen; only the FSM verdict. // Module auto-creates, so "not found" can't happen; only the FSM verdict.
+2
View File
@@ -774,6 +774,8 @@ TEST_CASE("ReportSubstrate (E90) and ReportModule (E157) drive the FSMs") {
// Reporting on a substrate that never ARRIVED is rejected. // Reporting on a substrate that never ARRIVED is rejected.
CHECK(sub("WFR-GHOST", pb::SubstrateReport::AT_WORK) == pb::Ack::INVALID_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);
auto mod = [&](const std::string& mid, pb::ModuleReport::State st) { auto mod = [&](const std::string& mid, pb::ModuleReport::State st) {
grpc::ClientContext ctx; grpc::ClientContext ctx;