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

Double storage for rally points and geofence #22533

Merged
merged 7 commits into from
Jan 30, 2024
Merged

Conversation

KonradRudin
Copy link
Contributor

Solved Problem

If there is an error during uploading of the rally points or geofence, the old fence points get overwritten and are not accessible anymore.

Solution

  • Extend dataman to have double storage for rally points and geofence the same way as is already done for missions
  • On upload write the uploaded points t othe inactive slot and switch at the end when upload was successful.

Changelog Entry

For release notes:

Feature: Enable double storage slots for rally points and geofence

Test coverage

  • Tested in SITL while observing the mission topic when uploading rally points and missions.

@dagar
Copy link
Member

dagar commented Dec 14, 2023

What's the memory cost?

@KonradRudin
Copy link
Contributor Author

@dagar I tested on the fmu_v5x with SYS_AUTOSTART 1100 in SIH:
When using the SDCard:
main:
Screenshot from 2023-12-14 10-40-24
PR:
Screenshot from 2023-12-14 15-20-01

When writing mission on RAM:
main:
Screenshot from 2023-12-14 15-33-26

PR:
Screenshot from 2023-12-14 15-26-16

So around 200 Bytes for SD Card, an around 4.5 KBytes when mission is in RAM. Do you think this is an issue?

@mcsauder
Copy link
Contributor

The memory cost goes up with each point on the polygon, correct? Factor of N?

@dagar
Copy link
Member

dagar commented Dec 14, 2023

Do you think this is an issue?

Nope, just wanted to make sure we're keeping an eye on it (memory is very tight on older STM32F4 boards).

@KonradRudin
Copy link
Contributor Author

The memory cost goes up with each point on the polygon, correct? Factor of N?

Sorry for the late reply @mcsauder depends on what you mean. For the mission in RAM the memory for the maximum mission/geofence/rally points is already reserved, so there wont be any additional memory cost there (and that is the reason for the difference between this PR and main, since we need to reserve double the storage in memory for geofence and rally points). When the mission is written on the SDcard, the dataman module does not need more memory depending on the mission. The only thing that is taking currently more memory when you upload bigger geofences and rally points is that the geofence and RTL mode cache all the points in memory (mission only caches the next 10 points irrespecitve of length). But that is the same both for the PR and main. So the memory difference is still the same.

@mcsauder
Copy link
Contributor

That makes sense to me. Are you able to demonstrate that a high degree polynomial geofence doesn't create a RAM problem? I haven't tried lately, but that was an issue I ran into.

-Mark

@mcsauder
Copy link
Contributor

Sorry. NM, you've already addressed that.

@KonradRudin
Copy link
Contributor Author

That makes sense to me. Are you able to demonstrate that a high degree polynomial geofence doesn't create a RAM problem? I haven't tried lately, but that was an issue I ran into.

-Mark

Well not completely but it is hard to check for all the different hardware and configs (the v5x i am testing on has still some margins at least for MC). That's why i checked the relative RAM difference but i do not know how close we already are for the other hardware and configs.

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Looks good, I did not spot anything.

@KonradRudin
Copy link
Contributor Author

rebased on main

@dagar dagar merged commit 169d2dd into main Jan 30, 2024
91 checks passed
@dagar dagar deleted the double_storage_rally_geofence branch January 30, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants