Skip to content

Commit

Permalink
Extend CsvReader capabilities, move into new library (#2805)
Browse files Browse the repository at this point in the history
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 <Data/CsvReader>`` to ``#include <CSV/Reader.h>``
- Change ``CsvReader`` class to :cpp:class:`CSV::Reader`
  • Loading branch information
mikee47 authored Jun 18, 2024
1 parent 69c4294 commit 7020094
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 298 deletions.
4 changes: 4 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion Sming/Core/Data/CStringArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 <typename T> CStringArray& operator+=(T value)
template <typename T> CStringArray& operator+=(const T& value)
{
add(String(value));
return *this;
Expand Down
124 changes: 0 additions & 124 deletions Sming/Core/Data/CsvReader.cpp

This file was deleted.

130 changes: 1 addition & 129 deletions Sming/Core/Data/CsvReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,132 +12,4 @@

#pragma once

#include "Stream/DataSourceStream.h"
#include "CStringArray.h"
#include <memory>

/**
* @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<IDataSourceStream> source;
char fieldSeparator;
bool userHeadingsProvided;
size_t maxLineLength;
CStringArray headings;
CStringArray row;
};
static_assert(false, "CsvReader class has been moved to the CsvReader library.");
1 change: 1 addition & 0 deletions Sming/Libraries/CsvReader
Submodule CsvReader added at 8f4d41
6 changes: 2 additions & 4 deletions Sming/Wiring/WString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions docs/source/upgrading/5.1-5.2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Data/CsvReader>`` to ``#include <CSV/Reader.h>``
- Change ``CsvReader`` class to :cpp:class:`CSV::Reader`
4 changes: 2 additions & 2 deletions samples/Basic_Templates/app/CsvTemplate.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include <Data/CsvReader.h>
#include <CSV/Reader.h>
#include <Data/Stream/SectionTemplate.h>

/**
Expand Down Expand Up @@ -35,5 +35,5 @@ class CsvTemplate : public SectionTemplate
}

private:
CsvReader csv;
CSV::Reader csv;
};
3 changes: 1 addition & 2 deletions samples/Basic_Templates/app/application.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include <SmingCore.h>
#include <Data/Stream/SectionTemplate.h>
#include <FlashString/Stream.hpp>
#include <Data/CsvReader.h>
#include "CsvTemplate.h"

namespace
Expand Down Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions samples/Basic_Templates/component.mk
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
COMPONENT_DEPENDS := CsvReader
HWCONFIG := basic_templates
DISABLE_NETWORK := 1

Loading

0 comments on commit 7020094

Please sign in to comment.