Hi @lietava

see a few more comments below. Since doing an incremental reviews becomes complicated, I would suggest to apply these fixes, make sure that the core produces an output you expect, remove the draft tag and merge after the CI's passed (unless @sawenzel has other comments). The skeleton should be good enough, the rest can be added in other PRs

Hi @shahor02
when I run o2-sim-digitizer-workflow (after o2-sim)
1.) I do not see CTP in the list
Detectors: Cont.RO Triggers
ITS: - -
TPC: - - ..
2.)
[INFO] CTP is in grp? no; is skipped? no

@lietava one should probably impose it as a dummy detector in the simulation. Is all your code committed? Will try to fix this later.

@lietava one should probably impose it as a dummy detector in the simulation. Is all your code committed? Will try to fix this later.

OK. All code is commited.

@lietava I've opened a PR in your repository. After merging it the digitization should work, tested it with

o2-sim -n 10 -m FT0 FV0 ITS CTP -g pythia8 -j10
o2-sim-digitizer-workflow -b

produces ctpdigits.root which seems to be meaningful (lot or after-pulses from FV0)

@lietava I've opened a PR in your repository. After merging it the digitization should work, tested it with

o2-sim -n 10 -m FT0 FV0 ITS CTP -g pythia8 -j10
o2-sim-digitizer-workflow -b

produces ctpdigits.root which seems to be meaningful (lot or after-pulses from FV0)

Thanks. I guess I should merge it - there are three options on web interface, to be safe - I use top option 'Create merge commit' ?

No, just to "Rebase and merge", anyway, we have to squash the multiple commits of your PR.

No, just to "Rebase and merge", anyway, we have to squash the multiple commits of your PR.

Done

@lietava could you fix this (complains from fullCI):

/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5758/0/Detectors/CTP/simulation/src/Digitizer.cxx:30:11: error: statement should be inside braces [readability-braces-around-statements]
/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5758/0//Detectors/CTP/simulation/src/CTPSimulationLinkDef.h:1:1: error: missing or malformed copyright notice

@lietava could you fix this (complains from fullCI):

/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5758/0/Detectors/CTP/simulation/src/Digitizer.cxx:30:11: error: statement should be inside braces [readability-braces-around-statements]
/mnt/mesos/sandbox/sandbox/o2-fullci/sw/SOURCES/O2/5758/0//Detectors/CTP/simulation/src/CTPSimulationLinkDef.h:1:1: error: missing or malformed copyright notice

Fixed.

Please, review.

@lietava did you run the test I mentioned above. If its output is ok for you, I would merge this PR once its passed CI tests.

@shahor02 I fixed two issues from fullCI. Which test should I run ?

@shahor02 I fixed two issues from fullCI. Which test should I run ?

#5758 (comment)

I run
o2-sim -n 10 -m FT0 FV0 ITS CTP -g pythia8 -j10
o2-sim-digitizer-workflow -b
I see in the output
...
[INFO] CTP is in grp? yes; is skipped? no
...
[INFO] Starting CTPDigitizer on pid 27460
File ctpdigits.root created.
I browsed it and I can see there something reasonable. I would leave the content check for later if you agree ?

OK, will merge after CIs are passed

HI Ruben, I fixed small bug. Please, merge.

Hi @lietava : I am a bit confused. This PR was already merged several weeks ago. Are you sure you commented on the right PR?

@davidrohr I understood from Ruben that minor bugs fixes are done in the same PR. I did minor fix related to this PR.

Hi Roman, but this PR here was already merged. If you push additional fixes to its branch, it will not do anything. You can only add minor fixes to the PR before it is merged.
If you want to apply a fix now, you have to open a new PR. If it is urgent you can ping @shahor02 or me to merge it quickly.

OK, thanks. It is not urgent I will open new PR or better leave it for more changes.