From 65db38d9f222adb43e1e18b60f4ea51f0fe09b52 Mon Sep 17 00:00:00 2001 From: Raphael Maenle Date: Sun, 7 Jun 2026 01:57:35 +0200 Subject: [PATCH] 100%/B: S9F3/F5/F11 emission + Router fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the S9 stream. Every documented protocol-error condition is now auto-emitted by Connection (with the assist of one Router predicate), without involving the application. Router (include/secsgem/gem/router.hpp) Adds two predicates: has_handler(stream, function) and has_handler_for_stream(stream). Lets the wrapping message handler decide whether an unhandled message is "unknown stream" (S9F3) or "unknown function in a known stream" (S9F5). Connection (include/secsgem/hsms/{connection.hpp, connection.cpp}) - emit_s9() goes public so the message_handler can call it. - New current_header() accessor returns the HSMS header of the primary currently being dispatched. Non-null only inside the on_message_ call; cleared on the way out. - handle_data sets current_header_ before invoking on_message_. - on_length on oversized frame: synthesizes a 10-byte MHEAD whose first 4 bytes are the offending length prefix, emits S9F11, and sets close_after_flush so the S9F11 goes out before the socket closes. Server (apps/secs_server.cpp) The conn->set_message_handler lambda now wraps router.dispatch. For any inbound primary without a registered handler, it captures the MHEAD via current_header() and emits either S9F3 (stream unknown) or S9F5 (function unknown). The wrapper still returns the Router's reply (SxF0 for primaries with W) so transactional semantics are preserved. COMPLIANCE.md Error Messages row flips from ๐ŸŸก to โœ…. S9F3/F5/F11 rows in the coverage matrix flip from ๐ŸŸก to โœ…. Each row in the matrix now states its trigger condition explicitly. Drops the "Finish S9 wiring" bullet from the "what would 100% take" list. Verified - Tests: 78 cases / 454 assertions still pass (no behavioural change on the happy path; new emission paths fire only on protocol errors that the demo doesn't induce). - Build clean. Co-Authored-By: Claude Opus 4.7 --- COMPLIANCE.md | 12 +++++------- apps/secs_server.cpp | 22 ++++++++++++++++++++-- include/secsgem/gem/router.hpp | 12 ++++++++++++ include/secsgem/hsms/connection.hpp | 21 ++++++++++++++------- src/hsms/connection.cpp | 18 +++++++++++++++++- 5 files changed, 68 insertions(+), 17 deletions(-) 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),