adafruit/Adafruit_nRF52_Arduino

Bluefruit.begin() causes high power consumption

jpconstantineau opened this issue · 19 comments

Describe the bug
master HEAD-unreleased 0.22.0 has issues with high power consumption when using Bluefruit BLE functions.

Set up (mandatory)

  • PC & IDE : Arduino 1.8.13 on Windows 10
  • BSP : 0.21.0 vs master-HEAD (0.22.0 WIP)
  • Board : nRF52840 module with no power circuitry (connected directly to Nordic Power Profiler)
  • Sketch: hid_keyboard with delay(20); added to top of loop() to ensure that CPU has a chance to sleep and lower power consumption

To Reproduce
1 - load hid_keyboard example
2 - add delay(20); to loop(); and comment out while !serial line in setup();
3 - compile/flash with 0.21.0
4 - connect power profiler and get 1.477mA average current when advertising in fast mode and 257uA average current once it settles
5 - Install master-HEAD version of BSP as per readme.md instructions
6 - compile/flash with master-HEAD
7 - connect power profiler and get 7.451mA average current when advertising in fast mode and 6.408mA average current once it settles

Screenshots
See screenshots of power consumption indicated above. (links to come...)

Serial Log
None needed

Notes
A brief analysis by commenting things out indicates that Bluefruit.begin(); is what triggers the issue.
This means that some of the changes implemented to handle background tasks do not put the CPU in sleep mode.

Screenshots of Nordic Power Profiler:
0.21.0

0 21 0_power
0 21 0_power_after_settling

master-HEAD (0.22.0 WIP)
HEAD_power
HEAD_power_after_settling

it is probably due to the PR #466 which make use of the CC310 cryptocell subsystem for performing LESC pairing. Maybe we should enable when needed only. For now I don't have the bandwidth to look at this, however if you could verify the current is indeed started increased after #466 it will help me to later on when looking at the issue.

I went through the commits from 0.21.0 and on and did find the one that caused the high power consumption:

ce81b83 1.4mA 245uA
1eae55e 1.37mA 240uA
7e64023 1.37mA 241uA
dc2802a 7.44mA 6.4mA

It is indeed PR #466 that caused the issue.

thanks @jpconstantineau for testing and confirmation, I am currently on other work, will take a look at this later on.

Tested HEAD by reverting #466 away from all the commits on my own fork and can confirm that power consumption is still at 1.4mA and 241uA without the content of that PR#466 merge. Doing a git revert might be messier than fixing it but could be an option to bring everything else that was added after that PR.

Revert won't be an option. We will fix it when we have time to work on this later on.

Do you think you will have time to revisit this issue soon? The increased power consumption is a deal breaker for a project of mine ☹️

BLE power consumption is also highly important for me. Waiting for any updates.

@jpconstantineau Is there a way to use power profiler kit on other boards? (Other than nrf52 devkit)

I have the first version of the power profiler. Although you need to have it on top of a dev kit, you can measure an external device (i.e. a custom board) by using the "External DUT" header to power your custom board.

The newer version of the power profiler does not need a dev kit.

Just saw the latest release (0.22.0) and was wondering if testing was done to validate is this still an issue or not.
I can rerun the tests I did previously and report what I see.

no, this issue is still open which already give you the hint. It will be closed if I got time to test and fix it. Haven't even tried to reproduce this yet !!. The release is for reworking of TinyUSB lib. If this issue is deal breaker for you, just ignore it.

I assumed as much. I have lots of other things to test (and potentially refactor) in my keyboard firmware with all the other changes that this release brought in too (looking forward to test those USB changes). I'll have to check the CI builds and see the impact (I should know later today). I am recommending my community to hold on before doing the upgrade until things are validated...

I suspect that this will be a deal breaker for everyone running on batteries.

Yeah I know, I just didn't got time to get back to nrf52 work for now. I suspect it is only a minor changes with peripheral e.g cryptocell is not disabled when not used. But getting to actual line of code can be tome consuming. Please hanged on. TinyUSB rework is the cross core/ports effort to get rp2040 and probably more like esp32s2 on board. So yeah, it takes the priority

should be fixed by #654 , please test it again, if not feel free to re-open this.

@jpconstantineau Would you mind retesting it, I don't have a sensitive enough meter and it would be really useful to know 😄

@mark9064 Will do - probably over the weekend. Description of the resolution seems like it will do but actual validation will be useful for everyone.

Loaded up the updated HEAD and loaded up HID keyscan. (it already has a delay in the loop).
Here is the current after a while:
image
Current consumption is 290uA. (which is good compared to what we had!)

Once I connect/pair my phone to it, I get 481uA on average.
image

Just for completeness, I did the same as above but for the 0.22.1 release. Here is the current consumption.
image

I am looking forward to the next release!
I am satisfied that the fix worked. Thanks

Awesome! Thanks for testing

thanks for testing and confirming the fix.