afarhan / sbitx

168 stars 58 forks source link

User settings written to disk very, very often #74

Closed n1ai closed 8 months ago

n1ai commented 9 months ago

https://github.com/afarhan/sbitx/blob/7abd77da8b45138bad5900b0b33563ba25013d57/sbitx_gtk.c#L1291

The code that refers to last_save_at is:

image

Yet the code never sets it after it does update the file.

This means we write the user settings to disk very, very often, like every time you move the tuning or volume knob.

Adding a line of code to set the variable to now at the end of the function is a fix:

image

Yet maybe only writing once every 30 seconds is not often enough?

RAndrewThomas commented 9 months ago

I like your bugfix and have incorporated in my own fork already. My opinion on the auto-save interval: 30 seconds is fine or even faster than necessary. Could even make the user settings auto-save interval a user setting ;) With reasonable bounds enforced.

With a save on normal exit too, I think most folks would understand that after a crash all bets are off.

It might be nice to catch SIGTERM and save user settings on the way out (after CTRL-C, etc), but that's probably unnecessarily fancy.

-n5rgn

n1ai commented 9 months ago

I like your bugfix and have incorporated in my own fork already. My opinion on the auto-save interval: 30 seconds is fine or even faster than necessary. Could even make the user settings auto-save interval a user setting ;) With reasonable bounds enforced.

With a save on normal exit too, I think most folks would understand that after a crash all bets are off.

It might be nice to catch SIGTERM and save user settings on the way out (after CTRL-C, etc), but that's probably unnecessarily fancy.

-n5rgn

I'm glad you like the bug fix.

https://github.com/afarhan/sbitx/pull/75 also has the fix, in a pull request ready to go.

It's an improvement on what is in the code code, but overall I think perhaps the best strategy is to save on band switch (since that's when the band stack gets updated) and save on exit.

I'll leave this issue opened till someone gets a chance to do something better.

afarhan commented 8 months ago

this was fixed by v3.02