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

Added parameter backup region on H7 with 32k FRAM #16304

Merged
merged 6 commits into from Jan 15, 2021

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Jan 12, 2021

This expands the FRAM storage region for H7 boards to 32k, and adds the following:

  • expands param storage by 1280 bytes (to help with new IMU temp cal param data)
  • expands mission storage by 9842 bytes, giving 820 more waypoints
  • adds a new ParamBak storage region as backup for primary param storeage

This fixes issue #13134

The backup param storage area is continuously updated along with the main param storage. On startup if the primary storage area has a corrupt header then the backup is used, copying it over the primary.

Confirmed OK on:

  • CubeOrange
  • Durandal
  • CUAV-X7
  • CUAV-Nora
  • PixracerPro
  • ControlZeroH7
  • mRoNexus

I've also tested that setting FORMAT_VERSION=0 does reset properly, so no user visible behaviour change
I've confirmed with @pkocmoud that mRo use 32k FRAM

@peterbarker
Copy link
Contributor

.... how did you manage to get this to conflict already? :-)

@proficnc
Copy link
Contributor

Thanks Tridge!

@tridge
Copy link
Contributor Author

tridge commented Jan 12, 2021

.... how did you manage to get this to conflict already? :-)

yeah ... forgot to rebase first :)
fixed

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

This looks good in terms of what it is trying to accomplish.

IMO the extra space allocations for fence and mission do not belong in this PR. They might be good changes but they're unrelated and deserve discussion.

@tridge
Copy link
Contributor Author

tridge commented Jan 13, 2021

IMO the extra space allocations for fence and mission do not belong in this PR. They might be good changes but they're unrelated and deserve discussion.

happy to discuss, but I think given the structure of StorageManager that allocating some more space for params and mission is worthwhile now if we are going to start using the 2nd half of the FRAM

@tridge tridge removed the DevCallEU label Jan 13, 2021
@yaapu yaapu mentioned this pull request Jan 13, 2021
@tridge
Copy link
Contributor Author

tridge commented Jan 15, 2021

in the one example of a STRG_BAK that we have showing this issue, I've confirmed that the corruption is confined to the first 48 bytes of the parameter storage.
That makes it very likely that this PR will prevent loss of parameters

@pompecukor
Copy link

pompecukor commented Jan 15, 2021

When can it be merged? Will be a life saver. Reset have been such a PITA.

@tridge tridge merged commit ed8d86e into ArduPilot:master Jan 15, 2021
@Mr-jh
Copy link

Mr-jh commented Jun 18, 2022

Is it valid for f7 based like cuav v5+

@timtuxworth
Copy link
Contributor

IMO the extra space allocations for fence and mission do not belong in this PR. They might be good changes but they're unrelated and deserve discussion.

happy to discuss, but I think given the structure of StorageManager that allocating some more space for params and mission is worthwhile now if we are going to start using the 2nd half of the FRAM

You always need more. (it doesn't matter what you are talking about, whenever there is a hard limit - you always need more).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants