InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

Add support for BMA425's wrist-tilt interrupt (raise-to-wake)

Open Zoarial94 opened this issue 2 years ago • 4 comments

This pull request is for adding support for the BMA425's wrist-tilt or raise-to-wake functionality. I have tested this on a PineTime Dev kit and have found no issues to far. The code will automatically use the interrupt if the device has a BMA425, and have no change for a BMA421 (untested). This behavior can be changed pretty easily with a checkbox/toggle. The software fallback for the raise-to-wake and shake-to-wake are still working with the fix mentioned above.

I have not tested this code on a sealed device, or even on my wrist, but from what I can tell should work fine. I understand this will probably interfere with other efforts to improve the BMA421's code (such as #826).


Code breaking change

The axes were remapped in the BMA425, and driver code. I have updated the software raise-to-wake and shake-to-wake to use the proper axes. The quick fix for using the remapped axes, it just to invert the sign. (Use -x, -y, & -z instead of x, y, & z)

Change log:

SystemTask.cpp

  • Added motionSensor as an argument to motionController's Init()
  • Call shouldRaiseWake instead of Should_RaiseWake when deciding to wake up

Bma421.cpp and .h

  • Fixed clang-format and initialization issues
  • Added wristTiltInterrupt flag for raise-to-wake functionality
  • Refactored Bma421::Init() to better match Bosch's example code.
    • Removed redundant step_detector_enable call. Step counting functionality appears to still be working.
    • Refactored all bma423_feature_enable calls to 1 function call
  • Remapped BMA425's axes for proper raise-to-wake functionality.
    • Also remapped the axes in driver-space
    • The remap can be done in runtime, which allows for remapping of the right and left hand.
  • Enabled BMA425's wrist-tilt feature and interrupt for raise-to-wake functionality

MotionController.cpp and .h

  • Fixed clang-format errors
  • Added shouldRaiseWake which executes a callback function for the desired raise-to-wake algorithm.
    • This was done so that the call back can be changed transparently in the driver during runtime.
  • Added a reference to Drivers::BMA421 in the constructor
    • I believe MotionController should completely encapsulate the BMA421 object. This can be discussed later.
  • Created separate function for software raise-to-wake and the BMA425's interrupt
  • Updated the software raise-to-wake and shake-to-wake to use the correct axes since they are now remapped.

Zoarial94 avatar Aug 30 '22 01:08 Zoarial94

I have been using this feature on my sealed PineTime for 4+, and I found that it doesn't work that well. Sometimes it will just not activate, and other times it will active in weird position. I still support remapping the axes in this PR, but I am not certain the interrupt is the way to go for a reliable experience. It a possibly I have not implemented it properly. I would love to see some more input on this.

Zoarial94 avatar Sep 04 '22 13:09 Zoarial94

I'm very interested in this feature, I haven't had a proper look yet but I've built it and will start testing. Given the problems you've listed seem similar to with the current software raise to wake implementation, is there any possibility both are active? I'm including a DFU here for others to test it as actions aren't approved yet for this PR, I've flashed it successfully on my own watch but obviously YMMV.

pinetime-mcuboot-app-dfu-1.10.0.zip

kieranc avatar Sep 12 '22 13:09 kieranc

I've done a bit of testing, it's certainly functional but not perfect - I've noticed a couple of times this evening that it was awake just lying on a desk, and there were some instances of it not waking when I hoped that it would. Maybe some sort of debug app with indicators for shake/software wrist raise/hardware wrist raise activations could be useful? I'm afraid I can't offer much in terms of code quality but hopefully someone else can weigh in. I can say that there's a fair amount of cleanup in the code which is great, but unrelated to this PR and for ease of review each PR should aim to address a single issue. You might want to break out the clang/gitignore stuff to a separate branch/PR.

kieranc avatar Sep 13 '22 20:09 kieranc

Thank you for the feedback. I have not contributed to projects other than my own and I am still trying to get a feel for it.

I have definitely seen the same behavior you are seeing, and I think the debug app is a great idea. Until now, I was just adding lables into the motion app.

...is there any possibility both are active?

No. The way I implemented this was that a callback is made to a single function. Only 1 algorithm is checked at any point.

I will see if I can work on something today or tomorrow.

Zoarial94 avatar Sep 13 '22 21:09 Zoarial94

Haha. Didn't expect feedback already. I've wanted to work on this again for a while, and I still won't really have the time for another month or so.

Zoarial94 avatar Mar 02 '23 16:03 Zoarial94

I haven't had time to test it on hardware, so I don't know how good the tilt detection itself is. If it's good, it might be alright to just enable it by default, but if it isn't, maybe a toggle in the settings would be good.

FintasticMan avatar Mar 02 '23 16:03 FintasticMan

From my small amount of testing (not mounted to my wrist), it isn't great. I don't know if it's a config issue or if it's just how the sensor is. False positives can be reduced by preventing it from waking while the screen is facing up, but not sure about false negatives yet.

I have a feeling software might be the way to go.

Zoarial94 avatar Mar 02 '23 16:03 Zoarial94

Both the hardware and software raise-to-wake functions work. I have flashed my sealed watch with the latest commits on this branch and I will try it out for a while and report what I find. From my initial findings, it seems to work fine. There is sometimes a slight delay before the screen turns on, but still acceptable. If it doesn't turn on after a second, a small shake seems to wake it.

pinetime-mcuboot-app-dfu-1.11.0.zip

Zoarial94 avatar Mar 02 '23 21:03 Zoarial94

I am happy to report using the hardware interrupt works pretty well. I would like to see what others think of it especially compared to the original software solution. In combination with a screen lock and lower-to-sleep functions, I would be able to go about my day without having to worry about accidentally opening an app or wasting too much battery life.

Zoarial94 avatar Mar 08 '23 23:03 Zoarial94

Build size and comparison to main:

Section Size Difference
text 413592B 6892B
data 940B 0B
bss 53608B 40B

github-actions[bot] avatar Mar 09 '23 07:03 github-actions[bot]