Skip to content
This repository has been archived by the owner on Jan 31, 2022. It is now read-only.

Template-based RPC calls #126

Closed
lmoureaux opened this issue May 21, 2019 · 7 comments
Closed

Template-based RPC calls #126

lmoureaux opened this issue May 21, 2019 · 7 comments

Comments

@lmoureaux
Copy link

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

Links

Related issues

The following issues are related to our proposal:

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
Copy link
Contributor

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<uint32_t> 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<sbitRateScan>(...);

// 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
Copy link
Contributor

mexanick commented May 27, 2019

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
Copy link
Contributor

lpetre-ulb commented May 27, 2019

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
Copy link
Contributor

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
Copy link
Contributor

lpetre-ulb commented May 27, 2019

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
Copy link
Contributor

jsturdy commented May 27, 2019

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
Copy link
Contributor

lpetre-ulb commented May 18, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants