From 7292ef480f21c6d365af267142de455756bac994 Mon Sep 17 00:00:00 2001
From: P-A Loizeau <p.-a.loizeau@gsi.de>
Date: Wed, 22 Sep 2021 14:08:05 +0200
Subject: [PATCH] In STS unpackers, use only unsigned for time storage and add
fatal where neg. could happen
Bug 2: In both Legacy and current unpackers, a signed integer was used at some point to store/return the raw timestamp in clock cycles.
While the last bit would probably never be reached in standard usage, it could lead to negative values and crazy results during the
conversion through double (full precision time in ns) and back to unsigned integer (ns precision time) for data coming from badly
synchronized setups
Bug 3: In the current unpacker, the subtraction of the Timeslice start time from the TS_MSB value was not protected against the case of
badly synchronized data where the TS_MSb does not match the Timeslice index. It now triggers a Fatal as these data are not recoverable
with this unpacker.
---
.../detectors/sts/unpack/CbmStsUnpackAlgo.cxx | 56 +++++++++++++------
.../sts/unpack/CbmStsUnpackAlgoLegacy.cxx | 6 +-
.../sts/unpack/CbmStsUnpackAlgoLegacy.h | 2 +-
3 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/reco/detectors/sts/unpack/CbmStsUnpackAlgo.cxx b/reco/detectors/sts/unpack/CbmStsUnpackAlgo.cxx
index 0e583287d1..9c5edfe703 100644
--- a/reco/detectors/sts/unpack/CbmStsUnpackAlgo.cxx
+++ b/reco/detectors/sts/unpack/CbmStsUnpackAlgo.cxx
@@ -449,7 +449,7 @@ void CbmStsUnpackAlgo::processHitInfo(const stsxyter::Message& mess)
const uint32_t uChanInFeb = usChan + fNrChsPerAsic * (uAsicIdx % fNrAsicsPerFeb);
// Compute the Full time stamp
- const int64_t ulHitTime = getFullTimeStamp(usRawTs);
+ const uint64_t ulHitTime = getFullTimeStamp(usRawTs);
/// Store hit for output only if it is mapped to a module!!!
if (0 != fviFebAddress[uFebIdx] && fdAdcCut < usRawAdc) {
@@ -490,7 +490,7 @@ void CbmStsUnpackAlgo::processHitInfo(const stsxyter::Message& mess)
auto tsreltime =
static_cast<uint64_t>((ulHitTime - (fTsStartTime / stsxyter::kdClockCycleNs)) * stsxyter::kdClockCycleNs);
*/
- const double_t tsreltime = ulHitTime * stsxyter::kdClockCycleNs;
+ const double_t tsreltime = static_cast<double_t>(ulHitTime) * stsxyter::kdClockCycleNs;
double dTimeInNs = tsreltime - fSystemTimeOffset;
if (uAsicIdx < fvdTimeOffsetNsAsics.size()) dTimeInNs -= fvdTimeOffsetNsAsics[uAsicIdx];
@@ -587,16 +587,17 @@ void CbmStsUnpackAlgo::processTsMsbInfo(const stsxyter::Message& mess, uint32_t
fvuCurrentTsMsbCycle[fuCurrDpbIdx]++;
}
- if (
- uVal != fvulCurrentTsMsb[fuCurrDpbIdx] + 1 && !(0 == uVal && 4194303 == fvulCurrentTsMsb[fuCurrDpbIdx])
- && /// Case where we reach a normal cycle edge
- 1 != uMessIdx && /// First TS_MSB in MS may jump if TS dropped by DAQ
- !(0 == uVal && 0 == fvulCurrentTsMsb[fuCurrDpbIdx] && 2 == uMessIdx) && /// case with cycle et edge of 2 MS
- !(uVal == fvulCurrentTsMsb[fuCurrDpbIdx] && 2 == uMessIdx)
- && /// Msg 1 and 2 will be same TS_MSB if data in 1st bin
- uVal < fvulCurrentTsMsb
- [fuCurrDpbIdx] /// New FW introduced TS_MSB suppression + large TS_MSB => warning only if value not increasing
- ) {
+ if (uVal != fvulCurrentTsMsb[fuCurrDpbIdx] + 1
+ /// Case where we reach a normal cycle edge
+ && !(0 == uVal && 4194303 == fvulCurrentTsMsb[fuCurrDpbIdx])
+ /// First TS_MSB in MS may jump if TS dropped by DAQ
+ && 1 != uMessIdx
+ /// case with cycle et edge of 2 MS
+ && !(0 == uVal && 0 == fvulCurrentTsMsb[fuCurrDpbIdx] && 2 == uMessIdx)
+ /// Msg 1 and 2 will be same TS_MSB if data in 1st bin
+ && !(uVal == fvulCurrentTsMsb[fuCurrDpbIdx] && 2 == uMessIdx)
+ /// New FW introduced TS_MSB suppression + large TS_MSB => warning only if value not increasing
+ && uVal < fvulCurrentTsMsb[fuCurrDpbIdx]) {
LOG(debug) << "TS MSb Jump in "
<< " TS " << std::setw(12) << fTsIndex << " MS Idx " << std::setw(4) << uMsIdx << " Msg Idx "
<< std::setw(5) << uMessIdx << " DPB " << std::setw(2) << fuCurrDpbIdx << " => Old TsMsb "
@@ -611,8 +612,17 @@ void CbmStsUnpackAlgo::processTsMsbInfo(const stsxyter::Message& mess, uint32_t
fulTsMsbIndexInTs[fuCurrDpbIdx] =
fvulCurrentTsMsb[fuCurrDpbIdx]
- + (fvuCurrentTsMsbCycle[fuCurrDpbIdx] * static_cast<uint64_t>(1 << stsxyter::kusLenTsMsbValBinning))
- - fulTsStartInTsMsb;
+ + (fvuCurrentTsMsbCycle[fuCurrDpbIdx] * static_cast<uint64_t>(1 << stsxyter::kusLenTsMsbValBinning));
+ if (fulTsMsbIndexInTs[fuCurrDpbIdx] < fulTsStartInTsMsb) {
+ LOG(fatal) << "CbmStsUnpackAlgo::processTsMsbInfo => "
+ << "Value computed from TS_MSB and TS_MSB cycle smaller than Timeslice start in TS_MSB, "
+ << "would lead to a negative value so it cannot be recovered!!!!"
+ << std::endl
+ /// Values Printout
+ << "TS_MSB: " << fvulCurrentTsMsb[fuCurrDpbIdx] << " Cycle: " << fvuCurrentTsMsbCycle[fuCurrDpbIdx]
+ << " Full TS_MSB: " << fulTsMsbIndexInTs[fuCurrDpbIdx] << " TS Start offset: " << fulTsStartInTsMsb;
+ }
+ fulTsMsbIndexInTs[fuCurrDpbIdx] -= fulTsStartInTsMsb;
if (fMonitor)
if (fMonitor->GetDebugMode()) { //also if( 1 < uMessIdx )?
@@ -627,6 +637,9 @@ void CbmStsUnpackAlgo::refreshTsMsbFields(const uint32_t imslice, const size_t m
{
const uint32_t uTsMsbCycleHeader =
std::floor(mstime / (stsxyter::kulTsCycleNbBinsBinning * stsxyter::kdClockCycleNs));
+ const uint32_t uTsMsbHeader =
+ std::floor((mstime - uTsMsbCycleHeader * (stsxyter::kulTsCycleNbBinsBinning * stsxyter::kdClockCycleNs))
+ / (stsxyter::kuHitNbTsBinsBinning * stsxyter::kdClockCycleNs));
if (0 == imslice) {
if (uTsMsbCycleHeader != fvuCurrentTsMsbCycle[fuCurrDpbIdx])
@@ -635,7 +648,7 @@ void CbmStsUnpackAlgo::refreshTsMsbFields(const uint32_t imslice, const size_t m
<< fuCurrDpbIdx << " Old TsMsb " << std::setw(5) << fvulCurrentTsMsb[fuCurrDpbIdx] << " Old MsbCy "
<< std::setw(5) << fvuCurrentTsMsbCycle[fuCurrDpbIdx] << " New MsbCy " << uTsMsbCycleHeader;
fvuCurrentTsMsbCycle[fuCurrDpbIdx] = uTsMsbCycleHeader;
- fvulCurrentTsMsb[fuCurrDpbIdx] = 0;
+ fvulCurrentTsMsb[fuCurrDpbIdx] = uTsMsbHeader;
}
else if (uTsMsbCycleHeader != fvuCurrentTsMsbCycle[fuCurrDpbIdx]) {
if (4194303 == fvulCurrentTsMsb[fuCurrDpbIdx]) {
@@ -654,8 +667,17 @@ void CbmStsUnpackAlgo::refreshTsMsbFields(const uint32_t imslice, const size_t m
}
fulTsMsbIndexInTs[fuCurrDpbIdx] =
fvulCurrentTsMsb[fuCurrDpbIdx]
- + (fvuCurrentTsMsbCycle[fuCurrDpbIdx] * static_cast<uint64_t>(1 << stsxyter::kusLenTsMsbValBinning))
- - fulTsStartInTsMsb;
+ + (fvuCurrentTsMsbCycle[fuCurrDpbIdx] * static_cast<uint64_t>(1 << stsxyter::kusLenTsMsbValBinning));
+ if (fulTsMsbIndexInTs[fuCurrDpbIdx] < fulTsStartInTsMsb) {
+ LOG(fatal) << "CbmStsUnpackAlgo::refreshTsMsbFields => "
+ << "Value computed from TS_MSB and TS_MSB cycle smaller than Timeslice start in TS_MSB, "
+ << "would lead to a negative value so it cannot be recovered!!!!"
+ << std::endl
+ /// Values Printout
+ << "TS_MSB: " << fvulCurrentTsMsb[fuCurrDpbIdx] << " Cycle: " << fvuCurrentTsMsbCycle[fuCurrDpbIdx]
+ << " Full TS_MSB: " << fulTsMsbIndexInTs[fuCurrDpbIdx] << " TS Start offset: " << fulTsStartInTsMsb;
+ }
+ fulTsMsbIndexInTs[fuCurrDpbIdx] -= fulTsStartInTsMsb;
}
// ---- unpack ----
diff --git a/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.cxx b/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.cxx
index 4b903078da..19bc9e5dad 100644
--- a/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.cxx
+++ b/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.cxx
@@ -587,7 +587,7 @@ void CbmStsUnpackAlgoLegacy::ProcessHitInfo(const stsxyter::Message& mess)
fvvusLastTsMsbCycleChan[uAsicIdx][usChan] = fvuCurrentTsMsbCycle[fuCurrDpbIdx];
// Compute the Full time stamp
- const int64_t ulHitTime = GetFullTimeStamp(usRawTs);
+ const uint64_t ulHitTime = GetFullTimeStamp(usRawTs);
/// Store hit for output only if it is mapped to a module!!!
if (0 != fviFebAddress[uFebIdx] && fdAdcCut < usRawAdc) {
@@ -708,10 +708,10 @@ void CbmStsUnpackAlgoLegacy::ProcessStatusInfo(const stsxyter::Message& mess, ui
}
// -------------------------------------------------------------------------
-int64_t CbmStsUnpackAlgoLegacy::GetFullTimeStamp(const uint16_t usRawTs)
+uint64_t CbmStsUnpackAlgoLegacy::GetFullTimeStamp(const uint16_t usRawTs)
{
// Use TS w/o overlap bits as they will anyway come from the TS_MSB
- const int64_t ulTime =
+ const uint64_t ulTime =
usRawTs
+ static_cast<uint64_t>(stsxyter::kuHitNbTsBinsBinning) * static_cast<uint64_t>(fvulCurrentTsMsb[fuCurrDpbIdx])
+ static_cast<uint64_t>(stsxyter::kulTsCycleNbBinsBinning)
diff --git a/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.h b/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.h
index b81fa7e761..f3768c5709 100644
--- a/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.h
+++ b/reco/detectors/sts/unpack/CbmStsUnpackAlgoLegacy.h
@@ -107,7 +107,7 @@ private:
void AddHitsToDigiVect(std::vector<stsxyter::FinalHit>* vmHitsIn, std::vector<CbmStsDigi>* vDigiVectOut);
/// Get full time stamp from raw time stamp
- int64_t GetFullTimeStamp(const uint16_t usRawTs);
+ uint64_t GetFullTimeStamp(const uint16_t usRawTs);
/// User settings: Data correction parameters
double fdTimeOffsetNs;
--
GitLab