Updated teensy40 board and machine/usb to utilize new machine.Serial interface #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ardnew
reviewed
Feb 20, 2022
| @@ -16,7 +17,7 @@ const descCDCACMCount = 1 | |||
|
|
|||
| // descHIDCount defines the number of USB cores that may be configured as a | |||
| // composite (keyboard + mouse + joystick) human interface device (HID). | |||
| const descHIDCount = 0 | |||
| const descHIDCount = 1 | |||
There was a problem hiding this comment.
The only solutions I can think of to this:
tinygodoes an in-place file edit:- Would need a robust technique to find and edit this and other consts.
- Would need to decide if/how to restore the edits afterwards.
- Restoring the edits will confuse IDEs/debuggers that are comparing the source to the compiled executable.
- NOT restoring the edits means compiling source code is a destructive operation on that source code!
- Kinda hacky IMO
- Move these respective constants to their own standalone source files with appropriate build tag constraints:
- Let
tinygoadjust active build tags automatically if command-line flag given.- For example:
GOFLAGS="-tags=...,usb.hid,...if command-line flag-usb=hidwas given.
- For example:
- Let
Of course, 2. would be my preference and also seems the easiest to implement.
There was a problem hiding this comment.
Note that if 2. would be preferable, it will be necessary to add a separate command-line flag to tinygo — piggybacking off of the -serial flag is insufficient.
Something like the following, along with logic to verify the flags given are compatible (e.g., -serial=usb -usb=hid contradicts itself)
tinygo -serial={none,{uart,usb}[:PORT]} -usb={cdc,hid}[:PORT]
As mentioned in the first comment, this new -usb flag would be responsible for setting the respective build tags which causes the corresponding source file(s) containing USB configuration constants to get included.
ardnew
pushed a commit
that referenced
this pull request
Jun 21, 2022
Instead of doing everything in the interrupt lowering pass, generate
some more code in gen-device to declare interrupt handler functions and
do some work in the compiler so that interrupt lowering becomes a lot
simpler.
This has several benefits:
- Overall code is smaller, in particular the interrupt lowering pass.
- The code should be a bit less "magical" and instead a bit easier to
read. In particular, instead of having a magic
runtime.callInterruptHandler (that is fully written by the interrupt
lowering pass), the runtime calls a generated function like
device/sifive.InterruptHandler where this switch already exists in
code.
- Debug information is improved. This can be helpful during actual
debugging but is also useful for other uses of DWARF debug
information.
For an example on debug information improvement, this is what a
backtrace might look like before this commit:
Breakpoint 1, 0x00000b46 in UART0_IRQHandler ()
(gdb) bt
#0 0x00000b46 in UART0_IRQHandler ()
#1 <signal handler called>
[..etc]
Notice that the debugger doesn't see the source code location where it
has stopped.
After this commit, breaking at the same line might look like this:
Breakpoint 1, (*machine.UART).handleInterrupt (arg1=..., uart=<optimized out>) at /home/ayke/src/github.com/tinygo-org/tinygo/src/machine/machine_nrf.go:200
200 uart.Receive(byte(nrf.UART0.RXD.Get()))
(gdb) bt
#0 (*machine.UART).handleInterrupt (arg1=..., uart=<optimized out>) at /home/ayke/src/github.com/tinygo-org/tinygo/src/machine/machine_nrf.go:200
#1 UART0_IRQHandler () at /home/ayke/src/github.com/tinygo-org/tinygo/src/device/nrf/nrf51.go:176
#2 <signal handler called>
[..etc]
By now, the debugger sees an actual source location for UART0_IRQHandler
(in the generated file) and an inlined function.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactored TinyGo to have a compiler flag for choosing the type of output - one of "usb", "uart", or "none". This PR updates the Teensy 4.0 implementation and HID example to utilize this mechanism, making it much easier to switch between HID and CDC.
Please note that I set the descriptor count for HID to be 1 by default, which uses a few kb more RAM than necessary... but IMO that is an ok tradeoff for now, especially since Teensy 4.0/4.1 have a whole 1MB of RAM to work with. I think this can be optimized/fixed in the future.