InfiniTime icon indicating copy to clipboard operation
InfiniTime copied to clipboard

Implement Lower to Sleep functionality

Open FintasticMan opened this issue 2 years ago • 14 comments

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

Closes #713.

FintasticMan avatar Nov 12 '21 00:11 FintasticMan

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.

Riksu9000 avatar Nov 14 '21 18:11 Riksu9000

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.

FintasticMan avatar Nov 15 '21 08:11 FintasticMan

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.

Riksu9000 avatar Nov 15 '21 10:11 Riksu9000

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.

FintasticMan avatar Nov 15 '21 10:11 FintasticMan

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

geekbozu avatar Dec 11 '21 22:12 geekbozu

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 avatar Dec 13 '21 08:12 FintasticMan

Rebased onto latest develop.

FintasticMan avatar Dec 13 '21 08:12 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 avatar Apr 28 '22 13:04 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.

FintasticMan avatar May 08 '22 15:05 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.

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

Riksu9000 avatar May 08 '22 19:05 Riksu9000

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

FintasticMan avatar May 08 '22 21:05 FintasticMan

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.

Riksu9000 avatar May 09 '22 10:05 Riksu9000

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?

FintasticMan avatar May 09 '22 10:05 FintasticMan

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.

Riksu9000 avatar May 09 '22 11:05 Riksu9000

Build size and comparison to main:

Section Size Difference
text 395940B 160B
data 996B 0B
bss 63420B 0B

github-actions[bot] avatar Feb 01 '23 20:02 github-actions[bot]

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.

FintasticMan avatar Aug 24 '23 10:08 FintasticMan

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

darkdragon-001 avatar Jan 08 '24 18:01 darkdragon-001

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.

FintasticMan avatar Jan 08 '24 20:01 FintasticMan

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?

darkdragon-001 avatar Jan 08 '24 21:01 darkdragon-001