search

espressif / esp-nimble

A fork of NimBLE stack, for use with ESP32 and ESP-IDF
Apache License 2.0
76 stars 49 forks source link

Deinit on nimble init failure #68

Open tannewt opened 2 months ago

tannewt commented 2 months ago

Otherwise the controller is left on and uses memory.

CLAassistant commented 2 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

rahult-github commented 2 months ago

Hi @tannewt ,

I have a query. The init for controller should not fail . If controller init fails, then as it is bluetooth cannot be used. So , when init fails, rather than freeing up the memory, it is better to understand what is failing and ensure bluetooth init never fails.

What use case are you targetting where it is ok to have bluetooth init fail ?

tannewt commented 2 months ago

I didn’t see the controller init fail but I guarded against it anyway. Nimble init failed and I wanted it to shutdown all of Bluetooth.

This makes circuitpython work better because it uses that freed memory to attempt to run user code again. Without it, only the first attempt fails gracefully. The second fails harder because it has less memory to work with.

On Tue, May 7, 2024, at 12:39 AM, Rahul wrote:

Hi @tannewt https://github.com/tannewt ,

I have a query. The init for controller should not fail . If controller init fails, then as it is bluetooth cannot be used. So , when init fails, rather than freeing up the memory, it is better to understand what is failing and ensure bluetooth init never fails.

What use case are you targetting where it is ok to have bluetooth init fail ?

— Reply to this email directly, view it on GitHub https://github.com/espressif/esp-nimble/pull/68#issuecomment-2097556228, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAM3KNVXZ6SM6XSC5L7GELZBBZJDAVCNFSM6AAAAABHGI2RSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJXGU2TMMRSHA. You are receiving this because you were mentioned.Message ID: @.***>

rahult-github commented 2 months ago

Ok. I feel there is no harm in doing so. The current PR is against nimble-1.5.0-idf. I will raise internally to match with nimble-1.6.0-idf and backport your PR to other 5.2 / 5.1 branch. Thanks for your contribution