fix pmem -> make backup_ram_t data members volatile #1135
Conversation
plus add calculated crc to the pmem debug screen
As this is how its refered to in the wiki and other screens
|
Hello, thanks a lot @u-foka , with your draft PR, With that PR, we are fixing that boot problem related to pmem , using gcc-arm v 10.3 , and 12.2 , very good news !!! Thanks @u-foka , I hope that this change has also no side effect to the other gcc 9.3 version (I can not test it ) . Cheers, |
Hey, here's a test build from this branch with gcc9.2 in docker https://discord.com/channels/719669764804444213/722101917135798312/1116705375975121007 |
|
Hi thanks , @u-foka So , you can finalise that draft and release that PR, very good job !!!! (In fact that PR is the first gcc-arm-none-eabi common 10.3 and 12.2 related fixes To me fully approved ! |
| @@ -381,22 +382,35 @@ DebugMenuView::DebugMenuView(NavigationView& nav) { | |||
| {"Peripherals", ui::Color::dark_cyan(), &bitmap_icon_peripherals, [&nav]() { nav.push<DebugPeripheralsMenuView>(); }}, | |||
| {"Temperature", ui::Color::dark_cyan(), &bitmap_icon_temperature, [&nav]() { nav.push<TemperatureView>(); }}, | |||
| {"Buttons Test", ui::Color::dark_cyan(), &bitmap_icon_controls, [&nav]() { nav.push<DebugControlsView>(); }}, | |||
| {"Pmem", ui::Color::dark_cyan(), &bitmap_icon_memory, [&nav]() { nav.push<DebugPmemView>(); }}, | |||
| {"p.mem", ui::Color::dark_cyan(), &bitmap_icon_memory, [&nav]() { nav.push<DebugPmemView>(); }}, | |||
There was a problem hiding this comment.
@u-foka Sorry for the late comment, but I would like to point out that Persistent Memory is called "P.Memory" in the "P.Memory Mgmt" app name on the Settings page, and personally I would have called it "P.Memory" on the Debug page too, for consistency.
There was a problem hiding this comment.
Hi,
The original Pmem was definitely inconsistent, but this time I looked around and p.mem seemed to be the most common one :)
The app is called P.Memory Mgmt, the wiki page for it is the same, and thats all the references to P.Memory Mgmt I have found :)
Then theres P.MemMgmt in the wiki tree, P.Mem Mgmt on the title, PersistentMemory in the description and 4 p.mem -s on the ui :)
I'm all in for consistency, but I wonder how should it be achieved. Additionally from the UX standpoint I'd put more emphasis on the "Persistent" part somehow to make the purpose more clear. But its probably too long for most places. We come up with a completely different name like Config Store that could fit easier and could also be shortened while retaining the more of the meaning like Cfg. Store or similar, but that would confuse existing users..
What do you think?
There was a problem hiding this comment.
I agree with all your comments. Seems it's inconsistent everywhere... Something like "Config Mem" might convey the functionality better, but might be confusing for existing users (or developers since we call it persistent memory in the code). I would lean towards just calling the app "P.Memory" on the Debug page, but it's no big deal and I leave it totally up to you. :-)
plus add calculated crc to the pmem debug screen
Soo the issue with pmem on newer gcc was that its data members werent declared volatile and seemingly writes to it got optimized out..
After adding volatile everything seems to work smooth.
We also loose some optimization potential as the cached settings use the same type (backup_ram_t) as what we use to read the p.mem area, which now also becomes volatile.