Rrivera/evb debugging2026 aug

[rrivera747]

No description provided.

[Copilot]

Pull request overview

Extends synchronized event-building configuration with CFO edge-fix phases, EVB diagnostics, ROC-flow handling, and documentation updates.

Changes:

• Adds CFO/DTC synchronization, edge-fix, and RTF calibration phases.
• Adds EVB initialization, status macros, and counters.
• Updates ROC classification and configuration-flow documentation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Summary and final comments
otsdaq-mu2e/FEInterfaces/DTCFrontEndInterfaceImpl.cc	Implements EVB, timing, ROC, and initialization changes. Critical (1 vote): EVB start-node addresses can exceed the 7-bit protocol field and be truncated. Moderate (2 votes each): the EnableROCConfigureStep default is inconsistent between phases, and the initialized DTC ID omits the established bit-7 convention.
otsdaq-mu2e/FEInterfaces/DTCFrontEndInterface.h	Adds EVB macro declarations and retry state.
otsdaq-mu2e/FEInterfaces/CFOFrontEndInterfaceImpl.cc	Implements CFO synchronization and edge-fix orchestration.
otsdaq-mu2e/FEInterfaces/CFOFrontEndInterface.h	Adds CFO edge-fix state tracking.
otsdaq-mu2e/CFOandDTCCore/CFOandDTCCoreVInterface.h	Revises configuration phase constants. Critical (3 votes): the start iteration constant is 13 while the CFO ready iteration remains 12, causing the startup ordering check to fail.
doc/event_building_mode_configure.md	Documents the updated configuration flow.

Suppressed comments (7)

otsdaq-mu2e/FEInterfaces/CFOFrontEndInterfaceImpl.cc:2482

• The clean-pass branch does not enforce MAX_CFO_EDGE_FIX_PASSES: if pass 10 is the first clean pass, it schedules pass 11 because the limit is checked only when toggledDTCs is nonempty. This exceeds the documented ten-pass cap and adds another blocking macro pass; reject a first clean pass at the limit or explicitly document the extra confirmation pass.

					++cfo_edge_fix_consecutive_clean_;
					if(cfo_edge_fix_consecutive_clean_ >= 2)
					{

otsdaq-mu2e/FEInterfaces/CFOFrontEndInterfaceImpl.cc:2473

• This condition now retries the punched-clock edge for an RTFMarker-only failure as well as RTFPhase, but the phase documentation still says the retry occurs only when RTFPhase is the sole error (doc/event_building_mode_configure.md:223). Please update the operator-facing documentation to describe the RTFMarker path too.

					__FE_COUT__ << "DTC " << dtcUID << ": " << result << __E__;

otsdaq-mu2e/FEInterfaces/DTCFrontEndInterfaceImpl.cc:6285

• The destination-node count is read as uint8_t and only zero is rejected. Register 0x9158 allocates bits 0–6 for this field, so values 128–255 are accepted and then truncated by SetEVBClusterInfo (and larger inputs can wrap during conversion), producing a cluster with a different size than requested. Parse a wider integer and reject values above 127 before programming the register.

	uint8_t NumOfDTCs =
	    __GET_ARG_IN__("EVB Number of DTCs in Cluster (Default := 1)", uint8_t, 1);

otsdaq-mu2e/FEInterfaces/DTCFrontEndInterfaceImpl.cc:6259

• atoi(parts.back().c_str()) silently returns zero for a malformed suffix, so a typo such as calo-xx is converted into a valid-looking subsystem base address and the EVB is programmed without an error. Validate the hostname format and numeric index explicitly (and reject unsupported or malformed input) before deriving an address.

	std::vector<std::string> parts;
	StringMacros::getVectorFromString(shortHostname, parts, {'-'});
	return subsystemOffset + atoi(parts.back().c_str()) * 2;

otsdaq-mu2e/FEInterfaces/DTCFrontEndInterfaceImpl.cc:2412

• This apply branch is reachable on every sub-step 1 through 4 whenever the histogram is already saturated and the marker position is valid. Each visit sets the permanent offset and issues another SoftReset(), so a normal successful calibration performs the apply/reset sequence multiple times instead of once; if the reset clears the histogram, this can also keep invalidating the next sample. Separate the saturation polling from the one-shot apply/advance state.

					else
					{
						dtc->SetCFOSamplePermanentOffset(impliedPos);
						int readback = dtc->ReadCFOSamplePermanentOffset();
						__FE_COUT_INFO__

otsdaq-mu2e/FEInterfaces/DTCFrontEndInterfaceImpl.cc:6013

• The EnableROCConfigureStep=true fallback is nested inside the if(!rocLink.isDisconnected()) block. When a DTC has no connected ROC group table, that outer block is skipped and has_real_roc_flow_ remains false, contrary to the documented default classification and the comment here. Apply the explicit opt-out/default rule even when the ROC link table is disconnected.

			if(!has_real_roc_flow_)
			{
				// EnableROCConfigureStep is true (checked above) — use it as final rule
				has_real_roc_flow_    = true;
				real_roc_flow_reason_ = "EnableROCConfigureStep is true";

otsdaq-mu2e/FEInterfaces/DTCFrontEndInterfaceImpl.cc:6291

• With the default baseDTCHostname set to the current DTC hostname, configuring a multi-DTC cluster independently produces different StartNode values on each member. For example, calo-01 and calo-02 with NumOfDTCs=2 derive different bases, so the EVB destination ranges do not agree. The automatic default needs one cluster-wide base from shared configuration, or the macro must require the same explicit base on every DTC.

	std::string baseDTCHostname = __GET_ARG_IN__(
	    "EVB Cluster Base DTC Address as hostname (e.g. calo-01, trk-04) (Default := "
	    "auto)",
	    std::string,
	    shortHostname);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[corrodis]

If I understand correctly, the DTCs toggle edges in multiple iterations when they show errors? I was wondering if it might be worth following the timing chains? Because, in a sense, there is no point in working on the "downstream" until DTC N-1 is ready? On the other hand, I understand that this makes it more complicated. So maybe we accept the many toggles in this iterative process, with the idea that one after another will fall into line?

[rrivera747]

The issue with following the chain is that it is disruptive (loopback toggles, drop lock, lose clocks) to discover the topology - or if you have to have the full configuration specified in a trustworthy way. Whereas only toggling when there is an error is not disruptive (i.e. only fix when broken) and takes no trusting of the configuration.