this has >200 commits and many conflicts, could you please rebase?

@davidrohr their dev is too old, rebasing produces lot of conflicts, I've sent to @fapfap69 instructions how to resolve them.

@davidrohr
Hi, seems that the Pull Request is blocked for Auth. issues, what I'm wrong ?

@fapfap69 the conflicts are gone but you need apply clang-format. The "merging is blocked" message is there as fall all PRs which were not approved yet.

@fapfap69 : Don't know why you say you don't understand the clang errors.
It clearly shows all the changes it requests: https://github.com/AliceO2Group/AliceO2/pull/5606/checks?check_run_id=2031459520
(At least when I checked 5 seconds ago).

BTW: You should also be able to do it locally by typing git-clang-format origin/dev (after rebasing to dev), and also the CI bot should open a PR to your PR automatically that includes all required changes.

I would do simply do (having the alienv loaded unless the system-wide clang-format is used) git clang-format HEAD~210 (where 210 is the number of commits in this RR), commit the changes and git push -f gvolpe79 RawFileWriter.

@fapfap69 @gvolpe79 : I am testing this now with simulation, there are 2 workflows, o2-hmpid-write-root-from-digits-workflow and o2-hmpid-write-raw-from-root-workflow. The one we need is the 2nd, the 1st one does I don't know what, but such naming is misleading. Could you rename the needed o2-hmpid-write-raw-from-root-workflow to o2-hmpid-digits-to-raw-workflow, then it matches to o2-hmpid-raw-to-digits-workflow.
This workflow has 2 exclusive options --hmp-raw-perlink and --hmp-raw-perflp which is not good. If you want to support all 3 options (all, per-flp, per-link), could you do e.g. like for the ITS:

instead of https://github.com/gvolpe79/AliceO2/blob/283bbca2c244ca231d3f2c49b9e7cd76c4080909/Detectors/HMPID/workflow/src/WriteRawFromRootSpec.cxx#L204-L205 use

{""file-for", VariantType::String, "all", {"single file per: all,flp,link"}},

here : https://github.com/gvolpe79/AliceO2/blob/283bbca2c244ca231d3f2c49b9e7cd76c4080909/Detectors/HMPID/workflow/src/WriteRawFromRootSpec.cxx#L74-L75

auto fileFor = ic.options().get<bool>("file-for");

and pass it to openOutputStream, where you can use it here https://github.com/gvolpe79/AliceO2/blob/283bbca2c244ca231d3f2c49b9e7cd76c4080909/Detectors/HMPID/simulation/src/HmpidCoder2.cxx#L261-L267
as

    if (fileFor=="per-link") {
      sprintf(mFileName, "%s_L%d%s", OutputFileName, ReadOut::FeeId(eq), ".raw");
    } else if (fileFor=="per-flp") {
      sprintf(mFileName, "%s_%d%s", OutputFileName, ReadOut::FlpId(eq), ".raw");
    } else if (fileFor=="all") {
       sprintf(mFileName, "%s%s", OutputFileName, ".raw");
    } else {
      throw std::runtime_error(fmt::format("unknown raw file grouping option {}", fileFor));
    }

@fapfap69 from the tests I see that I have to add few standard functionalities to your workflow, please don't modify the WriteRawFromRootSpec.cxx to avoid the conflicts, I will fix also the options and open a PR in your repo.

@shahor02 I was ready to push modificaations that includes your suggestions, nevertheless now I'll wait your pull request in order to integrate.

@fapfap69 In fact I've implemented my suggestions since anyway had to add some standard features your PR was missing, but when trying to rebase your branch, I again got plenty of conflicts. I see that your branch contains lot of merges, commits from other people (e.g. this 42a83a6) and also your many of your commits are encountered multiple times.
To solve this, I've simply created a new branch using your files, squashing them to a single commit, and added on top of it my commits. Once tested, will open a new PR. Please check my additions, if it is ok, I would merge this new PR and close this one.

Apart from the change of options I asked above, I've done the following fixes:

You were sending decoded digits as multiple messages withing single TF:

Your recoflow always reads digits from the file, so it is not possible to run it e.g. with digits from decoded raw data: added options --disable-root-input to not initialize root digits reader and --disable-root-output for the future: at the moment your reconstructed clusters cannot be stored. One should add a cluster writer and attach it to recoflow unless --disable-root-output is asked.

I see that you store in your digits the InteractionRecord. What is the reason for that? Your digits anyway contains the orbit and bc.
If you want to delimit the channels corresponding to specific trigger, you can add an extra structure, which tells refers to the entry of the 1st digit and their number for each trigger.

@shahor02

Apart from the change of options I asked above, I've done the following fixes:

You were sending decoded digits as multiple messages withing single TF:

Yes as I wrote in the ReadMe, my idea was to pipe workflows in order to perform different jobs, then for example

o2-hmpid-read-raw-file-workflow : reads the raw file and produce a continous stream of raw pages
o2-hmpid-raw-to-digits-workflow : reads the raw pages and produces chunck of digits vector
o2-hmpid-write-root-from-digits-workflow : reads chuncks of digits and build the root file
o2-hmpid-raw-to-pedestals-workflow : reads raw pages and calculate pedestals ...
... and I also wrote other debugging modules ...

, while they should be sent once after full decoding.
Your recoflow always reads digits from the file, so it is not possible to run it e.g. with digits from decoded raw data: added options --disable-root-input to not initialize root digits reader and --disable-root-output for the future: at the moment your reconstructed clusters cannot be stored. One should add a cluster writer and attach it to recoflow unless --disable-root-output is asked.

This is unclear for my ...

I see that you store in your digits the InteractionRecord. What is the reason for that? Your digits anyway contains the orbit and bc.

If you refer to the contents of root file, this give us the possibility to count the empty events

If you want to delimit the channels corresponding to specific trigger, you can add an extra structure, which tells refers to the entry of the 1st digit and their number for each trigger.

You were sending decoded digits as multiple messages withing single TF:

Yes as I wrote in the ReadMe, my idea was to pipe workflows in order to perform different jobs, then for example

Such a modular approach is OK, but what it has to do with sending within 1 TF a message with the same OutputSpec ? This will at least confuse the downstream devices which expect it, if not destroy the synchronization with others devices.

If you refer to the contents of root file, this give us the possibility to count the empty events

Why don't you do the same what many other detectors do: produce a single vector of fired channels, e.g. https://github.com/AliceO2Group/AliceO2/blob/dev/DataFormats/Detectors/MUON/MID/include/DataFormatsMID/ColumnData.h and another vector (of triggers) which tells that start and the number of channel entries for this trigger in the 1st vector:
https://github.com/AliceO2Group/AliceO2/blob/dev/DataFormats/Detectors/MUON/MID/include/DataFormatsMID/ROFRecord.h

Then you will have an access to the data of given trigger (which you don't have now) and don't need to store the same orbit/bc in every digits (will be stored in the trigger part). For empty data you will have the trigger part only, with 0 entries in the channels part.

@shahor02

You were sending decoded digits as multiple messages withing single TF:

Yes as I wrote in the ReadMe, my idea was to pipe workflows in order to perform different jobs, then for example

Such a modular approach is OK, but what it has to do with sending within 1 TF a message with the same OutputSpec ?
This will at least confuse the downstream devices which expect it, if not destroy the synchronization with others devices.

I understand it, then what are the boundary of a Time Frame ? I need to review the TF definition, is one TF contained into a raw file ?
Please Ruben, could give me a reference where I can better understand the data flow.

If you refer to the contents of root file, this give us the possibility to count the empty events

Why don't you do the same what many other detectors do: produce a single vector of fired channels, e.g.
Then you will have an access to the data of given trigger (which you don't have now) and don't need to store the same
orbit/bc in every digits (will be stored in the trigger part). For empty data you will have the trigger part only, with 0 entries
in the channels part.

Clearly the use of a table that drives the relation among Trigger & Digits is more efficient (I could improve that), but about the presence of trigger info (Orbit and BC) into the Digit structure, it is an heredity of RUN2 software, that we maintain (the overload of that info is 48 bits), clearly is the 50% of digit payload that is only 96 bits (I strongly reduce it packing the Pad address coordinates into a binary field structure)
Then, about the possibility to cut away the Trigger info from Digit structure, I need to discuss this issue with Giacomo

@fapfap69 The timeframe (somewhat simplified) from DPL point of view is what you receive or send with given Input- or OutputSpec within single run() call of DPL device. So, you should not issue multiple times the same {DataOrigin/DataDesc/SubSpec} from a single run() call. But I am not sure there is good documentation somewhere...

For the digits format: I would suggest to store vector of channels with only vpad and Q, delegating the orbit/bc to this wrapper Digit (or Trigger) structure. Once this is decided/finalized, I'll proceed to CTF creation. BTW, there I'll need to break your dense 32bit mPad to shorter columns, to profit from entropy compression.

Once you validate #5624, I'll merge it. Tested it locally with simulated data, works fine. Then, in separate PR, we can add the DetectorData part.

@shahor02 in order to reduce the entropy, could be better that I close this pull request ? What do you think ?

@fapfap69 yes, I've closed it, just check #5624, it is ok, I'll merge it as it is, then you can open other PR for passing the detectorWord in addData.