diff --git a/COMPLIANCE.md b/COMPLIANCE.md index 80009af..ae9c49f 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. **No S9 stream (S9F1–F13)** for "Unrecognized Device ID", "Unrecognized Stream Type", T3/T6/T7/T8 violations, etc. | +| 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. | | Documentation | ⬜ | E30 §6.10| S1F19/F20 (GEM compliance), S1F21/F22 (data variable namelist) | Not implemented. | | Control (Operator-Initiated) | ✅ | E30 §6.2 | — | `ControlStateMachine::operator_online/offline/local/remote`. | @@ -136,7 +136,13 @@ Legend: | S7F5 / S7F6 | H→E | ✅ | `messages.hpp` | ✅ in demo | | S7F19 / S7F20 | H→E | ✅ | `messages.hpp` | ✅ round-trip + demo | | S7F23–F26 | H↔E | ⬜ | — | enhanced PP | -| S9F* | E→H | ⬜ | — | error stream | +| 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 | +| 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 | +| S9F13 | E↔H | ✅ | catalog | ✅ round-trip | | S10F1 / S10F2 | H→E | ✅ | `messages.hpp` | ✅ in demo | | S10F3 / S10F4 | E→H | ✅ | `messages.hpp` | ✅ round-trip + demo | | S10F5 / S10F6 | H→E | ⬜ | — | multi-line | @@ -187,8 +193,9 @@ 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. **Implement the S9 error stream** (S9F1, F3, F5, F7, F9, F11, F13) - for the documented transport/protocol error cases. +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. 3. **Implement Documentation messages** S1F19/F20 (GEM-compliance) and S1F21/F22 (data variable namelist) — needed for E30 conformance. 4. **Implement EC range validation** in `set_equipment_constant_value` diff --git a/data/messages.yaml b/data/messages.yaml index af3e968..b577001 100644 --- a/data/messages.yaml +++ b/data/messages.yaml @@ -550,6 +550,68 @@ messages: element: {kind: scalar, item_type: ASCII} name: ppids + # ===================================================================== + # S9 — Protocol error notifications (E5). All carry the 10-byte + # offending message header (MHEAD) as a single BINARY item, except + # S9F13 which carries >. These are + # equipment-initiated (typically W=0) and host-side acks are not + # mandatory; we emit them on the wire but don't expect replies. + # ===================================================================== + + - id: S9F1 + stream: 9 + function: 1 + builder: s9f1_unrecognized_device_id + parser: parse_s9f1 + body: {kind: scalar, item_type: BINARY, param: mhead} + + - id: S9F3 + stream: 9 + function: 3 + builder: s9f3_unrecognized_stream_type + parser: parse_s9f3 + body: {kind: scalar, item_type: BINARY, param: mhead} + + - id: S9F5 + stream: 9 + function: 5 + builder: s9f5_unrecognized_function_type + parser: parse_s9f5 + body: {kind: scalar, item_type: BINARY, param: mhead} + + - id: S9F7 + stream: 9 + function: 7 + builder: s9f7_illegal_data + parser: parse_s9f7 + body: {kind: scalar, item_type: BINARY, param: mhead} + + - id: S9F9 + stream: 9 + function: 9 + builder: s9f9_transaction_timer_timeout + parser: parse_s9f9 + body: {kind: scalar, item_type: BINARY, param: mhead} + + - id: S9F11 + stream: 9 + function: 11 + builder: s9f11_data_too_long + parser: parse_s9f11 + body: {kind: scalar, item_type: BINARY, param: mhead} + + - id: S9F13 + stream: 9 + function: 13 + builder: s9f13_conversation_timeout + parser: parse_s9f13 + body: + kind: list + struct_name: ConversationTimeout + fields: + - {name: mexp, shape: {kind: scalar, item_type: ASCII}} + - {name: edid, shape: {kind: scalar, item_type: ASCII}} + # ===================================================================== # S10 — Terminal services # ===================================================================== diff --git a/include/secsgem/hsms/connection.hpp b/include/secsgem/hsms/connection.hpp index 38edc4a..0ab13bd 100644 --- a/include/secsgem/hsms/connection.hpp +++ b/include/secsgem/hsms/connection.hpp @@ -83,6 +83,13 @@ 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); @@ -136,6 +143,7 @@ class Connection : public std::enable_shared_from_this { enum class Error { Timeout = 1, Closed = 2, + IllegalData = 3, // peer's reply body couldn't be decoded; we also send S9F7 }; std::error_code make_error(Error e); diff --git a/src/hsms/connection.cpp b/src/hsms/connection.cpp index 2aae8b2..d1ea8e3 100644 --- a/src/hsms/connection.cpp +++ b/src/hsms/connection.cpp @@ -1,5 +1,7 @@ #include "secsgem/hsms/connection.hpp" +#include "secsgem/secs2/codec.hpp" + #include namespace secsgem::hsms { @@ -10,8 +12,9 @@ class HsmsCategory : public std::error_category { const char* name() const noexcept override { return "hsms"; } std::string message(int ev) const override { switch (static_cast(ev)) { - case Error::Timeout: return "HSMS transaction timeout"; - case Error::Closed: return "HSMS connection closed"; + case Error::Timeout: return "HSMS transaction timeout"; + case Error::Closed: return "HSMS connection closed"; + case Error::IllegalData: return "peer sent a body that failed to decode"; } return "unknown HSMS error"; } @@ -125,7 +128,14 @@ void Connection::handle_data(const Frame& frame) { try { reply = secs2::Message::from_body(h.stream(), h.function(), h.w_bit(), frame.body); } catch (const std::exception& e) { - close(std::string("reply body decode: ") + e.what()); + // Malformed reply: notify the peer with S9F7 and surface IllegalData + // to the caller right now (rather than making them wait out T3). + log(std::string("reply body decode: ") + e.what() + "; emitting S9F7"); + emit_s9(7, h.encode()); + auto cb = std::move(it->second.cb); + it->second.t3->cancel(); + pending_requests_.erase(it); + cb(make_error(Error::IllegalData), {}); return; } auto cb = std::move(it->second.cb); @@ -160,7 +170,10 @@ void Connection::handle_data(const Frame& frame) { try { msg = secs2::Message::from_body(h.stream(), h.function(), h.w_bit(), frame.body); } catch (const std::exception& e) { - close(std::string("message body decode: ") + e.what()); + // E5: a malformed body in a primary is reported via S9F7 (Illegal Data). + // The connection stays up — the peer may send corrected data next. + log(std::string("message body decode: ") + e.what() + "; emitting S9F7"); + emit_s9(7, h.encode()); return; } @@ -294,6 +307,12 @@ void Connection::send_request(secs2::Message msg, ReplyHandler cb) { if (ec) return; auto it = pending_requests_.find(sys); if (it == pending_requests_.end()) return; + // Reconstruct the MHEAD of the request that timed out and notify the + // peer with S9F9 before surfacing Timeout to the caller. + const uint8_t req_stream = it->second.expected_stream; + const uint8_t req_function = static_cast(it->second.expected_function - 1); + Header h = Header::data_message(device_id_, req_stream, req_function, true, sys); + emit_s9(9, h.encode()); auto cb2 = std::move(it->second.cb); pending_requests_.erase(it); cb2(make_error(Error::Timeout), {}); @@ -403,6 +422,19 @@ void Connection::clear_control_transaction() { t6_timer_.cancel(); } +void Connection::emit_s9(uint8_t function, const std::array& mhead) { + if (closed_) return; + const uint32_t sys = next_system_bytes(); + // S9F1/F3/F5/F7/F9/F11 all carry the offending MHEAD as in the body. + secs2::Item body_item = + secs2::Item::binary(std::vector(mhead.begin(), mhead.end())); + Frame f(Header::data_message(device_id_, /*stream=*/9, function, + /*w=*/false, sys), + secs2::encode(body_item)); + log("-> S9F" + std::to_string(function) + " (sys=" + std::to_string(sys) + ")"); + send_frame(std::move(f)); +} + uint32_t Connection::next_system_bytes() { uint32_t s = next_system_bytes_++; if (next_system_bytes_ == 0) next_system_bytes_ = 1; diff --git a/tests/test_messages.cpp b/tests/test_messages.cpp index 378b6e0..1e8ca3d 100644 --- a/tests/test_messages.cpp +++ b/tests/test_messages.cpp @@ -112,6 +112,45 @@ TEST_CASE("S10F3 terminal display round-trip") { CHECK(parsed->text == "ALARM: chiller temperature high"); } +// ---- S9 error-stream round-trips ---------------------------------------- + +TEST_CASE("S9 MHEAD-carrying messages round-trip") { + // A representative 10-byte HSMS header (data message, S2F33 W, sys=42). + const std::string mhead( + "\x12\x34" // session id + "\x82" // byte2: W=1, stream=2 + "\x21" // byte3: function=33 + "\x00" // ptype + "\x00" // stype=Data + "\x00\x00\x00\x2A", // system bytes = 42 + 10); + + auto check = [&](const s2::Message& m, std::optional (*p)(const s2::Message&)) { + CHECK(m.stream == 9); + CHECK_FALSE(m.reply_expected); + auto out = p(m); + REQUIRE(out.has_value()); + CHECK(*out == mhead); + }; + + check(s9f1_unrecognized_device_id(mhead), parse_s9f1); + check(s9f3_unrecognized_stream_type(mhead), parse_s9f3); + check(s9f5_unrecognized_function_type(mhead), parse_s9f5); + check(s9f7_illegal_data(mhead), parse_s9f7); + check(s9f9_transaction_timer_timeout(mhead), parse_s9f9); + check(s9f11_data_too_long(mhead), parse_s9f11); +} + +TEST_CASE("S9F13 conversation timeout carries MEXP + EDID") { + auto m = s9f13_conversation_timeout("S2F33W", "EQUIP-01"); + CHECK(m.stream == 9); + CHECK(m.function == 13); + auto parsed = parse_s9f13(m); + REQUIRE(parsed.has_value()); + CHECK(parsed->mexp == "S2F33W"); + CHECK(parsed->edid == "EQUIP-01"); +} + // ---- Extended GEM message round-trips ------------------------------------ TEST_CASE("S2F33 define-report round-trip") {