Skip to content

Implementation of parsing of LHC-IF file - #7148

Merged
shahor02 merged 4 commits into
AliceO2Group:devfrom
chiarazampolli:LHCIF_file
Oct 6, 2021
Merged

Implementation of parsing of LHC-IF file#7148
shahor02 merged 4 commits into
AliceO2Group:devfrom
chiarazampolli:LHCIF_file

Conversation

@chiarazampolli

Copy link
Copy Markdown
Collaborator

Parser

Workflow and data type

space checker

clang-format

@chiarazampolli
chiarazampolli requested review from a team and shahor02 as code owners September 28, 2021 22:04
@chiarazampolli

Copy link
Copy Markdown
Collaborator Author

Hello @shahor02 ,

This is the workflow to process the LHC IF file. There is one open point: the dcs-config-proxy expects that the name of the file starts with the three letters of the detector, but we don't have GRP yet. Should it be added? For the development I used "TOF", but it is clearly wrong.

This was tested simulating the DCS server sending the file, as we did long ago when developing the dcs-config-proxy.

Chiara

Parser

Workflow and data type

space checker

clang-format

remove file added by mistake
@shahor02

Copy link
Copy Markdown
Collaborator

There is one open point: the dcs-config-proxy expects that the name of the file starts with the three letters of the detector, but we don't have GRP yet. Should it be added?

Hi @chiarazampolli Expanding DetID will create a mess in many places, we better patch the dcs-config-proxy in such a way, that when failing to recognize the detector from the 1st 3 letters, before throwing an error it will check a list of allowed extra names.

@chiarazampolli
chiarazampolli force-pushed the LHCIF_file branch 2 times, most recently from ae6d407 to 087795d Compare September 29, 2021 13:54
@chiarazampolli

Copy link
Copy Markdown
Collaborator Author

Hi @shahor02 ,

Thanks for the comment and suggestion, I implemented GRP as an exception.

Chiara

@chiarazampolli

Copy link
Copy Markdown
Collaborator Author

Hello @shahor02 , @ktf , @TimoWilken

The error in the CI is unrelated to this PR, I don't know why it appears. The same happened on this same PR the other day, but with a different error (then it changed because I added a commit).

Chiara

@shahor02

Copy link
Copy Markdown
Collaborator

It is due to my bug which is already fixed

@ktf

ktf commented Sep 30, 2021

Copy link
Copy Markdown
Member

I restarted the test.

@chiarazampolli

Copy link
Copy Markdown
Collaborator Author

Thanks!

remove logging

clang-format

changing schema --> scheme
Comment thread DataFormats/Parameters/CMakeLists.txt Outdated
Comment thread DataFormats/Parameters/CMakeLists.txt Outdated
Comment thread Detectors/DCS/testWorkflow/src/dcs-config-proxy.cxx Outdated
Comment thread Detectors/DCS/testWorkflow/src/dcs-config-proxy.cxx Outdated
Comment thread Detectors/GRP/calibration/include/GRPCalibration/LHCIFfileReader.h Outdated
Comment thread Detectors/GRP/calibration/src/LHCIFfileReader.cxx Outdated
Comment thread Detectors/GRP/calibration/src/LHCIFfileReader.cxx Outdated
Comment thread Detectors/GRP/calibration/include/GRPCalibration/LHCIFfileReader.h
Comment thread DataFormats/Parameters/include/DataFormatsParameters/LHCIFData.h Outdated
Comment thread DataFormats/Parameters/include/DataFormatsParameters/LHCIFData.h Outdated
clang-format

leftover fixed

@shahor02 shahor02 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @chiarazampolli

Please see more comments below.
For the time: we use everywhere long, better to stick to it rather than uint64_t, to avoid later warnings about signed/unsigned comparison.

Comment thread DataFormats/Parameters/src/ParametersDataLinkDef.h Outdated
Comment thread Detectors/DCS/testWorkflow/src/dcs-config-proxy.cxx
@chiarazampolli

Copy link
Copy Markdown
Collaborator Author

uint64_t changed to long.

Chiara

@shahor02
shahor02 merged commit 787a076 into AliceO2Group:dev Oct 6, 2021
@chiarazampolli
chiarazampolli deleted the LHCIF_file branch May 16, 2022 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants