search

epics-modules / mrfioc2

EPICS driver for Micro Research Finland event timing system devices
http://epics-modules.github.io/mrfioc2/
Other
8 stars 30 forks source link

Add support for UNIV and RF mTCA EVR models #63

Closed ZanMaticPratnemer closed 8 months ago

ZanMaticPratnemer commented 1 year ago

So far only IFB model is supported. This pull request adds support for UNIV and RF models. If no model is selected, the default one is picked (IFB).

jerzyjamroz commented 10 months ago

Hi, the initial conversation took place: https://github.com/epics-modules/mrfioc2/issues/89

jerzyjamroz commented 10 months ago

I)

The issue I found was related to the way we use CLKA/B within the mTCA-EVR-300U MTCA backplane (the patch was in our internal repo). @ZanMaticPratnemer, please check: https://github.com/epics-modules/mrfioc2/pull/94 . We deleted lines 288-292, you actually modified those in your PR. Then the .substitution config looks like: https://github.com/epics-modules/mrfioc2/pull/95 and it works fine. Tested over a few years already. Could you confirm if this solution works for you?

ZanMaticPratnemer commented 10 months ago

Hi, the modification of those lines were purely visual on my end, possibly even automated by my formatter. So yeah that works for me, maybe @zioven has some other comments.

jerzyjamroz commented 10 months ago

II)

mTCA-EVR-300

mTCA-EVR-300 with IFP is discontinued and the code in line: https://github.com/epics-modules/mrfioc2/pull/63/files#diff-ec09296cb3ed989142d2d3a7eeb1fda890d403e91250f9328befd76e01fc5217R215 actually refers to a generic MTCA EVR, we have been using it for mTCA-EVR-300U and mTCA-EVR-300 (IFP) together as we have some obsolete IFP units (the FPGA images are the same, but we do not use IFP socket at all).

Your Implementation

Your implementation is cleaner so my proposal is to go for it accordingly:

mtca_evr_model variable

zioven commented 10 months ago

@jerzyjamroz As @ZanMaticPratnemer has mentioned I have inherited this task and pull request from him.

Regarding I): I will be testing this with our setup and determine how the changes to TCLKA and TCLKB work on our system

Regarding II): I can rebase the changes to the master (provide a new pull request) and we can open up the discussion what is best approach and where we see the challenges/improvements.

zioven commented 8 months ago

@jerzyjamroz , @mdavidsaver

I have rebased the pull request to the latest master.

In the last commit, I have updated both 300U and 300RF substitutions file, so that they are backwards compatible:

Please review and comment.

jerzyjamroz commented 8 months ago

@zioven I only wonder why CLKA/B works fine for mtca-evr-300u without this:

        for (unsigned int i = 16; i <= 17; ++i) {
            outputs[std::make_pair(OutputFPUniv, i)]
                = new MRMOutput(SB() << n << ":FrontUnivOut" << i, this, OutputFPUniv, i);
        }

What was not working for you that you decided to add it?

zioven commented 8 months ago

I had reports of missing objects evrN:FrontUnivOutX for TCLKA and TCLKB similar to this:

smpl{evrN-Out:FP0}Src-RB_: failed to find/create object 'evr2:FrontOut0' : Object not found : evr2:FrontOut0  
smpl{evrN-Out:FP0}Src2-RB_: failed to find/create object 'evr2:FrontOut0' : Object not found : evr2:FrontOut0 
smpl{evrN-Out:FP1}Src-RB_: failed to find/create object 'evr2:FrontOut1' : Object not found : evr2:FrontOut1  
smpl{evrN-Out:FP1}Src2-RB_: failed to find/create object 'evr2:FrontOut1' : Object not found : evr2:FrontOut1 
smpl{evrN-Out:FP2}Src-RB_: failed to find/create object 'evr2:FrontOut2' : Object not found : evr2:FrontOut2  
smpl{evrN-Out:FP2}Src2-RB_: failed to find/create object 'evr2:FrontOut2' : Object not found : evr2:FrontOut2 
smpl{evrN-Out:FP3}Src-RB_: failed to find/create object 'evr2:FrontOut3' : Object not found : evr2:FrontOut3  
smpl{evrN-Out:FP3}Src2-RB_: failed to find/create object 'evr2:FrontOut3' : Object not found : evr2:FrontOut3

NOTE: Errors above occurred when I loaded the evr-mtca-300u.db while initializing for mTCA-EVR-300RF card, which does not have any permanent Front Outputs.

jerzyjamroz commented 8 months ago

I use the actual master this moment and the objects are fine:

Object: EVR:FrontOut0
Type: 9MRMOutput
Object: EVR:FrontOut1
Type: 9MRMOutput
Object: EVR:FrontOut2
Type: 9MRMOutput
Object: EVR:FrontOut3
Type: 9MRMOutput

@zioven , try this file: evr-mtca-300.db, maybe your setup has some dependencies on some special characters.

zioven commented 8 months ago

@jerzyjamroz The message above is just an example of the missing objects that I got for my mTCA-EVR-300U card. If I didn't create the FrontUnivOut objects for TCLKA and TCLKB outputs, I got a similar error.

According to Jukka's document, TCLKA and TCLKB outputs are mapped as following:

In current config (see code reference below), the number of FPUV Outputs is set to 18 (assuming this is the IFB version of EVR, 16 on the interface box + 2 which correspond to TCLKA and TCLKB) https://github.com/epics-modules/mrfioc2/blob/277abf2216a5f977d76ba119a19be6760a84c362/evrMrmApp/src/drvemSetup.cpp#L176-L194

With the addition of the mTCA-EVR-300U and mTCA-EVR-300RF the number of FPUV Outputs is less than that.

In my opinion it doesn't make sense to create 18 objects, from which only 6 or 4 shall be used.

Based on this conclusion, I have added a separate creation of objects (for TLCKA and TCLKB) for FrontUnivOut16, FrontUnivOut17 on all platforms, and reduced the number of FPUV modules on generic mtca_evr_300 configuration to 16 (as is the actual number of Front Universal Outputs).

mTCA-EVR-300RF* details

(additional 2 on the front panel , that are GTX and linked to `Universal Output 18` and `Universal Output 19` are also created separately)

jerzyjamroz commented 8 months ago

@zioven , I got your point, you are right it is a cleaner solution.

I just noticed that you found the reverse solution for the .substitution implementation. I was just thinking of adding more .substitution files if needed e.g.

but both implementations can be used.