Skip to content

Conversation

@geekbozu
Copy link
Member

@geekbozu geekbozu commented Sep 27, 2021

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.

.

@geekbozu geekbozu changed the title Shake wake Shake wake [Not ready for merge] Sep 27, 2021
@Riksu9000
Copy link
Contributor

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.

@hubmartin
Copy link
Contributor

hubmartin commented Sep 27, 2021

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 :)
Thanks for details.

@GitZero1
Copy link

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!

@geekbozu
Copy link
Member Author

geekbozu commented Sep 28, 2021

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 think both should be done,
Shake to wake is meant to be an "aggressive wrist flick" action to make it turn on. EG only when desired. Where "automatic" I feel will always have to be a hair to sensitive and try to get the screen on when ever it might be possible that the user is looking. Which the current algo I think does a decent job at already!

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!

Matches what my testing showed! More tweaking!

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 :)
Thanks for details.

Yeah my thoughts as well, and this is going to take some iterations to make happy that is for sure.
The thresholds are poorly defined as of the moment. They are really just there so I can test a few values at a time.
The actual shake motion is still being tweaked as well. The current commit only accounts for Z and Y axis, which our natural wrist movement needs a smidge of X included in that or it loses some of the natural feel. Working on adding that as I type! Thanks for testing!!

@geekbozu
Copy link
Member Author

geekbozu commented Sep 28, 2021

Added a much nicer way of setting the threshold
Heavily tweaked the "algorithm" its closer to a generic shake algorithm now
Setting the threshold is based on user input or a slider

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

@Riksu9000
Copy link
Contributor

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 BMA4_DATA_RDY_INT interrupt in the motion controller to read the values, but couldn't get it to work. There's also BMA4_ACCEL_DATA_RDY_INT. I might have to try this again.

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.

@geekbozu
Copy link
Member Author

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?

For me personally no, Id much rather have an explicit turn on then something aggressive, Joy of the OSS here we can have both!

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.

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.

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?

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 once tried to enable the BMA4_DATA_RDY_INT interrupt in the motion controller to read the values, but couldn't get it to work. There's also BMA4_ACCEL_DATA_RDY_INT. I might have to try this again.
I think we currently just use polling mode as we do not always want the acell data? Will peek at my self as well! This works by leveraging that the motion controller is getting update in the system task when ever a Acellerometer mode is enabled.

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.

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!

@Riksu9000
Copy link
Contributor

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.

The sensor is set to 100Hz, but SystemTask has a delay of 100 ticks in xQueueReceive(), so UpdateMotion() gets run at just ~10Hz or more when receiving messages. Making use of the interrupts would decouple it from the SystemTask frequency.

I think we currently just use polling mode as we do not always want the acell data?

We can just disable the interrupt or ignore it if we don't want the data, but the sensor is always updating it anyway.

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.

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!

If UpdateMotion() is run more often, then ShouldShakeWake() also gets run more often. My concern was that it would accumulate speed faster, but it uses xTaskGetTickCount(), so maybe that won't happen in this case? Maybe this isn't an issue right now, but someone might create an algorithm that relies on the frequency at which it is called.

@geekbozu
Copy link
Member Author

geekbozu commented Sep 28, 2021

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.

The sensor is set to 100Hz, but SystemTask has a delay of 100 ticks in xQueueReceive(), so UpdateMotion() gets run at just ~10Hz or more when receiving messages. Making use of the interrupts would decouple it from the SystemTask frequency.

... Yeah math is hard...I moved a zero... I agree its only being called at 10hz.

I think we currently just use polling mode as we do not always want the acell data?

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!

We can just disable the interrupt or ignore it if we don't want the data, but the sensor is always updating it anyway.

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.

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!

If UpdateMotion() is run more often, then ShouldShakeWake() also gets run more often. My concern was that it would accumulate speed faster, but it uses xTaskGetTickCount(), so maybe that won't happen in this case? Maybe this isn't an issue right now, but someone might create an algorithm that relies on the frequency at which it is called.

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 ;)

@geekbozu geekbozu changed the title Shake wake [Not ready for merge] Shake wake [WIP] Sep 28, 2021
@Riksu9000
Copy link
Contributor

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.

@geekbozu
Copy link
Member Author

geekbozu commented Sep 28, 2021

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?

@dyamon dyamon mentioned this pull request Sep 28, 2021
6 tasks
@geekbozu
Copy link
Member Author

@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!

@geekbozu
Copy link
Member Author

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

@geekbozu geekbozu changed the title Shake wake [WIP] Shake wake [Uesr Feedback Required] Sep 30, 2021
@geekbozu geekbozu changed the title Shake wake [Uesr Feedback Required] Shake wake [Ussr Feedback Required] Sep 30, 2021
@geekbozu geekbozu changed the title Shake wake [Ussr Feedback Required] Shake wake [User Feedback Required] Sep 30, 2021
@JF002
Copy link
Collaborator

JF002 commented Sep 30, 2021

@JF002 any thoughts here?

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?
The current wrist raise algo is really simple and generates a lot of false positive. It could probably be improved and fine-tuned to avoid those unexpected wake-ups.
If I understand correctly, this new algo detect a shake motion, which might be a bit different than just raising the wrist to look at the watch. Both algo might also merge at some points, but I think it could be a good idea to keep both in parallel to see how they'll evolve. Let's just try to keep the settings simple ;)

IRQ vs current task vs a new task
As I said, the current algo is really simple and does not need a lot of accuracy regarding the timings. The driver for the BMA42x is also very basic and only allows simple polling method.
For this new algo, we might need more data throughput and more timing accuracy.
In my opinion, creating a new task should be the last resort, as it's the most resource intensive : it uses more RAM (for the task stack) and more CPU time (the scheduler has to switch from one task to another). So, before creating a new task, I would try to implement a more advanced driver that uses the FIFO from the sensor, the IRQ, the alarm functionality (I think the sensor can trig an IRQ when it detects a motion greater than a specified value),... Using DMA for the I²C bus could also be a good idea. A part of the signal processing could also be done in the IRQ handler, which would wake up a task only when necessary.
In the end, the important points are the memory footprint (especially RAM memory) and the CPU time, which is directly linked to the power consumption and battery life ;)
All of this is very theoretical as I haven't had a closer look to the PR yet. And of course, we don't need to implement all these things at once, as we can always release a first working version of the algo and these improve it later on.

@geekbozu
Copy link
Member Author

@JF002 any thoughts here?

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? The current wrist raise algo is really simple and generates a lot of false positive. It could probably be improved and fine-tuned to avoid those unexpected wake-ups. If I understand correctly, this new algo detect a shake motion, which might be a bit different than just raising the wrist to look at the watch. Both algo might also merge at some points, but I think it could be a good idea to keep both in parallel to see how they'll evolve. Let's just try to keep the settings simple ;)

IRQ vs current task vs a new task As I said, the current algo is really simple and does not need a lot of accuracy regarding the timings. The driver for the BMA42x is also very basic and only allows simple polling method. For this new algo, we might need more data throughput and more timing accuracy. In my opinion, creating a new task should be the last resort, as it's the most resource intensive : it uses more RAM (for the task stack) and more CPU time (the scheduler has to switch from one task to another). So, before creating a new task, I would try to implement a more advanced driver that uses the FIFO from the sensor, the IRQ, the alarm functionality (I think the sensor can trig an IRQ when it detects a motion greater than a specified value),... Using DMA for the I²C bus could also be a good idea. A part of the signal processing could also be done in the IRQ handler, which would wake up a task only when necessary. In the end, the important points are the memory footprint (especially RAM memory) and the CPU time, which is directly linked to the power consumption and battery life ;) All of this is very theoretical as I haven't had a closer look to the PR yet. And of course, we don't need to implement all these things at once, as we can always release a first working version of the algo and these improve it later on.

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.
All of this discussion is related to a not yet made PR, This shake to wake implementation works with the current base as is.

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!

@geekbozu geekbozu marked this pull request as draft October 2, 2021 03:52
@hubmartin
Copy link
Contributor

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

I just flashed it and I like the configuration. Calibration is awesome. It works quite reliable on that "not always 10 Hz" pace.
The calibration duration could be longer. First time I used it I did not noticed that "Shake!!!" text and when I did, it was already after calibration. I did not studied the code so just suggestion: The button "Shake!!!" could change colour, or you can show some countdown to the calibration start. But it's up to you. Great work 👍

@geekbozu
Copy link
Member Author

geekbozu commented Oct 3, 2021

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 :)

@geekbozu
Copy link
Member Author

geekbozu commented Oct 3, 2021

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

I just flashed it and I like the configuration. Calibration is awesome. It works quite reliable on that "not always 10 Hz" pace. The calibration duration could be longer. First time I used it I did not noticed that "Shake!!!" text and when I did, it was already after calibration. I did not studied the code so just suggestion: The button "Shake!!!" could change colour, or you can show some countdown to the calibration start. But it's up to you. Great work 👍

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 also made the button toggle properly not just the text. Give it ago :D (Also rebased on develop so all the features :D )

@darkdragon-001
Copy link

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?

@geekbozu
Copy link
Member Author

geekbozu commented Nov 13, 2021

I think this is finally ready for review...if you see anything let me know!

FintasticMan added a commit to FintasticMan/InfiniTime that referenced this pull request Dec 13, 2021
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
FintasticMan added a commit to FintasticMan/InfiniTime that referenced this pull request Dec 13, 2021
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
@JF002 JF002 added this to the 1.8.0 milestone Jan 3, 2022
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.
Added button toggle state for cleaner user interaction
Added delay to starting calibration
@JF002 JF002 merged commit bef3e70 into InfiniTimeOrg:develop Jan 4, 2022
@Riksu9000
Copy link
Contributor

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?

@JF002 JF002 changed the title Shake wake [User Feedback Required] Shake wake Jan 5, 2022
@geekbozu
Copy link
Member Author

geekbozu commented Jan 5, 2022

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.

@Riksu9000
Copy link
Contributor

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.

@geekbozu
Copy link
Member Author

geekbozu commented Jan 5, 2022

The exiting can be fixed in a slightly hacky way by disabling exiting while the arc is held.

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....

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.

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.

The calibration hints at shaking aggressively, which probably sets the threshold higher than expected.

I'm not sure "Shake!!" means aggressive but I can see the interpretation. Is there a better wording for that button?

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.

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.

@Riksu9000
Copy link
Contributor

Riksu9000 commented Jan 5, 2022

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.

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.

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.

The way I see it is that since flick is already handled by raise wake, shake wake doesn't need to respond to flick, only shake.

@kieranc
Copy link
Contributor

kieranc commented Jan 7, 2022

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 think the 2nd exclamation mark on 'Shake!!' can probably be removed, and avoiding going back when moving the slider would be nice, but it's also a fairly solid interface which (to me) is self explanatory, you can see how far a certain shake moves the value and set an appropriate threshold.
A simple on/off 'wake the watch when i want to look at it' would be great but i don't think it's possible right now, so until it is, a 2nd configurable option is useful IMO.

@Riksu9000
Copy link
Contributor

I was mainly talking about the calibration.

I don't think raise wake handles flick

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.

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'

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.

@tomfitzhenry
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants