cms-gem-daq-project / ctp7_modules

0 stars 13 forks source link

Address tables registers renamed in OH FW 3.2.x #133

Closed lpetre-ulb closed 4 years ago

lpetre-ulb commented 5 years ago

Brief summary of issue

While taking Sbits threshold scans with the 3.2.x OH firmware releases, @bdorney discovered issues with missing keys in the LMDB database. See this elog.

I first though is was because registers were programmatically generated in the new address table, but it appears they have simply been renamed.

In this case GEM_AMC.OH.OHx.FPGA.TRIG.CNT.VFATy_SBITS becomes GEM_AMC.OH.OHx.FPGA.TRIG.CNT.VFATy.

Please note that other registers in the trigger block (at least) have been renamed. For example registers controlling the tap delays have been moved into a new namespace.

Types of issue

Expected Behavior

The sbitRateScan RPC should perform seamlessly with the new firmware releases (once stabilized).

Current Behavior

The sbitRateScan RPC method fails with the new firmware releases.

Steps to Reproduce (for bugs)

  1. Update the OH FW to the 3.2.x branch.
  2. Update the LMDB address table.
  3. Try to take an Sbit threshold scan.

Possible Solution (for bugs)

Adapt the RPC methods to the new register names.

bdorney commented 5 years ago

I'm marking this as "On Hold" and "Blocked" because this seems like a bug/typo in the script(s) that were used to automatically generate these registers in the FW. There's no mention of these changes in the OH FW Release notes:

https://github.com/cms-gem-daq-project/OptoHybridv3/releases

So this seems like an unintended change. Additionally @andrewpeck and @evka85 as I mentioned in my email if we need to change node names in the future we should:

  1. Probably do it to improve clarity,
  2. Discuss it with the SW dev team and all agreed on it before implementing the node name change, and
  3. Document the agreed upon changes in the FW release notes.
lpetre-ulb commented 4 years ago

Change reverted in the OH firmware address tables.