-
Notifications
You must be signed in to change notification settings - Fork 134
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
Simulate BLE connection between central and peripheral zephyr samples #56
base: master
Are you sure you want to change the base?
Conversation
8f44b64
to
c38ed37
Compare
Hey @grifcj thanks for your contribution! It's very very interesting. We confirmed it to work on our side and we're processing it internally. There are some parts that are a little controversial and we need to spend some time on them (especially in the time handling areas), but we hope to move swiftly with this. As the PR is quite large, we'll let you know what can we just merge in and which parts we'd like to discuss more. |
@PiotrZierhoffer Thanks for taking a look. I suspected I might be trampling on an important optimization with the timer change or breaking something. Hoping the test cases I added will cover what's needed for the bluetooth simulation, so that impl may be reverted to something more similar to what it was while keeping tests going if need be. I'm sure you've seen some of the other things too that we're added mainly to support debugging, but perhaps shouldn't make it to master, least not in current form.
I did a little more characterization. Out of the zephyr samples, the basic central and peripheral ones seem to work. The central-multilink also worked with 3 peripherals, provided I commented out RSSI checks and programmed peripherals with different addresses. Some of the notable things that don't currently work which might be worth checking into next:
Also, no automated integration testing yet. Wonder if there was a preference to some simple robot tests or perhaps checking into using the existing zephyr test suite in some way. I believe I remember reading twister can deploy to boards and simulators. I'm not aware of any other demos using this radio model (NRF52840_Radio.cs) for other protocols, but if so, I haven't regression tested in that regard either. |
Thanks for those tests. I'm currently looking into changes in the timers infrastructure.
I agree that part of the proposed changes could be extracted to separate PRs (or dropped).
I've already created a simple robot test for |
Latest work here does a couple things. It enables some simple soft device execution as described here Adding a basic ECB peripheral enables execution with CONFIG_BT_SMP. CONFIG_BT_PRIVACY also works. Work here being defined as heart-rate example between central/peripheral can connect and send notifications. This was against latest zephyr/main:d435340f1b. |
@grifcj I merged fixes to |
Cool, so it be ok if I rebase on latest master, drop the related commits from PR, and force push the PR branch? Or is another process preferred? |
That would be great, thanks! |
37d4d74
to
8995959
Compare
@mateusz-holenko As you can see, pushed changes up. Added a couple, the interrupt fix to RTC being the important one. Spot checked against 3 sims and all appeared to work as intended / or at least equivalent to where I last left them. The last two configurations are less functional than the zephyr one.
|
Latest commits enhance radio model and enable simulation of RIOT/NimBLE peripheral w/ Zephyr central. The existing Zephyr central / peripheral functionality remains. Nordic softdevice peripheral with zephyr central seemed more functional, but no functional connection. Timing accuracy seems to require 1 us quantum, which slows down simulation considerably. Couple of issues.
|
I checked and merged upstream (with cosmetic formatting changes) commits fixing things in CLOCK, PPI and Timer. I also took a look at the ECB model and it seems fine. I believe it should be fairly simple to add actual encryption logic after extracting it from CC2538_Cryptoprocessor - I'll do that as a next step. I'm not sure whether logger-related changes should be merged upstream. If you find them useful, could you extract them from this PR so that we can discuss them separately? |
e077041
to
8d78990
Compare
Rebased on latest master, dropping merged commits. Extracted logging to separate branch and will create ticket for further discussion / development. |
I extended the ECB model with actual AES calculations and merged this together with RTC changes upstream. What's left for review now are changes to the Radio controller model. |
Code in interrupt handlers assume some time delay before packet goes out, hence the resulting actions as triggered to PPI need some delay. This change was found to ok for 10 us quantum.
Remove while loop, just pull one frame off queue. Loop causes simulation to deadlock because after receiving first frame, then radio is disabled and the receive just enqueues again.
Between RxEnable and PacketEnd and RadioDisable, embedded code may be written in way that assumes there is some time passing. Have not verified this like with transmit side, but seems a reasonable assumption that it should be similar.
When sending to wireless medium, use in-air representation. When reading/writing to memory use RAM representation. Radio allows s1 field to be optionally included in on-air packet. This becomes relevant for BLE data PDUs which may have 16 or 24 bit header.
Calculation of timing for transmit and receive side of radio events is similar enough to share implementation.
Bit counter can used for generating successive events. This implementation just fires a single match event configured prior to receiving radio frame.
8d78990
to
4d77fff
Compare
Rebased. Tested again zephyr and riot simulations. Everything still works. My platform file has some small mods to make RIOT simulation work. It needs the interrupt hookup for ECB peripheral. The DEVICEADDR stuff was described above @@ -87,11 +87,15 @@ rng: Miscellaneous.NRF52840_RNG @ sysbus 0x4000D000
-> nvic@13
ecb: Miscellaneous.NRF52840_ECB @ sysbus 0x4000E000
+ -> nvic@14
sysbus:
init:
ApplySVD @https://dl.antmicro.com/projects/renode/svd/NRF52840.svd.gz
Tag <0x10000000, 0x10000FFF> "FICR"
Tag <0x100000a0, 0x100000a3> "DEVICEADDRTYPE" 0x1 # random address
- Tag <0x100000a4, 0x100000a7> "DEVICEADDR[0]" 0xAABBCCDD
+ Tag <0x100000a4, 0x100000a4> "DEVICEADDR[0][0]" 0xDD
+ Tag <0x100000a5, 0x100000a5> "DEVICEADDR[0][1]" 0xCC
+ Tag <0x100000a6, 0x100000a6> "DEVICEADDR[0][2]" 0xBB
+ Tag <0x100000a7, 0x100000a7> "DEVICEADDR[0][3]" 0xAA
Tag <0x10001200, 0x10001203> "PSELRESET"
|
Most of the changes to We added an automated test running the Zephyr peripheral/central HR sample. For now we skipped the direct PPI events triggering, as those were quite hacky and seemed not to be necessary for the Zephyr demo. In the comment you mentioned that those were necessary to support RIOT stack correctly, is that right? Could you elaborate on this a bit more - perhaps we can come up with an alternative solution to the problem. |
Some of the RIOT ISRs have code like below, which rely on timer captures triggered by radio events. This particular one is in nimble/drivers/nrf52/src/ble_phy.c (I believe this was one of the problem ones, but didn't verify it again). Summary is that the simulated day from radio event to PPI task is 1 or 2 quantums, which allows too much ISR code to execute and results in incorrect simulated behavior. Just a bit more specificity on this particular scenario, the preprogrammed channels 26 and 27 tie radio events Address and End to captures on Timer0 CC1 and CC2. On hardware, PPI propagation is synced to 16 MHz clock, so a max latency of 62.5 ns. When run in simulation, the PPI model uses the "ExecuteInNearestSyncState" for executing the tasks triggered by events and hence the simulated latency will be dependent on chosen time quantum. If I recall correctly, it can be up to 2 quantum delay. So 2 us >> 62 ns (assuming 1 us quantum, the min), and the simulated CPU executes too much ISR code (which is fired by radio END event), passing the statements where the timer captures are evaluated. The direct writes to the timer tasks in the radio model bypass the sync delays, ensuring that that captures take place in time. Zephyr on the other hand doesn't have any code that stresses the simulation models in this fashion, so it executes fine without these statements. |
Changes to enable simulation of bluetooth connection between zephyr samples central_hr and periperhal_hr. I've been testing against main, most recently this commit. Simulation should work without any modifications to project configs. I did most development with heart-rate samples, but also did a quick check with central_ht/peripheral_ht and those seemed to connect.
west commands to build zephyr images
Changes here are paired with updates to parent renode repository, which contains an example script for executing the demo. See bluetooth-work branch in my fork.
The changes primarily deal with updates for coordinating the radio with timers and PPI. The zephyr stack makes heavy use of PPI and rtc0, timer0, & timer1 for scheduling actions in bluetooth controller firmware. The firmware is fairly time aware, and expects radio actions to consume some amount of time and that simulation has enough resolution to adequately order events below 100 us. I found that a simulation quantum of 100 us would not function, but a 10 us one does. Perhaps there are ways around this, as it unfortunately affects overall simulation performance.
I've only tested with stock zephyr configuration of samples and with privacy and security disabled. Things that rely on additional hardware features beyond default config, e.g. TIFS hardware support, isn't included in these changes.