JeffersonLab / analyzer

HallA C++ Analyzer
BSD 3-Clause "New" or "Revised" License
7 stars 54 forks source link

Some hana_decode changes to allow ADC trigger time slippage correction #154

Closed sawjlab closed 6 years ago

sawjlab commented 6 years ago

The trigger time in the header of each FADC250 module is correlates with the trigger time in the TI module, differing by a small fixed amount. This fixed amount sometimes slips by one clock tick (4ns) and correspondingly the pulse times recorded by the ADC shift. By monitoring the difference between the TI and FADC trigger times, this shift in pulse times can be corrected. These commits will allow hcana to get the information it needs to make this correction.

The THaEvData::GetCrateMap is added so that hcana can search the crate map to find the slot # for each crates TI module.

hansenjo commented 6 years ago

I don't want to be difficult, but since this seems to be rather specific to certain modules, I hestitate to make "trigger time" a piece of information that all modules are supposed to provide. That's what would be implied by adding a virtual function

virtual Int_t GetTriggerTime() const {return 0;};

in Module.h.

How about you write your hcana code along these lines:

Decoder::THaCrateMap* map = evdata->GetCrateMap();
for( everything_in_map ) {
   if( current_map_item == TImodule ) {
      TImodule* ti = dynamic_cast<TIModule*>( evdata->GetModule(current_map_crateslot) );
      assert(ti);  // else crate map lied to us
      Int_t ti_trigger_time = ti->GetTriggerTime();
      for( all modules ) {
         Decoder::Fadc250Module* fadc =
             dynamic_cast<Decoder::Fadc250Module*>( evdata->GetModule(crate,slot) );
         if( fadc ) {
            Int_t fadc_trigger_time = fadc->GetTriggerTime();
            // calculate the correction using ti_tigger_time and fadc_trigger_time ...
            break; //exit for(all modules)
         }
      }
      break;  // exit everything_in_map
   }
}

This can be improved by memorizing the crate(s)/slot(s) of all Fadc250 modules beforehand (in Init perhaps), so you don't need to do the dynamic_cast check for every event and module.

In this way, we only have to add a non-virtual method GetTriggerTime() to Fadc250Module (and your TI module).

Adding GetCrateMap() to THaEvData.h is fine.

If this is fine with you, I'll make the changes to the pull request and commit it for you.

sawjlab commented 6 years ago

I think what you are proposing is to drop the commit e101f97c6bb494493c224546aea67f9da77cd5dd and take the other two. Although I like the way I propose, I can work with a GetTriggerTime local to Fadc250Module too.

I do some "memorizing" already. I don't do it in Init because I don't know a good way to get the crate map information there, so I do it first time I process an event as I have access to evdata then.

hansenjo commented 6 years ago

After discussion with Steve, dropped commit e101f97 (i.e. no GetTriggerTime method in THaEvData nd Decoder::Module). Also modified commit 65a0fc1 so that Fadc250Module::GetTriggerTime is not virtual. In this way, we maintain binary compatibility, and fixed a compilation warning. All this is committed to master and Release-160_patches, with HEAD now at commit 5b488328e0e7262f41659cf8ec54250fb6ca31a4.

In the longer run, there should probably be a class MultifunctionModule that inherits from Module and offers virtual functions for common features of intelligent pipelined front end electronics, such as trigger time. This will need some thought. I'll put it as a TODO item in the Redmine task list.