-
Notifications
You must be signed in to change notification settings - Fork 56
Passive scanning #118
base: master
Are you sure you want to change the base?
Passive scanning #118
Conversation
This is required for the `BleScanner` to be used. The `recv_interrupt` method has been renamed to `recv_ll_interrupt` for a more consistent API.
For simplicity, data is currently not logged. However, the code can be used as a starting point.
@@ -84,6 +88,21 @@ pub struct BleRadio { | |||
rx_buf: Option<&'static mut PacketBuffer>, | |||
} | |||
|
|||
/// Acknowledge the DSIABLED event | |||
macro_rules! acknowledge_disabled_event { |
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.
Can't this be a method?
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 early-return of None
makes that a bit unergonomic... Or do you have an idea how to handle that?
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.
You can use ?
with a method returning Option<()>
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.
Whoa, I totally missed that this is possible with options as well, and not just results 🤯
/// Call this when the `RADIO` interrupt fires when using the [`BeaconScanner`]. | ||
/// | ||
/// The radio is automatically reconfigured to listen on the next | ||
/// advertisement channel. |
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.
Only the timer interrupt should do this, right?
where | ||
I: Iterator<Item = AdStructure<'a>>, | ||
{ | ||
// Detected an advertisement frame! Do something with it here. |
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.
Hmm, I think this should at least do something with the data, maybe even just an hprintln!
if that makes sense.
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.
We could add a rprintln!
call through rtt-target
. It would require connecting with an RTT client though (e.g. by flashing through cargo-embed, which is what I usually do).
Might actually be nice for the demo, since it doesn't require wiring up and configuring UART (and in contrast to semihosting code still runs without a debug probe attached).
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 rtt-target
has a bunch of open soundness issues and is still fairly new, I'd prefer to go with semihosting instead (we already pull that in for a panic handler). Since this is a demo, it won't run without a debugger anyways (and if it did, it wouldn't output anything regardless).
The demo currently doesn't do any logging. If we'd simply copy over the logging stuff from
nrf52-demo
, then we'd have a lot of duplication. However, it could be easily added.Even without logging, the example remains useful.
(I use RTT based logging, so I haven't used UART based logger implementation so far.)
Note that this PR contains a breaking change (method rename).