Tutorial3 cleanup

[rbx]

• Added tests.
• Moved relevant files from basemq to the example directory. These have very limited reusability value and are more suitable as an example. I added these to the breaking changes. The only known user is some old code in Panda. Confirmed with Tobias that it is not actually needed and are OK to remove.
• Reduced number of launch scripts to 1 - others produce more noise than value. Also simplified the scripts.
• Removed example with msgpack. It was used for some tests long ago, but now adds more noise than value.
• Simplified structure - shortened class/file names, simpler template structures.
• Removed ack channel. The idea behind it was to make sure all data arrives, but this can be done simpler (as it is done in the serialization example test).

The example classes could use further modernization (namespaces, memory management for (Fair)ROOT objects), but I'll leave that for future work.

[ChristianTackeGSI]

Why are you turning this back from a unique_ptr to a raw pointer?

One could even think of using a value member instead?

TClonesArray fOutput{FairTestDetectorHit::Class()};

The Branch() then should look like this:

fTree.Branch("Output", &fOutput, 64000, 99);

[rbx]

in FileSinkTMessage.h there is fTree.SetBranchAddress("Output", &fOutput);, which wants to have a pointer to a pointer....
Apparently it was just broken before, so I "repair" it here.
I don't like the solution either.
But I can't get value member or unique_ptr to work directly... maybe i'm still missing something x)

[ChristianTackeGSI]

Hmmm, very strange.

I would have expected this overload of SetBranchAddress to be used.

[rbx]

If I change it back to unique_ptr, and:

fTree.SetBranchAddress("Output", fOutput.get());

, which seems to be the overload you suggest, then I get this at runtime:

Error in <TTree::SetBranchAddress>: The address for "Output" should be the address of a pointer!

 *** Break *** segmentation violation

[ChristianTackeGSI]

I have the feeling, that this is a bug. I actually have an idea. I am going to post on the root forum and ask for some help in understanding their code.

Another question: Do we actually need the additional tree.SetBranchAddress when we already set the address in the tree.Branch() call?

[rbx]

Actually there is another aspect in this device. Before it calls SetBranchAddress() it also calls the Deserializer:

FairRoot/examples/advanced/Tutorial3/MQ/fileSink/FileSinkTMessage.h

Line 23 in 481eb09

RootSerializer().Deserialize(*msg, fOutput);

Which translates to either of:

    template<typename T>
    void Deserialize(const fair::mq::Message& msg, T*& output)
    {
        delete output;
        FairTMessage tm(msg.GetData(), msg.GetSize());
        output = static_cast<T*>(tm.ReadObjectAny(tm.GetClass()));
    }

    template<typename T>
    void Deserialize(const fair::mq::Message& msg, std::unique_ptr<T>& output)
    {
        FairTMessage tm(msg.GetData(), msg.GetSize());
        output.reset(static_cast<T*>(tm.ReadObjectAny(tm.GetClass())));
    }

So we already call delete ourselves on it before it gets to ROOT. This "generic part" would be the next thing which I would propose to delete. It would be better to see these ugly details directly in the example, to understand and improve them for whatever the use case is.

I suspect the TClonesArray* fOutput; is not ideal for every serializer and may have been chosen to keep the class "generic".

[rbx]

It is not obvious to me what the best solution for this issue will be, accounting both for SetBranchAddress and the serializers.

For this PR I prefer to leave it at raw ptr + new/delete, as it is also done in:
https://github.com/FairRootGroup/FairRoot/blob/dev/examples/MQ/Lmd/runMBSSink.cxx#L75
and
https://github.com/FairRootGroup/FairRoot/blob/dev/examples/MQ/serialization/sink.cxx#L83

The change from unique_ptr to raw ptr in this PR is necessary to get it to work at all - it was entirely broken before, which the added tests reveal.

[ChristianTackeGSI]

One thing I wondered: If the deserializers usually allocate the TCA, do we actually need to new one in the constructor?

[rbx]

Looks like it is not needed to be in the ctor. Probably it doesn't have to be a member at all. But rather be created and destroyed for each message.

[ChristianTackeGSI]

Having it as a member is good. Because TBranch needs its address.

Well, you're right: Let's keep it for this PR and clean this up in another go.

[ChristianTackeGSI]

Okay, I wasn't able to follow each detail, but generally this starts to look good for me.

Some final things I noticed.

[ChristianTackeGSI]

Looks good to me. I would still prefer if someone else could take a look as well.