From f7225be62c9ef1342787e78aa9e9c8429475c1d4 Mon Sep 17 00:00:00 2001 From: Mike Date: Tue, 18 Jun 2024 09:25:32 +0100 Subject: [PATCH 1/5] Use `intptr_t` etc. when casting pointers (#2811) Assuming that, for example, `void*` can be cast to `uint32_t` and vice-versa is not portable since platforms can have different sizes. Correct types to use are `intptr_t` and `uintptr_t` which are 32-bit for all the architectures we care about, and 64-bit for modern OS builds without 32-bit compatibility mode. It also makes it clearer that values are likely cast from pointers. --- .../Host/Components/driver/include/driver/os_timer.h | 2 +- Sming/Arch/Host/Components/driver/os_timer.cpp | 2 +- Sming/Arch/Host/Components/esp_hal/include/esp_tasks.h | 8 ++++---- Sming/Arch/Host/Components/esp_hal/tasks.cpp | 2 +- .../Network/Arch/Host/Platform/StationImpl.cpp | 8 ++++---- .../Network/src/Data/Stream/ChunkedStream.cpp | 3 ++- Sming/Components/Storage/src/include/Storage/SysMem.h | 2 +- .../rboot/include/Network/RbootHttpUpdater.h | 5 +++-- Sming/Components/ssl/Axtls/AxContext.cpp | 4 ++-- Sming/Core/CallbackTimer.h | 2 +- Sming/Core/Data/Buffer/CircularBuffer.h | 2 +- Sming/Core/Task.h | 4 ++-- Sming/Libraries/FlashIP | 2 +- Sming/Libraries/IOControl | 2 +- Sming/Libraries/Spiffs/src/FileSystem.cpp | 2 +- Sming/Libraries/TFT_S1D13781 | 2 +- Sming/Libraries/UPnP | 2 +- Sming/Platform/System.cpp | 9 +++++---- Sming/Platform/System.h | 10 +++++----- Sming/Wiring/FakePgmSpace.h | 2 +- samples/Basic_ProgMem/app/application.cpp | 4 ++-- tests/HostTests/modules/Clocks.cpp | 2 +- tests/HostTests/modules/Serial.cpp | 2 +- 23 files changed, 43 insertions(+), 40 deletions(-) diff --git a/Sming/Arch/Host/Components/driver/include/driver/os_timer.h b/Sming/Arch/Host/Components/driver/include/driver/os_timer.h index 4502fc2e23..fc8ae7affc 100644 --- a/Sming/Arch/Host/Components/driver/include/driver/os_timer.h +++ b/Sming/Arch/Host/Components/driver/include/driver/os_timer.h @@ -52,7 +52,7 @@ void os_timer_setfn(os_timer_t* ptimer, os_timer_func_t* pfunction, void* parg); static inline uint64_t os_timer_expire(const os_timer_t* ptimer) { - if(ptimer == nullptr || int(ptimer->timer_next) == -1) { + if(ptimer == nullptr || intptr_t(ptimer->timer_next) == -1) { return 0; } return ptimer->timer_expire; diff --git a/Sming/Arch/Host/Components/driver/os_timer.cpp b/Sming/Arch/Host/Components/driver/os_timer.cpp index e546c35256..1d0f77f4dd 100644 --- a/Sming/Arch/Host/Components/driver/os_timer.cpp +++ b/Sming/Arch/Host/Components/driver/os_timer.cpp @@ -67,7 +67,7 @@ void os_timer_disarm(struct os_timer_t* ptimer) { assert(ptimer != nullptr); - if(int(ptimer->timer_next) == -1) { + if(intptr_t(ptimer->timer_next) == -1) { return; } diff --git a/Sming/Arch/Host/Components/esp_hal/include/esp_tasks.h b/Sming/Arch/Host/Components/esp_hal/include/esp_tasks.h index be1d289e63..48882c54da 100644 --- a/Sming/Arch/Host/Components/esp_hal/include/esp_tasks.h +++ b/Sming/Arch/Host/Components/esp_hal/include/esp_tasks.h @@ -6,8 +6,8 @@ extern "C" { #endif -typedef uint32_t os_signal_t; -typedef uint32_t os_param_t; +typedef uintptr_t os_signal_t; +typedef uintptr_t os_param_t; typedef struct { os_signal_t sig; @@ -32,9 +32,9 @@ void host_init_tasks(); // Hook function to process task queues void host_service_tasks(); -typedef void (*host_task_callback_t)(uint32_t param); +typedef void (*host_task_callback_t)(os_param_t param); -bool host_queue_callback(host_task_callback_t callback, uint32_t param); +bool host_queue_callback(host_task_callback_t callback, os_param_t param); #ifdef __cplusplus } diff --git a/Sming/Arch/Host/Components/esp_hal/tasks.cpp b/Sming/Arch/Host/Components/esp_hal/tasks.cpp index bd242df133..1c561564a0 100644 --- a/Sming/Arch/Host/Components/esp_hal/tasks.cpp +++ b/Sming/Arch/Host/Components/esp_hal/tasks.cpp @@ -114,7 +114,7 @@ void host_service_tasks() } } -bool host_queue_callback(host_task_callback_t callback, uint32_t param) +bool host_queue_callback(host_task_callback_t callback, os_param_t param) { return task_queues[HOST_TASK_PRIO]->post(os_signal_t(callback), param); } diff --git a/Sming/Components/Network/Arch/Host/Platform/StationImpl.cpp b/Sming/Components/Network/Arch/Host/Platform/StationImpl.cpp index 25a71a9021..8b496710a4 100644 --- a/Sming/Components/Network/Arch/Host/Platform/StationImpl.cpp +++ b/Sming/Components/Network/Arch/Host/Platform/StationImpl.cpp @@ -89,8 +89,8 @@ void StationImpl::initialise(netif* nif) } auto netif_callback = [](netif* nif) { - host_queue_callback([](uint32_t param) { station.statusCallback(reinterpret_cast(param)); }, - uint32_t(nif)); + host_queue_callback([](os_param_t param) { station.statusCallback(reinterpret_cast(param)); }, + os_param_t(nif)); }; netif_set_status_callback(nif, netif_callback); @@ -328,7 +328,7 @@ bool StationImpl::startScan(ScanCompletedDelegate scanCompleted) } host_queue_callback( - [](uint32_t param) { + [](os_param_t param) { auto self = reinterpret_cast(param); BssList list; for(const auto& info : apInfoList) { @@ -336,7 +336,7 @@ bool StationImpl::startScan(ScanCompletedDelegate scanCompleted) } self->scanCompletedCallback(true, list); }, - uint32_t(this)); + os_param_t(this)); return true; } diff --git a/Sming/Components/Network/src/Data/Stream/ChunkedStream.cpp b/Sming/Components/Network/src/Data/Stream/ChunkedStream.cpp index be8df057d3..8b8c14b0ce 100644 --- a/Sming/Components/Network/src/Data/Stream/ChunkedStream.cpp +++ b/Sming/Components/Network/src/Data/Stream/ChunkedStream.cpp @@ -25,7 +25,8 @@ size_t ChunkedStream::transform(const uint8_t* source, size_t sourceLength, uint } // Header - unsigned offset = m_snprintf(reinterpret_cast(target), targetLength, "%X\r\n", sourceLength); + unsigned offset = + m_snprintf(reinterpret_cast(target), targetLength, "%X\r\n", static_cast(sourceLength)); // Content memcpy(target + offset, source, sourceLength); diff --git a/Sming/Components/Storage/src/include/Storage/SysMem.h b/Sming/Components/Storage/src/include/Storage/SysMem.h index 7b6f20361d..1193e5722c 100644 --- a/Sming/Components/Storage/src/include/Storage/SysMem.h +++ b/Sming/Components/Storage/src/include/Storage/SysMem.h @@ -80,7 +80,7 @@ class SysMem : public Device */ Partition add(const String& name, const FSTR::ObjectBase& fstr, Partition::FullType type) { - return PartitionTable::add(name, type, reinterpret_cast(fstr.data()), fstr.size(), + return PartitionTable::add(name, type, reinterpret_cast(fstr.data()), fstr.size(), Partition::Flag::readOnly); } }; diff --git a/Sming/Components/rboot/include/Network/RbootHttpUpdater.h b/Sming/Components/rboot/include/Network/RbootHttpUpdater.h index fda0ce60c8..c83f13bf95 100644 --- a/Sming/Components/rboot/include/Network/RbootHttpUpdater.h +++ b/Sming/Components/rboot/include/Network/RbootHttpUpdater.h @@ -36,7 +36,7 @@ class RbootHttpUpdater : protected HttpClient size_t size; // << max allowed size std::unique_ptr stream; // (optional) output stream to use. - Item(String url, uint32_t targetOffset, size_t size, RbootOutputStream* stream) + Item(String url, size_t targetOffset, size_t size, RbootOutputStream* stream) : url(url), targetOffset(targetOffset), size(size), stream(stream) { } @@ -99,7 +99,8 @@ class RbootHttpUpdater : protected HttpClient return false; } - return items.addNew(new Item{firmwareFileUrl, stream->getStartAddress(), stream->getMaxLength(), stream}); + return items.addNew( + new Item{firmwareFileUrl, uint32_t(stream->getStartAddress()), stream->getMaxLength(), stream}); } void start(); diff --git a/Sming/Components/ssl/Axtls/AxContext.cpp b/Sming/Components/ssl/Axtls/AxContext.cpp index 7283d3d910..03195386d1 100644 --- a/Sming/Components/ssl/Axtls/AxContext.cpp +++ b/Sming/Components/ssl/Axtls/AxContext.cpp @@ -89,7 +89,7 @@ Connection* AxContext::createClient(tcp_pcb* tcp) auto id = session.getSessionId(); auto connection = new AxConnection(*this, tcp); auto client = - ssl_client_new(context, int(connection), id ? id->getValue() : nullptr, id ? id->getLength() : 0, ssl_ext); + ssl_client_new(context, intptr_t(connection), id ? id->getValue() : nullptr, id ? id->getLength() : 0, ssl_ext); if(client == nullptr) { ssl_ext_free(ssl_ext); delete connection; @@ -105,7 +105,7 @@ Connection* AxContext::createServer(tcp_pcb* tcp) assert(context != nullptr); auto connection = new AxConnection(*this, tcp); - auto server = ssl_server_new(context, int(connection)); + auto server = ssl_server_new(context, intptr_t(connection)); if(server == nullptr) { delete connection; return nullptr; diff --git a/Sming/Core/CallbackTimer.h b/Sming/Core/CallbackTimer.h index f56ef2dc58..89eed46d0a 100644 --- a/Sming/Core/CallbackTimer.h +++ b/Sming/Core/CallbackTimer.h @@ -44,7 +44,7 @@ template struct CallbackTimerApi { String s; s += typeName(); s += '@'; - s += String(uint32_t(this), HEX); + s += String(uintptr_t(this), HEX); return s; } diff --git a/Sming/Core/Data/Buffer/CircularBuffer.h b/Sming/Core/Data/Buffer/CircularBuffer.h index fad71b793e..ec1c63e0fd 100644 --- a/Sming/Core/Data/Buffer/CircularBuffer.h +++ b/Sming/Core/Data/Buffer/CircularBuffer.h @@ -80,7 +80,7 @@ class CircularBuffer : public ReadWriteStream */ String id() const override { - return String(reinterpret_cast(&buffer), HEX); + return String(reinterpret_cast(&buffer), HEX); } size_t write(uint8_t charToWrite) override; diff --git a/Sming/Core/Task.h b/Sming/Core/Task.h index 1c04dc72e0..bb82f73896 100644 --- a/Sming/Core/Task.h +++ b/Sming/Core/Task.h @@ -164,12 +164,12 @@ class Task } scheduled = System.queueCallback( - [](uint32_t param) { + [](void* param) { auto task = reinterpret_cast(param); task->scheduled = false; task->service(); }, - uint32_t(this)); + this); return scheduled; } diff --git a/Sming/Libraries/FlashIP b/Sming/Libraries/FlashIP index 1e67f9a47d..5d1dae0541 160000 --- a/Sming/Libraries/FlashIP +++ b/Sming/Libraries/FlashIP @@ -1 +1 @@ -Subproject commit 1e67f9a47d9e9f479c1b2c34d36800994515c8c2 +Subproject commit 5d1dae05417171ea23081b44a5401661bbefb599 diff --git a/Sming/Libraries/IOControl b/Sming/Libraries/IOControl index cd42d86b26..d297b3ae71 160000 --- a/Sming/Libraries/IOControl +++ b/Sming/Libraries/IOControl @@ -1 +1 @@ -Subproject commit cd42d86b26901af9fedddb04a933807c5206ee5d +Subproject commit d297b3ae715e5fac82425e7c316cbc0cdd919cd4 diff --git a/Sming/Libraries/Spiffs/src/FileSystem.cpp b/Sming/Libraries/Spiffs/src/FileSystem.cpp index 7bc5f373cb..6e79df5ee8 100644 --- a/Sming/Libraries/Spiffs/src/FileSystem.cpp +++ b/Sming/Libraries/Spiffs/src/FileSystem.cpp @@ -152,7 +152,7 @@ int FileSystem::mount() .hal_erase_f = f_erase, .phys_size = uint32_t(partSize), .phys_addr = 0, - .phys_erase_block = partition.getBlockSize(), + .phys_erase_block = uint32_t(partition.getBlockSize()), .log_block_size = logicalBlockSize, .log_page_size = LOG_PAGE_SIZE, }; diff --git a/Sming/Libraries/TFT_S1D13781 b/Sming/Libraries/TFT_S1D13781 index 3d5ddab2f7..04e19b5003 160000 --- a/Sming/Libraries/TFT_S1D13781 +++ b/Sming/Libraries/TFT_S1D13781 @@ -1 +1 @@ -Subproject commit 3d5ddab2f7dec460ce649ca69bc5ab625c2c7215 +Subproject commit 04e19b5003be0d1edacdb07b9b511e3b1de60af1 diff --git a/Sming/Libraries/UPnP b/Sming/Libraries/UPnP index 2700835f8e..17ea2953be 160000 --- a/Sming/Libraries/UPnP +++ b/Sming/Libraries/UPnP @@ -1 +1 @@ -Subproject commit 2700835f8ef23efd415751db0a7843b1626cb8da +Subproject commit 17ea2953be95336e1b0cae3090fe992caab56b0a diff --git a/Sming/Platform/System.cpp b/Sming/Platform/System.cpp index c490fc3018..63a08d95a9 100644 --- a/Sming/Platform/System.cpp +++ b/Sming/Platform/System.cpp @@ -46,9 +46,9 @@ void SystemClass::taskHandler(os_event_t* event) --taskCount; restoreInterrupts(level); #endif - auto callback = reinterpret_cast(event->sig); + auto callback = reinterpret_cast(event->sig); if(callback != nullptr) { - callback(event->par); + callback(reinterpret_cast(event->par)); } } @@ -74,7 +74,7 @@ bool SystemClass::initialize() return true; } -bool SystemClass::queueCallback(TaskCallback32 callback, uint32_t param) +bool SystemClass::queueCallback(TaskCallback callback, void* param) { if(callback == nullptr) { return false; @@ -89,7 +89,8 @@ bool SystemClass::queueCallback(TaskCallback32 callback, uint32_t param) restoreInterrupts(level); #endif - return system_os_post(USER_TASK_PRIO_1, reinterpret_cast(callback), param); + return system_os_post(USER_TASK_PRIO_1, reinterpret_cast(callback), + reinterpret_cast(param)); } bool SystemClass::queueCallback(TaskDelegate callback) diff --git a/Sming/Platform/System.h b/Sming/Platform/System.h index ac5467527b..c4cca4fd9f 100644 --- a/Sming/Platform/System.h +++ b/Sming/Platform/System.h @@ -191,15 +191,15 @@ class SystemClass * Note also that this method is typically called from interrupt context so must avoid things * like heap allocation, etc. */ - static bool IRAM_ATTR queueCallback(TaskCallback32 callback, uint32_t param = 0); + static bool IRAM_ATTR queueCallback(TaskCallback32 callback, uint32_t param = 0) + { + return queueCallback(reinterpret_cast(callback), reinterpret_cast(param)); + } /** * @brief Queue a deferred callback, with optional void* parameter */ - __forceinline static bool IRAM_ATTR queueCallback(TaskCallback callback, void* param = nullptr) - { - return queueCallback(reinterpret_cast(callback), reinterpret_cast(param)); - } + static bool IRAM_ATTR queueCallback(TaskCallback callback, void* param = nullptr); /** * @brief Queue a deferred callback with no callback parameter diff --git a/Sming/Wiring/FakePgmSpace.h b/Sming/Wiring/FakePgmSpace.h index fae56002c7..82fd39066b 100644 --- a/Sming/Wiring/FakePgmSpace.h +++ b/Sming/Wiring/FakePgmSpace.h @@ -31,7 +31,7 @@ extern "C" { /** * @brief determines if the given value is aligned to a word (4-byte) boundary */ -#define IS_ALIGNED(_x) (((uint32_t)(_x)&3) == 0) +#define IS_ALIGNED(_x) (((uintptr_t)(_x)&3) == 0) /** * @brief Align a size up to the nearest word boundary diff --git a/samples/Basic_ProgMem/app/application.cpp b/samples/Basic_ProgMem/app/application.cpp index abdc5d24f4..8bccf27b0d 100644 --- a/samples/Basic_ProgMem/app/application.cpp +++ b/samples/Basic_ProgMem/app/application.cpp @@ -119,6 +119,6 @@ void init() Serial.println("> 0x3FFE8000 ~ 0x3FFFBFFF - User data RAM, 80kb. Available to applications."); Serial.println("> 0x40200000 ~ ... - SPI Flash."); - Serial << "> demoRam array address: 0x" << String(uint32_t(demoRam), HEX) << " is in the RAM" << endl; - Serial << "> demoPgm array address: 0x" << String(uint32_t(demoPgm), HEX) << " is in the Flash" << endl; + Serial << "> demoRam array address: 0x" << String(uintptr_t(demoRam), HEX) << " is in the RAM" << endl; + Serial << "> demoPgm array address: 0x" << String(uintptr_t(demoPgm), HEX) << " is in the Flash" << endl; } diff --git a/tests/HostTests/modules/Clocks.cpp b/tests/HostTests/modules/Clocks.cpp index b6c78fc775..b16097a04a 100644 --- a/tests/HostTests/modules/Clocks.cpp +++ b/tests/HostTests/modules/Clocks.cpp @@ -50,7 +50,7 @@ template class ClockTestTemplate : public TestG // Run for a second or two and check timer ticks correspond approximately with system clock constexpr uint64_t maxDuration = Clock::maxTicks().template as() - 5000ULL; - constexpr uint32_t duration = std::min(2000000ULL, maxDuration); + constexpr uint32_t duration = std::min(uint64_t(2000000ULL), maxDuration); auto startTime = system_get_time(); startTicks = Clock::ticks(); uint32_t time; diff --git a/tests/HostTests/modules/Serial.cpp b/tests/HostTests/modules/Serial.cpp index 069e98e22a..544f1f6bf2 100644 --- a/tests/HostTests/modules/Serial.cpp +++ b/tests/HostTests/modules/Serial.cpp @@ -41,7 +41,7 @@ class SerialTest : public TestGroup String compareBuffer; String readBuffer; for(unsigned i = 0; i < 10; ++i) { - m_printf("txfree = %u\n", txbuf.getFreeSpace()); + Serial << _F("txfree = ") << txbuf.getFreeSpace() << endl; for(char c = 'a'; c <= 'z'; ++c) { if(txbuf.getFreeSpace() < 10) { readBuffer += read(); From 17db0dbc89d43894e9ed728090f83f7eb9392d68 Mon Sep 17 00:00:00 2001 From: Mike Date: Tue, 18 Jun 2024 09:25:57 +0100 Subject: [PATCH 2/5] Fix GoogleCast reference (#2808) --- Sming/Libraries/GoogleCast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sming/Libraries/GoogleCast b/Sming/Libraries/GoogleCast index 4a95b75612..930d641cd9 160000 --- a/Sming/Libraries/GoogleCast +++ b/Sming/Libraries/GoogleCast @@ -1 +1 @@ -Subproject commit 4a95b756127799ef25697fa6bca28c35094eec52 +Subproject commit 930d641cd93862b554633e860afb6104de41b3c1 From 69c4294f70c2ffa56dda199e03c2b93c8ff0b747 Mon Sep 17 00:00:00 2001 From: Mike Date: Tue, 18 Jun 2024 09:26:22 +0100 Subject: [PATCH 3/5] Fix spiFlash type mismatch warning during init (#2809) This PR makes a small change to Host builds, enabling all `debug_x` statements during init/exit. This highlights one issue, fixed, caused by missing device entry in Rp2040, Host hardware configs: ``` [Device] 'spiFlash' type mismatch, 'unknown' in partition table but device reports 'flash' ``` --- Sming/Arch/Host/Components/hostlib/startup.cpp | 4 +++- Sming/Arch/Host/standard.hw | 7 ++++++- Sming/Arch/Rp2040/standard.hw | 9 ++++++--- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Sming/Arch/Host/Components/hostlib/startup.cpp b/Sming/Arch/Host/Components/hostlib/startup.cpp index 3fce4bec04..f74fdf3405 100644 --- a/Sming/Arch/Host/Components/hostlib/startup.cpp +++ b/Sming/Arch/Host/Components/hostlib/startup.cpp @@ -246,6 +246,8 @@ int main(int argc, char* argv[]) } } + m_setPuts(&host_nputs); + host_debug_i("\nWelcome to the Sming Host emulator\n\n"); auto i = get_first_non_option(); @@ -309,7 +311,7 @@ int main(int argc, char* argv[]) pause(config.exitpause); // Avoid issues with debug statements whilst running exit handlers - m_setPuts(nullptr); + m_setPuts(&host_nputs); return exitCode; } diff --git a/Sming/Arch/Host/standard.hw b/Sming/Arch/Host/standard.hw index 7244be0578..287e348737 100644 --- a/Sming/Arch/Host/standard.hw +++ b/Sming/Arch/Host/standard.hw @@ -3,7 +3,12 @@ "arch": "Host", "bootloader_size": "0x2000", "partition_table_offset": "0x2000", - "options": ["4m"], + "devices": { + "spiFlash": { + "type": "flash", + "size": "4M" + } + }, "partitions": { "rom0": { "address": "0x008000", diff --git a/Sming/Arch/Rp2040/standard.hw b/Sming/Arch/Rp2040/standard.hw index d143cd3ba0..2955c37e9d 100644 --- a/Sming/Arch/Rp2040/standard.hw +++ b/Sming/Arch/Rp2040/standard.hw @@ -4,9 +4,12 @@ "arch": "Rp2040", "bootloader_size": 0, "partition_table_offset": "self.devices[0].size - 0x1000", - "options": [ - "2m" - ], + "devices": { + "spiFlash": { + "type": "flash", + "size": "2M" + } + }, "partitions": { "rom0": { "address": 0, From 7020094accbf752deee1cb930676ec7765aeab9f Mon Sep 17 00:00:00 2001 From: Mike Date: Tue, 18 Jun 2024 09:28:11 +0100 Subject: [PATCH 4/5] Extend `CsvReader` capabilities, move into new library (#2805) This PR extends the capabilities `CsvReader` class, introduced in #2403. Goals: - Extend testing, verify efficient use of memory - Add Parser capability to allow filtering and processing of (very) large files streamed via network, in files, etc. - Add seeking support to allow indexing, bookmarking, etc. - Add iterator support The code has been moved into a separate library. Changes: **Fix String move to avoid de-allocating buffers** Use longer of two buffers for result. Example: ``` String s1 = "Greater than SSO buffer length"; String s2; s1 = ""; // Buffer remains allocated s2 = std::move(s1); // The move we need to fix ``` At present this move will result in s1's buffer being de-allocated. This can lead to performance degratation due to subsequent memory reallocations where a String is being passed around as a general buffer. This change uses the larger of the two buffers when deciding how to behave. Checks added to HostTests to enforce predictable behaviour. **Add CStringArray::release method** Allows efficient conversion to a regular `String` object for manipulation. **Fix CStringArray operator+=** Must take reference, not copy - inefficient and fails when used with FlashString. **Move CsvReader into separate library** The `CsvReader` class has been moved out of `Core/Data` and into `CsvReader` which has additional capabilities. Changes to existing code: - Add ``CsvReader`` to your project's :cpp:envvar:`COMPONENT_DEPENDS` - Change ``#include `` to ``#include `` - Change ``CsvReader`` class to :cpp:class:`CSV::Reader` --- .gitmodules | 4 + Sming/Core/Data/CStringArray.h | 10 +- Sming/Core/Data/CsvReader.cpp | 124 ------------------- Sming/Core/Data/CsvReader.h | 130 +------------------- Sming/Libraries/CsvReader | 1 + Sming/Wiring/WString.cpp | 6 +- docs/source/upgrading/5.1-5.2.rst | 10 ++ samples/Basic_Templates/app/CsvTemplate.h | 4 +- samples/Basic_Templates/app/application.cpp | 3 +- samples/Basic_Templates/component.mk | 1 + tests/HostTests/modules/CStringArray.cpp | 26 ++++ tests/HostTests/modules/String.cpp | 64 ++++++++-- tests/HostTests/modules/TemplateStream.cpp | 27 ---- 13 files changed, 112 insertions(+), 298 deletions(-) delete mode 100644 Sming/Core/Data/CsvReader.cpp create mode 160000 Sming/Libraries/CsvReader diff --git a/.gitmodules b/.gitmodules index 9d62de59ac..a1404fb3cd 100644 --- a/.gitmodules +++ b/.gitmodules @@ -217,6 +217,10 @@ path = Sming/Libraries/CS5460/CS5460 url = https://github.com/xxzl0130/CS5460.git ignore = dirty +[submodule "Libraries.CsvReader"] + path = Sming/Libraries/CsvReader + url = https://github.com/mikee47/CsvReader + ignore = dirty [submodule "Libraries.DFRobotDFPlayerMini"] path = Sming/Libraries/DFRobotDFPlayerMini url = https://github.com/DFRobot/DFRobotDFPlayerMini.git diff --git a/Sming/Core/Data/CStringArray.h b/Sming/Core/Data/CStringArray.h index 27afb9a66c..3b192c6b54 100644 --- a/Sming/Core/Data/CStringArray.h +++ b/Sming/Core/Data/CStringArray.h @@ -89,6 +89,14 @@ class CStringArray : private String return *this; } + /** + * @brief Give up underlying String object to caller so it can be manipulated + */ + String release() + { + return std::move(*this); + } + /** @brief Append a new string (or array of strings) to the end of the array * @param str * @param length Length of new string in array (default is length of str) @@ -127,7 +135,7 @@ class CStringArray : private String * @brief Append numbers, etc. to the array * @param value char, int, float, etc. as supported by String */ - template CStringArray& operator+=(T value) + template CStringArray& operator+=(const T& value) { add(String(value)); return *this; diff --git a/Sming/Core/Data/CsvReader.cpp b/Sming/Core/Data/CsvReader.cpp deleted file mode 100644 index 569ce02a0b..0000000000 --- a/Sming/Core/Data/CsvReader.cpp +++ /dev/null @@ -1,124 +0,0 @@ -/**** - * Sming Framework Project - Open Source framework for high efficiency native ESP8266 development. - * Created 2015 by Skurydin Alexey - * http://github.com/SmingHub/Sming - * All files of the Sming Core are provided under the LGPL v3 license. - * - * CsvReader.cpp - * - * @author: 2021 - Mikee47 - * - ****/ - -#include "CsvReader.h" -#include - -void CsvReader::reset() -{ - source->seekFrom(0, SeekOrigin::Start); - if(!userHeadingsProvided) { - readRow(); - headings = row; - } - row = nullptr; -} - -bool CsvReader::readRow() -{ - constexpr size_t blockSize{512}; - - String buffer(std::move(reinterpret_cast(row))); - constexpr char quoteChar{'"'}; - enum class FieldKind { - unknown, - quoted, - unquoted, - }; - FieldKind fieldKind{}; - bool escape{false}; - bool quote{false}; - char lc{'\0'}; - unsigned writepos{0}; - - while(true) { - if(buffer.length() == maxLineLength) { - debug_w("[CSV] Line buffer limit reached %u", maxLineLength); - return false; - } - size_t buflen = std::min(writepos + blockSize, maxLineLength); - if(!buffer.setLength(buflen)) { - debug_e("[CSV] Out of memory %u", buflen); - return false; - } - auto len = source->readBytes(buffer.begin() + writepos, buflen - writepos); - if(len == 0) { - if(writepos == 0) { - return false; - } - buffer.setLength(writepos); - row = std::move(buffer); - return true; - } - buflen = writepos + len; - unsigned readpos = writepos; - - for(; readpos < buflen; ++readpos) { - char c = buffer[readpos]; - if(escape) { - switch(c) { - case 'n': - c = '\n'; - break; - case 'r': - c = '\r'; - break; - case 't': - c = '\t'; - break; - default:; - // Just accept character - } - escape = false; - } else { - if(fieldKind == FieldKind::unknown) { - if(c == quoteChar) { - fieldKind = FieldKind::quoted; - quote = true; - lc = '\0'; - continue; - } - fieldKind = FieldKind::unquoted; - } - if(c == quoteChar) { - quote = !quote; - if(fieldKind == FieldKind::quoted) { - if(lc == quoteChar) { - buffer[writepos++] = c; - lc = '\0'; - } else { - lc = c; - } - continue; - } - } else if(c == '\\') { - escape = true; - continue; - } else if(!quote) { - if(c == fieldSeparator) { - c = '\0'; - fieldKind = FieldKind::unknown; - } else if(c == '\r') { - continue; - } else if(c == '\n') { - source->seekFrom(readpos + 1 - buflen, SeekOrigin::Current); - buffer.setLength(writepos); - row = std::move(buffer); - return true; - } - } - } - buffer[writepos++] = c; - lc = c; - } - } -} diff --git a/Sming/Core/Data/CsvReader.h b/Sming/Core/Data/CsvReader.h index 596a62a27e..2fcd76036a 100644 --- a/Sming/Core/Data/CsvReader.h +++ b/Sming/Core/Data/CsvReader.h @@ -12,132 +12,4 @@ #pragma once -#include "Stream/DataSourceStream.h" -#include "CStringArray.h" -#include - -/** - * @brief Class to parse a CSV file - * - * Spec: https://www.ietf.org/rfc/rfc4180.txt - * - * 1. Each record is located on a separate line - * 2. Line ending for last record in the file is optional - * 3. Field headings are provided either in the source data or in constructor (but not both) - * 4. Fields separated with ',' and whitespace considered part of field content - * 5. Fields may or may not be quoted - if present, will be removed during parsing - * 6. Fields may contain line breaks, quotes or commas - * 7. Quotes may be escaped thus "" if field itself is quoted - * - * Additional features: - * - * - Line breaks can be \n or \r\n - * - Escapes codes within fields will be converted: \n \r \t \", \\ - * - Field separator can be changed in constructor - */ -class CsvReader -{ -public: - /** - * @brief Construct a CSV reader - * @param source Stream to read CSV text from - * @param fieldSeparator - * @param headings Required if source data does not contain field headings as first row - * @param maxLineLength Limit size of buffer to guard against malformed data - */ - CsvReader(IDataSourceStream* source, char fieldSeparator = ',', const CStringArray& headings = nullptr, - size_t maxLineLength = 2048) - : source(source), fieldSeparator(fieldSeparator), userHeadingsProvided(headings), maxLineLength(maxLineLength), - headings(headings) - { - reset(); - } - - /** - * @brief Reset reader to start of CSV file - * - * Cursor is set to 'before start'. - * Call 'next()' to fetch first record. - */ - void reset(); - - /** - * @brief Seek to next record - */ - bool next() - { - return readRow(); - } - - /** - * @brief Get number of columns - */ - unsigned count() const - { - return headings.count(); - } - - /** - * @brief Get a value from the current row - * @param index Column index, starts at 0 - * @retval const char* nullptr if index is not valid - */ - const char* getValue(unsigned index) - { - return row[index]; - } - - /** - * @brief Get a value from the current row - * @param index Column name - * @retval const char* nullptr if name is not found - */ - const char* getValue(const char* name) - { - return getValue(getColumn(name)); - } - - /** - * @brief Get index of column given its name - * @param name Column name to find - * @retval int -1 if name is not found - */ - int getColumn(const char* name) - { - return headings.indexOf(name); - } - - /** - * @brief Determine if row is valid - */ - explicit operator bool() const - { - return bool(row); - } - - /** - * @brief Get headings - */ - const CStringArray& getHeadings() const - { - return headings; - } - - /** - * @brief Get current row - */ - const CStringArray& getRow() const - { - return row; - } - -private: - bool readRow(); - - std::unique_ptr source; - char fieldSeparator; - bool userHeadingsProvided; - size_t maxLineLength; - CStringArray headings; - CStringArray row; -}; +static_assert(false, "CsvReader class has been moved to the CsvReader library."); diff --git a/Sming/Libraries/CsvReader b/Sming/Libraries/CsvReader new file mode 160000 index 0000000000..8f4d416442 --- /dev/null +++ b/Sming/Libraries/CsvReader @@ -0,0 +1 @@ +Subproject commit 8f4d416442292927d15fe00d80130fb2fc7d8bb6 diff --git a/Sming/Wiring/WString.cpp b/Sming/Wiring/WString.cpp index b2d3b83aac..401851540b 100644 --- a/Sming/Wiring/WString.cpp +++ b/Sming/Wiring/WString.cpp @@ -276,16 +276,14 @@ void String::move(String& rhs) (void)reserve(rhs_len); } - // If we already have capacity, copy the data and free rhs buffers - if(capacity() >= rhs_len) { + // If we have more capacity than the target, copy the data and free rhs buffers + if(rhs.sso.set || capacity() > rhs.capacity()) { memmove(buffer(), rhs.buffer(), rhs_len); setlen(rhs_len); rhs.invalidate(); return; } - assert(!rhs.sso.set); - // We don't have enough space so perform a pointer swap if(!sso.set) { free(ptr.buffer); diff --git a/docs/source/upgrading/5.1-5.2.rst b/docs/source/upgrading/5.1-5.2.rst index 96f7fc41a4..cc8815a84f 100644 --- a/docs/source/upgrading/5.1-5.2.rst +++ b/docs/source/upgrading/5.1-5.2.rst @@ -91,3 +91,13 @@ These will now fail to compile. Copy construction and assignment has been explicitly deleted so avoid unintentional side-effects. Objects should always be passed by reference. + + +**CsvReader library** + +The :cpp:type:`CsvReader` class has been moved out of ``Core/Data`` and into :library:`CsvReader` +which has additional capabilities. Changes to existing code: + +- Add ``CsvReader`` to your project's :cpp:envvar:`COMPONENT_DEPENDS` +- Change ``#include `` to ``#include `` +- Change ``CsvReader`` class to :cpp:class:`CSV::Reader` diff --git a/samples/Basic_Templates/app/CsvTemplate.h b/samples/Basic_Templates/app/CsvTemplate.h index b29b4ae349..cdf56fdd8a 100644 --- a/samples/Basic_Templates/app/CsvTemplate.h +++ b/samples/Basic_Templates/app/CsvTemplate.h @@ -1,6 +1,6 @@ #pragma once -#include +#include #include /** @@ -35,5 +35,5 @@ class CsvTemplate : public SectionTemplate } private: - CsvReader csv; + CSV::Reader csv; }; diff --git a/samples/Basic_Templates/app/application.cpp b/samples/Basic_Templates/app/application.cpp index efca95a384..a2bbae2d40 100644 --- a/samples/Basic_Templates/app/application.cpp +++ b/samples/Basic_Templates/app/application.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include "CsvTemplate.h" namespace @@ -51,7 +50,7 @@ void printCars() void printClassics(const FlashString& templateSource, Format::Formatter& formatter) { // The CSV data source - CsvReader csv(new FileStream(Filename::classics_csv)); + CSV::Reader csv(new FileStream(Filename::classics_csv)); // Use a regular SectionTemplate class to process the template SectionTemplate tmpl(new FSTR::Stream(templateSource)); diff --git a/samples/Basic_Templates/component.mk b/samples/Basic_Templates/component.mk index 092ca88129..bcb5ca8f81 100644 --- a/samples/Basic_Templates/component.mk +++ b/samples/Basic_Templates/component.mk @@ -1,3 +1,4 @@ +COMPONENT_DEPENDS := CsvReader HWCONFIG := basic_templates DISABLE_NETWORK := 1 diff --git a/tests/HostTests/modules/CStringArray.cpp b/tests/HostTests/modules/CStringArray.cpp index 61c8394099..bb640bf83f 100644 --- a/tests/HostTests/modules/CStringArray.cpp +++ b/tests/HostTests/modules/CStringArray.cpp @@ -24,6 +24,7 @@ class CStringArrayTest : public TestGroup "b\0" "c\0" "d\0"); + DEFINE_FSTR_LOCAL(FS_BasicJoined, "a,b,c,d") TEST_CASE("Empty construction") { @@ -231,6 +232,31 @@ class CStringArrayTest : public TestGroup csa.add(F("test\0again")); REQUIRE_EQ(csa.join("}+{"), "a}+{}+{test}+{again"); REQUIRE_EQ(csa.join(nullptr), "atestagain"); + + csa = FS_Basic; + REQUIRE_EQ(csa.join(), FS_BasicJoined); + } + + TEST_CASE("release") + { + CStringArray csa(FS_Basic); + csa += FS_Basic; // Allocate > SSO + Serial << csa.join() << endl; + auto cstrWant = csa.c_str(); + String s = csa.release(); + REQUIRE(!csa); + REQUIRE(s.c_str() == cstrWant); + + REQUIRE(s == String(FS_Basic) + FS_Basic); + + csa = std::move(s); + REQUIRE(csa == String(FS_Basic) + FS_Basic); + + String js; + js += FS_BasicJoined; + js += ','; + js += FS_BasicJoined; + REQUIRE(csa.join() == js); } } }; diff --git a/tests/HostTests/modules/String.cpp b/tests/HostTests/modules/String.cpp index 8c48e8e5e5..b453142c27 100644 --- a/tests/HostTests/modules/String.cpp +++ b/tests/HostTests/modules/String.cpp @@ -15,6 +15,7 @@ class StringTest : public TestGroup nonTemplateTest(); testString(); + testMove(); testMakeHexString(); } @@ -154,17 +155,62 @@ class StringTest : public TestGroup } } + void testMove() + { + DEFINE_FSTR_LOCAL(shortText, "Not long") + DEFINE_FSTR_LOCAL(longText, "Greater than SSO buffer length") + + TEST_CASE("Move into unassigned string") + { + // Normal move + String s1 = longText; + String s2 = std::move(s1); + REQUIRE(!s1); + REQUIRE(s2.length() == longText.length()); + } + + TEST_CASE("Move between allocated strings of same length") + { + String s1 = longText; + auto cstrWant = s1.c_str(); + String s2 = std::move(s1); + REQUIRE(s2.c_str() == cstrWant); + } + + TEST_CASE("Move to allocated string of shorter length") + { + String s1 = longText; + String s2 = shortText; + auto cstrWant = s1.c_str(); + s2 = std::move(s1); + REQUIRE(s2.c_str() == cstrWant); + } + + TEST_CASE("Move to allocated string of longer length") + { + String s1 = longText; + String s2; + auto cstrWant = s1.c_str(); + s1 = ""; // Buffer remains allocated + s2 = std::move(s1); + REQUIRE(s2.c_str() == cstrWant); + } + } + void testMakeHexString() { - uint8_t hwaddr[] = {0xaa, 0xbb, 0xcc, 0xdd, 0x12, 0x55, 0x00}; - REQUIRE(makeHexString(nullptr, 6) == String::empty); - REQUIRE(makeHexString(hwaddr, 0) == String::empty); - REQUIRE(makeHexString(hwaddr, 6) == F("aabbccdd1255")); - REQUIRE(makeHexString(hwaddr, 6, ':') == F("aa:bb:cc:dd:12:55")); - REQUIRE(makeHexString(hwaddr, 7) == F("aabbccdd125500")); - REQUIRE(makeHexString(hwaddr, 7, ':') == F("aa:bb:cc:dd:12:55:00")); - REQUIRE(makeHexString(hwaddr, 1, ':') == F("aa")); - REQUIRE(makeHexString(hwaddr, 0, ':') == String::empty); + TEST_CASE("makeHexString") + { + uint8_t hwaddr[] = {0xaa, 0xbb, 0xcc, 0xdd, 0x12, 0x55, 0x00}; + REQUIRE(makeHexString(nullptr, 6) == String::empty); + REQUIRE(makeHexString(hwaddr, 0) == String::empty); + REQUIRE(makeHexString(hwaddr, 6) == F("aabbccdd1255")); + REQUIRE(makeHexString(hwaddr, 6, ':') == F("aa:bb:cc:dd:12:55")); + REQUIRE(makeHexString(hwaddr, 7) == F("aabbccdd125500")); + REQUIRE(makeHexString(hwaddr, 7, ':') == F("aa:bb:cc:dd:12:55:00")); + REQUIRE(makeHexString(hwaddr, 1, ':') == F("aa")); + REQUIRE(makeHexString(hwaddr, 0, ':') == String::empty); + } } }; diff --git a/tests/HostTests/modules/TemplateStream.cpp b/tests/HostTests/modules/TemplateStream.cpp index 66f1a6c9bc..36c725dd5a 100644 --- a/tests/HostTests/modules/TemplateStream.cpp +++ b/tests/HostTests/modules/TemplateStream.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #ifdef ARCH_HOST #include @@ -23,12 +22,6 @@ DEFINE_FSTR_LOCAL(template3_1, "Document Title