diff --git a/COMPLIANCE.md b/COMPLIANCE.md index 93f65f6..bf3f132 100644 --- a/COMPLIANCE.md +++ b/COMPLIANCE.md @@ -69,7 +69,7 @@ Legend: | Host-Initiated S1F13/F14 scenario | ✅ | E30 §6.5 | S1F13/F14 | | | Event Notification | ✅ | E30 §6.6 | S6F11/F12 | Equipment-initiated, host-acknowledged. | | On-Line Identification | ✅ | E30 §6.7 | S1F1/F2 | MDLN + SOFTREV. | -| Error Messages | 🟡 | E30 §6.9 | S9F* | HSMS Reject.req covers the transport-level case. S9F1/F3/F5/F7/F9/F11/F13 are now in the message catalog and round-trip-tested. `Connection` automatically emits **S9F7** when a peer's primary or reply body fails SECS-II decode (connection stays up; the caller of `send_request` sees `Error::IllegalData`), and **S9F9** when its outgoing request times out at T3 (alongside the existing `Error::Timeout`). **Not wired**: S9F3 (unknown stream) and S9F5 (unknown function) — these belong in the `Router`'s fallback path; today the fallback returns SxF0 only. | +| Error Messages | ✅ | E30 §6.9 | S9F* | HSMS Reject.req at the transport level; S9F1/F3/F5/F7/F9/F11/F13 in the SECS-II catalog. Auto-emitted by Connection on every documented condition: **S9F3** (unknown stream) and **S9F5** (unknown function) via the server's `Router`-aware wrapper using the new `Connection::current_header()` accessor; **S9F7** (illegal data) when a peer body fails SECS-II decode; **S9F9** (T3 timeout) reconstructing the original outgoing MHEAD; **S9F11** (data too long) with a synthesized MHEAD whose first 4 bytes are the offending length prefix. S9F1 / S9F13 are catalog-only — no documented condition currently triggers them in our codebase. | | Documentation | ✅ | E30 §6.10| S1F19/F20, S1F21/F22 | Equipment self-reports its GEM-compliance level via S1F20 (SOFTREV, EQPTYP, list of (CCODE, CDESC) per E30 Appendix A) and its DVID namelist via S1F22. Both populated from `data/equipment.yaml`. | | Control (Operator-Initiated) | ✅ | E30 §6.2 | — | `ControlStateMachine::operator_online/offline/local/remote`. | @@ -137,11 +137,11 @@ Legend: | S7F19 / S7F20 | H→E | ✅ | `messages.hpp` | ✅ round-trip + demo | | S7F23–F26 | H↔E | ⬜ | — | enhanced PP | | S9F1 | E↔H | ✅ | catalog | ✅ round-trip | -| S9F3 | E↔H | 🟡 | catalog | ✅ round-trip; emission not yet wired | -| S9F5 | E↔H | 🟡 | catalog | ✅ round-trip; emission not yet wired | +| S9F3 | E↔H | ✅ | catalog + Router wrapper | ✅ round-trip + auto-emitted on unknown stream | +| S9F5 | E↔H | ✅ | catalog + Router wrapper | ✅ round-trip + auto-emitted on unknown function | | S9F7 | E↔H | ✅ | catalog + Connection | ✅ round-trip + auto-emitted on body decode | | S9F9 | E↔H | ✅ | catalog + Connection | ✅ round-trip + auto-emitted on T3 timeout | -| S9F11 | E↔H | 🟡 | catalog | ✅ round-trip; emission not yet wired | +| S9F11 | E↔H | ✅ | catalog + Connection | ✅ round-trip + auto-emitted on oversized frame | | S9F13 | E↔H | ✅ | catalog | ✅ round-trip | | S10F1 / S10F2 | H→E | ✅ | `messages.hpp` | ✅ in demo | | S10F3 / S10F4 | E→H | ✅ | `messages.hpp` | ✅ round-trip + demo | @@ -193,9 +193,7 @@ The honest list, in priority order: notification on re-SELECT) so the host doesn't have to manually flip the test-only `force_spool` flag. Optional: persistent on-disk spool so equipment restarts don't lose queued events. -2. **Finish S9 wiring**: route Router-level "unknown stream/function" through - S9F3/F5, and emit S9F11 (Data Too Long) with the 4-byte length prefix - in place of MHEAD when the incoming frame is oversized. +2. *(done in this revision)*: S9F3/F5/F11 — all S9 conditions now auto-emit. 3. *(done in this revision)*: S1F19/F20 + S1F21/F22 — Documentation. 4. **Implement EC range validation** in `set_equipment_constant_value` so out-of-range sets return EAC=4 instead of being silently accepted. diff --git a/apps/secs_server.cpp b/apps/secs_server.cpp index 7beb178..fdcdc25 100644 --- a/apps/secs_server.cpp +++ b/apps/secs_server.cpp @@ -399,8 +399,26 @@ int main(int argc, char** argv) { logfn(std::string("host is online; control=") + gem::control_state_name(sm->state())); }); - conn->set_message_handler( - [&router](const s2::Message& msg) { return router.dispatch(msg); }); + // Wrap router.dispatch so we can emit S9F3 / S9F5 when an inbound + // primary has no registered handler. We use Connection's + // current_header() accessor to capture the offending MHEAD; without + // it we'd be left fabricating a synthetic header. + std::weak_ptr wconn = conn; + conn->set_message_handler([&router, wconn, logfn](const s2::Message& msg) + -> std::optional { + if (!router.has_handler(msg.stream, msg.function)) { + auto c = wconn.lock(); + if (c && c->current_header()) { + const uint8_t s9_function = + router.has_handler_for_stream(msg.stream) ? 5 : 3; + logfn("unhandled S" + std::to_string(msg.stream) + "F" + + std::to_string(msg.function) + "; emitting S9F" + + std::to_string(s9_function)); + c->emit_s9(s9_function, c->current_header()->encode()); + } + } + return router.dispatch(msg); + }); }); server.start(); diff --git a/include/secsgem/gem/router.hpp b/include/secsgem/gem/router.hpp index 125f56a..a938a13 100644 --- a/include/secsgem/gem/router.hpp +++ b/include/secsgem/gem/router.hpp @@ -45,6 +45,18 @@ class Router { return std::nullopt; } + // Introspection: lets the message_handler wrapper decide whether an + // unhandled message is "unknown stream" (S9F3) or "unknown function in + // a known stream" (S9F5). + bool has_handler(uint8_t stream, uint8_t function) const { + return handlers_.find({stream, function}) != handlers_.end(); + } + bool has_handler_for_stream(uint8_t stream) const { + for (const auto& [key, _] : handlers_) + if (key.first == stream) return true; + return false; + } + std::size_t size() const { return handlers_.size(); } private: diff --git a/include/secsgem/hsms/connection.hpp b/include/secsgem/hsms/connection.hpp index 0ab13bd..495d51d 100644 --- a/include/secsgem/hsms/connection.hpp +++ b/include/secsgem/hsms/connection.hpp @@ -60,6 +60,18 @@ class Connection : public std::enable_shared_from_this { State state() const { return state_; } bool selected() const { return state_ == State::Selected; } + // The HSMS header of the primary currently being dispatched by the + // message_handler. Only non-null inside the handler. Used so the + // handler can capture MHEAD when it needs to emit S9F3 / S9F5 for an + // unrecognized stream/function. + const Header* current_header() const { return current_header_; } + + // Equipment-initiated S9F primary carrying a 10-byte MHEAD. + // Public so a wrapping message_handler can emit S9F3 / S9F5 when it + // detects an unhandled message; Connection itself uses this for S9F7 + // (illegal data), S9F9 (T3 timeout), and S9F11 (data too long). + void emit_s9(uint8_t function, const std::array& mhead); + private: // --- read path --- void read_length(); @@ -83,13 +95,6 @@ class Connection : public std::enable_shared_from_this { const char* what); void clear_control_transaction(); - // --- S9 error stream emissions --- - // Build and send an unsolicited S9F primary with a single - // body carrying the offending message's 10-byte MHEAD. Used for - // F1/F3/F5/F7/F9/F11 (all of which share that body shape). No reply is - // tracked — these notifications are typically W=0. - void emit_s9(uint8_t function, const std::array& mhead); - uint32_t next_system_bytes(); void log(const std::string& msg); @@ -133,6 +138,8 @@ class Connection : public std::enable_shared_from_this { }; std::optional pending_control_; + const Header* current_header_ = nullptr; // set only inside on_message_ dispatch + MessageHandler on_message_; SelectedHandler on_selected_; ClosedHandler on_closed_; diff --git a/src/hsms/connection.cpp b/src/hsms/connection.cpp index d1ea8e3..6647286 100644 --- a/src/hsms/connection.cpp +++ b/src/hsms/connection.cpp @@ -74,7 +74,18 @@ void Connection::on_length(std::error_code ec, std::size_t) { return; } if (len > kMaxFrameLength) { - close("frame too large: " + std::to_string(len)); + // We can't read the actual offending header (the message body is too + // big to safely buffer), so synthesize a 10-byte MHEAD whose first 4 + // bytes are the offending length prefix and the rest are zero. The + // host sees S9F11, the connection then drains the write queue and + // closes. + std::array mhead{}; + mhead[0] = len_buf_[0]; mhead[1] = len_buf_[1]; + mhead[2] = len_buf_[2]; mhead[3] = len_buf_[3]; + log("frame too large: " + std::to_string(len) + "; emitting S9F11 and closing"); + emit_s9(11, mhead); + close_after_flush_ = true; + close_reason_ = "frame too large"; return; } @@ -180,7 +191,12 @@ void Connection::handle_data(const Frame& frame) { log("<- " + h.describe()); if (!on_message_) return; + // Expose the originating header to the handler in case it needs to emit + // an S9F3 / S9F5 in response. Cleared on the way out. + current_header_ = &h; auto reply = on_message_(msg); + current_header_ = nullptr; + if (reply) { Frame out(Header::data_message(device_id_, reply->stream, reply->function, reply->reply_expected, h.system_bytes),