-
Notifications
You must be signed in to change notification settings - Fork 24
Finished interrupts chapter #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Dangit. Forgot to mention the lovely chapter on peripherals and polling that @guptaarnav wrote that's in here. Thanks for that as well! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds great to me. Thank you for all your work.
However, this is too big for me to review in detail at the moment, though.
If somebody else wants to review, that would be great. Regardless of them being part of the WG or not.
Otherwise, you have my blessing to just merge it.
1. Check for button presses | ||
2. Blink the LED | ||
|
||
But the processor can only do one thing at a time. If we press a button during the blink delay, the processor won't be able to respond until the delay is over and the loop starts again. As a result, we get a much less responsive program (try for yourself and see how worse it is). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worse than what? Not checking for button presses? But that's a different thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Will replace with "see how much worse the button lag is".
mdbook/src/15-interrupts/README.md
Outdated
### Handling Interrupts | ||
|
||
Computation is always contextual: the core always needs memory to load inputs and store outputs to. | ||
Our microcontroller is of what's known as a load-store-architecture, and as such the core does not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The explanation of a load-store-architecture is a bit weird. Instead of explaining what it doesn't do confusingly using the words store and load, I'd turn it around and say: it's a load and store architecture because typically the data needs to be loaded from memory into registers before doing computation and store the results back to memory afterwards. I also wouldn't refer to registers as "scratch pad memory" but rather something like in-core storage locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds right. Will deal with it.
mdbook/src/15-interrupts/README.md
Outdated
|
||
The ISR handler function is "special". The name `GPIOTE` is required here, and the function must be | ||
decorated with `#[interrupt]` so that it returns using a return-from-interrupt instruction rather | ||
than the normal way. The function may not take arguments and must return `()`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm kind of missing the important detail here that this #[interrupt] does three things (not just one):
- It rewrites the function to make be an interrupt handler (including the return from interrupt)
- It renames the function to be not callable from the Rust program
- It adds it to the interrupt table of the program so the processor knows which function to execute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great comment. (Also (4) ensures that the type signature is fn()->().)
Will fix.
mdbook/src/15-interrupts/README.md
Outdated
|
||
When you push the A Button, you will see an "ouch" message and then a panic. Why does the interrupt | ||
handler call `panic!()`? Try commenting the `panic!()` call out and see what happens when you push | ||
the button. You will see "ouch" messages scroll off the screen. The GPIOTE records when an interrupt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the case in this particular example (hopefully? I haven't checked), however there can be different effects in play here as interrupts can work in many different ways depending on the implementation and the peripherals which trigger them, too. Especially with Input pins, you can have level or edge triggering, sometimes you need to manually clear the event, sometimes they're auto cleared when you read the state, sometimes they retrigger the interrupt, sometimes they do it only exactly once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I will add a few sentences explaining that other microcontrollers and peripherals may work differently. Your summary is great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upon further reading we probably need to talk about per-port and per-pin (channel) interrupts on the nRF52833. It looks like the nRF is more configurable than I thought: can allow edge, rising-edge, falling-edge, high and low triggers. It looks like edge-triggered interrupts are triggered once per transition, but level-triggered interrupts will run continuously until the latch is reset. In both cases it is advisable (and probably necessary) to clear the interrupt event.
It's an interesting question how much of this the HAL does (and should) support. I'll investigate.
mdbook/src/15-interrupts/README.md
Outdated
keep the interrupt handlers short and quick. | ||
|
||
When the ISR function returns (using a magic instruction), the CPU looks to see if interrupts have | ||
happened that need to be handled, and if so calls one of the handlers (according to a priority order |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it calls the specific handler assigned to each interrupt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite getting the distinction here. I'll clean up the text to be clearer about what happens. Maybe something like this?
Another interrupt may have happened while the current interrupt is being processed. These interrupts will "pend": the CPU will keep track of which interrupts have not yet been handled. When the current ISR function returns, the CPU will call the ISR handler for the highest-priority pending interrupt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naw, that's not enough. I will add a paragraph about interrupt priority, and indicate here that an interrupt handler can itself be interrupted (I think?) by a higher-priority interrupt. If a lower-priority interrupt occurs it will pend. I think? Interrupts are hard.
Maybe we should include an example showing interrupt priorities in action?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is: When you declare an interrupt handler in Rust via the #[interrupt]
macro (as mentioned in another comment), the macro creates a special "anonymous" function which is entered into .interrupt table of the linked program. Each interrupt has exactly one entry per available interrupt, you can re-use one interrupt handler for multiple (and figure out in the interrupt handler which interrupt was actually pended) but you can't have multiple interrupt handler entry points per interrupt.
Indeed the function of the NVIC (hence the N for nested) is to call the interrupt handlers per assigned priority and preempt or interrupt execution if higher priority interrupts are pending (and resume execution accordingly once their interrupt handler has finished). To avoid that, and e.g. access shared ressources, one can disable interrupts temporarily (for this core only) which is called a "critical section" or modify the priorities of the interrupts, which is a can of worms, though. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I won't get into priority inversion here 😀 but will add text to explain the existence and function of the interrupt table.
|
||
[pinmap table]: https://tech.microbit.org/hardware/schematic/#v2-pinmap | ||
|
||
Reading the state of a GPIO input involves checking whether the voltage level at the pin is high (1) or low (0). The buttons on the micro:bit are connected to pins; when the buttons are pressed, they pull the voltage at the pin low (to 0V ground). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if the (1) and (0) here are more confusing, as the sentence talks about voltage levels. Maybe putting it at 3V, 0V for high and low might be more consistent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Text adjusted for next push. Thanks much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't seem to see this example included in the book at the moment...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, should go at the end of "Polling Sucks" where we're talking about superloops. Thanks for the catch; fixed in next push.
The NVIC can receive requests to trigger an interrupt from many peripherals. It's even common for a | ||
peripheral to have multiple possible interrupts, for example a GPIO port having an interrupt for | ||
each pin, or a UART having both a "data received" and "data finished transmission" interrupt. The | ||
job of the NVIC is to prioritise these interrupts, remember which ones still need to be procesed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small typo in processed (second s missing)
handle another that's higher priority. This is called "preemption" and allows processors to respond | ||
very quickly to critical events. For example, a robot controller might use low-priority interrupts | ||
to keep track sending status information to the operator, but also have a high-priority interrupt to | ||
detect an emergency stop button being pushed so it can immediately stop moving the motors. You |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicking here, but aren't emergency stop buttons (at least the nice big red ones) most of the time hooked straight into power supply lines? Sensor detects imminent collision and needs to interrupt whatever the program is doing might be more real in the sense intended here.
@@ -0,0 +1,14 @@ | |||
# Waiting for an interrupt (wfi, wfe, nop) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title mentions wfe
as well, which is however not discussed in the text...
the tiny rectangular hole — the "speaker port" — on the side of the device. Do this fast enough, | ||
and the pressure changes will make a sound. | ||
|
||
<img class="white_bg" height="350" title="Speaker" src="../assets/speaker.svg" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a dark theme, the image is barely visible as there is no background color. Currently, mdbook doesnt seem to support the #only-dark
, #only-light
tags for images to create two images that are shown in either light or dark mode.
Looking at the Rust book as an example, the solution seems for the moment to give images solid white backgrounds. Attached an svg, if you like to use it, where I just took the existing figure and added a solid white background.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I had intended the background to be solid white when I drew it, but apparently screwed up. Will fix.
@@ -0,0 +1,33 @@ | |||
# Addendum: PWM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file doesn't currently show up in the book itself...
Co-authored-by: Daniel Egger <[email protected]>
Co-authored-by: Daniel Egger <[email protected]>
c56c7bf
to
2208e10
Compare
I think I've got everything noted in review comments above (and some other stuff also). Please let me know if you spot anything still lacking in the Input and Output or Interrupts chapters. Thanks much to everybody who wrote this and everybody who provided review and help. |
mdbook/src/15-interrupts/README.md
Outdated
|
||
**Note** As with most microcontrollers, there is a lot of flexibility in when the GPIOTE can generate an interrupt. Interrupts can be generated on low-to-high pin transition, high-to-low (as here), any change ("edge"), when low, or when high. On the nRF52833, interrupts generate an event that must be manually cleared in the ISR to ensure that the ISR is not called a second time for the same interrupt. Other microcontrollers may work a little differently — you should read Rust crate and microcontroller documentation to understand the details on a different board. | ||
|
||
When you push the A Button, you will see an "ouch" message and then a panic. Why does the interrupt handler call `panic!()`? Try commenting the `panic!()` call out and see what happens when you push the button. You will see "ouch" messages scroll off the screen. The NVIC records when an interrupt has been issued: that "event" is kept until it is explicitly cleared by the running program. Without the `panic!()`, when the interrupt handler returns the NVIC will (in this case) re-enable the interrupt, notice that there is still an interrupt event pending, and run the handler again. This will continue forever: each time the interrupt handler returns it will be called again. In the next section we will see how to clear the interrupt indication from within the interrupt handler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last sentence: "In the next section...": Technically, how to do this shows up in section 15.2 currently and not 15.1. Maybe replace with "Soon" to keep it in a non-section-number-referring way?
the count display into the main loop, to show that the count is shared between the interrupt handler | ||
and the rest of the program. | ||
|
||
Give this example (`examples/count.rs`) a run and note that the count is bumped up 1 on every push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since examples/count.rs
is not included in the text, it would be nice to provide a link to it.
Really, though, that `rprintln!()` in the interrupt handler is bad practice: while the interrupt | ||
handler is running the printing code, nothing else can move forward. Let's move the reporting to the | ||
main loop, just after the `wfi()` "wait for interrupt". The count will then be reported every time | ||
an interrupt handler finishes (`examples/count-bounce.rs`). Again, the count is bumped up 1 on every |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here for examples/count-bounce.rs
Noticed a couple of small things above. Overall, this is a really smooth and instructive read, very well written chapters! |
Thanks for the catches! Corrected, I think. |
This PR is multiple changes that end up with a new chapter on interrupts. It also does reorganization to get the chapters in the right order. It may have also accidentally touched other things.
This probably should not be merged lightly: a check for paper-bag issues by somebody would be absolutely great.
On the bright side, when we're done everybody gets a chance to make a siren.
Thanks much to Arnav Gupta @guptaarnav for writing the bulk of the interrupts chapter. I just finished it off.