Conversation
ithinuel
force-pushed
the
update-pico-usb-serial-interrupt
branch
from
fb5b54a to
9b68598
Compare
|
The changes look good, but I wonder why the CI actions did not run for this branch? |
|
It migth have been because it PR was initially targetting another branch (hal-0.9). |
ithinuel
force-pushed
the
update-pico-usb-serial-interrupt
branch
from
9b68598 to
f46db13
Compare
| // The USB Bus Driver (shared with the interrupt). | ||
| static mut USB_BUS: Option<UsbBusAllocator<hal::usb::UsbBus>> = None; |
There was a problem hiding this comment.
Is the last part of the comment "(shared with the interrupt)" not incorrect? The interrupt function (fn USBCTRL_IRQ()) shouldn't be able to reference the USB_BUS, because USB_BUS is out of scope.
| // Grab a reference to the USB Bus allocator. We are promising to the | ||
| // compiler not to take mutable access to this global variable whilst this | ||
| // reference exists! | ||
| let bus_ref = unsafe { USB_BUS.as_ref().unwrap() }; | ||
| let bus_ref = USB_BUS.as_mut().unwrap(); |
There was a problem hiding this comment.
Is this part of the comment "We are promising to the compiler not to take mutable access to this global variable whilst this reference exists!" not incorrect too? The compiler will stop you from taking a second mutable reference and the USB_BUS is local to the #[entry] function.
There was a problem hiding this comment.
Indeed, this comment is no longer relevant thanks to the #[entry] trick, the compiler is able to check we're not doing anything silly there.
| // Trait required to use `writeln!(…)` | ||
| use core::fmt::Write; | ||
|
|
||
| // A system wide mutex synchrinising IRQ & cores |
There was a problem hiding this comment.
There is a typo (synchrinising -> synchronising or synchronizing).
| #[allow(non_snake_case)] | ||
| #[exception] | ||
| fn SysTick() { | ||
| do_with_usb(|usb| usb.usb_device.poll(&mut [&mut usb.usb_serial])); |
There was a problem hiding this comment.
A comment like "Keeps the usb_device updated in case the usb interrupt is not called often." or a better explanation why this is advisable might be useful here for people like me who only have a superficial understanding of embedded software development.
| /// infinite loop. | ||
| #[entry] | ||
| fn main() -> ! { |
There was a problem hiding this comment.
Wouldn't a comment like "never ending function, except when the pico is reset to usb boot." not be more appropriate?
There was a problem hiding this comment.
Good spot, this text is actually inconsistent with the example.
|
@Agent59 thank you for your reviews! This last push should address your comments. Let me know if you see anything else :) |
ithinuel
force-pushed
the
update-pico-usb-serial-interrupt
branch
from
2bf2c0c to
37a124c
Compare
4 participants
Preceding PR: