Repository navigation
Conversation
…tion parser to FT0 task
…ues in FT0DataProcessor
…r of channels in FT0 configuration to 208
…static memory corruption issue in HV schema; fixed schema issues in both FEE and HV
…to FITDCSConfigProcessor, added back DeadChannelMap definition in link file'
Please consider the following formatting changes to AliceO2Group#15888
| // granted to it by virtue of its status as an Intergovernmental Organization | ||
| // or submit itself to any jurisdiction. | ||
|
|
||
| // \file FeeConfiguration.h |
There was a problem hiding this comment.
correct filename in comments
| uint16_t adcDelays[NChannels]; | ||
| uint16_t rangeCorrectionAdc0[NChannels]; | ||
| uint16_t rangeCorrectionAdc1[NChannels]; | ||
| uint16_t thresholdCalibration[NChannels]; |
There was a problem hiding this comment.
thresholdCalibration is not included later in operator==(const ChannelsConfig& other) comparison and also not defined as a getter. Any specific reasons ?
|
|
||
| template <int NChannels> | ||
| struct ChannelsConfig { | ||
| int16_t timeAligments[NChannels]; |
There was a problem hiding this comment.
Spelling of "Alignment" is incorrect almost throughout the draft.
| { | ||
| struct TriggersConfig { | ||
| int16_t vertexTimeLowThreshold{o2::fit::config_helpers::DefaultValue}; | ||
| int16_t vertexTimeHighThreshold{o2::fit::config_helpers::DefaultValue}; |
There was a problem hiding this comment.
Just cross-check the data types..
FDD has the same time thresholds declared as "float" and here is "int16_t"
| #pragma link C++ struct o2::fv0::Fv0FeeConfiguration + ; | ||
| #pragma link C++ struct o2::fit::ChannelsConfig < 48> + ; | ||
| #pragma link C++ struct o2::fv0::Fv0HvConfiguration + ; | ||
| #pragma link C++ struct o2::fit::HvChannelsConfig < 48> + ; |
There was a problem hiding this comment.
Missing ??
#pragma link C++ struct o2::fv0::TriggersConfig + ;
| void setValidateUpload(bool validate) | ||
| { | ||
| mValidateUpload = validate; | ||
| } |
There was a problem hiding this comment.
mValidateUpload is not initialized in the constructor.
If createObjectInfo() is called before setValidateUpload(), this reads an indeterminate value.
Should we provide an explicit default initialization?
|
|
||
| namespace o2::fit | ||
| { | ||
| rapidjson::Document FITDCSBaseConfigReader::parseJsonBuffer(gsl::span<const char> buffer, bool throwOnInvalidSchema) |
There was a problem hiding this comment.
Maybe we should also check document.HasParseError() in parseJsonBuffer(), as we already do for schemaDocument in loadSchema() ?
Currently, improper JSON could be returned as the schema validation is being skipped here..
| { | ||
| long dataTime = (long)(pc.services().get<o2::framework::TimingInfo>().creation); | ||
| if (dataTime == 0xffffffffffffffff) { // means it is not set | ||
| dataTime = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::high_resolution_clock::now().time_since_epoch()).count(); // in ms |
There was a problem hiding this comment.
The use of std::chrono::high_resolution_clock here might not guarantee a Unix-based epoch, whereas CCDB validity timestamps require Unix time in milliseconds. Could we double-check whether this is safe? Alternatively, std::chrono::system_clock is also an option.
| BOOST_CHECK_EQUAL(parsed.adcZeros[idx], adcZeros[idx]); | ||
| BOOST_CHECK_EQUAL(parsed.adcDelays[idx], adcDelays[idx]); | ||
| BOOST_CHECK_EQUAL(parsed.channelMaskData[idx], channelMaskData[idx]); | ||
| BOOST_CHECK_EQUAL(parsed.channelMaskTriggers[idx], channelMaskTriggers[idx]); |
There was a problem hiding this comment.
Just curious.. Any reason not to include thresholdCalibration in the test payload ? This is part of ChannelsConfig, but it currently isn’t covered by the channel parsing test.
| SimpleFITFEEConfigurationReader reader; | ||
| reader.parsePmConfig(doc, pmConfig); | ||
|
|
||
| BOOST_CHECK_EQUAL(pmConfig.orGate, orGateValue); |
There was a problem hiding this comment.
I understand that this is just checking this pmConfig.orGate. We should also check pmConfig.trgChargeLowLevel and pmConfig.trgChargeHighLevel on PM level.
Also, which one actually is this pmConfig.orGate variable ?
Implementation of FEE configuration CCDB object and DCS workflow that fetches JSON from DCS and transforms it to the CCDB object