cms-gem-daq-project / ctp7_modules

0 stars 13 forks source link

GE2/1 compatilibility #132

Closed mexanick closed 5 years ago

mexanick commented 5 years ago

Addressing #131

Description

The changes should be transparent and does not require changes in the other software for the GE1/1 usage

Types of changes

Motivation and Context

The hardware constants has to be defined in one place and corresponding compile-time switch has to be used

How Has This Been Tested?

Tested with the GE2/1 integration stand setup

Screenshots (if appropriate):

Checklist:

jsturdy commented 5 years ago

Is this targeting "working now with setups that may be using legacy", if so, needs to target release/1.1.X, which is what I understand based on the dependent PRs

bdorney commented 5 years ago

It’s possible I missed other blocks in the code due to not appearing here. Especially in the calibration toolkit.

mexanick commented 5 years ago

@bdorney @lpetre-ulb your comments has been addressed except first one from @bdorney which I believe deserves a separate issue

bdorney commented 5 years ago

After another pass on this PR I think the struct ADCChannel also needs to be done for both ge11 and ge21 namespaces as there are probably a different number (and location) of temperature sensors between GE2/1 and GE1/1 optohybrids:

https://github.com/cms-gem-daq-project/ctp7_modules/blob/2c5d65f667e50ca95f733608e8768bf2086fd3f8/include/amc/sca_enums.h#L211-L267

Also I would suggest a look at all structs and enums declared here as they may also need namespace protection for things like different channels on the SCA ADC used to monitor different voltages, additional channels being used, additional monitoring, different pin out, etc...

I think we should address this given the board design has been completed in this PR. Apologies for having missed this earlier.

mexanick commented 5 years ago

I agree that SCA enums has to be changed, but I don't have any info on this w.r.t. GE2/1. Can anyone point me to where I can get it?

lpetre-ulb commented 5 years ago

@mexanick, you should be able to find all the information in the GE2/1 OH specification and/or on the GE2/1 OH schematic : http://padley.rice.edu/cms/OH_GE21/ohdesign.html

mexanick commented 5 years ago

Should these changes still go to develop or you prefer them to go under legacy/1.1.X branch?

mexanick commented 5 years ago

Changed the target and performed a rebase

bdorney commented 5 years ago

Approving these changes per discussion in meeting; but if @mexanick you could open a new issue on the repo to address the SCA enum issues we discussed for GE1/1 vs. GE2/1 issues.

mexanick commented 5 years ago

Merging as is, further refactoring foreseen