Implement download, closes #78

probonopd commented 5 years ago

Trying to implement https://github.com/me-no-dev/ESPAsyncWebServer#respond-with-content-coming-from-a-file but I am not sure how to do this so that it works with both SPIFFS and SD. I assume I should be using StorageFS rather than SPIFFS. Any hint @GMagician?

Getting

ESP8266WirelessPrintAsync:583:69: error: no matching function for call to 'AsyncWebServerRequest::send(StorageFS&, String&, const char [20])'
     request->send(storageFS, uploadedFullname, "application/x-gcode");

https://travis-ci.org/probonopd/WirelessPrinting/builds/500736473#L3336

GMagician commented 5 years ago

Yes storageFS will switch between spiffs and SD depending on which one is active...

probonopd commented 5 years ago

What am I doing wrong then in https://github.com/probonopd/WirelessPrinting/pull/86/files?

GMagician commented 5 years ago

I think is not to be done in "/api/files"...this I think should just report files in unit...but maybe i should read octoprint manuals before answert....

probonopd commented 5 years ago

Oh right, typo, I meant /download (this is for the link that we have on the HTML page, not OctoPrint related)

GMagician commented 5 years ago

mmm SPIFFS is stream derived class... FileWrapper is not...

GMagician commented 5 years ago

Maybe it must be checkd if SDFile is also derived from stream class. If both are then also FileWrapper may be derived from stream and works as you need

probonopd commented 5 years ago

Maybe it must be checkd if SDFile is also derived from stream class.

It seems that it is: https://github.com/esp8266/Arduino/blob/master/libraries/SD/src/SD.h#L26

Now what do I do? Not use storageFS?

GMagician commented 5 years ago

If both spiFFS file and sdfile are derived from stream then alsow filewrapper should and since request->send(...) needs a stream you have to pass object returned from storageFS.open

GMagician commented 5 years ago

but maybe the better is to check what request->send needs

GMagician commented 5 years ago

..example talk about SPIFFS...but this is a class. to send a file I'm expecting an SPIFFS.file opbject..soruce code must be checked to see what it really need...if it is a stream we may derive FileWrapper from stream (implement all missing methods)

probonopd commented 5 years ago

So maybe I need to "respond with content coming from a Stream": https://github.com/me-no-dev/ESPAsyncWebServer#respond-with-content-coming-from-a-stream

probonopd commented 5 years ago

  server.on("/download", HTTP_GET, [](AsyncWebServerRequest * request) {
    static FileWrapper gcodeFile;
    gcodeFile = storageFS.open(uploadedFullname);
    request->send(storageFS, "application/x-gcode", gcodeFile.size());
    gcodeFile.close();
  });

Does not work either, build log: https://api.travis-ci.org/v3/job/500755530/log.txt

ESP8266WirelessPrintAsync.ino: In lambda function:
ESP8266WirelessPrintAsync:583:69: error: no matching function for call to 'AsyncWebServerRequest::send(StorageFS&, const char [20], long int)'
     request->send(storageFS, "application/x-gcode", gcodeFile.size());
                                                                     ^

probonopd commented 5 years ago

/home/travis/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Stream.h:38:7: note:   no known conversion for argument 1 from 'StorageFS' to 'const Stream&'
/home/travis/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Stream.h:38:7: note: constexpr Stream::Stream(Stream&&)
/home/travis/.arduino15/packages/esp8266/hardware/esp8266/2.5.0/cores/esp8266/Stream.h:38:7: note:   no known conversion for argument 1 from 'StorageFS' to 'Stream&&'
ESP8266WirelessPrintAsync:583:27: error: cannot allocate an object of abstract type 'Stream'
     request->send((Stream)storageFS, "application/x-gcode", gcodeFile.size());
                           ^

probonopd commented 5 years ago

Being able to download a file from SPIFFS or SD using a browser seems to be much harder than anticipated. It's a pity that we have to deal with low level stuff such as whether a file is stored on SD or SPIFFS. In my opinion, this should all be abstracted away by the esp8266/Arduino classes.

@earlephilhower said in https://github.com/esp8266/Arduino/issues/4740#issuecomment-392135052

I've just created a generic File wrapper virtual base class and written SD or SPIFFS subclasses that can be passed back to the library to handle the work.

This is way over my head, but maybe we could use it?

GMagician commented 5 years ago

No it's almos easy....first you open file with storageFS.open. returned object (FileWrapper) should be passed to send method but first...FileWrapper must derive from Stream class

probonopd commented 5 years ago

I don't know what I need to do. Can you help with code?

GMagician commented 5 years ago

Well I would like to close critical issues before new feature...

GMagician commented 5 years ago

I'll open a new PR with FileWrapper subclassing...you may close this and continue on mine if necessary

probonopd / WirelessPrinting

Implement download, closes #78 #86