From 42663b807e3cd876fbf3638dd044cf19b49944ed Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 10 Mar 2019 11:44:38 -0700 Subject: [PATCH 1/5] Reimplement SD.h write methods exactly in File Replace the individual override with the existing SD.h File's implementation for all methods of File::write. --- cores/esp8266/FS.h | 24 +++++++++++++++++++++++- tests/host/fs/test_fs.cpp | 15 +++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/cores/esp8266/FS.h b/cores/esp8266/FS.h index 07bfb61f47..12792d7c33 100644 --- a/cores/esp8266/FS.h +++ b/cores/esp8266/FS.h @@ -80,7 +80,29 @@ class File : public Stream bool isDirectory() const; // Arduino "class SD" methods for compatibility - size_t write(const char *str) { return write((const uint8_t*)str, strlen(str)); } + template size_t write(T &src){ + uint8_t obuf[512]; + size_t doneLen = 0; + size_t sentLen; + int i; + + while (src.available() > 512){ + src.read(obuf, 512); + sentLen = write(obuf, 512); + doneLen = doneLen + sentLen; + if(sentLen != 512){ + return doneLen; + } + } + + size_t leftLen = src.available(); + src.read(obuf, leftLen); + sentLen = write(obuf, leftLen); + doneLen = doneLen + sentLen; + return doneLen; + } + using Print::write; + void rewindDirectory(); File openNextFile(); diff --git a/tests/host/fs/test_fs.cpp b/tests/host/fs/test_fs.cpp index bde835dbbf..6b0d993a72 100644 --- a/tests/host/fs/test_fs.cpp +++ b/tests/host/fs/test_fs.cpp @@ -331,3 +331,18 @@ TEST_CASE("Listfiles.ino example", "[sd]") REQUIRE(readFileSD("/dir2/dir3/file4") == "bonjour"); } +TEST_CASE("Multisplendored File::writes", "[fs]") +{ + SDFS_MOCK_DECLARE(); + SDFS.end(); + SDFS.setConfig(SDFSConfig(0, SD_SCK_MHZ(1))); + REQUIRE(SDFS.format()); + REQUIRE(SD.begin(4)); + + File f = SD.open("/file.txt", FILE_WRITE); + f.write('a'); + f.write(65); + f.write("bbcc"); + f.close(); + REQUIRE(readFileSD("/file.txt") == "aAbbcc"); +} From d3a75565da280c799c10c55e3184baa7aa670235 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 10 Mar 2019 11:53:41 -0700 Subject: [PATCH 2/5] Add add'l tests --- tests/host/fs/test_fs.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/host/fs/test_fs.cpp b/tests/host/fs/test_fs.cpp index 6b0d993a72..5c39d3d582 100644 --- a/tests/host/fs/test_fs.cpp +++ b/tests/host/fs/test_fs.cpp @@ -343,6 +343,11 @@ TEST_CASE("Multisplendored File::writes", "[fs]") f.write('a'); f.write(65); f.write("bbcc"); + f.write("theend", 6); + char block[3]={'x','y','z'}; + f.write(block, 3); + uint32_t bigone = 0x40404040; + f.write((const uint8_t*)&bigone, 4); f.close(); - REQUIRE(readFileSD("/file.txt") == "aAbbcc"); + REQUIRE(readFileSD("/file.txt") == "aAbbcctheendxyz@@@@"); } From 77eca65655e46dc3d87f5b29ed579085c316504f Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 10 Mar 2019 12:06:42 -0700 Subject: [PATCH 3/5] Fix Print and File incompatible writes w/casts Print and File have ambiguous resolutions for single-argument write(0)s. Fix by adding explicit methods. A write of any integer will not be a const char* write (i.e. won't write a string) but will instead just write the integer truncated to 8 bits (as makes sense). --- cores/esp8266/FS.h | 5 +++++ tests/host/fs/test_fs.cpp | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/cores/esp8266/FS.h b/cores/esp8266/FS.h index 12792d7c33..7ee12d73fa 100644 --- a/cores/esp8266/FS.h +++ b/cores/esp8266/FS.h @@ -54,6 +54,11 @@ class File : public Stream // Print methods: size_t write(uint8_t) override; size_t write(const uint8_t *buf, size_t size) override; + size_t write(int8_t t) { return write((uint8_t)t); } + size_t write(int16_t t) { return write((uint8_t)(t & 0xff)); } + size_t write(int32_t t) { return write((uint8_t)(t & 0xff)); } + size_t write(uint16_t t) { return write((uint8_t)(t & 0xff)); } + size_t write(uint32_t t) { return write((uint8_t)(t & 0xff)); } // Stream methods: int available() override; diff --git a/tests/host/fs/test_fs.cpp b/tests/host/fs/test_fs.cpp index 5c39d3d582..322f057d9e 100644 --- a/tests/host/fs/test_fs.cpp +++ b/tests/host/fs/test_fs.cpp @@ -350,4 +350,12 @@ TEST_CASE("Multisplendored File::writes", "[fs]") f.write((const uint8_t*)&bigone, 4); f.close(); REQUIRE(readFileSD("/file.txt") == "aAbbcctheendxyz@@@@"); + File g = SD.open("/file.txt", FILE_WRITE); + g.write(0); + g.close(); + g = SD.open("/file.txt", FILE_READ); + uint8_t u = 0x66; + g.read(&u, 1); + g.close(); + REQUIRE(u == 0); } From af76764b99ec764a7a964e0f4704151522b7f510 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 10 Mar 2019 15:52:50 -0700 Subject: [PATCH 4/5] Use 256byte chunks in ::write template Reduce stack requirements for templated writes to 256bytes, matching the size uses in WiFiClient/etc. (from 512bytes). Reduces the chance of stack overflow. --- cores/esp8266/FS.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cores/esp8266/FS.h b/cores/esp8266/FS.h index 7ee12d73fa..56a285d4a0 100644 --- a/cores/esp8266/FS.h +++ b/cores/esp8266/FS.h @@ -54,6 +54,7 @@ class File : public Stream // Print methods: size_t write(uint8_t) override; size_t write(const uint8_t *buf, size_t size) override; + // These handle ambiguity for write(0) case size_t write(int8_t t) { return write((uint8_t)t); } size_t write(int16_t t) { return write((uint8_t)(t & 0xff)); } size_t write(int32_t t) { return write((uint8_t)(t & 0xff)); } @@ -86,16 +87,16 @@ class File : public Stream // Arduino "class SD" methods for compatibility template size_t write(T &src){ - uint8_t obuf[512]; + uint8_t obuf[256]; size_t doneLen = 0; size_t sentLen; int i; - while (src.available() > 512){ - src.read(obuf, 512); - sentLen = write(obuf, 512); + while (src.available() > sizeof(obuf)){ + src.read(obuf, sizeof(obuf)); + sentLen = write(obuf, sizeof(obuf)); doneLen = doneLen + sentLen; - if(sentLen != 512){ + if(sentLen != sizeof(obuf)){ return doneLen; } } From 10ac9293cae7cd7bb6174765f7c798768345475e Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sun, 10 Mar 2019 20:27:43 -0700 Subject: [PATCH 5/5] Move write(int) methods up to Print.h Remove some technical debt by moving the ::write(int/short/long) methods out of FS and HardwareSerial and up into Print.h. --- cores/esp8266/FS.h | 6 ------ cores/esp8266/HardwareSerial.h | 16 ---------------- cores/esp8266/Print.h | 7 +++++++ 3 files changed, 7 insertions(+), 22 deletions(-) diff --git a/cores/esp8266/FS.h b/cores/esp8266/FS.h index 56a285d4a0..d9c43ef3b8 100644 --- a/cores/esp8266/FS.h +++ b/cores/esp8266/FS.h @@ -54,12 +54,6 @@ class File : public Stream // Print methods: size_t write(uint8_t) override; size_t write(const uint8_t *buf, size_t size) override; - // These handle ambiguity for write(0) case - size_t write(int8_t t) { return write((uint8_t)t); } - size_t write(int16_t t) { return write((uint8_t)(t & 0xff)); } - size_t write(int32_t t) { return write((uint8_t)(t & 0xff)); } - size_t write(uint16_t t) { return write((uint8_t)(t & 0xff)); } - size_t write(uint32_t t) { return write((uint8_t)(t & 0xff)); } // Stream methods: int available() override; diff --git a/cores/esp8266/HardwareSerial.h b/cores/esp8266/HardwareSerial.h index 9d50be55e5..2ede7df43b 100644 --- a/cores/esp8266/HardwareSerial.h +++ b/cores/esp8266/HardwareSerial.h @@ -152,22 +152,6 @@ class HardwareSerial: public Stream { return uart_write_char(_uart, c); } - inline size_t write(unsigned long n) - { - return write((uint8_t) n); - } - inline size_t write(long n) - { - return write((uint8_t) n); - } - inline size_t write(unsigned int n) - { - return write((uint8_t) n); - } - inline size_t write(int n) - { - return write((uint8_t) n); - } size_t write(const uint8_t *buffer, size_t size) override { return uart_write(_uart, (const char*)buffer, size); diff --git a/cores/esp8266/Print.h b/cores/esp8266/Print.h index 73a955b4dd..b92bcc6e74 100644 --- a/cores/esp8266/Print.h +++ b/cores/esp8266/Print.h @@ -62,6 +62,13 @@ class Print { size_t write(const char *buffer, size_t size) { return write((const uint8_t *) buffer, size); } + // These handle ambiguity for write(0) case, because (0) can be a pointer or an integer + size_t write(short t) { return write((uint8_t)t); } + size_t write(unsigned short t) { return write((uint8_t)t); } + size_t write(int t) { return write((uint8_t)t); } + size_t write(unsigned int t) { return write((uint8_t)t); } + size_t write(long t) { return write((uint8_t)t); } + size_t write(unsigned long t) { return write((uint8_t)t); } size_t printf(const char * format, ...) __attribute__ ((format (printf, 2, 3))); size_t printf_P(PGM_P format, ...) __attribute__((format(printf, 2, 3)));