Implement Lower to Sleep functionality

[FintasticMan]

This feature makes the device go to sleep if you lower your wrist.

Closes #713.

[Riksu9000]

Works surprisingly well. Will test this more and if it doesn't cause issues, maybe it could be integrated with raise wake?

[Riksu9000]

Sometimes the watch goes to sleep when I try to look at it. I then shake it and it turns on, but goes to sleep right away. This has happened a few times today, so I think this could be tweaked a bit.

[FintasticMan]

Do you have a way to reproduce that? I don't think I've noticed that happening, but it sounds like a threshold issue. To check you could make the values in the line setting lastYForSleep higher.

[Riksu9000]

I think what usually happens is that the watch happens to go to sleep just before I want to look at it, and then I shake it because I think it's not waking up, which actually makes it sleep again. #648 would probably help with this as it makes the watch able to wake up much faster after going to sleep, so it wouldn't cause confusion.

However now just shaking it I've gotten it to stay asleep too.

[FintasticMan]

Now you mention it, I think I have noticed something similar happening.

I think that then there isn't necessarily an issue with lower to sleep and rather raise to wake. Raise to wake must be activating sometimes if you didn't mean to activate it, and then lower to sleep turning it off again just before you do actually try to look at it.

I just updated the raise to wake values a bit and will continue testing it to see if that changes things.

[geekbozu]

how has tweaking on this been going? Do you implement a custom checkbox for it currently so @kieranc can implement it in his risky builds :)

[FintasticMan]

I've been running this myself, and it's been working exactly how I expect it to. It already has a checkbox in the Wake Up settings submenu, so I think it is ready to be put into the riskytestbuilds.

[FintasticMan]

Rebased onto latest develop.

[Riksu9000]

I tried this again and I still have similar issues with it. Mainly the watch turns off if I shake it. I think this feature would need to be "smarter", so that it doesn't sleep immediately on certain motion values, but instead actually makes sure the watch has been lowered. I think this would require more work to be done on MotionController, like storing some previous motion values.

[FintasticMan]

Did you test it with #826 and #648? Those both improve the experience a lot, so that even though it does turn off if you shake it, it turns back on again immediately. The old raise to wake algorithm has a bug that means that if you angle your hand from facing to to facing you more, it doesn't recognise it as needing to wake up.

Anyway, I have implemented a system for storing previous motion values like you suggested, but don't want to commit it directly to this PR, because I don't think it's quite right yet. The branch with those changes is FintasticMan/LowerToSleepTest. Could you maybe have a look at it?

[FintasticMan]

@Riksu9000 I think there might be an issue with the clang-format GitHub actions you made. It says there's an issue with the formatting, but it doesn't upload any patches and on my machine clang-format doesn't make any changes.

[Riksu9000]

@Riksu9000 I think there might be an issue with the clang-format GitHub actions you made. It says there's an issue with the formatting, but it doesn't upload any patches and on my machine clang-format doesn't make any changes.

@FintasticMan You're right, sorry about that. Can you see if it's working now after rebasing with the fix?

[FintasticMan]

@Riksu9000 It does seem to be working, but the issues it found aren't with my changes in particular. If that's how it's supposed to work, that's fine. I do think it is possible to tell clang-format to only format changed lines of code using the --lines=<string> command-line option.

On another note, have you had a chance to look at the FintasticMan/LowerToSleepTest branch yet?

[Riksu9000]

Yes, that's expected. It's a much simpler implementation to just check the entire file. Soon when all the code is correctly formatted, this will no longer be a concern. For now please format the entire file.

I took a look at the branch. The circular buffer is good. I think you can remove the separate y variable and have just a y array which includes the current and previous values. We have exceptions disabled, so I'm not sure using the at function is any different than square brackets. Also something to note, the behaviour may be dependent on the rate at which the values get updated, which may change at some point. If you believe this algorithm will work better, feel free to push it here so we can test it.

I haven't tested these changes yet, since I expect that changes to these motion detection algorithms will take some time to thoroughly test. I think I'll try to test #826 first, since you're saying that this PR somewhat depends on it.

[FintasticMan]

Thanks for the review! The reason I chose to keep the y variable as well as the circular buffer is because it saves a modulo operation for accessing just the last y value, like raise to wake and shake to wake do. If that isn't a concern I can just change it.

As for the polling rate, changing the polling rate would be quite an issue for this. Making it poll faster would mean that the array needs to get bigger, which could go over space constraints. A solution might be to only update the array every so many polls, and have some of the values depend on the current polling rate. Is there any function to get the current polling rate, so I could try that out?

[Riksu9000]

I don't think the modulo is a concern. It should get optimized to a simple AND with 7 instruction. The accelerometer updates the values at 100Hz as set in Bma421.cpp, but SystemTask only reads them about every 100ms. It's hard to say what exactly will be changed and where the polling rate can be read from in the future.

[FintasticMan]

This is the final version of my algorithm. I'm happy with how it works, and I've got positive feedback from people in the PineTime chat room. I think that the location for the setting is fine, but if someone has a better suggestion I'm up for changing it.

[JF002]

LGTM, thanks!

[darkdragon-001]

Thanks a lot for this great addition! I just tried it out.

It seems to be based on rotation among the arm bone axis. After I figured it out, it works reliably.
My first attempts didn't work as I lowered my arm (bone pointing towards the floor) almost without rotation around the bone axis so this did not trigger a sleep. Maybe in addition to rotational acceleration one should also take final position into account?
(All of these are just based on my observations without actually looking at the code.)

[FintasticMan]

Hi, thanks for the feedback! It is indeed the case that it is based on the rotation around your wrist. It's quite difficult to figure out if it's facing down or up without knowing if the watch is being worn on the left or right hand. I am debating adding a setting for both which arm the watch is being worn on as well as whether it's being worn on the inside or the outside of the wrist. I think that the helper functions I've made, make it simple enough to change the exact conditions for sleeping.

[darkdragon-001]

I didn't think of this. Thinking about it now, it shouldn't make a difference if being worn on the inside of outside as facing down is the same orientation. Also if ignoring the sign, it also doesn't matter if it's worn on the left of right hand. Thinking about it only quickly, I can't imagine any situation where the one doesn't want to watch to be turned off when facing sideways down. Can you?