-
Notifications
You must be signed in to change notification settings - Fork 19
Description
Brief explanation
Migen / LiteX doesn't quite follow pep8 / pylint for good reasons. Extend pep8 and/or pylint to understand when it should allow violations. Extra additional checks for good Migen / LiteX style should also be added.
Expected results
Running pep8-migen on Migen / LiteX HDL code does the following;
- Checks all Python code that isn't Migen / LiteX complies with pep8 standard
- Checks Migen / LiteX code compiles with the standard formatting style for Migen / LiteX
- Checks for more common Migen / LiteX code smells.
Detailed Explanation
Due to the way the MiSoC / LiteX code works, this is preferred way to format the following statement;
self.sync += [
If(signal == 10,
output1.eq(1),
).ElIf(signal == 5,
output2.eq(0),
).Else(
output3.eq(1),
),
]See the migen documentation for more examples.
When the code is formatting this way it will fail pep8 with the following output;
$ pep8 misoc.py
misoc.py:6:9: E124 closing bracket does not match visual indentation
misoc.py:7:13: E128 continuation line under-indented for visual indent
misoc.py:8:9: E124 closing bracket does not match visual indentation
We need a pep8 / pylint extension which understand MiSoC / LiteX HDL formatting.
There are also a number of LiteX / Migen "code smells" that pep8 / pylint should also detect. Some are listed in the following section.
Migen / LiteX code smells
pep8 code smells which definitely apply
- Using a bare
try/except:clause - Pretty much most things
- FIXME: Put more things here
pep8 code smells which do not apply
Incorrect wrapping of final brace
All lists / dictionaries should have commas on the last element;
No;
self.comb += [
counter0.eq(counter[0]),
counter1.eq(counter[1]),
counter2.eq(counter[2]),
counter3.eq(counter[3]),
]Yes;
self.comb += [
counter0.eq(counter[0]),
counter1.eq(counter[1]),
counter2.eq(counter[2]),
counter3.eq(counter[3]),
]Missing Comma on final element in list / dictionary
All lists / dictionaries should have commas on the last element;
No;
self.comb += [
counter0.eq(counter[0]),
counter1.eq(counter[1]),
counter2.eq(counter[2]),
counter3.eq(counter[3])
]Yes;
self.comb += [
counter0.eq(counter[0]),
counter1.eq(counter[1]),
counter2.eq(counter[2]),
counter3.eq(counter[3]),
]No;
analyzer_signals = {
0 : vcd_group,
1 : sigrok_group
}Yes;
analyzer_signals = {
0 : vcd_group,
1 : sigrok_group,
}Using line continuation rather than list
FIXME: Check this one makes sense...
No;
self.sync.clk200 += \
If(reset_counter != 0,
reset_counter.eq(reset_counter - 1)
).Else(
ic_reset.eq(0)
)Yes;
self.sync.clk200 += [
If(reset_counter != 0,
reset_counter.eq(reset_counter - 1)
).Else(
ic_reset.eq(0)
),
]Using the wrong docstring style
Migen / LiteX use the "Numpy DocString style" for docstring comments.
See examples;
- https://github.com/m-labs/migen/blob/master/migen/fhdl/bitcontainer.py#L42-L61
- https://github.com/m-labs/misoc/blob/master/misoc/interconnect/csr.py#L63-L88
Code should not be using the "Google DocString Style" and not the "PEP 287 DocString Style"
This is supported by the napoleon module in Sphinx.
Not using yield from in test benches
https://m-labs.hk/migen/manual/simulation.html#pitfalls
When calling other testbenches, it is important to not forget the yield from. If it is omitted, the call would silently do nothing.
Further reading
Knowledge Prerequisites
- List of
- what knowledge is required
- to complete this project.
Contacts
- Potential Mentors: @mithro
- Mailing list: [email protected]