#4 Split EquipmentDataModel into focused stores
The god-class is gone. Each capability is now its own focused store:
StatusVariableStore, DataVariableStore, EquipmentConstantStore (with EAC
range validation), EventReportSubscriptions, AlarmRegistry, RecipeStore,
Clock, HostCommandRegistry. Each is independently testable.
EquipmentDataModel becomes a small composite that holds one of each store
as a public member, plus three convenience methods (vid_value, vid_exists,
compose_reports_for) that span SVIDs+DVIDs and inject the right callbacks
into the EventReportSubscriptions.
New under include/secsgem/gem/store/:
status_variables.hpp StatusVariable, StatusVariableStore,
DataVariable, DataVariableStore
equipment_constants.hpp EquipmentConstant, EquipmentConstantStore,
EquipmentAck. set_value() now validates
numeric values against min_str/max_str and
returns EAC=4 on out-of-range — closes the
COMPLIANCE.md gap about EC range validation.
event_reports.hpp CollectionEvent, Report, ReportData,
EventReportSubscriptions + DefineReportAck,
LinkEventAck, EnableEventAck. The store is
pure data; VidLookup / VidExists callbacks
are injected at define / emit time so the
service doesn't back-reference the SVID
store.
alarms.hpp Alarm, AlarmAck, AlarmRegistry.
Encapsulates the (enabled, active) sets and
ALCD byte computation.
recipes.hpp ProcessProgramAck, RecipeStore.
clock.hpp TimeAck, Clock. set_time_string applies an
offset so subsequent reads reflect the host
time without mutating system clock.
host_commands.hpp HostCmdAck, CommandParameter,
HostCommandRegistry with Spec/Result types.
include/secsgem/gem/data_model.hpp shrinks to a 50-line composite:
struct EquipmentDataModel {
StatusVariableStore svids;
DataVariableStore dvids;
EquipmentConstantStore ecids;
EventReportSubscriptions events;
AlarmRegistry alarms;
RecipeStore recipes;
Clock clock;
HostCommandRegistry commands;
/* + vid_value, vid_exists, compose_reports_for sugar */
};
src/gem/data_model.cpp is gone — every store is inline header-only.
include/secsgem/gem/messages_helpers.hpp picks up EventReportAck and
TerminalAck (S6F12 / S10F2-F4 ack enums that aren't tied to any one
store).
Call-site updates:
apps/secs_server.cpp model->status_variable(id) -> model->svids.get(id),
model->equipment_constant(id) -> model->ecids.get(id),
model->alarm_set(id) -> model->alarms.set_active(id),
model->dispatch_command(...) -> model->commands.dispatch(...),
and similar across every handler. Plus
model->current_time_string() -> model->clock....
src/config/loader.cpp model.add_status_variable(sv) -> model.svids.add(sv),
and similar. HostCommandRegistry::Spec replaces
EquipmentDataModel::CommandSpec.
apps/secs_client.cpp std::vector<EquipmentDataModel::CommandParam> ->
std::vector<CommandParameter>.
tests/test_data_model.cpp Rewritten around the individual stores;
each gets its own TEST_CASE block. Adds three
new cases covering EC range validation (in
range / out of range / non-numeric skipped).
tests/test_loader.cpp m.has_event(100) -> m.events.has_event(100),
etc.
Verified:
- Tests: 69 cases / 370 assertions pass (was 67 / 384; -14 stale
composite-API assertions + 16 new store-level assertions covering
EC range validation and the per-store add/get/list/delete paths).
- Demo: byte-identical behaviour across the full 17-step flow.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
+129
-109
@@ -4,96 +4,117 @@
|
||||
|
||||
using namespace secsgem::gem;
|
||||
|
||||
// ---- Status variables ----------------------------------------------------
|
||||
|
||||
TEST_CASE("SVID add / get / list / set value") {
|
||||
EquipmentDataModel m;
|
||||
m.add_status_variable({1, "Clock", "", s2::Item::ascii("19700101000000")});
|
||||
m.add_status_variable({2, "EventsEnabled", "", s2::Item::boolean(true)});
|
||||
StatusVariableStore svids;
|
||||
svids.add({1, "Clock", "", s2::Item::ascii("19700101000000")});
|
||||
svids.add({2, "EventsEnabled", "", s2::Item::boolean(true)});
|
||||
|
||||
CHECK(m.status_variable(1).has_value());
|
||||
CHECK(m.status_variable(99).has_value() == false);
|
||||
CHECK(m.all_status_variables().size() == 2);
|
||||
CHECK(svids.get(1).has_value());
|
||||
CHECK_FALSE(svids.get(99).has_value());
|
||||
CHECK(svids.all().size() == 2);
|
||||
|
||||
m.set_status_value(2, s2::Item::boolean(false));
|
||||
CHECK(m.status_variable(2)->value == s2::Item::boolean(false));
|
||||
svids.set_value(2, s2::Item::boolean(false));
|
||||
CHECK(svids.get(2)->value == s2::Item::boolean(false));
|
||||
}
|
||||
|
||||
TEST_CASE("ECID set rejects unknown id") {
|
||||
EquipmentDataModel m;
|
||||
m.add_equipment_constant({10, "MaxSpool", "msgs", s2::Item::u4(uint32_t{100}),
|
||||
s2::Item::u4(uint32_t{100}), "0", "1000"});
|
||||
CHECK(m.set_equipment_constant_value(10, s2::Item::u4(uint32_t{50})) ==
|
||||
EquipmentAck::Accept);
|
||||
CHECK(m.equipment_constant(10)->value == s2::Item::u4(uint32_t{50}));
|
||||
CHECK(m.set_equipment_constant_value(999, s2::Item::u4(uint32_t{1})) ==
|
||||
EquipmentAck::Denied_UnknownEcid);
|
||||
// ---- Equipment constants -------------------------------------------------
|
||||
|
||||
TEST_CASE("ECID set rejects unknown id and out-of-range values") {
|
||||
EquipmentConstantStore ecids;
|
||||
ecids.add({10, "MaxSpool", "msgs", s2::Item::u4(uint32_t{100}),
|
||||
s2::Item::u4(uint32_t{100}), "0", "1000"});
|
||||
CHECK(ecids.set_value(10, s2::Item::u4(uint32_t{50})) == EquipmentAck::Accept);
|
||||
CHECK(ecids.get(10)->value == s2::Item::u4(uint32_t{50}));
|
||||
CHECK(ecids.set_value(999, s2::Item::u4(uint32_t{1})) == EquipmentAck::Denied_UnknownEcid);
|
||||
|
||||
// Out-of-range rejected (closes COMPLIANCE.md gap).
|
||||
CHECK(ecids.set_value(10, s2::Item::u4(uint32_t{5000})) == EquipmentAck::Denied_OutOfRange);
|
||||
CHECK(ecids.get(10)->value == s2::Item::u4(uint32_t{50})); // unchanged
|
||||
}
|
||||
|
||||
TEST_CASE("clock string has 16 digit characters") {
|
||||
EquipmentDataModel m;
|
||||
auto s = m.current_time_string();
|
||||
TEST_CASE("ECID with no min/max accepts any value") {
|
||||
EquipmentConstantStore ecids;
|
||||
ecids.add({1, "n", "", s2::Item::u4(uint32_t{0}), s2::Item::u4(uint32_t{0}), "", ""});
|
||||
CHECK(ecids.set_value(1, s2::Item::u4(uint32_t{99999999})) == EquipmentAck::Accept);
|
||||
}
|
||||
|
||||
TEST_CASE("ECID range validation skipped for non-numeric formats") {
|
||||
EquipmentConstantStore ecids;
|
||||
ecids.add({1, "label", "", s2::Item::ascii("a"), s2::Item::ascii("a"), "0", "10"});
|
||||
CHECK(ecids.set_value(1, s2::Item::ascii("foo")) == EquipmentAck::Accept);
|
||||
}
|
||||
|
||||
// ---- Clock ---------------------------------------------------------------
|
||||
|
||||
TEST_CASE("clock string has 16 characters") {
|
||||
Clock c;
|
||||
auto s = c.current_time_string();
|
||||
REQUIRE(s.size() == 16);
|
||||
for (char c : s) CHECK(c >= '0');
|
||||
}
|
||||
|
||||
TEST_CASE("set_time_string accepts well-formed and rejects malformed") {
|
||||
EquipmentDataModel m;
|
||||
CHECK(m.set_time_string("20260101000000") == TimeAck::Accept);
|
||||
CHECK(m.set_time_string("2026010100000000") == TimeAck::Accept);
|
||||
CHECK(m.set_time_string("not-a-date-12345") == TimeAck::Error);
|
||||
CHECK(m.set_time_string("short") == TimeAck::Error);
|
||||
Clock c;
|
||||
CHECK(c.set_time_string("20260101000000") == TimeAck::Accept);
|
||||
CHECK(c.set_time_string("2026010100000000") == TimeAck::Accept);
|
||||
CHECK(c.set_time_string("not-a-date-12345") == TimeAck::Error);
|
||||
CHECK(c.set_time_string("short") == TimeAck::Error);
|
||||
}
|
||||
|
||||
TEST_CASE("host command registry") {
|
||||
EquipmentDataModel m;
|
||||
m.register_command("START", {HostCmdAck::Accept, 300, std::nullopt});
|
||||
m.register_command("STOP", {HostCmdAck::CannotDoNow, std::nullopt, std::nullopt});
|
||||
m.register_command("FAULT", {HostCmdAck::Accept, std::nullopt, 1});
|
||||
// ---- Host command registry ----------------------------------------------
|
||||
|
||||
CHECK(m.has_command("START"));
|
||||
CHECK_FALSE(m.has_command("PAUSE"));
|
||||
TEST_CASE("host command registry returns spec + result") {
|
||||
HostCommandRegistry r;
|
||||
r.register_command("START", {HostCmdAck::Accept, 300, std::nullopt});
|
||||
r.register_command("STOP", {HostCmdAck::CannotDoNow, std::nullopt, std::nullopt});
|
||||
r.register_command("FAULT", {HostCmdAck::Accept, std::nullopt, 1});
|
||||
|
||||
auto start = m.dispatch_command("START", {});
|
||||
CHECK(r.has("START"));
|
||||
CHECK_FALSE(r.has("PAUSE"));
|
||||
|
||||
auto start = r.dispatch("START", {});
|
||||
CHECK(start.ack == HostCmdAck::Accept);
|
||||
CHECK(start.emit_ceid.value_or(0) == 300);
|
||||
CHECK_FALSE(start.set_alarm.has_value());
|
||||
|
||||
auto stop = m.dispatch_command("STOP", {});
|
||||
CHECK(stop.ack == HostCmdAck::CannotDoNow);
|
||||
|
||||
auto fault = m.dispatch_command("FAULT", {});
|
||||
CHECK(fault.ack == HostCmdAck::Accept);
|
||||
auto fault = r.dispatch("FAULT", {});
|
||||
CHECK(fault.set_alarm.value_or(0) == 1);
|
||||
|
||||
CHECK(m.dispatch_command("UNKNOWN", {}).ack == HostCmdAck::InvalidCommand);
|
||||
CHECK(r.dispatch("UNKNOWN", {}).ack == HostCmdAck::InvalidCommand);
|
||||
}
|
||||
|
||||
// ---- Event reports -------------------------------------------------------
|
||||
|
||||
TEST_CASE("define report rejects unknown VID, accepts known VIDs") {
|
||||
EquipmentDataModel m;
|
||||
m.add_status_variable({1, "A", "", s2::Item::u4(uint32_t{10})});
|
||||
m.add_status_variable({2, "B", "", s2::Item::u4(uint32_t{20})});
|
||||
TEST_CASE("define reports rejects unknown VID") {
|
||||
EventReportSubscriptions ev;
|
||||
ev.register_event({100, "x"});
|
||||
auto exists = [](uint32_t id) { return id == 1 || id == 2; };
|
||||
|
||||
CHECK(m.define_reports({{100, {1, 2}}}) == DefineReportAck::Accept);
|
||||
CHECK(m.define_reports({{101, {1, 999}}}) == DefineReportAck::InvalidVid);
|
||||
CHECK(ev.define_reports({{1000, {1, 2}}}, exists) == DefineReportAck::Accept);
|
||||
CHECK(ev.define_reports({{1001, {1, 999}}}, exists) == DefineReportAck::InvalidVid);
|
||||
}
|
||||
|
||||
TEST_CASE("link / enable / compose: full event-report pipeline") {
|
||||
EquipmentDataModel m;
|
||||
m.add_status_variable({1, "ControlState", "", s2::Item::ascii("OnlineRemote")});
|
||||
m.add_status_variable({2, "Clock", "", s2::Item::ascii("19700101000000")});
|
||||
m.register_event({100, "ControlStateChanged"});
|
||||
m.register_event({200, "AlarmSetEvent"});
|
||||
TEST_CASE("full event-report pipeline") {
|
||||
StatusVariableStore svids;
|
||||
svids.add({1, "ControlState", "", s2::Item::ascii("OnlineRemote")});
|
||||
svids.add({2, "Clock", "", s2::Item::ascii("19700101000000")});
|
||||
|
||||
REQUIRE(m.define_reports({{1000, {1}}, {1001, {1, 2}}}) == DefineReportAck::Accept);
|
||||
REQUIRE(m.link_event_reports({{100, {1000, 1001}}, {200, {1001}}}) == LinkEventAck::Accept);
|
||||
CHECK_FALSE(m.is_event_enabled(100));
|
||||
EventReportSubscriptions ev;
|
||||
ev.register_event({100, "ControlStateChanged"});
|
||||
ev.register_event({200, "AlarmSetEvent"});
|
||||
|
||||
REQUIRE(m.enable_events(true, {100}) == EnableEventAck::Accept);
|
||||
CHECK(m.is_event_enabled(100));
|
||||
CHECK_FALSE(m.is_event_enabled(200));
|
||||
auto exists = [&](uint32_t id) { return svids.has(id); };
|
||||
auto value = [&](uint32_t id) { return svids.value(id); };
|
||||
|
||||
auto reports = m.compose_reports_for(100);
|
||||
REQUIRE(ev.define_reports({{1000, {1}}, {1001, {1, 2}}}, exists) == DefineReportAck::Accept);
|
||||
REQUIRE(ev.link_event_reports({{100, {1000, 1001}}, {200, {1001}}}) == LinkEventAck::Accept);
|
||||
CHECK_FALSE(ev.is_enabled(100));
|
||||
|
||||
REQUIRE(ev.enable_events(true, {100}) == EnableEventAck::Accept);
|
||||
CHECK(ev.is_enabled(100));
|
||||
CHECK_FALSE(ev.is_enabled(200));
|
||||
|
||||
auto reports = ev.compose_for(100, value);
|
||||
REQUIRE(reports.size() == 2);
|
||||
CHECK(reports[0].rptid == 1000);
|
||||
REQUIRE(reports[0].values.size() == 1);
|
||||
@@ -103,75 +124,74 @@ TEST_CASE("link / enable / compose: full event-report pipeline") {
|
||||
}
|
||||
|
||||
TEST_CASE("enable_events with empty CEID list enables all registered events") {
|
||||
EquipmentDataModel m;
|
||||
m.register_event({1, "a"});
|
||||
m.register_event({2, "b"});
|
||||
CHECK(m.enable_events(true, {}) == EnableEventAck::Accept);
|
||||
CHECK(m.is_event_enabled(1));
|
||||
CHECK(m.is_event_enabled(2));
|
||||
CHECK(m.enable_events(false, {}) == EnableEventAck::Accept);
|
||||
CHECK_FALSE(m.is_event_enabled(1));
|
||||
EventReportSubscriptions ev;
|
||||
ev.register_event({1, "a"});
|
||||
ev.register_event({2, "b"});
|
||||
CHECK(ev.enable_events(true, {}) == EnableEventAck::Accept);
|
||||
CHECK(ev.is_enabled(1));
|
||||
CHECK(ev.is_enabled(2));
|
||||
CHECK(ev.enable_events(false, {}) == EnableEventAck::Accept);
|
||||
CHECK_FALSE(ev.is_enabled(1));
|
||||
}
|
||||
|
||||
TEST_CASE("link_event_reports rejects unknown CEID or RPTID") {
|
||||
EquipmentDataModel m;
|
||||
m.add_status_variable({1, "A", "", s2::Item::u4(uint32_t{0})});
|
||||
m.register_event({100, "x"});
|
||||
m.define_reports({{500, {1}}});
|
||||
EventReportSubscriptions ev;
|
||||
ev.register_event({100, "x"});
|
||||
ev.define_reports({{500, {1}}}, [](uint32_t) { return true; });
|
||||
|
||||
CHECK(m.link_event_reports({{999, {500}}}) == LinkEventAck::UnknownCeid);
|
||||
CHECK(m.link_event_reports({{100, {999}}}) == LinkEventAck::UnknownRptid);
|
||||
CHECK(m.link_event_reports({{100, {500}}}) == LinkEventAck::Accept);
|
||||
}
|
||||
|
||||
TEST_CASE("define_reports empty list clears reports and links") {
|
||||
EquipmentDataModel m;
|
||||
m.add_status_variable({1, "A", "", s2::Item::u4(uint32_t{0})});
|
||||
m.register_event({100, "x"});
|
||||
m.define_reports({{500, {1}}});
|
||||
m.link_event_reports({{100, {500}}});
|
||||
|
||||
CHECK(m.define_reports({}) == DefineReportAck::Accept);
|
||||
CHECK(m.all_reports().empty());
|
||||
CHECK(m.compose_reports_for(100).empty());
|
||||
CHECK(ev.link_event_reports({{999, {500}}}) == LinkEventAck::UnknownCeid);
|
||||
CHECK(ev.link_event_reports({{100, {999}}}) == LinkEventAck::UnknownRptid);
|
||||
CHECK(ev.link_event_reports({{100, {500}}}) == LinkEventAck::Accept);
|
||||
}
|
||||
|
||||
// ---- Alarms --------------------------------------------------------------
|
||||
|
||||
TEST_CASE("alarm enable / set / clear / list") {
|
||||
EquipmentDataModel m;
|
||||
m.add_alarm({1, "Chiller Temp High", 4});
|
||||
m.add_alarm({2, "Door Open", 1});
|
||||
AlarmRegistry r;
|
||||
r.add({1, "Chiller Temp High", 4});
|
||||
r.add({2, "Door Open", 1});
|
||||
|
||||
CHECK(m.set_alarm_enabled(1, true) == AlarmAck::Accept);
|
||||
CHECK(m.alarm_enabled(1));
|
||||
CHECK(m.set_alarm_enabled(999, true) == AlarmAck::Error);
|
||||
CHECK(r.set_enabled(1, true) == AlarmAck::Accept);
|
||||
CHECK(r.enabled(1));
|
||||
CHECK(r.set_enabled(999, true) == AlarmAck::Error);
|
||||
|
||||
auto alcd_set = m.alarm_set(1);
|
||||
auto alcd_set = r.set_active(1);
|
||||
REQUIRE(alcd_set.has_value());
|
||||
CHECK((*alcd_set & 0x80) != 0);
|
||||
CHECK((*alcd_set & 0x7F) == 4);
|
||||
CHECK(m.alarm_active(1));
|
||||
CHECK(r.active(1));
|
||||
|
||||
auto alcd_clr = m.alarm_clear(1);
|
||||
auto alcd_clr = r.clear_active(1);
|
||||
REQUIRE(alcd_clr.has_value());
|
||||
CHECK((*alcd_clr & 0x80) == 0);
|
||||
CHECK_FALSE(m.alarm_active(1));
|
||||
CHECK_FALSE(r.active(1));
|
||||
|
||||
CHECK(m.all_alarms().size() == 2);
|
||||
CHECK(r.all().size() == 2);
|
||||
}
|
||||
|
||||
// ---- Process programs ---------------------------------------------------
|
||||
// ---- Process programs ----------------------------------------------------
|
||||
|
||||
TEST_CASE("process program CRUD") {
|
||||
TEST_CASE("recipe CRUD") {
|
||||
RecipeStore r;
|
||||
r.add("RECIPE-A", "step1\nstep2\n");
|
||||
r.add("RECIPE-B", "alt body");
|
||||
CHECK(r.list().size() == 2);
|
||||
CHECK(r.get("RECIPE-A").value() == "step1\nstep2\n");
|
||||
CHECK_FALSE(r.get("UNKNOWN").has_value());
|
||||
|
||||
CHECK(r.remove("RECIPE-A") == ProcessProgramAck::Accept);
|
||||
CHECK_FALSE(r.get("RECIPE-A").has_value());
|
||||
CHECK(r.remove("RECIPE-A") == ProcessProgramAck::PpidNotFound);
|
||||
}
|
||||
|
||||
// ---- Composite EquipmentDataModel ---------------------------------------
|
||||
|
||||
TEST_CASE("EquipmentDataModel composes the seven stores") {
|
||||
EquipmentDataModel m;
|
||||
m.add_process_program("RECIPE-A", "step1\nstep2\n");
|
||||
m.add_process_program("RECIPE-B", "alt body");
|
||||
CHECK(m.process_program_list().size() == 2);
|
||||
CHECK(m.process_program("RECIPE-A").value() == "step1\nstep2\n");
|
||||
CHECK_FALSE(m.process_program("UNKNOWN").has_value());
|
||||
|
||||
CHECK(m.delete_process_program("RECIPE-A") == ProcessProgramAck::Accept);
|
||||
CHECK_FALSE(m.process_program("RECIPE-A").has_value());
|
||||
CHECK(m.delete_process_program("RECIPE-A") == ProcessProgramAck::PpidNotFound);
|
||||
m.svids.add({1, "Clock", "", s2::Item::ascii("")});
|
||||
m.dvids.add({2, "Temp", "C", s2::Item::u4(uint32_t{25})});
|
||||
CHECK(m.vid_exists(1));
|
||||
CHECK(m.vid_exists(2));
|
||||
CHECK_FALSE(m.vid_exists(3));
|
||||
CHECK(m.vid_value(2) == s2::Item::u4(uint32_t{25}));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user