GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Implement Lower to Sleep functionality by FintasticMan · Pull Request #827 · InfiniTimeOrg/InfiniTime

Conversation

@FintasticMan

@FintasticMan FintasticMan commented Nov 12, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

Closes #713.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

WowieMan, KoalaV2, scuba10steve, atomdmac, mario-ebenhofer, darkdragon-001, and tituscmd reacted with thumbs up emoji
All reactions
  • 👍 7 reactions
@geekbozu geekbozu added the enhancement Enhancement to an existing app/feature label Nov 13, 2021
Riksu9000
Riksu9000 reviewed Nov 13, 2021
View reviewed changes

@Riksu9000 Riksu9000 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/components/motion/MotionController.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/displayapp/screens/settings/SettingWakeUp.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

@Riksu9000

Riksu9000 commented Nov 14, 2021

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan

FintasticMan commented Nov 15, 2021

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Riksu9000

Riksu9000 commented Nov 15, 2021

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan

FintasticMan commented Nov 15, 2021

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@geekbozu

geekbozu commented Dec 11, 2021

Copy link
Copy Markdown
Member

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan

FintasticMan commented Dec 13, 2021

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from ab86ab6 to fab8269 Compare December 13, 2021 08:33
@FintasticMan

FintasticMan commented Dec 13, 2021

Copy link
Copy Markdown
Member Author

Rebased onto latest develop.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

FintasticMan added a commit to FintasticMan/InfiniTime that referenced this pull request Dec 13, 2021
@FintasticMan
PR InfiniTimeOrg#827: Implement Lower to Sleep functionality
53788ed
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
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from 7d020c7 to d5abe3b Compare January 7, 2022 21:09
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from 6e54344 to be855a7 Compare April 25, 2022 15:54
@FintasticMan
FintasticMan requested a review from Riksu9000 April 27, 2022 23:46
Riksu9000
Riksu9000 reviewed Apr 28, 2022
View reviewed changes

@Riksu9000 Riksu9000 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for 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.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@FintasticMan

FintasticMan commented Apr 28, 2022

Copy link
Copy Markdown
Member Author

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Riksu9000
Riksu9000 self-requested a review April 28, 2022 16:45
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from be855a7 to a689a09 Compare May 8, 2022 14:54
@FintasticMan

FintasticMan commented May 8, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Riksu9000

Riksu9000 commented May 8, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from a689a09 to 2feb17c Compare May 8, 2022 21:26
@FintasticMan

FintasticMan commented May 8, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Riksu9000

Riksu9000 commented May 9, 2022

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan

FintasticMan commented May 9, 2022

Copy link
Copy Markdown
Member Author

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Riksu9000

Riksu9000 commented May 9, 2022

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Riksu9000
Riksu9000 removed their request for review June 12, 2022 16:10
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from b32a22e to adc49c7 Compare August 28, 2022 11:44
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from adc49c7 to 398136d Compare October 17, 2022 16:37
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from 398136d to 050d9c3 Compare December 27, 2022 20:12
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from 050d9c3 to 7469157 Compare January 7, 2023 20:01
@github-actions

github-actions Bot commented Feb 1, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Build size and comparison to main:

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from b76fc85 to 1c19392 Compare April 1, 2023 23:23
mark9064 added a commit to mark9064/InfiniTime that referenced this pull request May 8, 2023
@mark9064
Implement Lower to Sleep functionality InfiniTimeOrg#827
2b38146
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
mark9064 added a commit to mark9064/InfiniTime that referenced this pull request May 27, 2023
@mark9064
Implement Lower to Sleep functionality InfiniTimeOrg#827
c8155fd
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
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from 1c19392 to 27f6fd8 Compare June 18, 2023 11:38
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch 2 times, most recently from 2bb9a12 to 81c5368 Compare June 25, 2023 14:18
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from 81c5368 to d71f861 Compare August 20, 2023 16:43
@FintasticMan FintasticMan added new feature This thread is about a new feature and removed enhancement Enhancement to an existing app/feature labels Aug 22, 2023
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from d71f861 to f119a5d Compare August 22, 2023 10:28
@FintasticMan
lowersleep: Implement Lower to Sleep functionality
dca17ce
@FintasticMan
FintasticMan force-pushed the LowerToSleep branch from f119a5d to dca17ce Compare August 22, 2023 13:04
@FintasticMan
FintasticMan requested a review from a team August 24, 2023 09:27
@FintasticMan

FintasticMan commented Aug 24, 2023

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan
lowersleep: Bump settings version
ef2bcca
JF002
JF002 approved these changes Aug 27, 2023
View reviewed changes

@JF002 JF002 left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@JF002 JF002 added this to the 1.14.0 milestone Aug 27, 2023
@FintasticMan
FintasticMan merged commit 0f9f606 into InfiniTimeOrg:main Aug 27, 2023
@FintasticMan
FintasticMan deleted the LowerToSleep branch August 27, 2023 16:15
@FintasticMan FintasticMan mentioned this pull request Aug 30, 2023
lowersleep: Fix InfiniSim build InfiniTimeOrg/InfiniSim#118
Merged
Zetabite pushed a commit to Zetabite/InfiniTime that referenced this pull request Nov 12, 2023
@FintasticMan @Zetabite
lowersleep: Implement Lower to Sleep functionality (InfiniTimeOrg#827)
ad49721
@darkdragon-001

darkdragon-001 commented Jan 8, 2024

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@FintasticMan

FintasticMan commented Jan 8, 2024

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@darkdragon-001

darkdragon-001 commented Jan 8, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

FintasticMan added a commit to FintasticMan/InfiniTime that referenced this pull request Feb 12, 2024
@FintasticMan
lowersleep: Improve algorithm by checking wrist angle
c3c4ef5
Inspired by InfiniTimeOrg#827 (comment)
FintasticMan added a commit to FintasticMan/InfiniTime that referenced this pull request Feb 12, 2024
@FintasticMan
lowersleep: Improve algorithm by checking wrist angle
684f28b
Inspired by InfiniTimeOrg#827 (comment).
@FintasticMan FintasticMan mentioned this pull request Feb 12, 2024
lowersleep: Improve algorithm by checking wrist angle #2014
Merged
FintasticMan added a commit to FintasticMan/InfiniTime that referenced this pull request Feb 12, 2024
@FintasticMan
lowersleep: Improve algorithm by checking wrist angle
563e143
Inspired by InfiniTimeOrg#827 (comment).
JF002 pushed a commit that referenced this pull request Mar 12, 2024
@FintasticMan @JF002
lowersleep: Improve algorithm by checking wrist angle
f2df0c4
Inspired by #827 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@geekbozu geekbozu geekbozu left review comments
@JF002 JF002 JF002 approved these changes
+1 more reviewer
@Riksu9000 Riksu9000 Riksu9000 left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

new feature This thread is about a new feature

Projects

None yet

Milestone

1.14.0

Development

Successfully merging this pull request may close these issues.

Lock/Sleep on when lowering the hand (opposite to wake up on raise)

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@FintasticMan @Riksu9000 @geekbozu @darkdragon-001 @JF002
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.