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

feat: persistent data storage across reboots for apps #368

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

thiswillbeyourgithub
Copy link
Contributor

This creates 2 new methods for wasp.system to store and get settings. They will be stored in files in a settings folder.

I'm testing this with stopwatch (start time) and alarms.

What do you think?

Signed-off-by: thiswillbeyourgithub github@32mail.33mail.com

Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
@thiswillbeyourgithub
Copy link
Contributor Author

Btw I initially wanted to use asset statements when saving data but apparently they were working on the simulator but ignored on the pinetime.

Reverting the assert in [this commit][https://github.com/thiswillbeyourgithub/wasp-os/commit/86dc51535435351d37be5f636afeac9ff80f8867) fixed the issue.

Is it normal? Is there something I'm missing? Can we improve the simulator consistency?

Copy link
Collaborator

@daniel-thompson daniel-thompson left a comment

Choose a reason for hiding this comment

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

Overall I like this... just a few small suggestions

wasp/wasp.py Outdated Show resolved Hide resolved
wasp/wasp.py Outdated Show resolved Hide resolved
wasp/wasp.py Outdated
"title": "Storing failed",
"body": "Error when storing '{}': '{}'".format(name, err)})

def get_settings(self, name, delete=False):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding default = None might make it better for some callers (and then return default rather than None in the error paths).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually don't understand what you mean by "default" or how people could use it.

wasp/wasp.py Show resolved Hide resolved
thiswillbeyourgithub added 5 commits October 1, 2022 20:41
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>
@thiswillbeyourgithub
Copy link
Contributor Author

By the wat I made an enhancement to the alarm app to support more than 4 alarms but I'm waiting for this PR to be approved to avoid having too many incompatible PR waiting to be merged :)

thiswillbeyourgithub added 2 commits April 21, 2023 13:30
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Signed-off-by: thiswillbeyourgithub <github@32mail.33mail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants