Skip to content
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

All BLE event handling should happen in thread mode #89

Closed
betzw opened this issue Oct 28, 2015 · 29 comments
Closed

All BLE event handling should happen in thread mode #89

betzw opened this issue Oct 28, 2015 · 29 comments

Comments

@betzw
Copy link

betzw commented Oct 28, 2015

Hi @rgrover,
I have the impression that the current version of ble is not "really" ported to mbedOS/minar as it still makes use of the "old" interfaces of Tickers and callbacks which get then executed in interupt context (e.g. https://github.com/ARMmbed/ble/blob/master/ble/services/EddystoneService.h#L490).
Can you confirm this and is there a version of the BLE stack which would use minar callbacks instaed?

@rgrover
Copy link
Contributor

rgrover commented Oct 28, 2015

@betzw There is only one version of ble. And we intend to keep it compatible both with mbedOS and mbed-classic. Use of minar to schedule callbacks would violate compatibility with mbed-classic. Tickers are abstractions that remain alive across mbed-classic and mbed OS.

The fact is that the current Ticker implementation in mbed OS uses interrupt context (like it used to in mbed-classic). If Ticker is used in conjunction with BLE in an application, it could result in race conditions where BLE stack callbacks are executed in thread mode while the Ticker callbacks get to run in handler mode. If the Ticker callbacks use BLE_API, it could mess the system.

Thanks for pointing this out.

Ticker can't be implemented using minar because Ticker requires microsecond resolution (for legacy reasons), but minar can only work with millisecond resolution.

We need to implement a cross platform Ticker (or low-power ticker) which can be used in both mbed-classic and mbed OS.

Thanks so much.

@betzw
Copy link
Author

betzw commented Oct 28, 2015

Yes, this is exactly what happens with the st-ble-shield, it gets messed up :(
So do you have any concrete plans to implement tickers which do (at least in mbedOS) not execute their callbacks in interrupt context?

@rgrover
Copy link
Contributor

rgrover commented Oct 28, 2015

I believe applications needing periodic callbacks can easily switch to Minar supported callbacks for mbed OS. We'll be updating EddystoneService (and the likes) to not use Ticker; or at least not call BLE_API from Ticker handlers.

@betzw
Copy link
Author

betzw commented Oct 28, 2015

Sounds great!
Pls. let us know when we can try these modifications.

@screamerbg

@rgrover
Copy link
Contributor

rgrover commented Oct 28, 2015

One of my colleagues is working on a version of Eddystone which doesn't use the Ticker for BLE_API calls. I'm sure he would send us an update when he has something to share.

@rainierwolfcastle
Copy link

ARM Internal Ref: IOTSFW-1099

@rgrover
Copy link
Contributor

rgrover commented Nov 16, 2015

Radio-notification handling is now done at a lower priority. They post events to MINAR.
Refer to https://github.com/ARMmbed/ble-nrf51822/blob/develop/source/nRF5xGap.h#L114
@note: this is still in the develop branch of ble-nrf51822.
@betzw : Please confirm that this solves your issue.

@rgrover rgrover changed the title Real mbedOS port of BLE API All interrupts originating from the Nordic Softdevice should post callbacks to Minar Nov 16, 2015
@rgrover rgrover changed the title All interrupts originating from the Nordic Softdevice should post callbacks to Minar All BLE event handling should happen in thread mode Nov 16, 2015
@rgrover
Copy link
Contributor

rgrover commented Nov 16, 2015

In our recent posts for this issue, we did not clarify how RadioNotification callbacks are related to this issue. Apologies.
Our goal is to provide an alternate implementation for Eddystone which doesn't rely upon Ticker to call into BLE_API. This updated implementation uses the radioNotificationCallback, which uncovered another issue with their use on the Nordic platform.

We've now resolved the problem with Radio Notification, and are about to release this new version of Eddystone.

@betzw
Copy link
Author

betzw commented Nov 17, 2015

@rgrover could you pls. clarify which repo/branch I should test?

@ghost
Copy link

ghost commented Nov 17, 2015

@betz develop branch of https://github.com/ARMmbed/ble-nrf51822/

@rgrover
Copy link
Contributor

rgrover commented Nov 17, 2015

I'll soon be publishing an updated version of Eddystone developed by @andresag01. It should arrive under https://github.com/ARMmbed/ble-examples; and eventually under https://github.com/ARMmbed/ble
Presently it is a pull request for ble-examples.

@betzw
Copy link
Author

betzw commented Nov 17, 2015

Looking at the code at https://github.com/ARMmbed/ble-nrf51822/blob/develop/source/nRF5xGap.h#L114 to me this seems to be in the right direction. But how can I test it and confirm that it resolves the issue on our expansion board?

@ghost
Copy link

ghost commented Nov 17, 2015

by replacing your version of ble-nr51822 with the one from the develop branch.

@betzw
Copy link
Author

betzw commented Nov 17, 2015

Are you kidding?

@ghost
Copy link

ghost commented Nov 17, 2015

no? you configure your your module.json for ble-nrf51822 like so

"dependencies": {
"ble-nrf51822": "ARMmbed/ble-nrf51822#develop"
}

@betzw
Copy link
Author

betzw commented Nov 17, 2015

I do not have a nrf51822.

@rgrover
Copy link
Contributor

rgrover commented Nov 17, 2015

@betzw as long as your port of BLE handles radio-notification events in a CriticalSection-safe manner, you're fine. You don't need to test this against Nordic if you don't have an nRF51.

@rgrover
Copy link
Contributor

rgrover commented Nov 17, 2015

@betzw as a general guide for porters of BLE_API: BLE stack generated interrupts should respect the HAL's CriticalSection; if any interrupt violates this expectation, then mbedOS APIs shouldn't be called directly from such a handler.

@betzw
Copy link
Author

betzw commented Nov 17, 2015

OK, but how can I test it on our x-nucleo-idb04a1? I.e. should I wait for you to merge it into ARMmbed/ble-examples and/or ARMmbed/ble?

@rgrover
Copy link
Contributor

rgrover commented Nov 17, 2015

@betzw you'll soon find an update version of Eddystone under ble-examples. Please test that.

@andresag01
Copy link

@betzw We have made a new implementation of EddystoneService that does not use any Timeout or Ticker callbacks when used with mbedOS. Please use this one on your tests. It can be found on ble-examples.

@andresag01
Copy link

@betzw to avoid any confusion, we have consolidated the Eddystone examples in ARMmbed/ble-examples repo master branch. We have removed all the "old" examples that used the previous implementation (i.e. BLE_EddystoneBeacon and BLE_EddystonBeaconConfigService) and replaced them with the example using the new Eddystone implementation that can be found under BLE_EddystoneService. Please use the code in BLE_EddystoneService for any tests from now on.

@betzw
Copy link
Author

betzw commented Nov 17, 2015

Seems as if there is a nrf dependence in the code:

/mnt/disk2/betzw/Projects/mbed/mbed_3.0/yotta_my/apps/ble-examples/BLE_EddystoneService/source/nrfConfigParamsPersistence.cpp:17:22: fatal error: pstorage.h: No such file or directory
 #include "pstorage.h"
                      ^
compilation terminated.

@rgrover
Copy link
Contributor

rgrover commented Nov 17, 2015

@andresag01: could you remove persistence from the demo?

@andresag01
Copy link

@rgrover @betzw Dependencies on nrf51 modules have been removed from the example.

@rgrover
Copy link
Contributor

rgrover commented Nov 17, 2015

@andresag01 👍

@betzw
Copy link
Author

betzw commented Nov 17, 2015

OK, seems to work fine.
Thanks!

@seajayshore
Copy link

seajayshore commented Jul 14, 2016

Hi there, sorry to comment on a closed issue like this.

There are multiple seemingly related issues with using mbed-classic Tickers & the BLE_API / Softdevice on the Nordic devices where basically, after some time, the ticker interrupts stop firing and the application never returns to main() (but the Softdevice keeps running happily).

I've tried to get to the bottom of it but I'm not so familiar with the libraries & I've found myself here reading about this Ticker/Timeout usage within the BLE_API callbacks.

Could anyone more knowledgeable please say if they think these such issues are due to the mbed-classic BLE_API problems described in this thread above?

Issues links:
https://developer.mbed.org/questions/69710/Tickers-crashhang-on-the-nRF51822-especi/
ARMmbed/mbed-os#1533

I'd massively appreciate anyone helping on this. :-)

@pan-
Copy link
Member

pan- commented Jul 14, 2016

@seajayshore I'm able to reproduce the ticker issue on mbed OS, I don't think it is related to a race condition due to a ticker but rather to an internal overflow in the ticker. I'm still investigating it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants