cfelton / rhea

A collection of MyHDL cores and tools for complex digital circuit design
MIT License
85 stars 34 forks source link

RFC Tidy test pins #4

Closed robtaylor closed 8 years ago

robtaylor commented 8 years ago

This is a request for comments.

The nonlocality of led_port_pin_map bugged me, so hacked this approach up whilse bored, which adds moves the definitions of test output pins to the board defintions.

cfelton commented 8 years ago

@robtaylor I think you are correct, there is room for improvement here. This originally started off as an example for a workshop and the blinky grew into a generic first pass test for the automated build test, room for improvement indeed.

I am not sure I like the test_output_pins added to the board definitions because it is specific to the test, and not meaningful in most other cases. I think the test_output_pins definitions (which port/pins to be used for the test) should be moved to the test file instead of test specific information in the board definition.

I probably should rework the example/build and use something other than blinky. The reason, some else commented that having the blinky in each board directory is more intuitive to find and the explicit example is useful (although redundant and a maintenance pain). Currently, I point folks to the blinky examples in the example/build.

For the near-term I think capturing the test_output_pins in test_boards.py and removing board_build_example.py would clear up some of the confusion (using your proposed add_test_port). Long term replace blinky with an example design that has more meat (as in FPGA resources) but limit it to 3-4 pins (clock reset (optional), serialin, serialout).

robtaylor commented 8 years ago

ack, I can see the reasoning there, will tidy up to that effect

cfelton commented 8 years ago

I am back tracking on the removal of the board_build_example.py, it is a good example (being in the example directory) how to use the board defs to extract information on the board. What needs to be done is that get_port should have some accept a glob syntax and more options: options to return closest match).

robtaylor commented 8 years ago

Hopefully this is is better :)