-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Shake wake #691
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
Shake wake #691
Conversation
|
Why should there be another algorithm? Why not just improve the first one? I think "Raise wrist" is a poor name and should really be thought of as "automatic" wakeup, where it's always awake when we want it, instead of literally requiring raising it. |
|
I like to have option of other algorithm which might turn display on only when I need it and don't waste batteries like for example when I'm driving and turning wheel. Also tuning new algorithm will take some time and few releases, so it might be good idea to keep the old one there as a fallback. I merged and tested the code, was able to turn on the watch few times with shake, but I feel like a video of proper movement would be great :) I'm not sure it must be so violenent and I might have shaking wrong :) Also I'm not able to fully understand the vector/math behind shake detection. Also, what the threshold in setting means? Is it sensitivity? Tried HIGH and I had to shake really lot (or I did not shaked properly), with settings LOW I did not managed to wake it up :) |
|
Did some quick testing. High and med threshold works for me. Low may be a bit too low but I think the x axis will help a lot. Great work so far! |
I think both should be done,
Matches what my testing showed! More tweaking!
Yeah my thoughts as well, and this is going to take some iterations to make happy that is for sure. |
|
Added a much nicer way of setting the threshold Hit the calibrate button, Shake at desired wake sensitivity and it will auto set to the max you generated - 200. Thanks guys! Win.20210928.00.41.27.Pro-1.mp4 |
|
I'm still not really convinced. When the "automatic" algorithm gets improved, would there be any reason to use this? Maybe this is better for now, but is it just a temporary solution? The automatic algorithm wouldn't have to be so sensitive that it's always on when it's at an angle where the user might see it, as long as it always wakes up on a wrist flick if it didn't wake up at first. I also suspect that the update frequency might be too low and it misses some quick motions. I tried this code, and it seemed like it didn't always wake up even though I think I flicked it hard enough. Looking at the graph in Motion, when I try to make similar peaks back to back, sometimes nothing happens. Is it just me? I once tried to enable the It would be a good idea to improve the motion controller interfacing as soon as possible, because all the code that depend on it would have to be tweaked when it is changed. |
For me personally no, Id much rather have an explicit turn on then something aggressive, Joy of the OSS here we can have both!
Yeah I have an additional algorithm I want to look into that will tweak the Automatic, This algorithm is really just grabbing raw Acceleration and using that to turn on. I want to look into one that is also a hair more "Face forward, Face user, Face forward, Face user" to turn on. I feel it will still trigger to often however so this was the compromise.
Na this happens to me as well. I'm not sure where the issue is, We poll the sensor at 100hz, Which is what we have the output data rate set to. That should be fast enough so I'm thinking the BMA either needs some setting tweaks, or its just how it is.
I'm not sure what needs to change. What are your thought? Currently MotionController exposes what it needs and gets updated at 100hz assuming UpdateMotion is enabled. I would love to hear your thoughts :) And maybe try to get something together! |
The sensor is set to 100Hz, but SystemTask has a delay of 100 ticks in
We can just disable the interrupt or ignore it if we don't want the data, but the sensor is always updating it anyway.
If |
... Yeah math is hard...I moved a zero... I agree its only being called at 10hz.
Correct, We really only want it when we are looking to wake. Steps are their own thing. However now that I see we are not calling it as often as I would like I wonder if we should either move to an interrupt or go to a FreeRTOS task to just handle the motion sensor. I am willing to implement either. Which makes more sense to you? Either of them would have to calculate the acell speed for me at this point as the refresh time I am currently using would not be fast enough to make use of that data granularity, and the faster samples would just get discarded anyway. I'm wary of calling into user tasks from that task/interrupt for battery life reasons. So what ever implements the better motion controller I think it needs to stay self contained. Again more then willing to handle this across infinitime. Just want to make sure I pick the right direction first!
Yeah I am using the time delta to calculate acceleration. So for me update time should be accounted for. If it updates faster or slower it should not matter....the scalar at the end might need to change however...simple fix. I will add a comment to the code for future us. Should I move this to an issue riksu so we can keep this conversation going there? Its stretched a bit from what this PR is supposed to be. For now this is almost functional and can get refactored as part of updating the motion controller. Also thanks for the feedback riksu, Its always appreciated! Helps keep me on my toes ;) |
|
I don't have much more to add so we don't necessarily need to open a new issue if one of us will improve the interfacing just based on this. The only differences between using an interrupt and a timer from what I can tell is the timing and the frequency. The interrupt would have more accurate timing, but the timer has more freedom for selecting the frequency. Not sure which is better, but I think I would use the interrupts if possible. I believe the sensor also has features like updating faster when there's more motion, which would work better with interrupts. The sensor data output frequency could be lowered to 25Hz, so it wouldn't send too many interrupts. 12.5Hz is the next step and it might be too low. |
So the sensors we use actually support internal buffering of up to 1024samples. Currently i believe we only grab the current data, So its speed is polling based. Where it can be set to free run at pick an output data rate. So it really boils down to what interfaces with Infinitime better. I was thinking a FreeRTOS task just because then its not yet another (hardware) interrupt in the system, and with the chip self buffering we do not need time critical anyway as long as the interface to the "new" MotionController class exposes things well. @JF002 any thoughts here? |
|
@Riksu9000 I'll pick up the motioncontroller updates FYI. I got everything in order and potentially a watch with the new acclerometer on the way. So unless your looking forward to playing with it focus elsewhere you do awesome work! |
|
So with this last commit. I'm content with it. I could tweak it for days with no real end in site. Anyone up for another round of feedback :D |
There are a lot of discussions in this thread, and I'm not sure I'll have a strong opinion for every points :) Do we need a new wake up algo? IRQ vs current task vs a new task |
I can see those as good reasons to not use the task then unless its needed. Ill write the code agnostic enough to handle that then. I am planning on updating the BMA drivers. I am intending to not leverage interrupts much at all...that said LPMs will be useful with the fifo. I didn't think to use DMA for the i2c bus, however I'm not sure it will be heavily required. Just having good data properly paced even if its 25/50hz will be way better then the ~10hz with misses we have now :) The power savings from DMA might be substantial however so after a FIFO based implementation with some other QOL changes getting implemented I will plan to revisit DMA. Id love to go over this in more detail somewhere...more realtime then github. Let me know in the chats! |
I just flashed it and I like the configuration. Calibration is awesome. It works quite reliable on that "not always 10 Hz" pace. |
|
By no means it it up to just me! by all means this is exactly the type of feedback I want! I can do technical stuff...user interaction...not so much. Ill raise the calibration time a little..I also want to tweak the offset from the shake. 200 is no longer enough with some of the other adjustments I made I feel. So a new commit/build will be up shortly :) |
So i just upped the timeout to 7.5 Seconds. A few people in the chats have said it was to short as well. |
|
I suggest implementing a lock algorithm for every unlock algorithm. For the shake algo in this PR, this should be quite easy to add since the movement could be exactly the same. What do you think? |
|
I think this is finally ready for review...if you see anything let me know! |
Squashed commit of the following: commit f141743 Author: Tim Keller <geekboy1011@gmail.com> Date: Fri Nov 12 00:53:27 2021 +0000 Fix setting removing it self from wake settings when opening calibration window twice. commit 2cc120a Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Nov 8 02:00:35 2021 +0000 Made calibration window enable Accel wakeups for setting and calibration even when wake mode is inactive. commit 8219ff1 Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Oct 5 03:29:49 2021 +0000 Remove "fancy" settings display and always show ShakeWakeThresholdSetting commit 38f539b Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Oct 4 17:11:27 2021 +0000 Fixed button color changing commit 4b6a1f7 Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Oct 4 02:36:51 2021 +0000 Added visual aide for shake strength Added delay to starting calibration commit 69ad222 Author: Tim Keller <geekboy1011@gmail.com> Date: Sun Oct 3 21:06:31 2021 +0000 Raise calibration timeout to 7.5 seconds Added button toggle state for cleaner user interaction commit 83899c9 Author: Tim Keller <geekboy1011@gmail.com> Date: Thu Sep 30 00:04:51 2021 +0000 Fix crash upon leaving app. Code formatting commit d39677f Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Sep 28 04:33:00 2021 +0000 Make arc moveable, and clear previous setting on calibrate commit 3a67445 Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Sep 28 04:21:47 2021 +0000 Actually save the threshold Prevent a few crashes due to an LV task being active when it shouldnt be. commit 92f9920 Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Sep 28 03:50:08 2021 +0000 Better Sensitivity UI, Calibration button added commit 993313f Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 03:30:49 2021 +0000 Add averaging to wake threshold. Makes it take more then just a "flick" to turn on commit 9f0fd19 Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 02:52:02 2021 +0000 Add start of settings app for senstivity. really just debugging. I want to make it more configurable then high med low. Position of setting needs a new location...dynamicly adding it currently at the end. Which honestly im fine with. commit 065636f Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 01:27:08 2021 +0000 Cleanup commit f7bf6e7 Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 01:20:44 2021 +0000 Added Shake to wake
Squashed commit of the following: commit f141743 Author: Tim Keller <geekboy1011@gmail.com> Date: Fri Nov 12 00:53:27 2021 +0000 Fix setting removing it self from wake settings when opening calibration window twice. commit 2cc120a Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Nov 8 02:00:35 2021 +0000 Made calibration window enable Accel wakeups for setting and calibration even when wake mode is inactive. commit 8219ff1 Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Oct 5 03:29:49 2021 +0000 Remove "fancy" settings display and always show ShakeWakeThresholdSetting commit 38f539b Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Oct 4 17:11:27 2021 +0000 Fixed button color changing commit 4b6a1f7 Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Oct 4 02:36:51 2021 +0000 Added visual aide for shake strength Added delay to starting calibration commit 69ad222 Author: Tim Keller <geekboy1011@gmail.com> Date: Sun Oct 3 21:06:31 2021 +0000 Raise calibration timeout to 7.5 seconds Added button toggle state for cleaner user interaction commit 83899c9 Author: Tim Keller <geekboy1011@gmail.com> Date: Thu Sep 30 00:04:51 2021 +0000 Fix crash upon leaving app. Code formatting commit d39677f Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Sep 28 04:33:00 2021 +0000 Make arc moveable, and clear previous setting on calibrate commit 3a67445 Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Sep 28 04:21:47 2021 +0000 Actually save the threshold Prevent a few crashes due to an LV task being active when it shouldnt be. commit 92f9920 Author: Tim Keller <geekboy1011@gmail.com> Date: Tue Sep 28 03:50:08 2021 +0000 Better Sensitivity UI, Calibration button added commit 993313f Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 03:30:49 2021 +0000 Add averaging to wake threshold. Makes it take more then just a "flick" to turn on commit 9f0fd19 Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 02:52:02 2021 +0000 Add start of settings app for senstivity. really just debugging. I want to make it more configurable then high med low. Position of setting needs a new location...dynamicly adding it currently at the end. Which honestly im fine with. commit 065636f Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 01:27:08 2021 +0000 Cleanup commit f7bf6e7 Author: Tim Keller <geekboy1011@gmail.com> Date: Mon Sep 27 01:20:44 2021 +0000 Added Shake to wake
really just debugging. I want to make it more configurable then high med low. Position of setting needs a new location...dynamicly adding it currently at the end. Which honestly im fine with.
Prevent a few crashes due to an LV task being active when it shouldnt be.
Code formatting
Added button toggle state for cleaner user interaction
Added delay to starting calibration
…ion even when wake mode is inactive.
…ion window twice.
|
There's no settings icon and dragging the sensitivity arc/slider downwards will close the screen. Can we find a suitable sensitivity and remove the calibration screen? |
The swipe action can be easily removed...its not something that has been an issue on my end...never had an accidental close due to moving the arc. This can be done in another PR. I find the sensitivity range changes per uses and per choice. Hence the slider. I like a stronger shake action where I'm sure others like it milder. |
|
The exiting can be fixed in a slightly hacky way by disabling exiting while the arc is held. Requiring a calibration isn't a great user experience and for a "sensitive" wakeup, just use raise wrist. By removing the calibration, the choice becomes simple and nice for the user and it should cover all use cases I think. Lowering the sensitivity threshold on shake wake so it doesn't require a proper shake feels like misusing it, and might be worse than raise wake for accidental wakeups. The threshold should be just high enough that it doesn't activate accidentally. The calibration hints at shaking aggressively, which probably sets the threshold higher than expected. I noticed that tapping the screen a single time is detected as a BIG shake and will most likely wake up the watch. This conflicts with double tap to wake. Now if I double tap the screen to wake up, it usually wakes up and goes back to sleep, which is a problem. Also since the motioncontroller and the update frequency haven't been updated, the behaviour isn't very consistent, or maybe that's just how it is. |
This would work, We could also just disable the ability to exit on swipe. I'm fine with either. I wonder why LVGL is getting a swipe action while the arc is held however. I feel like that should be getting ignored anyway....
This will be a differing of opinions then. I agree a to low value is bad, but the shake is user dependent due to how they flick/shake their wrist. So having a calibration lets it get set to what they are comfortable with.
I'm not sure "Shake!!" means aggressive but I can see the interpretation. Is there a better wording for that button?
The tap on the screen is the touch screen fubaring the i2c bus causing the accelerometer to get bad data. It is a bug with the i2c driver not the actual shake handler. |
Which point do you disagree with? Increasing the threshold more than required to not have accidental wakeups isn't beneficial even if the user prefers a higher threshold. Then on the low end, is there a range of values where using shake wake with a low threshold is reasonably a better choice than just using raise wake? If the answer ends up being no, then raise wake might as well replace the low threshold range of shakewake, and I think we can find a sweetspot for the threshold without requiring calibration from everyone.
|
|
I don't think raise wake handles flick - they are 2 different features, shake wake is a configurable threshold where the users says 'i want the watch to wake when i make this movement' and raise wake is the watch going 'i think the user might be looking at me, better turn on'. If raise wake worked better maybe shake wake wouldn't be needed but as it stands I like having both available. |
|
I was mainly talking about the calibration.
I flick it if it isn't already on and it wakes up. What I said about responding to shake or flick isn't relevant after all, but being just above the threshold of accidental wakeups is instead.
This is from the watches perspective. From the users perspective, raise wrist is an effortless/low effort way to interact with the watch, and I see shake wake as a "higher" effort method to reduce screen on time. The user doesn't care about the movement, but accidental wakeups vs convenience. As I have already stated, "The threshold should be just high enough that it doesn't activate accidentally." There will always be cases where it will activate accidentally, but let's stay within reason. Any higher than that is unnecessary as the issue is already fixed. If the user wants more convenience and doesn't mind potentially more accidental wakeups, they should use raise wrist. This way the calibration becomes unnecessary. |
|
Note for folks trying to get this working: you'll need to go to "Settings -> Wake Up" and select "Shake Wake", in addition to doing the calibration. |
Adds shake to wake by calculating Y/Z acceleration. Essentially a gentle wrist flick action will turn the screen on.
I think its set to be a hair over sensitive as of the moment. Looking for some feedback there. In the long run I want to break out a settings option to select how sensitive it is, but not there yet.
.