Skip to content

Fix bug #309 #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
patzinak wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix bug #309 #310

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c1f9acf
Fix bug #309
patzinak Jan 27, 2016
ea9a2ca
Allow access to AD clock bits, improve readability
patzinak Jan 27, 2016
4ba6826
Make the bitwise operation priorities explicit
patzinak Jan 28, 2016
3b7d395
Use named arguments in regRun calls
patzinak Jan 28, 2016
c2320b2
Remove an unnecessary self reference when calling a method
patzinak Feb 7, 2016
6c665de
Bug fix version bump
patzinak Feb 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 26 additions & 23 deletions fpgalib/adc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,10 @@ def runCalibrate(self):
@inlineCallbacks
def func():
# build register packet
filterFunc=np.zeros(self.FILTER_LEN, dtype='<u1')
filterStretchLen=0
filterStretchAt=0
demods={}
regs = self.regRun(RUN_MODE_CALIBRATE, 1, filterFunc,
filterStretchLen, filterStretchAt, demods)

filterFunc = np.zeros(self.FILTER_LEN, dtype='<u1')
regs = self.regRun(mode=RUN_MODE_CALIBRATE, reps=1,
filterFunc=filterFunc, filterStretchAt=0,
filterStretchLen=0, demods={})
# create packet for the ethernet server
p = self.makePacket()
p.write(regs.tostring()) # send registry packet
Expand Down Expand Up @@ -305,9 +302,10 @@ def runPacket(self, page, slave, delay, sync):

filterFunc, filterStretchLen, filterStretchAt = self.filter
startDelay = self.startDelay + delay
regs = self.dev.regRun(self.mode, self.reps, filterFunc,
filterStretchLen, filterStretchAt,
self.channels, startDelay)
regs = self.dev.regRun(mode=self.mode, reps=self.reps,
filterFunc=filterFunc, filterStretchAt=filterStretchAt,
filterStretchLen=filterStretchLen, demods=self.channels,
startDelay=startDelay)
return regs

def collectPacket(self, seqTime, ctx):
Expand Down Expand Up @@ -441,9 +439,10 @@ def runAverage(self, filterFunc, filterStretchLen, filterStretchAt,
@inlineCallbacks
def func():
# build registry packet
regs = self.regRun(self, self.RUN_MODE_AVERAGE_AUTO, 1, filterFunc,
filterStretchLen, filterStretchAt, demods)

regs = self.regRun(self, mode=self.RUN_MODE_AVERAGE_AUTO,
Copy link
Copy Markdown
Contributor

@maffoo maffoo Jan 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is just wrong. We're calling regRun, a classmethod, through an instance of that class, and then passing an unneccessary self reference as the first argument. I realize you didn't create the issue here, but it stands out that this needs to be fixed. I'll file a separate issue.

Copy link
Copy Markdown
Contributor

@maffoo maffoo Jan 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#312

Copy link
Copy Markdown
Author

@patzinak patzinak Jan 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see quite a few things in the code that do not really look great to me. However, I keep in mind the following: (1) I'm not 100% familiar with the code, there might be tricky, uncommented bug fixes, not obviously standing out in the commit history; (2) the code works for your group; (3) we would love to see our core code synced with yours to benefit from all future updates. It seems that the right way for us to act is to ask for the minimum number of changes that should do the job for us.

Copy link
Copy Markdown
Member

@DanielSank DanielSank Jan 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please post issues. It's perfectly fine to post an issue just asking why a bit of code is a certain way (we even have a "question" label for issues).

Copy link
Copy Markdown
Contributor

@maffoo maffoo Jan 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was being a bit overdramatic about classmethods. We should just remove the first self argument to the method. If you can do that here while you're touching these lines, @patzinak, that'd be great.

Copy link
Copy Markdown
Author

@patzinak patzinak Feb 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been addressed in a corresponding commit.

reps=1, filterFunc=filterFunc,
filterStretchAt=filterStretchAt,
filterStretchLen=filterStretchLen, demods=demods)
p = self.makePacket()
p.write(regs.tostring())
p.timeout(T.Value(10, 's'))
Expand All @@ -460,9 +459,10 @@ def runDemod(self, filterFunc, filterStretchLen, filterStretchAt, demods):
@inlineCallbacks
def func():
# build register packet
regs = self.regRun(self.RUN_MODE_DEMOD_AUTO, 1, filterFunc,
filterStretchLen, filterStretchAt, demods)

regs = self.regRun(mode=self.RUN_MODE_DEMOD_AUTO, reps=1,
filterFunc=filterFunc,
filterStretchAt=filterStretchAt,
filterStretchLen=filterStretchLen, demods=demods)
# create packet for the ethernet server
p = self.makePacket()
self.makeFilter(filterFunc, p) # upload filter function
Expand Down Expand Up @@ -650,7 +650,8 @@ def runPacket(self, page, slave, delay, sync):
This is cheesey and ought to be fixed.
"""
startDelay = self.startDelay + delay
regs = self.dev.regRun(self.mode, self.info, self.reps, startDelay=startDelay)
regs = self.dev.regRun(mode=self.mode, info=self.info,
reps=self.reps, startDelay=startDelay)
# print("ADC run packet: %s" % (regs,))
return regs

Expand Down Expand Up @@ -955,8 +956,8 @@ def registerReadback(self):
@inlineCallbacks
def func():
# build registry packet
regs = self.regRun(self.RUN_MODE_REGISTER_READBACK, 0) # 0 reps?

regs = self.regRun(mode=self.RUN_MODE_REGISTER_READBACK,
info={}, reps=0)
p = self.makePacket("registerReadback")
p.write(regs.tostring())
p.timeout(T.Value(10, 's'))
Expand All @@ -973,7 +974,8 @@ def runAverage(self):
@inlineCallbacks
def func():
# build registry packet
regs = self.regRun(self.RUN_MODE_AVERAGE_AUTO, {}, 1)
regs = self.regRun(mode=self.RUN_MODE_AVERAGE_AUTO, info={},
reps=1)
p = self.makePacket("runAverage")
p.write(regs.tostring())
p.timeout(T.Value(10, 's'))
Expand All @@ -1000,8 +1002,8 @@ def runDemod(self, info):
@inlineCallbacks
def func():
# build register packet
regs = self.regRun(self.RUN_MODE_DEMOD_AUTO, info, 1)

regs = self.regRun(mode=self.RUN_MODE_DEMOD_AUTO, info=info,
reps=1)
# create packet for the ethernet server
p = self.makePacket("runDemod packet")
self.makeTriggerTable(triggerTable,p)
Expand Down Expand Up @@ -1052,7 +1054,8 @@ def processReadback(resp):
a = np.fromstring(resp, dtype='<u1')
return {
'build': a[0],
'noPllLatch': a[1]&1 == 1,
'noPllLatch': bool(a[1] & 1),
'dClkBits': (a[1] >> 1) & 0b1111,
'executionCounter': int(a[2]) + int(a[3] << 8),
'nPackets': a[4],
'badPackets': a[5]
Expand Down