Forum Rule: Always post complete source code & details to reproduce any issue!
Page 2 of 4 FirstFirst 1 2 3 4 LastLast
Results 26 to 50 of 93

Thread: FS.h - File class goes boom - when virtual functions not overwritten

  1. #26
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    1,623
    Frank, I think we are taking past each other. IMHO your implementation of MemFile is a valid implementation of File, it provides all methods it needs to provide. I can write a function which takes a File as parameter and it will happily accept your memFile.

    One can argue if the API should enforce the implementer to explicitly define the API (that would be pure virtual) or if the API provides default values and the implementer only implements what he wants (that's the current solution with the virtual functions which provide default implementations). For me that is a matter of taste only. At the end of the day both methods provide implementations of all functions required by the interface.

    Plus, it needs a different open() and has no filenames.
    That's a different story. The open is not part of the File API but belongs to the Filesystem. Of course opening a file on a SD works differently than your memory file or some file streamed via a serial interface. Personally I would remove the open functions from the FS interface since it depends on the Filesystem you are implementing. Technically, as you have seen, you don't need to implement the FS interface at all. Just generate some open function which returns a File object and it works.

  2. #27
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    25,064
    Quote Originally Posted by KurtE View Post
    I started off with the assumption that we will need to maintain compatibility.
    Obviously we can't break compatibility with the API as seen by users.

    But we can change the class name LittleFS and SD and Frank's MemFS inherit. FileImpl is a likely choice, not because I personally like it, but because that's the name ESP is using.

    We can (and almost certainly will) add more functions which are required to be implemented by LittleFS, SD, MemFS.

  3. #28
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    9,564
    There are a few others that I know of like MSC. Not sure how many others... With our current playing with integration, will try to keep track of all of them.

  4. #29
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    Quote Originally Posted by PaulStoffregen View Post
    Obviously we can't break compatibility with the API as seen by users.

    But we can change the class name LittleFS and SD and Frank's MemFS inherit. FileImpl is a likely choice, not because I personally like it, but because that's the name ESP is using.

    We can (and almost certainly will) add more functions which are required to be implemented by LittleFS, SD, MemFS.
    This is just for reference and random thoughts....

    If anyone is interested there is ESP implementation of FS for the ESP32: https://github.com/espressif/arduino...braries/FS/src. It does have timestampl as part of it as well. Guess this is the path you are talking about implementing in terms of virtual functions. Looks like vfs_api.h would really be the link to the SDFat library. This is just generalities I am talking about. Looks like they have their own SD Library (https://github.com/espressif/arduino...r/libraries/SD) which is not applicable. Just looking at comparisons.

    EDIT: In one of first posts @dirkenstein I believe actually did some of that porting already see https://forum.pjrc.com/threads/58033...l=1#post258409

    Might be interesting to take a look at. Was part of his initial implementation of LittleFS for the Teensy. [EDIT] Just noticed that this implementations is based on the ESP8266 - probably should have been based off the ESP32 implementation. Just observations. Also in no way shape or form am I implying using on own SD implementation!
    Last edited by mjs513; 08-31-2021 at 11:57 AM. Reason: Updated comments

  5. #30
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    25,064
    Here's a first attempt to get rid of the union and use 2 classes, where the implementation class is named FileImpl to be sort-of like ESP.

    So far only tested with SD. LittleFS should be similar, just inherit from FileImpl rather than File.
    Attached Files Attached Files
    • File Type: h FS.h (5.2 KB, 16 views)
    • File Type: h SD.h (6.0 KB, 15 views)

  6. #31
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    Quote Originally Posted by PaulStoffregen View Post
    Here's a first attempt to get rid of the union and use 2 classes, where the implementation class is named FileImpl to be sort-of like ESP.

    So far only tested with SD. LittleFS should be similar, just inherit from FileImpl rather than File.
    That was fast - and thank you. Ok back to work/playing

  7. #32
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    @PaulStoffregen
    Started the conversion process by replacing SD.h in the SD Library and FS.h in the Teensy4 core. I am assuming that the changes will be transparent to the example sketches - I hope.

    Anyway I tried the listfiles.ino example after I mad the changes and am receiving numerous errors probably all stemming from this first one:
    Code:
    F:\arduino-1.8.15a\hardware\teensy\avr\libraries\SD\src/SD.h:54:1: error: expected class-name before '{' token
     {
     ^
    I am attaching a text file with all the errors. Am sure I am missing something obvious but don't see it - and usually its something easy.
    Attached Files Attached Files

  8. #33
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    25,064
    Check the copy of FS.h. Those errors look like you're using the new SD.h with the original FS.h.

    Try editing FS.h and quickly add a syntax error on the first line, just to make sure Arduino really is using that copy.

  9. #34
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    Quote Originally Posted by PaulStoffregen View Post
    Check the copy of FS.h. Those errors look like you're using the new SD.h with the original FS.h.
    Ok turned out that was it. Sometimes I hate Windows because I know I did the copy/replace.

  10. #35
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    Try write(char) - seems to have a endless loop.

  11. #36
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    size_t write(const char *str) {
    return write(str, strlen(str));
    }

    - hm. Is this the most efficient way? Surely depends on how the underlying things are implemented, but at a first glance, the strlen-loop + the later copy is somehow twice the work?
    Normally strcpy() is used for such cases. But i guess that needs additional work?
    Last edited by Frank B; 08-31-2021 at 04:22 PM. Reason: rmv blah blah

  12. #37
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    Quote Originally Posted by PaulStoffregen View Post
    ...
    So far only tested with SD. LittleFS should be similar, just inherit from FileImpl rather than File.
    Ok this might go along with what Frank just commented on. In trying to update LittleFS I just changed File to FileImpl in the .h's but then get an error on:
    Code:
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h:197:15: error: type 'Print' is not a base type for type 'LittleFSFile'
      using Print::write;
    ok looking at LittleFS and FS.h stream class is only referenced in the base class when you use FILE. So if I do a
    Code:
    class LittleFSFile : public FileImpl, public Stream
    I get a whole bunch of other errors:
    Code:
    :\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h: In member function 'virtual File LittleFSFile::openNextFile(uint8_t)':
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h:165:50: error: invalid new-expression of abstract class type 'LittleFSFile'
         return File(new LittleFSFile(lfs, f, pathname));
                                                      ^
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h:30:7: note:   because the following virtual functions are pure within 'LittleFSFile':
     class LittleFSFile : public FileImpl, public Stream
           ^
    In file included from F:\arduino-1.8.15a\hardware\teensy\avr\cores\teensy4/Stream.h:24:0,
                     from F:\arduino-1.8.15a\hardware\teensy\avr\cores\teensy4/HardwareSerial.h:115,
                     from F:\arduino-1.8.15a\hardware\teensy\avr\cores\teensy4/WProgram.h:46,
                     from C:\Users\Merli\AppData\Local\Temp\arduino_build_420315\pch\Arduino.h:6:
    F:\arduino-1.8.15a\hardware\teensy\avr\cores\teensy4/Print.h:58:17: note: 	virtual size_t Print::write(uint8_t)
      virtual size_t write(uint8_t b) = 0;
                     ^
    In file included from F:\arduino-1.8.15a\hardware\teensy\avr\cores\teensy4/HardwareSerial.h:115:0,
                     from F:\arduino-1.8.15a\hardware\teensy\avr\cores\teensy4/WProgram.h:46,
                     from C:\Users\Merli\AppData\Local\Temp\arduino_build_420315\pch\Arduino.h:6:
    F:\arduino-1.8.15a\hardware\teensy\avr\cores\teensy4/Stream.h:31:14: note: 	virtual int Stream::read()
      virtual int read() = 0;
                  ^
    In file included from D:\Users\Merli\Documents\Arduino\libraries\MTP_t4\examples\LFS_Program_MTP_Simple_Datalogger\LFS_Program_MTP_Simple_Datalogger.ino:9:0:
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h:172:50: error: invalid new-expression of abstract class type 'LittleFSFile'
         return File(new LittleFSFile(lfs, d, pathname));
                                                      ^
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h: In member function 'virtual File LittleFS::open(const char*, uint8_t)':
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h:232:55: error: invalid new-expression of abstract class type 'LittleFSFile'
          return File(new LittleFSFile(&lfs, file, filepath));
                                                           ^
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h:240:54: error: invalid new-expression of abstract class type 'LittleFSFile'
          return File(new LittleFSFile(&lfs, dir, filepath));
                                                          ^
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\src/LittleFS.h:252:54: error: invalid new-expression of abstract class type 'LittleFSFile'
         return File(new LittleFSFile(&lfs, file, filepath));
    So don;t think its as simple as changing File to FileImpl.

    Or is there something else to be added.

    As for SD.h I did notice that you did not include Stream or using Print::write as part of the class - should that be added?

  13. #38
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    Can you add a writen(const char *str)? (similar to the other exisiting "n" string functions) - that would make it more safe.
    Last edited by Frank B; 08-31-2021 at 04:27 PM. Reason: rmv blah blah

  14. #39
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    It should read
    Code:
        size_t write(uint8_t b) {                
            return (f) ? f->write(&b, 1) : 0;
        }
        size_t write(const char *str) {        
            return (f) ? f->write(str, strlen(str)) : 0;
        }
        size_t nwrite(const char *str, size_t maxlen) {
            if (!f) return 0;
            size_t len = strlen(str);
            if (len > maxlen) len = maxlen;
            return f->write(str, len);
        }
    Last edited by Frank B; 08-31-2021 at 05:34 PM.

  15. #40
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    1,623
    Quote Originally Posted by mjs513 View Post
    ...
    So don;t think its as simple as changing File to FileImpl.
    Or is there something else to be added.
    As for SD.h I did notice that you did not include Stream or using Print::write as part of the class - should that be added?
    I gave that a try. I cloned LittleFS from https://github.com/PaulStoffregen/LittleFS, changed File to FileImpl in the class declaration, removed Stream from the inheritance list and commented using print::write. (the stream interface should be handled by File if I understand the code correctly)

    With that changes LittleFS_usage.ino compiled without an issue. I didn't check if it works as expected but at least it compiles...

  16. #41
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    Quote Originally Posted by luni View Post
    I gave that a try. I cloned LittleFS from https://github.com/PaulStoffregen/LittleFS, changed File to FileImpl in the class declaration, removed Stream from the inheritance list and commented using print::write. (the stream interface should be handled by File if I understand the code correctly)

    With that changes LittleFS_usage.ino compiled without an issue. I didn't check if it works as expected but at least it compiles...
    Thanks for testing and letting me know. I keep learning with you all. Commented out
    Code:
    	//using Print::write;
    in LittleFS.h and that resolved the compile issues. I also incorporated Franks fix just in case. As for it working - not opening a file for writing. Using a simple test sketch using program mem I get an error on writing to the file:
    Code:
    Logging Data!!!
    error opening datalog.txt
    error opening datalog.txt
    error opening datalog.txt
    error opening datalog.txt
    error opening datalog.txt
    error opening datalog.txt
    error opening datalog.txt
    error opening datalog.txt
    It does initialize the drive based on the space allocated:
    Code:
    D:\Users\Merli\Documents\Arduino\libraries\LittleFS\examples\Simple DataLogger Examples\LittleFS_Program_Simple_Datalogger\LittleFS_Program_Simple_Datalogger.ino Aug 31 2021 14:18:15
    Initializing LittleFS ...
    LittleFS initialized.
    
    Menu Options:
    	l - List files on disk
    	e - Erase files on disk
    	s - Start Logging data (Restarting logger will append records to existing log)
    	x - Stop Logging data
    	d - Dump Log
    	h - Menu
    
    
     Space Used = 8192
    Filesystem Size = 1048576
    It does look like it created the file:
    Code:
    Stopped Logging Data!!!
    Records written = 0
    
     Space Used = 8192
    Filesystem Size = 1048576
    Directory
    ---------
    datalog.txt                           0
    Here is the test sketch if you want to try it - going to keep looking:
    Code:
    #include <LittleFS.h>
    
    LittleFS_Program myfs;
    
    // NOTE: This option is only available on the Teensy 4.0, Teensy 4.1 and Teensy Micromod boards.
    // With the additonal option for security on the T4 the maximum flash available for a 
    // program disk with LittleFS is 960 blocks of 1024 bytes
    #define PROG_FLASH_SIZE 1024 * 1024 * 1 // Specify size to use of onboard Teensy Program Flash chip
                                            // This creates a LittleFS drive in Teensy PCB FLash. 
    
    File dataFile;  // Specifes that dataFile is of File type
    
    int record_count = 0;
    bool write_data = false;
    uint32_t diskSize;
    
    void setup()
    {
    
      // Open serial communications and wait for port to open:
      Serial.begin(115200);
      while (!Serial) {
        // wait for serial port to connect.
      }
      Serial.println("\n" __FILE__ " " __DATE__ " " __TIME__);
    
      Serial.println("Initializing LittleFS ...");
    
      // see if the Flash is present and can be initialized:
      // lets check to see if the T4 is setup for security first
      #if ARDUINO_TEENSY40
        if ((IOMUXC_GPR_GPR11 & 0x100) == 0x100) {
          //if security is active max disk size is 960x1024
          if(PROG_FLASH_SIZE > 960*1024){
            diskSize = 960*1024;
            Serial.printf("Security Enables defaulted to %u bytes\n", diskSize);  
          } else {
            diskSize = PROG_FLASH_SIZE;
            Serial.printf("Security Not Enabled using %u bytes\n", diskSize);
          }
        }
      #else
        diskSize = PROG_FLASH_SIZE;
      #endif
    
      // checks that the LittFS program has started with the disk size specified
      if (!myfs.begin(diskSize)) {
        Serial.printf("Error starting %s\n", "PROGRAM FLASH DISK");
        while (1) {
          // Error, so don't do anything more - stay stuck here
        }
      }
      Serial.println("LittleFS initialized.");
      
      menu();
      
    }
    
    void loop()
    { 
      if ( Serial.available() ) {
        char rr;
        rr = Serial.read();
        switch (rr) {
          case 'l': listFiles(); break;
          case 'e': eraseFiles(); break;
          case 's':
            {
              Serial.println("\nLogging Data!!!");
              write_data = true;   // sets flag to continue to write data until new command is received
              // opens a file or creates a file if not present,  FILE_WRITE will append data to
              // to the file created.
              dataFile = myfs.open("datalog.txt", FILE_WRITE);
              logData();
            }
            break;
          case 'x': stopLogging(); break;
          case 'd': dumpLog(); break;
          case '\r':
          case '\n':
          case 'h': menu(); break;
        }
        while (Serial.read() != -1) ; // remove rest of characters. 
      } 
    
      if(write_data) logData();
    }
    
    void logData()
    {
        // make a string for assembling the data to log:
        String dataString = "";
      
        // read three sensors and append to the string:
        for (int analogPin = 0; analogPin < 3; analogPin++) {
          int sensor = analogRead(analogPin);
          dataString += String(sensor);
          if (analogPin < 2) {
            dataString += ",";
          }
        }
      
        // if the file is available, write to it:
        if (dataFile) {
          dataFile.println(dataString);
          // print to the serial port too:
          Serial.println(dataString);
          record_count += 1;
        } else {
          // if the file isn't open, pop up an error:
          Serial.println("error opening datalog.txt");
        }
        delay(100); // run at a reasonable not-too-fast speed for testing
    }
    
    void stopLogging()
    {
      Serial.println("\nStopped Logging Data!!!");
      write_data = false;
      // Closes the data file.
      dataFile.close();
      Serial.printf("Records written = %d\n", record_count);
    }
    
    
    void dumpLog()
    {
      Serial.println("\nDumping Log!!!");
      // open the file.
      dataFile = myfs.open("datalog.txt");
    
      // if the file is available, write to it:
      if (dataFile) {
        while (dataFile.available()) {
          Serial.write(dataFile.read());
        }
        dataFile.close();
      }  
      // if the file isn't open, pop up an error:
      else {
        Serial.println("error opening datalog.txt");
      } 
    }
    
    void menu()
    {
      Serial.println();
      Serial.println("Menu Options:");
      Serial.println("\tl - List files on disk");
      Serial.println("\te - Erase files on disk");
      Serial.println("\ts - Start Logging data (Restarting logger will append records to existing log)");
      Serial.println("\tx - Stop Logging data");
      Serial.println("\td - Dump Log");
      Serial.println("\th - Menu");
      Serial.println();
    }
    
    void listFiles()
    {
      Serial.print("\n Space Used = ");
      Serial.println(myfs.usedSize());
      Serial.print("Filesystem Size = ");
      Serial.println(myfs.totalSize());
    
      printDirectory(myfs);
    }
    
    void eraseFiles()
    {
      myfs.quickFormat();  // performs a quick format of the created di
      Serial.println("\nFiles erased !");
    }
    
    void printDirectory(FS &fs) {
      Serial.println("Directory\n---------");
      printDirectory(fs.open("/"), 0);
      Serial.println();
    }
    
    void printDirectory(File dir, int numSpaces) {
       while(true) {
         File entry = dir.openNextFile();
         if (! entry) {
           //Serial.println("** no more files **");
           break;
         }
         printSpaces(numSpaces);
         Serial.print(entry.name());
         if (entry.isDirectory()) {
           Serial.println("/");
           printDirectory(entry, numSpaces+2);
         } else {
           // files have sizes, directories do not
           printSpaces(36 - numSpaces - strlen(entry.name()));
           Serial.print("  ");
           Serial.println(entry.size(), DEC);
         }
         entry.close();
       }
    }
    
    void printSpaces(int num) {
      for (int i=0; i < num; i++) {
        Serial.print(" ");
      }
    }

  17. #42
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    1,623
    Tried the same example

    In FS.h class File

    Code:
    operator bool() {
    		return (f) ? (bool)*f : false;
    }
    seems to be a left over from the union thing ?

    Changing it to
    Code:
    operator bool{return f != nullptr;}
    Let you open the file. But then writing hangs, probably due to the issue Frank mentioned.

  18. #43
    Senior Member+ KurtE's Avatar
    Join Date
    Jan 2014
    Posts
    9,564
    Sorry been busy working in yard...

    Note: Will also need to update MSC Library UsbMscFat\src\mscFS.h
    In my version line 44: class MSCFile : public File

  19. #44
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    Quote Originally Posted by luni View Post
    Tried the same example

    In FS.h class File

    Code:
    operator bool() {
            return (f) ? (bool)*f : false;
    }
    seems to be a left over from the union thing ?

    Changing it to
    Code:
    operator bool{return f != nullptr;}
    Let you open the file. But then writing hangs, probably due to the issue Frank mentioned.
    Syntax.. probably copy&paste issue.. you want:
    Code:
    operator bool(){return f != nullptr;}

  20. #45
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    1,623
    Syntax.. probably copy&paste issue.. you want:
    Not even copy paste, I wrote it simply down which will ALWAYS be wrong :-) Thanks for spotting.

    After adding your fixes from above this example seems to work. At least it pretends to write something. Is there a possibility to read the file back / display contents?

  21. #46
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    Yes, the dumpfile example.

    Updated and renamed MemFile (no all capitals anymore)

  22. #47
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    Quote Originally Posted by luni View Post
    Tried the same example

    In FS.h class File

    Code:
    operator bool() {
    		return (f) ? (bool)*f : false;
    }
    seems to be a left over from the union thing ?

    Changing it to
    Code:
    operator bool{return f != nullptr;}
    Let you open the file. But then writing hangs, probably due to the issue Frank mentioned.
    Well yes and no to Frank's changes. I made the change above and used the changes that Frank suggest - it looks like it opened the file and data was writing to the file. But when I went to read the created log it was empty and the directory showed 0 Bytes. Here is short transcript of the test case of writing to the log file, reading it back and then doing a list files:
    Code:
    Logging Data!!!
    10,10,9
    10,10,10
    10,10,10
    10,10,10
    11,10,10
    11,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    11,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,10
    10,10,9
    10,10,10
    
    Stopped Logging Data!!!
    Records written = 57
    
    Dumping Log!!!
    
     Space Used = 8192
    Filesystem Size = 524288
    Directory
    ---------
    datalog.txt                           0
    So we are missing something else

    EDIT: to write data to the file it is using:
    Code:
          dataFile.println(dataString);

  23. #48
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    Hm.
    My datalog.txt shows data. DId that before luni's change.

    Also, MemFile works now - does not work without the write() mod
    Instead it even crashed.

  24. #49
    Senior Member+ mjs513's Avatar
    Join Date
    Jul 2014
    Location
    New York
    Posts
    7,551
    Quote Originally Posted by Frank B View Post
    Hm.
    My datalog.txt shows data. DId that before luni's change.
    Thanks Frank - try that next .....


    EDIT: Ok without luni's change to bool I still get the error writing to the file. This is with your changes in for the write functions. As I said the only changes to the LittleFS was to
    1. change File to FileImpl
    2. comment out using Print::write - if i leave this in the you get other issues as I mentioned.

  25. #50
    Senior Member
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    9,293
    Here is my FS.h


    i'll do other things now.
    Attached Files Attached Files
    Last edited by Frank B; 08-31-2021 at 07:17 PM.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •