[pyOCD] Semihosting support (#172)
Chris Reed
Added semihosting support. A new SemihostAgent class handles semihosting requests on behalf of the GDB server or any other user. File IO requests can be handled internally by pyOCD or converted to gdb syscall requests. A telnet server is also added for standard IO requests.
Semihosting was tested by enabling the --coverage compiler and linker options, and calling gcov_exit() at the end of main(). This causes .gcda files to be written with coverage information. Make sure --specs=rdimon.specs is enabled. Also, do not use nano.specs because it writes files 1 byte at a time, which is exceedingly slow through semihosting. Other semihosting APIs were tested by modifying the KSDK 1.2 adc_pit_trigger_frdm64f demo.
Note that you must manually create a telnet console in Eclipse, as the OpenOCD plugin doesn't support automatically doing this (like the J-Link plugin does).
Some semihosting requests are not implemented:
- TARGET_SYS_TMPNAM
- TARGET_SYS_REMOVE
- TARGET_SYS_RENAME
- TARGET_SYS_SYSTEM
- TARGET_SYS_GET_CMDLINE
- TARGET_SYS_HEAPINFO
- TARGET_SYS_EXIT
- TARGET_SYS_ELAPSED
- TARGET_SYS_TICKFREQ
This PR also includes some OpenOCD compatibility enhancements. All OpenOCD features used by the GNU ARM Eclipse OpenOCD plugin are handled by pyocd-gdbserver and the GDBServer class. Most importantly, the "echo" command is handled on the command line, which fixes the timeout when the OpenOCD plugin launches pyocd-gdbserver automatically.
Commands handled by pyocd-gdbserver via -c/--command option:
- gdb_port
- telnet_port
- echo
Additional gdb server monitor commands:
- arm semihosting (enable|disable)
- reg (recognized but ignored)
- reset init
Monitor commands (qRcmd RSP packet) are now handled in GDBServer in a new handleRemoteCommand() method. Some minor improvements were made, but the monitor command handling needs to be completely rewritten.
You can view, comment on, or merge this pull request online at:
https://github.com/mbedmicro/pyOCD/pull/172
Commit Summary
- Added my dev and windev virtualenvs to gitignore.
- Initial semihosting support.
- Commented out unaligned mem read/write ragged edge debug logs in CortexM.
- Trailing whitespace cleanup in target and gdbserver.
- Refactor GDBServer packet send and receive.
- Continued work on semihosting.
- Added GDBServer packet log option.
- Fixes for GDBServer packet send and receive.
- GDBServer file I/O and semihosting fixes.
- Refactored GDB server packet I/O into separate thread.
- Renamed GDBServer.performFileIO() to syscall().
- Semihosting telnet server and I/O handler classes.
- Flushing out semihosting.
- GDB server syscall semihost handler integration.
- GDB server remote command handler.
- Semihosting telnet console input.
- GDB server tool improvements and semihosting support.
- Semihosting cleanup.
- Semihosting bug fixes and improvements.
- Merge branch 'master' into dev_semihost
- Semihost and GDB server shutdown handling and cleanup.
File Changes
- M pyOCD/gdbserver/gdbserver.py (227)
- A pyOCD/gdbserver/syscall.py (125)
- M pyOCD/target/__init__.py (1)
- M pyOCD/target/cortex_m.py (16)
- A pyOCD/target/semihost.py (703)
- A pyOCD/test/test_utility/test_cmdline.py (40)
- M pyOCD/tools/gdb_server.py (217)
- M pyOCD/utility/__init__.py (2)
- A pyOCD/utility/cmdline.py (52)
Patch Links:
—
Reply to this email directly or view it on GitHub.
Martin Kojtal
Semihosting was tested by enabling the --coverage compiler and linker options, and calling gcov_exit() at the end of main(). This causes .gcda files to be written with coverage information. Make sure --specs=rdimon.specs is enabled.
Can you share changed version of adc_pit_trigger_frdm64f, via github? Plus, we could set up an example with coverage.
Some minor improvements were made, but the monitor command handling needs to be completely rewritten.
Worth sharing more ?
Chris Reed
Instead of sharing the adc_pit_trigger_frdm64f modifications (which would require almost all of KSDK 1.2 to build), I was intending to build a position-independant semihosting test app that we can run on any device.
Regarding the monitor commands, the handler current treats each word of the command line as a separate command. So 'reset halt' is handled as if it were two commands, 'reset' and 'halt'. Plus, the handler is just not well designed (uses masks to identify and handle commands, which is weird).
Chris Reed
Added unit tests for semihosting. I tried creating a position-independant test app, but had trouble with the stdlib not being position-independant compatible. So I ended up building a test suite that sets up registers and a bit of code to execute semihosting requests.
As part of this change, I switched dev-requirements.txt to require pytest instead of nose. The semihost unit tests heavily depend on pytest's powerful fixture and parameterization capabilities. It's completely compatible with the couple unit tests we already have.
The semihosting telnet server is not fully tested yet. It is only started and then shutdown. Needs further development.
Chris Reed
Extended semihosting telnet console unit test and added other new unit tests. Also fixed a couple more bugs.
Martin Kojtal
I should release new pyOCD patched version with latest patches today.
Is this feature complete to be merged to master?
Chris Reed
Yes, I think it's complete. I've tested pretty extensively. And all the automated tests plus the new unit tests pass.
c1728p9
In pyOCD/transport/cmsis_dap.py:
> @@ -381,6 +381,8 @@ def _write(self, request, data = 0): > """ > Write a single command > """ > + assert type(request) is int, "request is not an int" > + assert type(data) is int, "data is not an int"
When I run basic_test.py I get an assert on this because a long is passed in. The value is still in the integer range, but due to previous operations it was converted to a long implicitly.
Chris Reed
In pyOCD/transport/cmsis_dap.py:
> @@ -381,6 +381,8 @@ def _write(self, request, data = 0): > """ > Write a single command > """ > + assert type(request) is int, "request is not an int" > + assert type(data) is int, "data is not an int"
Thanks, Russ, I'll take a look.
c1728p9
In pyOCD/gdbserver/gdbserver.py:
> + # TODO rewrite the remote command handler
> + def handleRemoteCommand(self, cmd):
> + logging.debug('Remote command: %s', cmd)
> +
> + safecmd = {
> + 'reset' : ['Reset target', 0x1],
> + 'halt' : ['Halt target', 0x2],
> + 'resume': ['Resume target', 0x4],
> + 'help' : ['Display this help', 0x80],
> + 'reg' : ['Show registers', 0],
> + 'init' : ['Init reset sequence', 0]
> + }
> + resultMask = 0x00
> + resp = 'OK'
> + if cmd == 'help':
> + for k,v in safecmd.items():
resp should be set to an empty string before this for loop, otherwise 'OK' will show up in the help text.
(gdb) mon help
OKreset Reset target
help Display this help
resume Resume target
init Init reset sequence
reg Show registers
halt Halt target
c1728p9
Sorry it took me so long to look at this patch, I have been kinda busy lately. A lot of good features and fixes in this pull request.
Would it make sense to move the hardware dependent testing into the upper level test folder where basic_test.py and flash_test.py are, and leave only the unit tests that can be run quickly and without any hardware dependency in the pyOCD/test folder? That way we would have one set unit tests that runs very quickly, and then a group of integration tests the take longer and actually test hardware. Let me know what you think.
Chris Reed
In pyOCD/transport/cmsis_dap.py:
> @@ -381,6 +381,8 @@ def _write(self, request, data = 0): > """ > Write a single command > """ > + assert type(request) is int, "request is not an int" > + assert type(data) is int, "data is not an int"
Ah, it's dependant on whether your machine is 32- or 64-bit. I'm changing it to in (int, long).
Chris Reed
No worries, I figured you were busy.
Originally I had test_semihost.py in the top-level test directory, but then I decided to place it with the other unit tests meant to be run with pytest/nose. As for splitting between hw-dependant and not, I don't see there being a lot of non-hw-dependant tests unless we were to go insane with mocking.
The only things aside from utilities that do not require hw are parts of flash_builder, memory_map, and perhaps a few methods in gdb_server.
c1728p9
Was testing with this patch this week at work and didn't see any problems. Also tested semihosting tonight and it work for me as well. @flit I think you're right about unit tests. Running with hardware is the way to go. I wonder, is there a way the unit tests can be broken into two groups, one that requires hardware and one that doesn't? I still like the idea of having a set of unit tests that can run quickly on any machine, but not at the expense of no hardware unit tests. If we could have both that would be excellent. One more thought, would it be possible to rebase some of the older patches to be on top of master? That way the history would be more linear, making it easier to do a binary search for any bugs that appeared. If it isn't easy to do don't worry about it. Great work on the patch! I think it is ready for being merged unless you had any further change you wanted to make. Thanks again and sor
ry for t
he long response time. In 2 or 3 weeks I'll be able to review your patches at full speed 
.
Chris Reed
Thanks for the continued testing.
We can use py.test's marking capability to mark tests that do or do not require hardware.
@pytest.mark.nohw def test_something(foo): pass
Then you run py.test with the -m option: py.test -m nohw.
Sure, I can rebase this and other outstanding pull requests. But that will create a new branch as far as GitHub is concerned, so a new pull request will have to be created and we'll lose comment history. Unless I'm doing something wrong.
Yes, this PR is ready to be merged. 
c1728p9
Awesome, great news that the non-hw tests can be run independently. If you rebase and force push to the same branch, you can use the same pull request.
Martin Kojtal
Comment history will stay. Give it a try, rebase and force push as @c1728p9 noted. Let us know once done.
Chris Reed
Do you mind if I don't rebase for this PR? I've been working on it for a while today, and it's turning out to be a very complicated rebase with many tricky conflicts. So I'm really concerned that I'll break something. The current branch is already well tested, though it obviously doesn't have the cleanest history. In the future, I'll rebase more often to keep histories cleaner.
c1728p9
If it isn't easy to rebase then I wouldn't worry about it. It is more of a nice to have anyway, and not worth the risk or effort. Thanks for giving it a try.
Martin Kojtal
Merged #172.