SDFileSystem and PokittoDisk not working 100% with latest library

Troubleshooting
7

Compiling Xploritto with the FemtoIDE works better but it crashes when it tries to read a POP file. If I try and debug it I get

Program received signal SIGTRAP, Trace/breakpoint trap.
0x000156de in _free_r ()

Which fails at the following line:

GDB: #8 SDBrowse::draw (this=0x10005e98) at /home/justin/Downloads/FemtoIDE/projects/Xploritto/SDBrowse.cpp:472
472	Pop::open(f);

This crash happens in the emulator causing a segfault when not run in debug mode and freezes when run on hardware.

The SDFileIO example still doesn’t work, but I just tried the pre-compiled version in the build folder and it also doesn’t work. If I remove the file creation part of SDFileIO and simply have it read a file that already exists then the SDFileSystem read works but the PFFS doesn’t. I just discovered that the PFFS only allows filenames in the 8.3 format and can now read using it. Still doesn’t explain the weird crash when building Xploritto from source, and should still be looked into, but it looks like my issues with the sd card has to do with the CLI setup.

Since it looks like the FemtoIDE allows you to use an external editor I think I will switch to using it instead. This will save me some headache in the future. Plus it has built-in debugging available and it took me about 20 seconds to figure out how to launch the emulator with an SD card image. Since I’m on linux I have the img mounted as a loop device allowing me to modify it and then re-launch the emulator. Unfortunately the emulator doesn’t support updating the img file with any changes made by the running program, but maybe someday.

1 Like
8

When i try to build Xporitto with latest femtoIDE and latest PokittoLib, i had the same problems as you.
(loading files in Xploritto cause a SIGTRAP crash)
Xploritto version 0.1.1 was origanally built using EmBitz, before femtoIDE was a thing.
When i compare the build flags i notice that i was compiling it with -std=c++11 .
So when i change to -std=c++11 from -std=c++14 in project.json everything work fine.
Maybe something in SDFilesSystem is not compatible with C++ 14 :thinking:.

Edit:
Here is my working project
Xploritto.zip (194.8 KB)

1 Like
9

@FManga , if this is true we have a potential showstopper!

10

Showstopper… or an excuse to take the Cleanup even further? :thinking:

2 Likes
11

I am not sure I am totally enthusiastic about losing mbed connection completely and/or rewriting things like SDFileSystem just for the sake of C++11->C++17

12

Not for the sake of C++17, but for the sake of having a single filesystem library that doesn’t have the limitations of PFFS, the memory leaks of SDFS, or the confusion of having both.

6 Likes
13

Well, we’ve seen miracles here before so I have no doubt it can be done

3 Likes
14

I’ve still got the tilemap thing to finish, then I need to update micropython. After that I’ll give the filesystem situation a look to see if it’s something that can be tackled now or if it would be better to just find out what breaks in C++14.
It is very odd that something breaks when going from 11 to 14 and it suggests the code is relying on undefined behaviour.

1 Like
15

When compiling with std=c++11 it still crashes the same. However if I launch the emulator through FemtoIDE instead of the command line I get a better message then segfault. Running through with GDB still causes the breakpoint trap at the previously mentioned location.

UNKNOWN OP effe@ PC=1e
Attempt to write to flash (0x204) on PC=0x2236

If @bl_ackrain was able to successfully compile it using std=c++11 but I’m still getting the same results whether I use c++11 or c++14 means there’s something deeper going on with the libs/setup. Looking through the compilation process there’s mostly just a lot of redefinition warnings, although most of the warnings aren’t visible anymore due to scroll limits in FemtoIDE (it only shows the last X output lines, and I don’t see a setting anywhere to change that).

I noticed there’s a lot of redefinition warnings when using both PFFS and SDFileSystem. When doing a library overhaul it would be nice to still get the best of both worlds. That is use SDFileSystem when integrity is important, or when creating new files/folders, but switching to PFFS when streaming large amounts of data in real-time.

1 Like
16

I’ve been contemplating this for a while but so far most of my time has been spent contemplating an API rather than actually building anything that works.

I’ve only made one prototype API, most of which is hosted here:

(Some of the function names aren’t ideal,
e.g. Filesystem::delete naturally clashes with the delete keyword.)

I was designing it with C++11 in mind though,
with access to C++17 I’d redesign quite a few things,
e.g. make more use of std::optional and std::variant.

Though I think the most important thing is to have an actual File object that self-closes itself upon destruction to prevent memory leaks,
and to perhaps extend that philosophy to other parts of the library.
I.e. don’t expect the user to remember to close and dispose of things (especially if the user is more familiar with GCed languages like Python)

1 Like
17

This error is unrelated to SDFS and C++17, I get the same thing with C++11. Xploritto is crashing due to running out of memory:
static bool open(const std::string path);
This should be a const std::string& so as to not allocate a copy on the heap.

As for SDFS not creating files, it turned out to be a bug in the emulator. I’m testing a fix and now the following works on both the emulator and hardware:

#include "Pokitto.h"
int main(){
    using PC=Pokitto::Core;
    
    PC::begin();    
    char str[33] = {};
    auto sdfs = new SDFileSystem(P0_9, P0_8, P0_6, P0_7, "sd", NC, SDFileSystem::SWITCH_NONE, 24000000 );
    sdfs->write_validation(false);
    sdfs->crc(false);
    {
        auto f = sdfs->open("x.txt", O_CREAT | O_RDWR );
        f->write("this is a test", 14);
        f->close();
    }
    {
        auto f = sdfs->open("x.txt", O_RDONLY );
        f->read(str, 32);
        f->close();
    }

    printf("%s\n", str);
}
2 Likes
18

Nice. Just tested it and now it’s working. Also noticed ~13 hours ago a commit to allow dumping the SD image on exit :grin: :+1::+1:

Will definitely need to test this. Been stress testing everything so I know just how far I can push this little guy before starting my long-term project. Part of the tests is seeing how things behave in the emulator vs actual hardware, will continue to provide feedback as I discover things.

Keep up the amazing work.

2 Likes
19

I second this, amazing is the word!

1 Like
20

Spoiler! :stuck_out_tongue:

Also added read-only support for img files to the IDE:

3 Likes
21

MORE AWESOMENESS!!!
Too bad read-write support might be too much of a challenge for you :wink:

22

Heh. It’s just how FemtoIDE releases work: crudely add a feature in one release, refine it in the next. That way others don’t have to wait too long for functionality they need, and can find problems and make suggestions before too much effort has been put into it.

2 Likes
23

I wonder :thinking: why it was working before? (old EmBitz build).

Nice.

24

Possible with a bunch of changes to the library the memory requirements have changed. If Xploritto was already pushing the limits of the RAM usage then that’s definitely a possibility.

25

The answer that first comes to mind is ‘copy elision’.

Depending on how open is being used the compiler might be able to elide the copy.
This is the typical behaviour when a temporary object is passed as an argument to a function.

For example, if you were to do open("/some/file.txt"), the exact process would be:

  • Pass the const char * "/some/file.txt" to the std::string constructor string(const char *), creating a temporary std::string
  • Copy the temporary string to open's path parameter
  • Destroy the temporary string

In which case the compiler can figure out that the last two steps are unnecessary and elide the copy by directly initialising the path parameter with the (unnamed) const char * value "/some/file.txt".
(From C++11 onwards this will potentially result in a move rather than a copy, but the compiler may be able to treat this as an opportunity to do copy elision rather than an outright move.)

However, if std::string had already been constructed and assigned to a variable (i.e. it’s an lvalue rather than a temporary/rvalue) then the copy cannot be elided because a variable is not a temporary - the construction has already occured in a separate statement.
Consequently the string must be copied and the heap must suffer the penalty of the subsequent allocation.

Basically:

// Copy is elided:
open("/some/file.txt");

// Copy cannot be avoided:
std::string object { "/some/file.txt" };
open(object);

Ultimately the best solution is probably to have a const char * overload and to make a const std::string & overload which just forwards the string.c_str(), if you can get away with it.
That way open("/some/file.txt"); doesn’t even create a std::string, it just passes a pointer.

(There’s an even better way than that which involves also passing the length of the string, but I don’t want to drag out the explanations and examples. If you’re interested in that better solution, let me know.)

It doesn’t really need mentioning, but I’ll mention it anyway…

In static bool open(const std::string path); the const is actually meaningless because the std::string is passed by value so the const doesn’t affect the semantics in any way.
(The same is true for volatile, and for return types when returning by value.)

With static bool open(const std::string & path); it is the std::string referred to by the reference that is const, not the reference itself, which is why const makes sense in that case.

1 Like
26

@Pharap Thank you for the explanation.
My C++ knowledge has improved a lot since i joined this wonderful community.

2 Likes