Skip to content

MuonTGC_Cabling: Fix LVL1TGC SROD mapping.

SROD IDs are in the range 17 to 19. However, MuonTGC_CablingSvc::getSReadoutIDfromSLID can return IDs outside this range. For example, with phi=15, isAside=true, and isEndcap=true, it returns srodID=21.

I think this is due to a typo, writing NumberOfReadoutSector instead of NumberOfSReadoutSector. Fixed here.

This was resulting in crashes in the tests for !64380 (merged) (in particular, running  --filesInput=/cvmfs/ Trigger.triggerMenuSetup="Dev_pp_run3_v1" Trigger.doRuntimeNaviVal=True

in release 23).

This was cleaning up some cppcheck warnings in TrigT1GC. But then i was seeing a crash coming from LVL1TGCTrigger::recordRdoInner:

    int subDetectorId=0, rodId=0, sswId=0, sbLoc=0;

    bool status = m_cabling->getSReadoutIDfromSLID(phi, isAside, isEndcap,
                                                   subDetectorId, rodId, sswId, sbLoc);
    if (!status) { ... }

    //  secID for TGCRawData
    //  0-3(EC), 0-1(FWD) for 1/12 sector
    //  0-15(EC), 0-7(FWD) for 1/3 sector covered by SROD in RUn3
    int startEndcapSector, coverageOfEndcapSector;
    int startForwardSector, coverageOfForwardSector;
                                     ) ;

    int secId = 0;
    if (isEndcap){
      secId = sectorId % coverageOfEndcapSector;

For the phi mentioned above, we were getting rodId=21. In that case, getCoveragefromSRodID returns false and does not store anything in the output variables. So in particular, coverageOfEndcapSector is then uninitialized. Without the changes in !64380 (merged), it is garbage, but just happens to always be nonzero. But with the change, the value of the uninitialized coverageOfEndcapSector happens to be zero, which results in a trap when it is used with the modulus operator.

The return value of getCoveragefromSRodID really ought to be checked.

Further, i'll note in passing that LVL1TGCTrigger::recordRdoInner starts by requiring isEndcap==true, but then has tests on isEndcap further down. This is the sort of thing that some of the static checkers warns about.

Merge request reports