Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

newapp: added SleepTk app (sleep tracking and alarm clock, second PR) #416

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

thiswillbeyourgithub
Copy link
Contributor

Hi!

Sorry for the many mails but with all the recent changes to the icons and screenshots folder I couldn't seem to make the old PR work (#351) so here's a new one. I'll close the other one just after this one.

To note:
* I cherry-picked an older unmerged PR: #370 It is needed for the wasp.cancel_alarm to work regardless of time specification. I'm sure it can be made more efficient and am open to implementation tips!
* fixed a typo in the README
* tested on the simulator but not on my own watch.

  • added sleeptk
  • update to fit new naming convention
  • added screenshot
  • feat: remove any alarm given an action
  • updated icon
  • fix: typo in the README

thiswillbeyourgithub added 6 commits April 21, 2023 12:28
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
@fgaz fgaz added the new app Requests or adds a new application label Apr 21, 2023
thiswillbeyourgithub added 6 commits April 21, 2023 13:34
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
@beardeddude
Copy link
Contributor

Very cool app. It is a bit too memory heavy so I had to dedicate another pinetime just to the task and swap watches at bed time but as an open source data collection device this is amazing. Looking at the code you could probably lower the memory usage quite a bit by storing bools and small numbers using bit masking (similar to how the alarm app stores its day/enable logic). But certainly not something that has to be done now.

The UX is surprisingly good for something this complex on such a small device. And its really rich in features. I particularly like how I can mark points in the night in the logs just by pressing the button. That is super useful for lucid dream work. I also like how all the complex settings are in the code itself and that the UI just sticks to the basics.

For what my opinion is worth this app should be added to the library.

Small updates that would be cool:

  • I would love to see the press the button to mark a point in time feature in the readme
  • I would like to see the location of the stored data and file naming pattern in the readme

@thiswillbeyourgithub
Copy link
Contributor Author

thiswillbeyourgithub commented Apr 29, 2023

Hi!

Thanks a lot for the kind words. This means a lot to me as I'm just a hobbyist studying things not at all related to computer science (medschool)!

Good points regarding the README, I just added this. Thanks! (feedback welcome)

Regarding memory optimization, I'm not comfortable investigating this alone but don't hesitate to open an issue or a PR in my original repo to show me how it's done.

@beardeddude
Copy link
Contributor

Sure, I would like this app on my daily driver and I'm very comfortable with memory optimization. My life is really busy right now but in a month or 2 I would gladly send you a PR

@GaryM0101
Copy link

GaryM0101 commented Apr 29, 2023

I'm on an older version and keeping it there because it's consistently successful. One of the things I've done is disable the step counter, there are occasional collisions with each other requiring constant rebooting. Other issue is the step graphic will corrupt, you can tell by the step count being at it's max 65,000 steps. I won't create a PR due to me being on a prior release.

@beardeddude
Copy link
Contributor

I cant speak to collisions between the step counter app and this app as I have not yet seen that bug. But I have seen the 65k steps bug on my daily driver which does not have this app. So I dont believe this app is responsible for it.

@thiswillbeyourgithub
Copy link
Contributor Author

I have disabled the step counter on my watch too so I can't add anything meaningful, apart from the fact that I had random date change : the date somehow got changed to a few months in the past. I have not found a reason for this but it is worrying because I don't want this to happen in the middle of the night and impact my alarm...

To me that sounded like an overflow error somewhere and not related to SleepTk. So this could be related to the step counter displaying 65k right?

I reboot my watch every night just before starting sleeptk btw.

@beardeddude
Copy link
Contributor

I dont think any of that would be related to this app based on the code. And I have not seen the date change bug on either of my devices so I cant speak to that.

@GaryM0101
Copy link

I too reboot my watch before starting sleeptk, it guarantees and successful log and the accelerometer values are more consistent. In the morning I reboot again so I can d/l the log file. If I don't reboot in the morning sometimes I'll get taceback errors using the wasptool.

@beardeddude
Copy link
Contributor

I have had issues downloading the logs in the morning. It looks like it is just an out of memory issue. So memory optimization would fix it

@beardeddude
Copy link
Contributor

For what my opinion is worth I still think this app is useful in its current state. But if we want to wait to add it until after I have had a chance to do some memory optimization work that wouldn't be a bad idea. I can certainly get to it this summer.

@thiswillbeyourgithub
Copy link
Contributor Author

This kind of error was why i tried to deleted the instance in background to reset it. This does not seem to work and restore the memory, maybe that could be easily fixable? I would love your opinion.

@beardeddude
Copy link
Contributor

I saw that. I couldn't give you any definitive answers without sitting down and combing through it. But it looks like there is still a fair bit of data held by the app just for being loaded at all and while you are setting the vars using const the vars themselves are still in RAM. I see a lot of bools being used as separate vars and that is very memory inefficient as each bool gets stored as a full width integer and there is overhead just to having the variable name exist in an interpreted language. Additionally the garbage collector is imperfect and sometimes doesn't clean up very well.

I think that the solution here is to have one or more integer "registers" that store all this data (bools and small ints) using bitmasking/shifting in addition to cleaning up data when going into the background. That way there is a lot more margin for the garbage collector to be imperfect

@thiswillbeyourgithub
Copy link
Contributor Author

Alright, no easy fix then :) Thanks!

@beardeddude
Copy link
Contributor

I optimized this app and sent it to @thiswillbeyourgithub . I did not have time to thoroughly test. @thiswillbeyourgithub will need to test and review changes before having them reviewed here.

@thiswillbeyourgithub thiswillbeyourgithub marked this pull request as draft May 1, 2023 10:55
@GaryM0101
Copy link

GaryM0101 commented Jan 9, 2024 via email

@ajoliveau
Copy link

Thank you so much ! I deleted my comment before you had a chance to respond, because I found the answer in the meantime :)
Everything works a-ok in the simulator, can't wait to receive the Pinetime on friday and try wasp-os on the real thing !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new app Requests or adds a new application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants