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

Support for scripting in flash using littlefs #28724

Open
wants to merge 54 commits into
base: master
Choose a base branch
from

Conversation

andyp1per
Copy link
Collaborator

@andyp1per andyp1per commented Nov 24, 2024

This completely replaces the flash based support, including logging with a littlefs backend. This results in much more reliable FS access as well as support for things like scripting.

With thanks to @ntamas for the original code

@andyp1per andyp1per added the WIP label Nov 24, 2024
@tpwrules
Copy link
Contributor

Oh excellent!! Have you tried with both NOR and NAND?

@andyp1per
Copy link
Collaborator Author

Oh excellent!! Have you tried with both NOR and NAND?

No, still a lot of work to do as there is lots of duplication currently. The code is from @ntamas but needs a bunch of rework to get into master. I have a plan, but the main thing is that it now works for me locally and I know how to proceed from here.

@ntamas
Copy link
Contributor

ntamas commented Nov 25, 2024

Let me know if I can help with reworking this PR for inclusion. This is something that we at @skybrush-io would also be interested in.

@jpkh
Copy link
Contributor

jpkh commented Nov 26, 2024

Oh yes, there are a lot small H743 based FPVDrone FCs that only have dataflash. This will be important update to all those (and others). Thanks Andy.

@robustini
Copy link
Contributor

Thanks Andy, this will be most useful!

@andyp1per
Copy link
Collaborator Author

@ntamas actually it's not so bad - I think what you have done is correct. I really want the block logger to die, but I don't think that's going to be possible on boards with very small flash chips

@tridge
Copy link
Contributor

tridge commented Nov 29, 2024

it will be very interesting to see how this goes! and some stats on storage overhead (and flash cost) would be useful

@tridge
Copy link
Contributor

tridge commented Nov 29, 2024

@andyp1per assuming this works out and testing is good, we should fork littlefs so we point to our own github org, we've tended to regret it when we don't do that

@andyp1per andyp1per marked this pull request as ready for review November 29, 2024 21:49
@andyp1per andyp1per removed the WIP label Nov 29, 2024
@andyp1per
Copy link
Collaborator Author

Working quite nicely for me now. Not yet flown it.

@andyp1per andyp1per force-pushed the pr-littlefs-flash-v3 branch 3 times, most recently from 01c281d to 424f673 Compare November 30, 2024 10:25
@andyp1per andyp1per added the WIP label Dec 1, 2024
@andyp1per
Copy link
Collaborator Author

Unfortunately I have yet to be convinced that the architecture of littlefs is suitable for logging, there are a lot of unaddressed issues which particularly bit hard.

@ntamas
Copy link
Contributor

ntamas commented Dec 2, 2024

Are you experiencing performance issues with LittleFS? If so, how about splitting the flash memory into two partitions, one for logging with the old block logger, and one for storing files (scripts etc) that are assumed to be mostly static?

@andyp1per
Copy link
Collaborator Author

Are you experiencing performance issues with LittleFS? If so, how about splitting the flash memory into two partitions, one for logging with the old block logger, and one for storing files (scripts etc) that are assumed to be mostly static?

That's obviously the fallback, but at an even higher flash cost and would require modifying either lfs or the block logger to cope with some kind of partitioning scheme. I'm going to see what is achievable with LFS first before going that route.

@andyp1per
Copy link
Collaborator Author

We don't have a block logger based build to check the size against

@andyp1per andyp1per force-pushed the pr-littlefs-flash-v3 branch from bc0fd66 to 92f9c58 Compare December 7, 2024 14:45
@andyp1per andyp1per added Enhancement and removed WIP labels Dec 7, 2024
@andyp1per
Copy link
Collaborator Author

Actually this is looking pretty promising

Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

it would be good to know what the flash cost is - is it comparable to FATFS/sdcard? (likely it is)
worth thinking about supporting this in SITL with a command line option to enable instead of normal file system support, like we do for the logger block support in SITL

@andyp1per andyp1per force-pushed the pr-littlefs-flash-v3 branch from d13260d to 97e57c2 Compare January 8, 2025 08:15
@andyp1per andyp1per force-pushed the pr-littlefs-flash-v3 branch 2 times, most recently from 22d62f5 to 7b5a799 Compare January 9, 2025 17:11
@tpwrules
Copy link
Contributor

tpwrules commented Jan 9, 2025

Please consider this alternate approach for locking, based on the "ensure sitl uses ./scripts" commit: cc679cc...tpwrules:ardupilot:xpr/littlefs-alt

It's two code lines and avoids touching the rest of the code. It should be simple to remove later if and when the underlying issue is fixed as well.

Did some more testing and it all still looks good. I can't crash the board and scripting restart in flight works properly.

@andyp1per andyp1per force-pushed the pr-littlefs-flash-v3 branch from 659959d to 2a94fb4 Compare January 10, 2025 17:52
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.

9 participants