[MCH] improved digits time computation

The code introduces various fixes and improvements
in the handling of the HeartBeat packets, which are
used to compute the reference SAMPA time at the
beginning of each time frame.

Author: aferrero2707

Detectors/MUON/MCH/Raw/Decoder/include/MCHRawDecoder/DataDecoder.h

@@ -109,14 +109,16 @@ class DataDecoder
     TimeFrameStartRecord() = default;
 
     /// store the new orbit/bc pair, and copy the existing one in the "*Prev" data members
-    void update(uint32_t orbit, uint32_t bunchCrossing);
-    bool check();
+    bool update(uint32_t orbit, uint32_t bunchCrossing, bool verbose = false);
+    bool check(int32_t orbit, uint32_t bc, int32_t orbitRef, uint32_t bcRef, bool verbose = false);
 
     int64_t mOrbit{-1};
     int64_t mBunchCrossing{-1};
 
     int64_t mOrbitPrev{-1};
     int64_t mBunchCrossingPrev{-1};
+
+    bool mValid{true};
   };
 
   /// Structure used internally to store the information of the decoded digits.

Detectors/MUON/MCH/Raw/Decoder/src/DataDecoder.cxx

@@ -154,31 +154,86 @@ bool DataDecoder::RawDigit::operator==(const DataDecoder::RawDigit& other) const
   return digit == other.digit && info == other.info;
 }
 
-void DataDecoder::TimeFrameStartRecord::update(uint32_t orbit, uint32_t bunchCrossing)
+bool DataDecoder::TimeFrameStartRecord::update(uint32_t orbit, uint32_t bunchCrossing, bool verbose)
 {
-  mOrbitPrev = mOrbit;
-  mBunchCrossingPrev = mBunchCrossing;
+  if (verbose) {
+    LOGP(info, "[TimeFrameStartRecord::update()] new {}/{}  current {}/{}  prev {}/{}  valid {}",
+         orbit, bunchCrossing, mOrbit, mBunchCrossing, mOrbitPrev, mBunchCrossingPrev, mValid);
+  }
+
+  // if this is the first occurence, simply initialize the orbit and binch crossing and mark the start recortd as valid
+  if (mOrbit < 0) {
+    mOrbit = orbit;
+    mBunchCrossing = bunchCrossing;
+
+    mValid = true;
+    return true;
+  }
+
+  if (mOrbit == orbit) {
+    if (mValid) {
+      // there is already one valid record for this orbit, so the current one is discarded
+      return false;
+    } else {
+      // there is already one record for this orbit, but it is invalid
+      // check if the current one is compatible with the previous, if yes replace the existing one
+      if (check(orbit, bunchCrossing, mOrbitPrev, mBunchCrossingPrev, verbose)) {
+        mOrbit = orbit;
+        mBunchCrossing = bunchCrossing;
+
+        mValid = true;
+      }
+    }
+  } else {
+    // we received an HB packet for a new TF, check if it is compatible with the last stored one
+    bool valid = check(orbit, bunchCrossing, mOrbit, mBunchCrossing, verbose);
+
+    bool replace = valid || (mOrbitPrev < 0) || (mValid == false);
+
+    if (replace) {
+      mOrbitPrev = mOrbit;
+      mBunchCrossingPrev = mBunchCrossing;
+
+      mOrbit = orbit;
+      mBunchCrossing = bunchCrossing;
+
+      mValid = valid;
+    }
+  }
+
+  if (verbose) {
+    LOGP(info, "[TimeFrameStartRecord::update()] set to {}/{}  prev {}/{}  valid {}",
+         mOrbit, mBunchCrossing, mOrbitPrev, mBunchCrossingPrev, mValid);
+  }
 
-  mOrbit = orbit;
-  mBunchCrossing = bunchCrossing;
+  return mValid;
 }
 
-bool DataDecoder::TimeFrameStartRecord::check()
+bool DataDecoder::TimeFrameStartRecord::check(int32_t orbit, uint32_t bc, int32_t orbitRef, uint32_t bcRef, bool verbose)
 {
-  if (mOrbitPrev < 0) {
-    return true;
+  if (verbose) {
+    LOGP(info, "[TimeFrameStartRecord::check()] current {}/{}  ref {}/{}", orbit, bc, orbitRef, bcRef);
   }
 
-  if (mOrbit < mOrbitPrev) {
-    return false;
+  if (orbitRef < 0) {
+    return true;
   }
 
-  uint64_t dOrbit = mOrbit - mOrbitPrev;
-  uint64_t bcExpected = dOrbit * bcInOrbit + mBunchCrossingPrev;
+  int64_t dOrbit = orbit - orbitRef;
+  if (verbose) {
+    LOGP(info, "[TimeFrameStartRecord::check()] dOrbit{}", dOrbit);
+  }
+  int64_t bcExpected = dOrbit * bcInOrbit + bcRef;
 
   uint64_t bcExpected20bits = bcExpected & twentyBitsAtOne;
 
-  return (bcExpected20bits == mBunchCrossing);
+  bool result = (bcExpected20bits == bc);
+
+  if (verbose) {
+    LOGP(info, "  dOrbit {}  expected {}  expected20bits {}  bc {}  valid {}", dOrbit, bcExpected, bcExpected20bits, bc, result);
+  }
+
+  return result;
 }
 
 //_________________________________________________________________________________________________
@@ -374,6 +429,12 @@ void DataDecoder::updateMergerRecord(uint32_t mergerChannelId, uint32_t mergerBo
   mergerCh.digitId = digitId;
   mergerCh.bcEnd = digit.info.bunchCrossing + (digit.info.sampaTime + digit.digit.getNofSamples() - 1) * 4;
   mMergerRecordsReady[mergerBoardId] |= mergerChannelBitmask;
+  if (mDebug) {
+    std::cout << fmt::format("[updateMergerRecord] updated S{}-DS{}-CHIP{}  time {}-{}-{}  cs {}",
+                             (int)digit.info.solar, (int)digit.info.ds, (int)digit.info.chip,
+                             (int)digit.info.orbit, (int)digit.info.bunchCrossing, (int)digit.info.sampaTime, (int)digit.digit.getNofSamples())
+              << std::endl;
+  }
 }
 
 //_________________________________________________________________________________________________
@@ -501,18 +562,20 @@ void DataDecoder::decodePage(gsl::span<const std::byte> page)
 
     uint32_t bcTF = subtractBcOffset(bunchCrossing, mSampaTimeOffset);
 
-    updateTimeFrameStartRecord(chipId, mFirstOrbitInTF, bcTF);
-
     if (mDebug) {
       auto s = asString(dsElecId);
       LOGP(info, "HeartBeat: {}-CHIP{} -> {}/{} offset {} -> bcTF {}",
            s, chip, mFirstOrbitInTF, bunchCrossing, mSampaTimeOffset, bcTF);
     }
 
-    if (!mTimeFrameStartRecords[chipId].check()) {
+    if (chipId >= DataDecoder::sReadoutChipsNum) {
+      return;
+    }
+
+    if (!mTimeFrameStartRecords[chipId].update(mFirstOrbitInTF, bcTF)) {
       if (mErrorCount < MCH_DECODER_MAX_ERROR_COUNT) {
         auto s = asString(dsElecId);
-        LOGP(warning, "Inconsistent HeartBeat packet received: {}-CHIP{} {}/{} (last {}/{})",
+        LOGP(warning, "Bad HeartBeat packet received: {}-CHIP{} {}/{} (last {}/{})",
              s, chip, mFirstOrbitInTF, bcTF, mTimeFrameStartRecords[chipId].mOrbitPrev, mTimeFrameStartRecords[chipId].mBunchCrossingPrev);
         mErrorCount += 1;
       }
@@ -637,6 +700,13 @@ bool DataDecoder::getTimeFrameStartRecord(const RawDigit& digit, uint32_t& orbit
     return false;
   }
 
+  if (tfStart.mValid == false) {
+    if (mErrorCount < MCH_DECODER_MAX_ERROR_COUNT) {
+      LOGP(warning, "Invalid TF start record for S{}-J{}-DS{}-CHIP{}", info.solar, info.ds / 5 + 1, info.ds % 5, info.chip);
+      mErrorCount += 1;
+    }
+  }
+
   // orbit and BC from the last received HB packet
   uint32_t orbitHBP = tfStart.mOrbit;
   // SAMPA BC at the beginning of the current TF
@@ -661,6 +731,7 @@ bool DataDecoder::getTimeFrameStartRecord(const RawDigit& digit, uint32_t& orbit
 void DataDecoder::computeDigitsTime()
 {
   static constexpr int32_t timeInvalid = DataDecoder::tfTimeInvalid;
+  constexpr int BCINORBIT = o2::constants::lhc::LHCMaxBunches;
 
   auto setDigitTime = [&](Digit& d, int32_t tfTime) {
     d.setTime(tfTime);

Detectors/MUON/MCH/Raw/Decoder/src/ROFFinder.cxx

@@ -156,7 +156,6 @@ void ROFFinder::sortDigits()
 
     auto orbit = digit.getOrbit();
     if (orbit < mFirstTForbit) {
-      LOG(error) << "[ROFFinder::fillDigitsArray] orbit smaller than first TF orbit: " << orbit << ", " << mFirstTForbit;
       continue;
     }