GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||
GeezerVison is showing github.com — Improve Raise to Wake algorithm by FintasticMan · Pull Request #826 · InfiniTimeOrg/InfiniTime Conversation
|
|
Seems to be working well for me. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
After running it for a couple of days, I think the speed and threshold could be lowered a bit. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
8f51ef5 to
c26a3e8
Compare
|
I've been testing this a bit too, it seems like an improvement but I am still getting some false positives, usually when I'm lying down/on the sofa when my arm is close to horizontal but angled slightly up, a small movement will wake the display. It's difficult to pin down as it only seems to happen accidentally, if I try to replicate it I can't! I'll keep testing and try get more info. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
70c6c2d to
9d5ddb3
Compare
|
Rebased onto latest develop. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Squashed commit of the following: commit 9d5ddb3 Author: Finlay Davidson <[email protected]> Date: Mon Nov 15 10:01:52 2021 +0100 Tweak the raise to wake values a bit more commit 52bd0c3 Author: Finlay Davidson <[email protected]> Date: Sun Nov 14 01:39:37 2021 +0100 Adjust trigger values for Raise to Wake Also handle isSleeping in SystemTask commit d41fee0 Author: Finlay Davidson <[email protected]> Date: Fri Nov 12 00:59:03 2021 +0100 Improve and simplify Raise to Wake algorithm
69d8e03 to
cad485b
Compare
bb87989 to
168c387
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
There are two things that I'm noticing.
While the watch is already facing slightly towards me, it will wake up on tiny movements. In this case I think it's too sensitive. If the watch is positioned towards me and it's off, I think it should require a flick to turn on.
Previously the accepted range of values for X wasn't limited in the negatives. By requiring X to be larger than 384, I think it's harder to turn on when lying down. I think this also depends on handedness.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I've just updated it to use either the y or z values depending on which is more accurate. This means that it also works when the facing down and towards you. The threshold values and speed values are now also variables so it's easier to adjust them, and I've increased them. I'm aware that the code is a bit of a mess of if-statements and returns, it should be possible to make it a oneliner like my last version was. Edit: just noticed I left some define macros in that I used for testing, I'll take them out when I change the code again. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
b905d2e to
ef0a0c4
Compare
|
I don't think using Z in place of Y is correct, because the axis don't receive comparable acceleration. Y is basically detecting wrist rolling, which is what we want. Z detects moving the arm up, which can be useful data, but it's not the same as Y. I think motion algorithm changes are difficult to test, because it only depends on feeling. What should we look for when testing this? Please explain your thoughts behind the changes in detail and for each future revision, so we can get an idea of how this should work. People with two PineTimes can help compare the behaviour. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I believe that using Z is fine, because Y and Z are both the same axis, just offset by 90 degrees. This means that when Y is at (-)1024, Z is at 0, and vice versa. I've noticed that the larger the values are, the less they change with the same motion (A.K.A they are less accurate). For example, if the watch is facing up, Y is at 0 and Z is at -1024, and a slight movement will have more of an effect on Y than on Z. In the current state of this PR, you should need around the same movement speed to get it to wake up all the time. It should also work when the watch is facing down and towards you, so that should help when you're lying down. When the Pine64 EU store (finally) opens, I'll be getting another PineTime, so then I can make sure that I'm not just catering to my specific device's quirks :). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
I don't think I understand what you mean. Z is from the front of the screen to the back of the device. Y is from the top of the device to the bottom. When you tilt the device, the acceleration due to gravity will affect one of the axis more than the other, but the tilting of the wrist only affects the Y acceleration, since there's no movement along Z. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
A visual might help, but my drawing skills are not up to par, especially not with a mouse ;-). It's quite difficult to explain using words, but maybe you can try it on your own device using the motion app. If the screen is facing up, the Y value is 0 and the Z value will be shown as -64. Then, if you slowly roll the screen towards you, the Y value will drop below zero, and the Z value will increase. Once the screen is completely facing you, the Y value should show as -64 and the Z value should be 0. I personally haven't looked at the spec sheet, so I don't know what the values are supposed to represent, I just figured it out through a lot of testing. I can't see anything that the Z value could be other than roll offset by 90 degrees, but maybe I missed something. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
If there was no gravity, these would all show 0 by default. What you're showing is acceleration due to gravity. An accelerometer shows device movement and gravity combined. Please note the difference between a gyroscope and an accelerometer. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ah, I see now. I did make a mistake, I misunderstood what Z does. In the code, I do use the values of Z in a way that doesn't align with what it means, but I think that because of the X threshold it can't cause incorrect behaviour. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This change should be the final change (bar maybe some tweaking of the values if people think it's necessary). I think the is good and readable enough to be merged. I have made it so that the required speed to wake the device is slightly dependent on the current y value, because the bigger it is, the less sensitive it is. I would like some testing on it, because at the moment the values are right for me, and I would like to know if that is also the case for other people. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
There are still a lot of changes here and reading the commits individually isn't proving any simpler. Not sure why shakewake was changed in the same PR and also the same commit where a lot of other stuff was done. LVGL has cheap sine functions that are already used in the analog watchface if that would be useful. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
0b3ba3c to
5120d28
Compare
798e0ab to
7f53c60
Compare
commit 1b5c84d Author: Finlay Davidson <[email protected]> Date: Wed Apr 5 11:34:05 2023 +0200 raisewake: Add some comments to explain logic commit 7f53c60 Author: Finlay Davidson <[email protected]> Date: Wed Apr 5 10:21:42 2023 +0200 raisewake: Clean up code for better readability commit 5120d28 Author: Finlay Davidson <[email protected]> Date: Tue Apr 4 21:48:05 2023 +0200 raisewake: Improve raise to wake algorithm This new algorithm calculates the number of degrees that the wrist has rolled, and checks if that is above a threshold. First it checks if the wrist is still enough for the acceleration values to be considered mostly from gravity. It does this by calculating the mean over the past 2 SystemTask loops, and checking that the variance from that mean is below a threshold. Then it calculates the angle the wrist is being held at, and calculates the difference from the angle some time ago. If this difference is above the threshold, it wakes the watch. commit f507db2 Author: Finlay Davidson <[email protected]> Date: Mon Mar 20 19:53:42 2023 +0100 notification: Switch to CircularBuffer for notification buffer commit 853e788 Author: Finlay Davidson <[email protected]> Date: Fri Mar 17 22:14:19 2023 +0100 circularbuffer: Add circular buffer utility struct
1b5c84d to
cc9f88f
Compare
|
I think this is ready to be reviewed now. I have made some changes that mean that this has as few false positives as possible, while making sure it does wake when the user wants it to. It checks that the accelerometer values can reasonably be assumed to be from gravity, then checks that the wrist was rolled more than 45 degrees within 6 SystemTask cycles (around 600ms). To make the recovery firmware not fail when linking, I've added an ifdef to the arcsin function so it doesn't use lvgl. This is the best solution I could think of, other than creating a separate SystemTask for the recovery firmware. There are a lot of separate cases in the DegreesRolled function, but I'm quite sure that's the only way to make it work in every situation. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
commit 1b5c84d Author: Finlay Davidson <[email protected]> Date: Wed Apr 5 11:34:05 2023 +0200 raisewake: Add some comments to explain logic commit 7f53c60 Author: Finlay Davidson <[email protected]> Date: Wed Apr 5 10:21:42 2023 +0200 raisewake: Clean up code for better readability commit 5120d28 Author: Finlay Davidson <[email protected]> Date: Tue Apr 4 21:48:05 2023 +0200 raisewake: Improve raise to wake algorithm This new algorithm calculates the number of degrees that the wrist has rolled, and checks if that is above a threshold. First it checks if the wrist is still enough for the acceleration values to be considered mostly from gravity. It does this by calculating the mean over the past 2 SystemTask loops, and checking that the variance from that mean is below a threshold. Then it calculates the angle the wrist is being held at, and calculates the difference from the angle some time ago. If this difference is above the threshold, it wakes the watch. commit f507db2 Author: Finlay Davidson <[email protected]> Date: Mon Mar 20 19:53:42 2023 +0100 notification: Switch to CircularBuffer for notification buffer commit 853e788 Author: Finlay Davidson <[email protected]> Date: Fri Mar 17 22:14:19 2023 +0100 circularbuffer: Add circular buffer utility struct
cc9f88f to
82047c2
Compare
82047c2 to
2cb535f
Compare
The accumulated speed was calculated by dividing first and multiplying after, which results in more rounding errors than if you multiply first and then divide. The values aren't big enough to overflow.
Store history of acceleration values for the y and z axes.
These are functions for converting acceleration due to gravity to angles in degrees, and some statistical analysis including the mean and variance.
This new algorithm calculates the number of degrees that the wrist has rolled, and checks if that is above a threshold. First it checks if the wrist is still enough for the acceleration values to be considered mostly from gravity. It does this by calculating the mean over the past 2 SystemTask loops, and checking that the variance from that mean is below a threshold. Then it calculates the angle the wrist is being held at, and calculates the difference from the angle some time ago. If this difference is above the threshold, it wakes the watch.
2cb535f to
0d85124
Compare
|
Hey @FintasticMan ! Thanks for this PR and for taking the time to refine it over and over again :) I've just tested this PR and compared it with the current implementation. At first sight, this new implementation reject more false positives that the current one. It's also a bit slower, but the reaction time is still reasonable, in my opinion. About the code, I'm very bad at signal processing, and I can't really judge if this new algorithm "makes sense" or is "mathematically correct". It does however seem to work fine :+1! Did you have the opportunity to get feedback from other users? All in all, if this new implementation is still comfortable enough bug avoid waking up the watch when it's not necessary, it'll probably help with unwanted display and touch activation, and will also improve the battery life! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
@FintasticMan told me he uses this implementation for a few months, and also received positive feedback from a few users.
So I suggest we merge this branch, and refine the implementation later on if necessary.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Wake on wrist raise too sensitive
There was an error while loading. Please reload this page.