-
-
Notifications
You must be signed in to change notification settings - Fork 614
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
Persistent memory check value verification, defaulting when fails. #662
Conversation
This looks awesome! I really appreciate you taking the time to do this. I'll do some testing shortly with it. Again, thanks very much |
Pulling this into my local and building it now. |
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 have just tested and seems to be working great for me. Tested the boot config options and also tested that audio from FM radio works. (H1 clone) Approved. But lets wait for some more testers before merging
I can verify that my testing did not find any issue on both my h2+ units. |
Booting correctly here, looks fine. |
I suppose I should’ve thought a bit more about whether I consider this “feature” done before submitting the pull request. There’s a couple of nice-to-haves I didn’t put in, but probably should. One is a revision field in the data structure. So if the structure is modified, it can be incremented so that incompatible data already in the persistent RAM can be migrated or reset to default values. I could also do a compaction pass, where I collapse existing fields that are taking up more space than they need to. I also find the API around the backlight timer to be peculiar — the getter returns seconds, while the setter requires an index into an array that maps indices to seconds. There’s probably other things, too… because I can’t stop fiddling with things. :-) |
Hey if you are willing to add those nice to have features in that would be awesome! If not then there is no problem as I know you are a busy guy and already appreciate what you have done so far |
Please apply that PR as it seem to also solve the boot problem with black/white screens in latest next. |
OK, I'll stop for now. At this point, I'm making changes just to make changes, pretending C++ is Rust, and then remembering it's not. |
Alright, I'll merge it in now. Fell free to open another PR in the future if you want to add any more changes or fixes |
Of course, there's still more I'd do, but those changes are more invasive. Instead of having free functions serving as an API to persistent memory, I would hand out a pointer to a data_t, collect all those free functions into the data_t class, and let callers use that. |
…ortapack-mayhem#662) * Make default constructor for touch calibration * Add persistent memory check value and access abstraction. * Add persistent data_t default constructor with reasonable defaults. * serial_format_t default constructor. * Tidy up backlight timeout type. * Add persistent data struct version/checking. * Make range_t functions constexpr. * Move ui_config and functions into class. * Add backlight_config_t struct, separate enable and time settings.
Long-overdue enhancement: compute a check value for the contents of the 256-byte persistent RAM (RTC "REGFILE"). Check at power-on, and only use the persisted data if the check value is correct. If it is not, replace with default values and use those. Instead of directly modifying persistent memory, push changes from a local (non-persistent) RAM cache every second, along with an updated check value.
This addresses random issues users have had with brand new PortaPacks that seem dead. In my experience, the hardware is never at fault, but instead the (random) contents of persistent RAM on their HackRFs are causing the firmware to fail to boot.
A side benefit of using a cached copy of persistent settings is that the data structure members in
data_t
no longer need to be uint32_t aligned and sized. Apparently that's a requirement for RTC REGFILE access -- the Cortex-M will fault. This will allow compacting that structure somewhat, in case more information needs to be stored in persistent RAM.