FPGA Runner: Remove timeout handling, add run subcommand#504
FPGA Runner: Remove timeout handling, add run subcommand#504ziuziakowska wants to merge 4 commits intolowRISC:mainfrom
run subcommand#504Conversation
This is now handled by Cmake/Ctest test timeouts which would kill the process, so this can be now be removed which simplifies some state management in the runner Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
uart_poll_checking_for will only return once it has a value, so it will always be not none, therefore we can be certain bootstrap succeeded if this returns Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
…command Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
Signed-off-by: Alice Ziuziakowska <a.ziuziakowska@lowrisc.org>
| await set_pin(0, False) | ||
| await reset_core() | ||
| await poll_uart_checking_for(uart, MOCHA_BOOTROM_BOOSTRAP_STR) | ||
| if uart is not None: |
There was a problem hiding this comment.
Why not use the uart for the run subcommand?
I just read the PR header, I think that there should be an arg to not use uart for the run subcommand --no-uart, or try to open it and if fails use the delay.
There was a problem hiding this comment.
I think that that would add a bit more complexity, and for the use-case this is intended for, not using the UART would almost always be preferred (indeed locally I found myself always commenting out the code that checked for the ROM bootstrapping and test status strings).
There was a problem hiding this comment.
I think that we can recommend this script in the release documentation to load the examples, so it would be nice to print the uart output by default. So, for development, one just need to open the uart prior to running this script.
| await load_fpga_test(test, uart) | ||
| pattern = r"TEST RESULT: (PASSED|FAILED)" | ||
| result = await asyncio.create_task(poll_uart_checking_for(uart, pattern)) | ||
| return "PASSED" in result |
There was a problem hiding this comment.
Nit: you're changing this line twice in the same PR. Is there any way to avoid this?
| args = parser.parse_args() | ||
| try: | ||
| asyncio.run(args.func(args)) | ||
| except KeyboardInterrupt: |
There was a problem hiding this comment.
Is this not the default behaviour of Python?
marnovandermaas
left a comment
marnovandermaas
left a comment
There was a problem hiding this comment.
Nice PR. Just a few minor comments from me (non blocking in case Douglas approves)
3 participants
This PR makes some changes to the FPGA runner.
Test timeout handling is removed as this functionality is now handled by CMake/CTest, so the runner can be simplified to not do this itself.
This PR also moves the testing aspect of the runner to a
testsubcommand, and introduces arunsubcommand which just loads a program over SPI and exits. This is useful for simply loading and running interactive software, such as examples and demos, and in the future interactive Operating Systems. This subcommand does not attempt to open the UART, which means that the UART can be kept open continuously across reboots in a serial console e.g screen or picocom, which is quite convenient.