-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix high idle/sleep current due to flash and use xiao-ble from upstream Zephyr #2091
Conversation
I don't have a strong opinion on removing At the very least, I think it would be helpful to split this cleanly into two commits: one which fixes the battery use and one which removes the |
Have you measured the power consumption with these changes? Ideally measure and compare with Bluetooth disabled but even figures with Bluetooth could be useful. Also worth mentioning that upstream Zephyr definition for I would even try measuring on Zephyr 3.5 without any changes. See zephyrproject-rtos/zephyr#55199 for some related issues. I haven't looked at this closely but one of those issues won't be in upstream Zephyr until 3.6. |
Measuring with BT off is pointless since the USB controller + Battery charger will drain a lot more power than this fix and it would overshadow the fix. I do not see how that would break with the upstream QSPI Zephyr, in my view it is the opposite actually. Currently there is ZMK own board definition inside Zephyr repo that could contain flags that are no longer supported. On the upstream, QSPI is defined and configured and enter DPD upon init(), unless I missed what you meant. |
ok, I have made a separate commit. Please find below the difference:
Measured at 3.3V as the LDO is undocumented and cant be trusted:My board beforeWith this commitI have also flashed BeforeAfter |
Thanks for these additional figures. The deep sleep figures were more or less what I was looking for - essentially both BT and USB off as that most accurately isolates the QSPI power consumption.
Your earlier commit had |
I see just one commit in the PR now, with the qspi node changes. Is that enough or were the older changes dropped by accident? |
The 1x commit is enough |
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.
This all seems reasonable to me. We have our own board definition for historical reasons... Sadly, removing it now would break a lot of folks' existing builds. It would be nice to at least get the upstream board definition fully supported (by adding the necessary config and overlay ZMK specific additions on app/boards
) and changing our metadata file to guide folks to that board ID, so we can slowly phase it out.
For now... Does this in fact fix things still for our board definition? If so, happy to merge, and grateful for the fix!
Hi Pete, is this something that could be used with my revxlp v0.1? If so I'd gladly test it to see if I get longer battery life. |
Yes, the overlay fix the current board definition with:
|
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.
Flashed to my revxlp here with a XIAO BLE, and runs no problem. I trust OP measurements here, so merging so others can get the fix easily. We can follow up further if folks still report high power draw.
Thanks for digging and for the fix @ldab !
…am Zephyr zmkfirmware#2091 Merged petejohanson merged 1 commit into zmkfirmware:main from ldab:xiao-flash-sleep 2 days ago Getting the changes in my repo to benefit from the battery savings.
with latest changes to seeeduino_xiao_ble.overlay (instead qspi disabled) my split KB with XIAO BLEs refuse to go to sleep anymore after first sleep-wakeup cycle ... borads need to be powered off-on to get to sleep again I again built with qspi overridden to disabled and it works again |
I am also seeing randomness in being able to sleep, sometimes it sleeps fine, sometimes it won't. The symptoms look to be the same as #1901. |
On my tests as shown on the measurements above, it worked ok. To help you, can you please create an issue and include a detailed description of your issue, including which board you use, the schematic, measurements and enable USB logs and attach it? |
The sleep code only looks for VBus detected, no? It could be related to zephyrproject-rtos/zephyr#55543 can you pull some logs? Can you try to patch with zephyrproject-rtos/zephyr#64782 and see if it helps? |
I'm running handwire split (10 keys on each half, directly connected to pins and GND - no matrix) on XIAO BLEs with latest ZMK build
I have split battery reporting on and also using Brave's RGB widget - that's how I noticed that they are not sleeping .. as after returning to PC they do not blink battery status and connection after pressing keys (what RGB widget always does after wakeup) - this led me to checknig it on battery widget in MacOS and indeed they always stay on after first wakeup after setting QSPI to disabled behavior want back to normal and boards go to sleep after timeout my assumption is that when I'll try running logging they will not go to sleep as USB is connected |
Your assumption is correct, but the logs may show something that we are missing, you can change this and it will go to sleep when USB is connected: Line 70 in 466cf92
Do you have your keyboard configuration? I can try to reproduce it here. Can you try to patch with zephyrproject-rtos/zephyr#64782 and see if it helps? |
I do not have local env .... building via GHA, so editing .c files is out of my reach i guess my repo is here |
It prints
So I suppose it relates to the patch I mentioned above. If I remove the power (and battery) it works no problem. so there is a point where the QSPI is not handled properly. |
I am now using a Zephyr 3.5-based branch (#2027, that is based on #1995) and the deep sleep is working OK after waking up from sleep. I am not sure if the power consumption is normal after waking up since I don't have a power profiler, but I'll track overall battery life. |
I didn't flash the board you mentioned to my keyboard because the GPIOs are different, but I have build based on #1995 my keyboard and the result is: So using #1995 the sleep current is 20uA vs ~1uA I measured not, but it seems to fix the sleep issue after second wake. I guess we can open an issue to track it and investigate more when 3.5.0 is merged, what do you think @caksoylar ? |
That's interesting that the sleep current went up but the usage consumption looks pretty good, thanks for measuring!
I think that would be a good idea indeed. It shouldn't be too long until 1995 is merged, I am guessing. |
I'm working a XIAO-BLE based keyboard and I noticed high idle and sleep currents, bearing in mind that my board:
The 500uA current is similar to reported here: https://forum.seeedstudio.com/t/low-power-with-xiao-nrf52840-on-zephyr-rtos/270491/6
Which led me to #1927. Even tho the PR seems to address the issue somehow, I find it strange that it does since the nRF driver puts the flash on Deep Power-Down (DPD) drivers/flash/nrf_qspi_nor.c#L1324, the reason for DPD not properly working I believe to be related to the fact that the ZMK fork of Zephyr use what seems the incorrect boards/arm/seeeduino_xiao_ble/seeeduino_xiao_ble.dts#L97 board configuration (which does not match the upstream Zephyr).
I noticed the
seeeduino_xiao_ble
was added on zmkfirmware/zephyr@1f19b89 and I do not know the reason for adding since it is already available on boards/arm/xiao_ble and also on Zephyr 3.2 and newer.With that being said, this PR:
Removeseeeduino_xiao_ble
and use thexiao_ble
from Zephyr(To add measurement)Note/Question:
zmkfirmware/zephyr
to revert zmkfirmware/zephyr@1f19b89app/boards/seeeduino_xiao_ble.overlay