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 — Implement Lower to Sleep functionality by FintasticMan · Pull Request #827 · InfiniTimeOrg/InfiniTime Conversation
|
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
ab86ab6 to
fab8269
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 fab8269 Author: Finlay Davidson <[email protected]> Date: Sun Nov 14 01:31:29 2021 +0100 Have isSleeping be checked by SystemTask, and fix incorrect array size commit 7d50102 Author: Finlay Davidson <[email protected]> Date: Fri Nov 12 01:48:04 2021 +0100 Add settings item commit 5269885 Author: Finlay Davidson <[email protected]> Date: Fri Nov 12 00:48:04 2021 +0100 Improve and simplify algorithm commit c925c62 Author: Finlay Davidson <[email protected]> Date: Thu Nov 11 13:42:25 2021 +0100 Initial lower to sleep implementation
7d020c7 to
d5abe3b
Compare
6e54344 to
be855a7
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@FintasticMan You're right, sorry about that. Can you see if it's working now after rebasing with the fix? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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 On another note, have you had a chance to look at the FintasticMan/LowerToSleepTest branch yet? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for the review! The reason I chose to keep the 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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
b32a22e to
adc49c7
Compare
adc49c7 to
398136d
Compare
398136d to
050d9c3
Compare
050d9c3 to
7469157
Compare
There was an error while loading. Please reload this page.
|
Build size and comparison to main:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
b76fc85 to
1c19392
Compare
commit 1c19392 Author: Finlay Davidson <[email protected]> Date: Thu Nov 11 13:42:25 2021 +0100 lowersleep: Implement Lower to Sleep functionality commit 27ae633 Author: Finlay Davidson <[email protected]> Date: Mon Mar 20 19:53:42 2023 +0100 notification: Switch to CircularBuffer for notification buffer commit 99236cb Author: Finlay Davidson <[email protected]> Date: Fri Mar 17 22:14:19 2023 +0100 circularbuffer: Add circular buffer utility struct
commit 1c19392 Author: Finlay Davidson <[email protected]> Date: Thu Nov 11 13:42:25 2021 +0100 lowersleep: Implement Lower to Sleep functionality commit 27ae633 Author: Finlay Davidson <[email protected]> Date: Mon Mar 20 19:53:42 2023 +0100 notification: Switch to CircularBuffer for notification buffer commit 99236cb Author: Finlay Davidson <[email protected]> Date: Fri Mar 17 22:14:19 2023 +0100 circularbuffer: Add circular buffer utility struct
1c19392 to
27f6fd8
Compare
2bb9a12 to
81c5368
Compare
81c5368 to
d71f861
Compare
d71f861 to
f119a5d
Compare
f119a5d to
dca17ce
Compare
|
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. |
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.
LGTM, thanks!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
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 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? |
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.
Lock/Sleep on when lowering the hand (opposite to wake up on raise)
There was an error while loading. Please reload this page.