cms-gem-daq-project / xhal

XHAL interface library
0 stars 10 forks source link

Addressing issue #16 #21

Closed mexanick closed 7 years ago

mexanick commented 7 years ago

Description

Implementing flattened array return in genScan instead of text file creation Addressing https://github.com/cms-gem-daq-project/xhal/issues/16

Types of changes

Motivation and Context

Required for seamless integration into the scan routines

How Has This Been Tested?

It wasn't tested although I'm pretty sure from my previous experience that it will work. Some things to pay attention:

Screenshots (if appropriate):

Checklist:

bdorney commented 7 years ago

it is user responsibility to check if the array size will be an integer number (i.e. (scanMax-scanMin+1)/scanStep is an integer number

We should probably add a check that this integer requirement is enforced. e.g. something like:

if( floor((scanMax-scanMin+1)/scanStep) != ceil((scanMax-scanMin+1)/scanStep)){
    cout<<"Improper usage case\n";
    cout<<"The quantity (scanMax-scanMin+1)/scanStep must be an integer\n";
    cout<<errorFlagUsage;
    return;
}

(Adapt above syntax as appropriate)

I'm not sure if this should be placed in the genScan(...) function in this PR or in getUltraScanResults(...) which will call genScan(...). In the later case the above syntax will be adapted to python. I guess here is more appropriate because the usage case that needs to be checked is in genScan(...).

mexanick commented 7 years ago

@bdorney , setting correct range and step of the scan is caller's responsibility. So the code snippet you put above has to be adapted to python and reside in getUltraScanResults(...)

Now please clarify which changes you have requested. "needs usage case check" is not quite clear to me. Do you want a test case? You can run it with test.py - it is written there. I can do it for you when the V3 system is configured and ready to use. "

bdorney commented 7 years ago

The needs usage case check is the code I showed in my comment above.

@bdorney , setting correct range and step of the scan is caller's responsibility. So the code snippet you put above has to be adapted to python and reside in getUltraScanResults(...)

I agree that a caller should setup a function responsibly. But a function being called should notify if it is being used improperly. At the very least there should be some comment or documentation with the function that informs the user of this integer requirement. Right now there is none in the code.

mexanick commented 7 years ago

@bdorney in order to call the genScan method, you must supply an array for resulting data. How are you going to create an array of non-integer size???

bdorney commented 7 years ago

@bdorney in order to call the genScan method, you must supply an array for resulting data. How are you going to create an array of non-integer size???

Very carefully.

(Yeah fair point, I was not thinking).