search

cms-gem-daq-project / reg_utils

0 stars 9 forks source link

[WIP] Port the GBT tools to the new RPC methods #42

Closed lpetre-ulb closed 5 years ago

lpetre-ulb commented 5 years ago

Description

This PR is the continuity of the parent PR in cmsgemos. It has been done to port the functionalities of gbt.py, previously running on the CTP7, to the new RPC methods.

The tool brings these functions :

Types of changes

Motivation and Context

It was mainly done to fix this issue https://github.com/cms-gem-daq-project/ctp7_modules/issues/72. It also fixes issue #38 regarding the argument parsing.

How Has This Been Tested?

These tests were performed to see if the functionalities implemented in the gbt.py tool were still working with the RPC modules and so the new CTP7 releases :

./gbt.py eagle63 0 setPhase 15 14
Open pickled address table if available  /home/lpetre/CMS_GEM_DAQ/maps/amc_address_table_top.pickle...
Initializing AMC eagle63

The result has been checked with the I²C dongle (VFAT 15 has a phase of 14).

The environment was the following :

Checklist:

bdorney commented 5 years ago

Until https://github.com/cms-gem-daq-project/ctp7_modules/pull/73 gets merged in I think this is still WIP.

bdorney commented 5 years ago

One comment is the output should produce a file that is compatible with TTree::ReadFile.

bdorney commented 5 years ago

reg_utils is a base layer and cmsgemos and xhal sit on top of this repository.

This repository should never require anything from cmsgemos or xhal. This needs a redesign.

mexanick commented 5 years ago

I suggest migrating the gbt-related functionality to xhal as all the functionality which extends the barebone reg_utils should go there

bdorney commented 5 years ago

I suggest migrating the gbt-related functionality to xhal as all the functionality which extends the barebone reg_utils should go there

Can you outline how you'd like that to be integrated? I assume since this is a python tool it should fall under xhal/reg_interface/... and then be built and included in that rpm. Does this meet your expectation?

What about @jsturdy ?

jsturdy commented 5 years ago

If a thing is using a CTP7 module function, it should be done through the HW device class that exposes that (currently being developed into cmsgemos and exposed in gempython).

Following an architecture discussion with @mexanick, the part of xhal that it was suggested to migrate this code to will be migrated, as it should really be its own separate package. So this migration will take place shortly, and then these tools can be placed there, but they should interface with the gempython library from cmsgemos (API not yet public), so the frustratingly short answer is, "just be patient" :-\/

Also migrating there will be the *info.py scripts from cmsgemos, as they migrate to the new cmsgemos gempython library.

If you want anything quick and hacky (and not just as a test of module), you should develop it in one of the short term branches that have been set up for this express purpose.

If you're just testing that your module works, you can build a test executable from ctp7_modules a la this (there is probably no need to commit the tool, unless you are preparing a nice generic test suite :+1: )

lpetre-ulb commented 5 years ago

Thank you all for your comments and advices.

I brought some changes to the GBT RPC module this weekend and it is now time to adapt the Python tool. However, I still don't clearly see where the tool must go.

If you want anything quick and hacky (and not just as a test of module), you should develop it in one of the short term branches that have been set up for this express purpose.

Also migrating there will be the *info.py scripts from cmsgemos, as they migrate to the new cmsgemos gempython library.

@jsturdy, I understand that we must be patient, but this tool is important for operating the 3.7.x CTP7 firmwares; it is a test for the module, but not only. Thereby, it looks like the short term branches are the way to go for exposing the hardware. (And PR is ready.)

Regarding the script itself, maybe it should follow the *info.py scripts ? Where will they go ? Maybe in xhal as @mexanick suggests ?

I suggest migrating the gbt-related functionality to xhal as all the functionality which extends the barebone reg_utils should go there

Can you outline how you'd like that to be integrated? I assume since this is a python tool it should fall under xhal/reg_interface/... and then be built and included in that rpm. Does this meet your expectation?

If the script is added to xhal, xhal will depend of gempython which might not be good either. Should I instead directly use the CDLL module to write the script and bypass gempython ?

If you're just testing that your module works, you can build a test executable from ctp7_modules a la this (there is probably no need to commit the tool, unless you are preparing a nice generic test suite :+1: )

Oh, yes, a real test suite would be good, but that would require hardware and could not be integrated in CI/CD easily.

For this feature specific comment :

One comment is the output should produce a file that is compatible with TTree::ReadFile.

Ok, I'll make the modifiction. For now, the output can optionally be written as .csv, but the modification is trivial.

bdorney commented 5 years ago

Addressed by https://github.com/cms-gem-daq-project/xhal/pull/102/files