Skip to content

Commit

Permalink
Address comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
JulianSKelly committed Mar 8, 2016
1 parent 0d0b03d commit 101338d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 11 deletions.
10 changes: 5 additions & 5 deletions fpgalib/dac.py
Original file line number Diff line number Diff line change
Expand Up @@ -1203,8 +1203,7 @@ class DAC_Build15(DAC_Build8):
COUNTER_BYTES = 4
IDLE_BITS = 15
JUMP_TABLE_ENTRY_BYTES = 8
JUMP_TABLE_COUNT = (JUMP_TABLE_PACKET_LEN - (NUM_COUNTERS * COUNTER_BYTES)
) / JUMP_TABLE_ENTRY_BYTES
JUMP_TABLE_COUNT = 64
JT_FROM_ADDR_OFFSET = -2
JT_END_ADDR_OFFSET = JT_FROM_ADDR_OFFSET - 1
JT_IDLE_OFFSET = -0
Expand Down Expand Up @@ -1516,8 +1515,9 @@ def make_jump_table(cls, jt_entries, counters=None, start_address_ns=0):
:rtype: jump_table.JumpTable
"""

if len(jt_entries) > cls.JUMP_TABLE_COUNT:
raise ValueError("Jump table too long: {} > {}"
# -1 hardcoded due to issue #325

This comment has been minimized.

Copy link
@maffoo

maffoo Mar 8, 2016

Contributor

I think this comment is confusing, not least because no -1 appears in the next line. I would suggest something like the following:

# jt_entries does not include initial NOP; see #325
if len(jt_entries) + 1 > cls.JUMP_TABLE_COUNT:
    ...

This comment has been minimized.

Copy link
@JulianSKelly

JulianSKelly Mar 8, 2016

Author Contributor

Fixed.

if len(jt_entries) + 1 > cls.JUMP_TABLE_COUNT:
raise ValueError("Jump table too long: {} + starting NOP > {}"
.format(len(jt_entries), cls.JUMP_TABLE_COUNT))

# no two opcodes executed within JT_MIN_CLK_CYCLES_BETWEEN_OPCODES
Expand Down Expand Up @@ -1547,7 +1547,7 @@ def make_jump_table(cls, jt_entries, counters=None, start_address_ns=0):
start_addr=cls.convert_to_address(start_address_ns),
jumps=jt_entries,
counters=counters,
PACKET_LEN=cls.JUMP_TABLE_PACKET_LEN,
packet_len=cls.JUMP_TABLE_PACKET_LEN,
)

@classmethod
Expand Down
8 changes: 4 additions & 4 deletions fpgalib/jump_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,19 +275,19 @@ class JumpTable(object):
NUM_COUNTERS = 4

def __init__(self, start_addr=None, jumps=None, counters=None,
PACKET_LEN=528):
packet_len=528):
"""
:param start_addr: start address
:param list[JumpEntry] jumps: the jump table entries
:param list[int] counters: the counter values
:param int PACKET_LEN: Number of packets in a JT write
:param int packet_len: Number of byes in jump table write packet
"""
self._startAddr = 0
self.counters = self._initialize_counters(counters=counters)
self.start_addr = start_addr
self.jumps = jumps
self.PACKET_LEN = PACKET_LEN
self.packet_len = packet_len

@classmethod
def _initialize_counters(cls, counters=None):
Expand Down Expand Up @@ -323,7 +323,7 @@ def __str__(self):

def toString(self):
"""Serialize jump table to a byte string for the FPGA"""
data = np.zeros(self.PACKET_LEN, dtype='<u1')
data = np.zeros(self.packet_len, dtype='<u1')
# Set counter values. Each one is 4 bytes
for i, c in enumerate(self.counters):
data[i * 4:(i + 1) * 4] = littleEndian(c, 4)
Expand Down
15 changes: 13 additions & 2 deletions fpgalib/test/test_dac.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,20 @@ def test_nop_nop_min_clk_ok(self):
('END', [400])
])

def test_too_many_entry(self):
def test_max_entries(self):
entry_sep = self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES * 4 * 2
from_addr = (entry_sep * np.arange(self.dac.JUMP_TABLE_COUNT + 1) +
from_addr = (entry_sep * np.arange(self.dac.JUMP_TABLE_COUNT - 2) +
entry_sep)
entries = self.make_entries(
[('NOP', [k]) for k in from_addr] +
[('END', [from_addr[-1] + entry_sep])]
)
self.dac.make_jump_table(entries)
# we made it this far and we're feeling pretty good about it

def test_too_many_entries(self):
entry_sep = self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES * 4 * 2
from_addr = (entry_sep * np.arange(self.dac.JUMP_TABLE_COUNT - 1) +
entry_sep)
entries = self.make_entries(
[('NOP', [k]) for k in from_addr] +
Expand Down

0 comments on commit 101338d

Please sign in to comment.