cms-gem-daq-project / xhal

XHAL interface library
0 stars 10 forks source link

Bug Fix: Fixes #75 #76

Closed bdorney closed 6 years ago

bdorney commented 6 years ago

Description

This should resolve issue #75.

Related too: https://github.com/cms-gem-daq-project/reg_utils/issues/17

Types of changes

Motivation and Context

Since jtagCommand() returns a list if drReadOhList is nonsequential the indices of of the return of jtagCommand() will not match the elements in the input drReadOhList and generate an IndexError

How Has This Been Tested?

With one OH, link 0:

$ sca.py eagle63 0x1 sysmon
...
...
=== OH #0 ===Core temp = -273.15, voltage #1 = 0.298828125, voltage #2 = 0.298828125 

=== OH #0 ===Core temp = 34.9440917969, voltage #1 = 1.0224609375, voltage #2 = 2.4609375 

With one OH, link 1:

$ sca.py eagle63 0x2 sysmon
...
...
=== OH #1 ===Core temp = 31.9911132813, voltage #1 = 1.0341796875, voltage #2 = 2.5546875 

=== OH #1 ===Core temp = 31.9911132813, voltage #1 = 1.0341796875, voltage #2 = 2.5546875 

With two OH's, links 0 & 1:

$ sca.py eagle63 0x3 sysmon
...
...
=== OH #0 ===Core temp = 34.4519287109, voltage #1 = 1.0224609375, voltage #2 = 2.4609375 

=== OH #1 ===Core temp = 31.9911132813, voltage #1 = 1.0341796875, voltage #2 = 2.5546875 

=== OH #0 ===Core temp = 34.4519287109, voltage #1 = 1.0224609375, voltage #2 = 2.4609375 

=== OH #1 ===Core temp = 31.9911132813, voltage #1 = 1.0341796875, voltage #2 = 2.5546875

Checklist:

bdorney commented 6 years ago

This file no longer exists in develop, but in reg_utils. So I have applied the hotfix there as well.

mexanick commented 6 years ago

Indeed this file won't be present in this repo, so closing issue

bdorney commented 6 years ago

But for the legacy release of xhal this bug persists...wouldn't it be good to fix it? Sorry for re-opening but I thought it was potentially relevant.

jsturdy commented 6 years ago

But for the legacy release of xhal this bug persists...wouldn't it be good to fix it? Sorry for re-opening but I thought it was potentially relevant.

If it is an issue at P5 because of this change not being implemented in this package, potentially, as the SW at P5 is using the state-of-the-art

bdorney commented 6 years ago

If it is an issue at P5 because of this change not being implemented in this package, potentially, as the SW at P5 is using the state-of-the-art

At p5 it seems the sca.py tool is coming from reg_utils so my PR in reg_utils will resolve this.