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

Status
Not open for further replies.
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.
 
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.
 
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.
 
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-esp32/tree/master/libraries/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-esp32/tree/master/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-LittleFS-port-to-Teensy-SPIFlash?p=258409&viewfull=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:
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.
 

Attachments

  • FS.h
    5.2 KB · Views: 54
  • SD.h
    6 KB · Views: 52
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 :)
 
@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.
 

Attachments

  • Error-SD.txt
    3.2 KB · Views: 43
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.
 
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:
...
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?
 
Can you add a writen(const char *str)? (similar to the other exisiting "n" string functions) - that would make it more safe.
 
Last edited:
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;
    }
[I]    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);
    }[/I]
 
Last edited:
...
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...
 
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(" ");
  }
}
 
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.
 
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
 
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[B]()[/B]{return f != nullptr;}
 
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?
 
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);
 
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.
 
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.
 
Here is my FS.h


i'll do other things now.
 

Attachments

  • FS.h
    5.4 KB · Views: 44
  • datalog.txt
    3.7 KB · Views: 38
Last edited:
Status
Not open for further replies.
Back
Top