Template-based RPC calls

lmoureaux commented 5 years ago

This issue is about our proposal (@lmoureaux, @lpetre-ulb) to use C++ templates for type-safe RPC calls.

Links

Initial proposal describing the goals
Second talk after a minimal working example has been implemented
Laurent's clone with the code

Related issues

The following issues are related to our proposal:

68: We do use namespaces in our examples, but they are not mandatory
85: An exception of type RPC::RemoteException (inherits std::runtime_error) is thrown if any exception is caught on the CTP7. If available, it has information about the stack trace on the CTP7 (need to compile the modules with -g to get meaningful stack traces).
109: Exceptions are caught in a central place within the templated code.

Syntax

Declaring an RPC method (remotely callable function):

#include "RPC.hpp"
namespace Module42
{
    struct Module42Base : public RPC::Method
    {
        static constexpr char const * module = "module42"; // May be dropped
    };
    struct AnswerQuestion : public MemoryBase
    {
        int operator()(const std::string &question) const;
    }
};

RPC method implementation:

#include "module42.h"
int Module42::AnswerQuestion operator()(const std::string &question) const
{
    if (question.empty()) throw std::runtime_error("You must ask a question");
    return 42;
}

Call from DAQ PC:

#include "module42.h"
// ...
int answer = conn.call<Module42::AnswerQuestion>("Ultimate Question");

Call from CTP7: (Can also add a tiny wrapper for full symmetry with the DAQ PC)

#include "module42.h"
// ...
int answer = Module42::AnswerQuestion{}("Ultimate Question");

Constraints

We work within the following limits:

Do not touch rpcsvc, RPCMsg, RPCSvc
Don't use any feature beyond C++ 11
No extra binary dependency

lpetre-ulb commented 5 years ago

During the developer meeting we have been asked to present what would be the return types for real life examples and how these types would be parsed by the client. @bdorney and @mexanick provided us this list.

Here is a first attempt which is topic to further discussion :

dacScanMultiLink : Returns a map where the key is the OH number and where the value is identical to what dacScanLocal currently returns :

Returns a std::vector object of size 24*(dacMax-dacMin+1)/dacStep where dacMax and dacMin are described in the VFAT3 manual. For each element bits [7:0] are the dacValue, bits [17:8] are the ADC readback value in either current or voltage units depending on dacSelect (again, see VFAT3 manual), bits [22:18] are the VFAT position, and bits [26:23] are the optohybrid number.

This type can be naturally parsed in C++ and you avoid sending fillers for missing OHs :

// Call the remote method
std::map<std::uint32_t, std::vector<std::uint32_t>> 
    multiLinkResults = conn.call<dacScanMultiLink>(...);

// Fill a ROOT tree with the data
for (auto const& singleLinkResult : multiLinkResults)
{
    const uint32_t ohN = singleLinkResult.first;
    const std::vector<std::uint32_t> &scanData = singleLinkResult.second;

    myMagicTreeFillerFunction(ohN, dacScanData);
}

getmonGBTLink : Returns a map, indexed by the OH number, of an array of a specialized structure. Code worths a thousand words :

// Define what characterizes the GBT status (it's so easy to write a serializer)
struct gbt_status_t
{
    bool ready;
    bool wasNotReady;
    bool hadUnderflow;
    bool hadOverflow;
};

// We have multiple OHs, each one with gbt::GBTS_PER_OH GBTXs
std::map<std::uint32_t,
    std::array<gbt_status_t, gbt::GBTS_PER_OH>> allStatuses = conn.call<getmonGBTLink>(...);

// Output the ready flag in a .csv-like format
std::cout << "# ohN gbtN readyFlag" << std::endl;

for (auto const& ohStatus : allStatuses)
{
    const uint32_t ohN = ohStatus.first;

    for (uint32_t gbtN=0; gbtN < gbt::GBTS_PER_OH; gbtN++)
    {
        std::cout << ohN << "," << gbtN << "," 
            << ohStatus.second.at(gbtN).ready << std::endl;
    }
}

sbitRateScan : Maybe the hardest one since it returns so much data... it however becomes obvious with a structure :


// We have many rates
struct sbit_rates_t
{
std::vector<std::uint32_t> dacValues;

// The rates vectors keep the same definition
// That is their length is (dacMax-dacMin+1)/dacStep
std::vector<std::uint32_t> CTP7Rates;
std::vector<std::uint32_t> OHRates;
std::array<std::vector<std::uint32_t>, 24> VFATRates;
};

// Retrieve the data for all unmasked OH std::map<std::uint32_t, sbit_rates_t> allRates = conn.call(...);

// Since xDAQ 15 will support C++17, let's use it at our advantage for (auto const& [ohN, rates] : allRates) { // Let say we want only VFAT 5 rates for every OH for (auto const usedData : boost::combine(rates.dacValues, rates.VFATRates[5])) { uint32_t dacValue, rate; boost::tie(dacValue, rate) = usedData;

    std::cout << ohN << " " << dacValue << " " << rate << std::endl;
}

}



These types are the first ideas which come to my mind. Of course, considering the versatility of the interface, different solutions are possible. For instance, one does not have to use a structure in `getmonGBTLink` and in `sbitRateScan`, the structure can be replaced by a map, even though it is less C/C++ style.

mexanick commented 5 years ago

Parallel to this issue we may want to address as well the IPBus compatibility for the RPC-based software. In this case, the memory, utils and extras modules will require overloading to use the IPBus register access instead of memsvc.

lpetre-ulb commented 5 years ago

Parallel to this issue we may want to address as well the IPBus compatibility for the RPC-based software. In this case, the memory, utils and extras modules will require overloading to use the IPBus register access instead of memsvc.

I'm not sure I fully understand your concern here. Do you want to be able to use the RPC software on another system than the CTP7 ? So without the Zynq and its memory mapped address space ?

A few months back, when I ported the CTP7 v3 firmware to the GLIB, I went with a more straightforward solution where I implemented a small wrapper which converts the the memsvc calls to IPBus calls. You can find the implementation here. What is interesting here is that the RPC-based software works out the box.

mexanick commented 5 years ago

I'm not sure I fully understand your concern here. Do you want to be able to use the RPC software on another system than the CTP7 ? So without the Zynq and its memory mapped address space ?

Yes, exactly

A few months back, when I ported the CTP7 v3 firmware to the GLIB, I went with a more straightforward solution where I implemented a small wrapper which converts the the memsvc calls to IPBus calls. You can find the implementation here. What is interesting here is that the RPC-based software works out the box.

Yes, that was actually initial plan, just didn't have time and real need to implement. I didn't check yet the details, but I think there's also a need to wrap the update of LMDB address table method to achieve the full compatibility. I think this can be a good branching point to bring your developments to the main repository

lpetre-ulb commented 5 years ago

I didn't check yet the details, but I think there's also a need to wrap the update of LMDB address table method to achieve the full compatibility.

In fact, there is no need to bring any change to the update of LMDB table method (or any other RPC method/function). The wrapper converts the addresses on the fly, so that you can use the vanilla RPC modules.

I think this can be a good branching point to bring your developments to the main repository

In the upcoming weeks, I will update the v3 GLIB firmware and propose a PR to the GEM_AMC repository. Once done, I will attempt to upstream the software developments. Also, I think it is better if we could have such a discussion in a dedicated issue since it is not related to this template-based RPC calls proposal.

jsturdy commented 5 years ago

I think this can be a good branching point to bring your developments to the main repository

In the upcoming weeks, I will update the v3 GLIB firmware and propose a PR to the GEM_AMC repository. Once done, I will attempt to upstream the software developments. Also, I think it is better if we could have such a discussion in a dedicated issue since it is not related to this template-based RPC calls proposal.

Indeed, and I have long had several questions about this.

For the discussion here (re: templates), the primary comment I have is that the complicated return types outlined here cry out for a different implementation, rather than trying to cram them through this interface (this comment remains even if we decide not to go this templated route)

lpetre-ulb commented 4 years ago

The migration to the templated RPC framework is now done (#167).

cms-gem-daq-project / ctp7_modules