From 90a2a75e97e263a1ccf0cf389ea717b427422f9d Mon Sep 17 00:00:00 2001
From: Dave Casper <dcasper@uci.edu>
Date: Thu, 10 Sep 2020 05:12:12 -0700
Subject: [PATCH] Switch TrackerDataFragment to use logger

---
 .../TrackerByteStream/CMakeLists.txt          |   2 +-
 .../src/TrackerDataDecoderTool.cxx            | 220 +++++++++---------
 faser-common                                  |   2 +-
 3 files changed, 116 insertions(+), 108 deletions(-)

diff --git a/Tracker/TrackerEventCnv/TrackerByteStream/CMakeLists.txt b/Tracker/TrackerEventCnv/TrackerByteStream/CMakeLists.txt
index e80463b6..2ae44b44 100644
--- a/Tracker/TrackerEventCnv/TrackerByteStream/CMakeLists.txt
+++ b/Tracker/TrackerEventCnv/TrackerByteStream/CMakeLists.txt
@@ -8,7 +8,7 @@ atlas_subdir( TrackerByteStream )
 atlas_add_component( TrackerByteStream
                      src/*.cxx src/*.h
                      src/components/*.cxx
-                     LINK_LIBRARIES AthenaKernel GaudiKernel StoreGateLib FaserByteStreamCnvSvcBaseLib FaserEventStorageLib TrackerRawData TrackerReadoutGeometry TrackerIdentifier
+                     LINK_LIBRARIES AthenaKernel GaudiKernel StoreGateLib FaserByteStreamCnvSvcBaseLib FaserEventStorageLib TrackerRawData TrackerReadoutGeometry TrackerIdentifier EventFormats
                      PRIVATE_LINK_LIBRARIES AthenaBaseComps )
 
 
diff --git a/Tracker/TrackerEventCnv/TrackerByteStream/src/TrackerDataDecoderTool.cxx b/Tracker/TrackerEventCnv/TrackerByteStream/src/TrackerDataDecoderTool.cxx
index edd3ae05..7f7763b8 100644
--- a/Tracker/TrackerEventCnv/TrackerByteStream/src/TrackerDataDecoderTool.cxx
+++ b/Tracker/TrackerEventCnv/TrackerByteStream/src/TrackerDataDecoderTool.cxx
@@ -117,132 +117,140 @@ TrackerDataDecoderTool::convert(const DAQFormats::EventFull* re,
     int station = 2 + trb / TrackerDataFragment::PLANES_PER_STATION; 
     int plane = trb % TrackerDataFragment::PLANES_PER_STATION;
     
-    TrackerDataFragment trackFrag{ frag->payload<const uint32_t*>(), frag->payload_size() };
-
-    if (!trackFrag.valid())
-    {
-      ATH_MSG_WARNING("Invalid tracker data fragment for TRB " << trb);
-      // FIXME: What else to do if error?  Return FAILURE?
-      // FIXME: Validity checking in TrackerDataFragment is a placeholder
-      continue;
-    }
-    if (trackFrag.event_id() != re->event_id())
+    // Exceptions are a no-no in Athena/Calypso, so catch any thrown by faser-common
+    try
     {
-      ATH_MSG_ERROR("Event ID mismatch for tracker data fragment from trb " << trb << 
-                    "; found " << trackFrag.event_id() << " but expected " << re->event_id());
-      // FIXME: Is returning FAILURE the right thing to do here?
-      return StatusCode::FAILURE;
-    }
-    validFragments++;
-    // auto bcid = trackFrag.bc_id();
-    ATH_MSG_DEBUG("Processing tracker data fragment for TRB " << trb);
+      TrackerDataFragment trackFrag{ frag->payload<const uint32_t*>(), frag->payload_size() };
 
-    for (const SCTEvent* sctEvent : trackFrag)
-    {
-      if (sctEvent != nullptr)
+      if (!trackFrag.valid())
       {
-        unsigned short onlineModuleID = sctEvent->GetModuleID(); 
-        if (plane == 2) onlineModuleID = (onlineModuleID + 4) % 8;
-        if (onlineModuleID >= TrackerDataFragment::MODULES_PER_FRAGMENT)
-        {
-          // FIXME: Should we return failure here?
-          ATH_MSG_ERROR("Invalid module ID (" << onlineModuleID << ") from trb " << trb);
-          continue;
-        }
-        if (sctEvent->BCIDMismatch())
-        {
-          // FIXME: Should we return failure here?
-          ATH_MSG_ERROR("Module data BCID mismatch between sides for online module " << onlineModuleID);
-          continue;
-        }
-        else if (sctEvent->HasError())
-        {
-          // FIXME: Should we return failure here?
-          ATH_MSG_ERROR("Online module " << onlineModuleID << " reports one or more errors.");
-          continue;
-        }
-        else if (sctEvent->MissingData())
-        {
-          // FIXME: Should we return failure here?
-          ATH_MSG_ERROR("Online module " << onlineModuleID << " reports missing data.");
-          continue;
-        }
-        else if (!sctEvent->IsComplete())
-        {
-          // FIXME: Should we return failure here?
-          ATH_MSG_ERROR("Online module " << onlineModuleID << " reports not complete.");
-          continue;
-        }
-        // Not satisfied, at least for cosmics data
-        // else if (sctEvent->GetBCID() != bcid)
-        // {
-        //   // FIXME: Should we return failure here?
-        //   ATH_MSG_ERROR("Module data BCID (" << sctEvent->GetBCID() << ")for online module " << 
-        //                  onlineModuleID << " differs from TrackerDataFragment (" << bcid << ")");
-        //   continue;
-        // }
-        ATH_MSG_DEBUG("Processing online module #" << onlineModuleID);
-        // Offline module number
-        uint32_t module = m_moduleMap[onlineModuleID];
-        size_t chipIndex{0};
-        for (auto hitVector : sctEvent->GetHits())
+        ATH_MSG_WARNING("Invalid tracker data fragment for TRB " << trb);
+        // FIXME: What else to do if error?  Return FAILURE?
+        // FIXME: Validity checking in TrackerDataFragment is a placeholder
+        continue;
+      }
+      if (trackFrag.event_id() != re->event_id())
+      {
+        ATH_MSG_ERROR("Event ID mismatch for tracker data fragment from trb " << trb << 
+                      "; found " << trackFrag.event_id() << " but expected " << re->event_id());
+        // FIXME: Is returning FAILURE the right thing to do here?
+        return StatusCode::FAILURE;
+      }
+      validFragments++;
+      // auto bcid = trackFrag.bc_id();
+      ATH_MSG_DEBUG("Processing tracker data fragment for TRB " << trb);
+
+      for (const SCTEvent* sctEvent : trackFrag)
+      {
+        if (sctEvent != nullptr)
         {
-          if (hitVector.size() > 0)
+          unsigned short onlineModuleID = sctEvent->GetModuleID(); 
+          if (plane == 2) onlineModuleID = (onlineModuleID + 4) % 8;
+          if (onlineModuleID >= TrackerDataFragment::MODULES_PER_FRAGMENT)
           {
-            int side = chipIndex / TrackerDataFragment::CHIPS_PER_SIDE;
-            uint32_t chipOnSide = chipIndex % TrackerDataFragment::CHIPS_PER_SIDE;
-            for (auto hit : hitVector)
+            // FIXME: Should we return failure here?
+            ATH_MSG_ERROR("Invalid module ID (" << onlineModuleID << ") from trb " << trb);
+            continue;
+          }
+          if (sctEvent->BCIDMismatch())
+          {
+            // FIXME: Should we return failure here?
+            ATH_MSG_ERROR("Module data BCID mismatch between sides for online module " << onlineModuleID);
+            continue;
+          }
+          else if (sctEvent->HasError())
+          {
+            // FIXME: Should we return failure here?
+            ATH_MSG_ERROR("Online module " << onlineModuleID << " reports one or more errors.");
+            continue;
+          }
+          else if (sctEvent->MissingData())
+          {
+            // FIXME: Should we return failure here?
+            ATH_MSG_ERROR("Online module " << onlineModuleID << " reports missing data.");
+            continue;
+          }
+          else if (!sctEvent->IsComplete())
+          {
+            // FIXME: Should we return failure here?
+            ATH_MSG_ERROR("Online module " << onlineModuleID << " reports not complete.");
+            continue;
+          }
+          // Not satisfied, at least for cosmics data
+          // else if (sctEvent->GetBCID() != bcid)
+          // {
+          //   // FIXME: Should we return failure here?
+          //   ATH_MSG_ERROR("Module data BCID (" << sctEvent->GetBCID() << ")for online module " << 
+          //                  onlineModuleID << " differs from TrackerDataFragment (" << bcid << ")");
+          //   continue;
+          // }
+          ATH_MSG_DEBUG("Processing online module #" << onlineModuleID);
+          // Offline module number
+          uint32_t module = m_moduleMap[onlineModuleID];
+          size_t chipIndex{0};
+          for (auto hitVector : sctEvent->GetHits())
+          {
+            if (hitVector.size() > 0)
             {
-              uint32_t stripOnChip = hit.first;
-              if (stripOnChip >= TrackerDataFragment::STRIPS_PER_CHIP)
+              int side = chipIndex / TrackerDataFragment::CHIPS_PER_SIDE;
+              uint32_t chipOnSide = chipIndex % TrackerDataFragment::CHIPS_PER_SIDE;
+              for (auto hit : hitVector)
               {
-                // FIXME: Return failure?
-                ATH_MSG_ERROR("Invalid strip number on chip: " << stripOnChip );
-                continue;
-              }
-              uint32_t hitPattern  = hit.second;
-              if (hitMode == HitMode::EDGE && (((hitPattern & 0x2) == 0 ) || ((hitPattern & 0x4) != 0) ) ) continue; // 01X
-              if (hitMode == HitMode::LEVEL && ((hitPattern & 0x2) == 0)) continue; // X1X
-              int phiModule = module % 4; // 0 to 3 from bottom to top
-              int etaModule = -1 + 2*((module%2 + module/4) % 2); // -1 or +1
-              // ATH_MSG_DEBUG("Getting wafer_id for station, plane, phi, eta, side = " << station << " " << plane << " " << phiModule << " " << etaModule << " " << side);
-              Identifier id = m_sctID->wafer_id(station, plane, phiModule, etaModule, side);
-              IdentifierHash waferHash = m_sctID->wafer_hash(id);  // this will be the collection number in the container
-              int stripOnSide = chipOnSide * TrackerDataFragment::STRIPS_PER_CHIP + stripOnChip;
-              // ATH_MSG_DEBUG("Checking phi reversal for waferhash = " << waferHash << "(" << waferHash.value() << ") strip on side = " << stripOnSide);
-              if (m_phiReversed[waferHash]) stripOnSide = TrackerDataFragment::STRIPS_PER_SIDE - stripOnSide - 1;
-              if (stripOnSide < 0 || static_cast<uint32_t>(stripOnSide) >= TrackerDataFragment::STRIPS_PER_SIDE)
-              {
-                // FIXME: Return failure?
-                ATH_MSG_ERROR("Invalid strip number on side: " << stripOnSide);
-                continue;
-              }
-              Identifier digitID {m_sctID->strip_id(id, stripOnSide)};
-              int errors{0};
-              int groupSize{1};
-              unsigned int rawDataWord{static_cast<unsigned int>(groupSize | (stripOnSide << 11) | (hitPattern <<22) | (errors << 25))};
+                uint32_t stripOnChip = hit.first;
+                if (stripOnChip >= TrackerDataFragment::STRIPS_PER_CHIP)
+                {
+                  // FIXME: Return failure?
+                  ATH_MSG_ERROR("Invalid strip number on chip: " << stripOnChip );
+                  continue;
+                }
+                uint32_t hitPattern  = hit.second;
+                if (hitMode == HitMode::EDGE && (((hitPattern & 0x2) == 0 ) || ((hitPattern & 0x4) != 0) ) ) continue; // 01X
+                if (hitMode == HitMode::LEVEL && ((hitPattern & 0x2) == 0)) continue; // X1X
+                int phiModule = module % 4; // 0 to 3 from bottom to top
+                int etaModule = -1 + 2*((module%2 + module/4) % 2); // -1 or +1
+                // ATH_MSG_DEBUG("Getting wafer_id for station, plane, phi, eta, side = " << station << " " << plane << " " << phiModule << " " << etaModule << " " << side);
+                Identifier id = m_sctID->wafer_id(station, plane, phiModule, etaModule, side);
+                IdentifierHash waferHash = m_sctID->wafer_hash(id);  // this will be the collection number in the container
+                int stripOnSide = chipOnSide * TrackerDataFragment::STRIPS_PER_CHIP + stripOnChip;
+                // ATH_MSG_DEBUG("Checking phi reversal for waferhash = " << waferHash << "(" << waferHash.value() << ") strip on side = " << stripOnSide);
+                if (m_phiReversed[waferHash]) stripOnSide = TrackerDataFragment::STRIPS_PER_SIDE - stripOnSide - 1;
+                if (stripOnSide < 0 || static_cast<uint32_t>(stripOnSide) >= TrackerDataFragment::STRIPS_PER_SIDE)
+                {
+                  // FIXME: Return failure?
+                  ATH_MSG_ERROR("Invalid strip number on side: " << stripOnSide);
+                  continue;
+                }
+                Identifier digitID {m_sctID->strip_id(id, stripOnSide)};
+                int errors{0};
+                int groupSize{1};
+                unsigned int rawDataWord{static_cast<unsigned int>(groupSize | (stripOnSide << 11) | (hitPattern <<22) | (errors << 25))};
 
-              if (collectionMap.count(waferHash) == 0)
-              {
+                if (collectionMap.count(waferHash) == 0)
                 {
-                  std::unique_ptr<FaserSCT_RDO_Collection> current_collection = std::make_unique<FaserSCT_RDO_Collection>(waferHash);
-                  current_collection->setIdentifier(id);
-                  collectionMap[waferHash] = std::move(current_collection);
+                  {
+                    std::unique_ptr<FaserSCT_RDO_Collection> current_collection = std::make_unique<FaserSCT_RDO_Collection>(waferHash);
+                    current_collection->setIdentifier(id);
+                    collectionMap[waferHash] = std::move(current_collection);
+                  }
                 }
+                collectionMap[waferHash]->emplace_back(new FaserSCT3_RawData(digitID, rawDataWord, std::vector<int>() ));
               }
-              collectionMap[waferHash]->emplace_back(new FaserSCT3_RawData(digitID, rawDataWord, std::vector<int>() ));
             }
+            chipIndex++;
           }
-          chipIndex++;
         }
       }
+    } 
+    catch (const TrackerDataException& exc)
+    {
+      ATH_MSG_ERROR("TrackerDataException: " << exc.what());
+      continue;
     }
-
   }
 
   if (validFragments == 0) 
   {
-    ATH_MSG_ERROR("Failed to find Tracker fragments in raw event!");
+    ATH_MSG_ERROR("Failed to find any valid Tracker fragments in raw event!");
     return StatusCode::FAILURE;
   }
 
diff --git a/faser-common b/faser-common
index 48f7e3c7..7e7eafb4 160000
--- a/faser-common
+++ b/faser-common
@@ -1 +1 @@
-Subproject commit 48f7e3c7458c17ea30ce50deef68744d26d1c72a
+Subproject commit 7e7eafb4ec4c05f2131b9fb7cb7e5304b3cc7e07
-- 
GitLab