Conversation
kebab01
commented
Aug 24, 2023
kebab01
commented
- Added 4 new functions for FTP support
- Added a function to get the remaining space on the FS
| return SARA_R5_ERROR_OUT_OF_MEMORY; | ||
| } | ||
|
|
||
| sprintf(command, "%s=1", SARA_R5_FILE_SYSTEM_LIST_FILES); |
There was a problem hiding this comment.
Please add a comment here that AT+ULSTFILE=1 returns the Remaining free FS space expressed in bytes.
(I was worried it was doing a full LS - and would overflow the minimumResponseAllocation)
| free(response); | ||
| return err; | ||
| } | ||
| char *responseStart = strstr(response, "+ULSTFILE:"); |
There was a problem hiding this comment.
David's changes - which I like - move us away from hard-coded strings in the code. Is there an elegant way to append the ":" to SARA_R5_FILE_SYSTEM_LIST_FILES ? I.e. avoid hard-coding here; also avoid 'cheating' by defining SARA_R5_FILE_SYSTEM_LIST_FILES_RESPONSE (which includes the colon). Maybe create a new helper function which will append the colon to whatever command is passed?
| } | ||
|
|
||
| size_t availableSpace; | ||
| responseStart += strlen("+ULSTFILE:"); // Move searchPtr to first char |
|
|
||
| return SARA_R5_ERROR_SUCCESS; | ||
| } | ||
| SARA_R5_error_t SARA_R5::getAvailableSize(size_t * size) |
There was a problem hiding this comment.
Consider renaming this to getAvailableFreeSpace
| return err; | ||
| } | ||
|
|
||
| SARA_R5_error_t SARA_R5::ftpList(const String &dirName, char * response) { |
There was a problem hiding this comment.
Consider renaming this to ftpListFiles
There was a problem hiding this comment.
Humm. Thinking about this more...
The response could be quite large, depending on how many files are in the system. You could easily overrun response if you're not careful...
(I know I cheated with this overload of getFileContents because there you can know in advance how much data is coming.)
So, here, it would be best to use a String - which can grow as required - or pass in the size of response so you can stop writing before you overrun. String gets my vote - same as this overload of getFileContents.
There was a problem hiding this comment.
Where possible I avoided the use of String in my changes to avoid heap problems, even though we're using an ESP32 so heap problems go away on deep sleep :)
I had originally changed a lot of existing methods to use char buffers too, but decided that was too much of an ask for a pull request so reverted them.
I'd be wary of using so many Strings on our Cortex M0+ based nodes which don't reboot after waking up.
You haven't found any problems with long running sketches?
There was a problem hiding this comment.
Hi David (@dajtxx ),
We haven't noticed any problems but: we also tend to run this on ESP32; and we generally don't put the processor to sleep. In this case you're welcome to twist my arm and go with a char buffer, with size.
Cheers,
Paul
| SARA_R5_error_t ftpGetFile(const String& filename); | ||
| SARA_R5_error_t ftpChangeWorkingDirectory(const String& dirName); | ||
| SARA_R5_error_t ftpCreateDirectory(const String& dirName); | ||
| SARA_R5_error_t ftpList(const String& dirName, char * response); |
| SARA_R5_error_t getFileContents(String filename, char *contents); // OK for binary files. Make sure contents can hold the entire file. Get the size first with getFileSize. | ||
| SARA_R5_error_t getFileBlock(const String& filename, char* buffer, size_t offset, size_t length, size_t& bytes_read); // OK for binary files. Make sure buffer can hold the requested block size. | ||
|
|
||
| SARA_R5_error_t getAvailableSize(size_t * size); // Get available FS space |
3 participants
