cms-gem-daq-project / ctp7_modules

0 stars 13 forks source link

[feature request] Optimize return types of functions in templated framework #161

Closed jsturdy closed 4 years ago

jsturdy commented 4 years ago

Brief summary of issue

We need to do a survey of all functions that are sending data back in the RPC response, and optimize the return type of the function in the templated framework.

The current xhal library provides serializers for certain types, and we should try hard to make all functions able to work with these types, only adding additional custom types if absolutely necessary.

Types of issue

Each reply to this thread should contain:

lpetre-ulb commented 4 years ago

Before proposing specific return types for each RPC method, I think it is worth thoroughly listing the types with a serializer implemented in xhal. The current list is:

For easier usage, I believe we should add in xhal serializers for, at least, the following types:

Serializers for custom types (e.g. simple struct's) can be easily implemented. However, I agree the number of those custom serializers should be minimized as much as possible.

jsturdy commented 4 years ago

A clarification: is "a type that can be serialized" a recursive definition? E.g. does std::vector<T> imply that it could be std::vector<std::vector<std::vector<std::vector<uint32_t>>>>? Or is this only true after all interior types have had a serializer explicitly defined?

lpetre-ulb commented 4 years ago

A clarification: is "a type that can be serialized" a recursive definition? E.g. does std::vector<T> imply that it could be std::vector<std::vector<std::vector<std::vector<uint32_t>>>>?

Yes, it is a recursive definition. I haven't explicitly tested std::vector<std::vector<...>> ... because there is no serializer for std::vector<T> yet, but it worked well for std::array<N, std::array<M, ...>>. In the unlikely situation the recursive serialization does not work, I'll have to fix the issue.

lpetre-ulb commented 4 years ago

@jsturdy, do we agree on those new types? I should be able to quickly implement the new (de)serializers.

jsturdy commented 4 years ago

@jsturdy, do we agree on those new types? I should be able to quickly implement the new (de)serializers.

I've finished the porting and the only special serializables I had to add were for some struct { enum (which as has been mentioned previously, should be migrated to scoped enums, a migration which can happen during this transition) One thing that worked around in the current migration was the std::vector<std::vector<uint32_t>> type, but yes, the additional types listed above I think can encompass all likely/supported use cases. (I might add std::unordered_map<K, T> if it is not a byproduct of the already implemented std::map<K, T> and if the implementation is not onerous.

jsturdy commented 4 years ago

@lpetre-ulb, actually, I added some for uint8_t, uint16_t and float as placeholders, if they are implemented in xhal before my migrations are done, then i'll clean them out, otherwise they can be removed later

lpetre-ulb commented 4 years ago

Partially implemented (#167) and partially migrated to GitLab (cmsgemos#88).