WIP: Keep steps through reboots.

[kalaspuffar]

Hi Dev Team.

Thank you for a fun and great project. I love to play around with it, and it's a solid system to run on my clock.

In this PR, I've tried to fix the issue with the step counter. Currently, we save the information inside the counter unit, which is reset if the clock ever reboots. Sadly the state of the current build is that you lose connection to the companion app and need to restart the clock to re-establish that connection.

So I wanted to persist the step count, and as we newly added a little file system, I thought some files with dates would solve this issue.

The code below creates a new file each day and saves the number of steps taken, and reset the step counter for each update. By doing this, we will remove the requirement to have an accurate count in the unit and keep the data preserved over boots.

I did not find a way to set the number to the unit or anything like that, so saving the value before reboots and restoring after seems not feasible at the moment.

This code might not be perfect as I've not written much C the last 20 years, but I've run the patch for more than 48 hours, and it seems to work.

As a side note, I want to assure you that I don't have any problems with critic and discussions, so if you have any suggestions or other comments, I'm here to learn and help out.

Considerations:

• Is this the best way to store data if we want to export this information from the device.
• How often do we need to remove files so we don't fill upp the memory?
• How many days should be kept on device?

Thanks again, and keep up the great work.

Best regards
Daniel

[JF002]

Thanks for this PR, @kalaspuffar ! This is a nice use-case for our brand new file system 👍 !

The code below creates a new file each day and saves the number of steps taken

I'm wondering : is it more effective to create a new file every day, or create a single file and append/update new values everyday?

You changes certainly work, but, from what I understand, the file is read/written every time UpdateMotion() is called, which happens every 100ms. This might not be very efficient for the battery life and also for the flash memory lifespan. We'll probably need to find when is the best time to write new values to the file (when the acquired value changes, once every hour, once every day,..?).

Your questions are also valid, and I don't have the answer for them : how much data do we keep in memory? When will InfiniTime decide to erase older values?

This is a really interesting PR that will allow us to think about the data storage in InfiniTime !

By the way, I though about another way to keep the step value after a reset : the step count is acquired a memorized in the motion sensor. This values is automatically incremented by the motion sensor each time it detects a new step. It's only reset when we explicitly request it by calling Bma421::ResetStepCounter(), or when the device is initialized (in Bma421::Init()). In theory, it should be possible to detect that the motion sensor has already been initialized and to skip the initialization if it's the case. That way, the step value won't be cleared on reset.
Anyway, storing the historical values in the memory will be interesting for companion app (see this project.

[hatmajster]

Wouldn't it be safer to just have a reboot option in settings to do things gracefully - persist time, date, steps, etc? That way nothing needs to be saved all the time, no waste of energy.
Or maybe its even possible to somehow attach this to the reset through long button press...

[Riksu9000]

Wouldn't it be safer to just have a reboot option in settings to do things gracefully - persist time, date, steps, etc? That way nothing needs to be saved all the time, no waste of energy.

There's no reason to reboot the watch under normal circumstances, so having a graceful reboot option is like admitting defeat, though maybe we need it just temporarily.

Or maybe its even possible to somehow attach this to the reset through long button press...

This gives me an idea. What if there were multiple levels of long pressing? I'll see if I can implement a "longer press" on top of the "long press" in #480 that could open a new screen with a reboot option.

[kalaspuffar]

Hi @JF002

I'm wondering : is it more effective to create a new file every day, or create a single file and append/update new values everyday?

I've been thinking about different solutions, and you are right that writing to a single file would make it easier to look at the drive and read a single file. But you add complexity when it comes to export, appending values, and also removal of values. My main focus in life is to keep it simple and make things you often do easy, and the things you seldomly do might be a bit more challenging. Writing one file per day will help us with the reset that should happen each day anyway, and it also makes storing the value easier.

Removing files could be a bit more challenging, and if you viewed the filesystem, you would have many files. Then again, I don't see a use case for a file explorer on a clock, so the filesystem is more of a key-value store in my mind.

You changes certainly work, but, from what I understand, the file is read/written every time UpdateMotion() is called, which happens every 100ms. This might not be very efficient for the battery life and also for the flash memory lifespan. We'll probably need to find when is the best time to write new values to the file (when the acquired value changes, once every hour, once every day,..?).

Update every hour or once a day doesn't work if you don't fix the issue requiring reboots or other unforeseen problems. What happens if you run out of power and so on. I've updated this PR, so we now only update the file every 10 seconds with is a bit more reasonable. I found that the power consumption on the watch was quite heavy, with the older solution 30% on one day. Now we are back to almost the same power consumption as before the change. And we can change it to do updates every minute or what we like pretty easily.

By the way, I though about another way to keep the step value after a reset : the step count is acquired a memorized in the motion sensor. This values is automatically incremented by the motion sensor each time it detects a new step. It's only reset when we explicitly request it by calling Bma421::ResetStepCounter(), or when the device is initialized (in Bma421::Init()). In theory, it should be possible to detect that the motion sensor has already been initialized and to skip the initialization if it's the case. That way, the step value won't be cleared on reset.

This is an exciting note, something we should look into if we could change, then the update frequency could be reduced even further.

Thank you for your input.

Best regards
Daniel

[kalaspuffar]

Hi @hatmajster

Wouldn't it be safer to just have a reboot option in settings to do things gracefully - persist time, date, steps, etc? That way nothing needs to be saved all the time, no waste of energy.
Or maybe its even possible to somehow attach this to the reset through long button press...

Great suggestions, I don't know enough about the platform to answer any of them, but we should pursue these options. Having something that stores the value regularly could be good anyway to ensure that the value is persisted. But what frequency and the solution could be up to debate depending on what is the most power and storage efficient.

Best regards
Daniel

[kalaspuffar]

Hi @Riksu9000

This gives me an idea. What if there were multiple levels of long pressing? I'll see if I can implement a "longer press" on top of the "long press" in #480 that could open a new screen with a reboot option.

That sounds like an exciting suggestion; we need to ensure that the long press never gets overruled as I see that as a last resort for when things go wrong, you only have one button, and if that stops working, your device is bricked.

Best regards
Daniel

[Itai-Nelken]

maybe the value could stop being written if the step count isn't incrementing?
that way battery is saved when you aren't walking and there is no need to save the same value over and over.

[kalaspuffar]

Hi @Itai-Nelken

maybe the value could stop being written if the step count isn't incrementing?
that way battery is saved when you aren't walking and there is no need to save the same value over and over.

Thank you, a great suggestion and easy enough to implement. I have updated the code, built a version that I took for a walk today. I passed my 10k goal, so it seems to work just fine :)

Best regards
Daniel

[jonvmey]

Why construct a std::string from buff, to then only ever call c_str() on it? That just incurs an extra allocation for no benefit.

[kalaspuffar]

Hi @jonvmey

Thank you for helping me. As i mentioned I've not written C code in 20 years or so. Usually do java code. Had a har time to get my sprintf string usable in the function.

If you can show me how it's done I will learn something. I got the suggestion for this solution from stack overflow.

Best regards
Daniel

[hatmajster]

Just don't do fileName.c_str() use buff instead (maybe change name). What is going on here is wrapping one container (char[]) into second (std::string), and then to third (char[] same as at the beginning). That's so unnecessary, takes a lot of memory for nothing

[kalaspuffar]

Hi @hatmajster

Thank you for the explanation, I thought it was just one extra array, but two extra arrays are wrong. Also, it did not answer my question on how to do it as I'm not familiar with the language. Doing it in java, you just cast, but here you need to get the pointers correctly handled.

First, I tested to use the variable, which did not work because it could not cast the value correctly.

 char fileName[40];
 snprintf(fileName, sizeof(fileName), "/steps_%.4d%.2d%.2d.dat", dateTimeController.Year(), static_cast<uint8_t>(dateTimeController.Month()), dateTimeController.Day());
 if ( fs.FileOpen(&stepsFile, fileName, LFS_O_RDWR | LFS_O_CREAT) != LFS_ERR_OK) {

Then I tested to cast it and use the pointer, but I must have done it multiple times because it put my clock in a mode where I could not update, probably writing many small files with number names.

 if ( fs.FileOpen(&stepsFile, (const char*) &fileName, LFS_O_RDWR | LFS_O_CREAT) != LFS_ERR_OK) {

Currently testing casting without fetching the pointer, and that seems to work.

 if ( fs.FileOpen(&stepsFile, (const char*) fileName, LFS_O_RDWR | LFS_O_CREAT) != LFS_ERR_OK) {

If anyone could give me some clarity here on what is the correct solution, I'm grateful.

best regards
Daniel

[jonvmey]

The first version of the code you put should have worked fine without any additional casting. What error did you get?

[kalaspuffar]

Hi @jonvmey

You are right; I must not have tested it. I was testing using &filename as the variable. Without the reference mark & it compiles without any issues. Thank you for helping. Have updated the PR.

Best regards
Daniel

[jonvmey]

You should use the standard defined type [std::]uint8_t in the cast rather than u_int8_t.

[kalaspuffar]

Hi @jonvmey

That code was copied from the settings.cpp file where we use the filesystem. I'll test to change this and see if it works later.

Best regards
Daniel

[kalaspuffar]

Hi Everyone.

The last commit made by instruction from @jonvmey and @hatmajster is not working.

I'm not sure why because my C++ knowledge is limited.

If I install the build of the last commit, I can't update the device again. All updates will end with the message "Error."

As I have a sealed device, I can't debug it more than that. On the other hand, if I build with the commit before the last one, I can install it as many times as I like.

Could someone with more knowledge or an unsealed device debug this and hint at what is going wrong?

Best regards
Daniel

[hatmajster]

Please, if You want help then describe as much as You can what You are using/doing and maybe someone will help. "Not working" usually means that 9999 things could go wrong. If You really can't fix it, go on chat and try to find solution there. Pinetime as most embedded devices is not compiling anything, You can literally flash just 0xFFFFFFFFFFFF.... to the device and it will take it. Most likely your flashing/building/etc process has some mistake .

Also, if You have no experience in c++ and embedded then its probably not a good idea to develop with sealed device. I would advice to maybe try wasp-os, and script using python there, I THINK just scripting shouldn't break anything.

I've successfully built Your code using docker on linux and flashed it onto my devkit using OTA, so Your problem is not code related. What i've noticed: setting steps doesn't work after time synchro immediately, and when it later does I see that I've walked 4294966863 steps today. Something is wrong.

[kalaspuffar]

Please, if You want help, then describe as much as You can what You are using/doing and maybe someone will help. "Not working" usually means that 9999 things could go wrong. If You really can't fix it, go on chat and try to find solution there. Pinetime as most embedded devices is not compiling anything, You can literally flash just 0xFFFFFFFFFFFF.... to the device and it will take it. Most likely your flashing/building/etc process has some mistake .

Also, if You have no experience in c++ and embedded then its probably not a good idea to develop with sealed device. I would advice to maybe try wasp-os, and script using python there, I THINK just scripting shouldn't break anything.

I've successfully built Your code using docker on linux and flashed it onto my devkit using OTA, so Your problem is not code related. What i've noticed: setting steps doesn't work after time synchro immediately, and when it later does I see that I've walked 4294966863 steps today. Something is wrong.

Hi @hatmajster

Thank you for verifying that the last commit isn't working. That means that the updates done by the latest review are incorrect and that my first implementation is more correct.

Best regards
Daniel

[hatmajster]

Erm, no, sorry if it wasn't clear. I build the newest changes with this hash: 319e3a2
Again, the code is fine and should flash, I did it.

EDIT: To be more clear, I've found some problems with the code like high step count. But the code itself correctly built and flashed and I was running it on my devkit pinetime

[kalaspuffar]

Hi @hatmajster

Yes, I understood what you wrote. Unfortunately, the code did not work as implemented. You were able to flash it as I was. I couldn't reflash the device after I had flashed that version, so something is wrong with the filename change as the rest of the changes seem to be OK.

As the old commit 9677a5a works just fine and the new commit 319e3a2 don't work, then the commit seem to be the culprit. Still, I can't figure out what is going wrong with the implementation suggested to me.

We have a data object that we send in as a pointer to a function which seems strange, but then again, I might have missed something. As other things I've tested seem not to work, I asked for help.

Thank you for your time.

Best regards
Daniel

[geekbozu]

Maybe a better place to handle saving steps would be in SRAM in a noinit section, I just opened a PR to work on getting us support for a noinit segment for Clock, Steps would be a good candidate of something else to store here!
#595

Then steps could be flushed to FLASH for logging only, Should be better on battery life and wear.

[schneidr]

You changes certainly work, but, from what I understand, the file is read/written every time UpdateMotion() is called, which happens every 100ms. This might not be very efficient for the battery life and also for the flash memory lifespan.

I was about to implement this myself when I found this PR. I also thought a little about the amout of write operations. I came to the conclusion that personally I wouldn't mind to lose some steps over the day, so I planned to store the amount every 100 steps or maybe at regular intervals (hourly?).

If I'm at, let's say, 5367 steps when I have to reboot the watch I wouldn't mind if it started with 5300 for the rest of the day. That would be way better than starting with zero.

[Riksu9000]

With new releases of InfiniTime, there's not much need to restart the watch, so this is might not be an issue anymore.

[Riksu9000]

Closing this in favor of a solution that uses noinit or doesn't reset the motioncontroller, if this is worth fixing, or a solution that has been designed with logging in mind.