Fix event-driven Close feedback for dynamic reports - #20
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ec16a5e70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ["dchg"] = 0, | ||
| ["data-change"] = 0, | ||
| ["datachange"] = 0, |
There was a problem hiding this comment.
When dynamic monitoring writes TrgOps, this mapping shifts every trigger one bit too far left: the existing report runtime documents and parses TrgOps with bit 0 reserved and dchg at bit 1, so a request such as dchg qchg GI is encoded as reserved+dchg+integrity rather than data-change/quality-change/general-interrogation. On live IEDs or the in-repo simulator this can leave the intended triggers disabled or make the RCB reject the write before reports are enabled.
Useful? React with 👍 / 👎.
| if (!rcb.Attributes.Contains("TrgOps", StringComparer.OrdinalIgnoreCase) || | ||
| !MmsReportControlFieldCodec.TryEncodeTriggerOptions(rcb.TriggerOptions, out var triggerOptions)) |
There was a problem hiding this comment.
In dynamic mode this check runs after the temporary DataSet has already been created and RCB.DatSet has already been written, but this failure path returns without registering a session, deleting the DataSet, or restoring the original DatSet. Any relay whose TrgOps is missing or whose current value cannot be tokenized will be left with the temporary DataSet bound even though start reports failure.
Useful? React with 👍 / 👎.
| ["sequence-number"] = 1, | ||
| ["sequencenumber"] = 1, | ||
| ["sqnum"] = 1, | ||
| ["report-timestamp"] = 2, | ||
| ["reporttimestamp"] = 2, | ||
| ["time-of-entry"] = 2, | ||
| ["timeofentry"] = 2, | ||
| ["reason-for-inclusion"] = 3, | ||
| ["reasonforinclusion"] = 3, |
There was a problem hiding this comment.
The new encoder silently drops unknown optional-field tokens while still writing any recognized ones, but existing discovery/simulator text uses aliases such as seqNum, timeStamp, reasonCode, and report-time-stamp; those are not in this map. With those common inputs the OptFlds write strips sequence number, timestamp, and reason diagnostics instead of preserving/requesting them, so subsequent report decoding loses the evidence this path is trying to enable.
Useful? React with 👍 / 👎.
1 participant
Field evidence
ArIED command transactions complete in about 420 ms with positive CommandTermination and immediate native process feedback, while the Live Monitor sometimes shows Closed only several seconds later. The same diagnostic repeatedly reports that MMS validation found a value change not delivered by the armed report. Open transitions are fast; Closed transitions are asymmetric.
Root causes corrected
TrgOps. A free RCB could therefore remain GI/integrity-only even though the application described it as event-driven.01and Closed10could remain raw bit strings in the report projection path.Changes
TrgOps = dchg qchg dupd integrity GIbeforeRptEna=truefor dynamic report plans.dchgcannot be written successfully.OptFldsfor sequence number, report timestamp, reason for inclusion, DataSet, data reference, and ConfRev.0x40) and Closed (0x80) DBPOS report projection and the RCB bit-string encodings.Safety
This does not alter command execution, SBOw/Operate ordering, interlock, synchrocheck, or CommandTermination handling. It only repairs report configuration and value projection.