Fix options parser (#1240)

* clang-format cleanup - no substantive changes

* Eliminate redundant ] at end of $SS lines

* Cleaner code for string_view IOStream << operator

* Fix pin options error report format glitch ...

while changing PinOptionsParser to use string_view instead of
explicit start and end pointers for substrings.

* Fixed typo

* Fixed test suite and a bug that it found

FluidNC/esp32/StartupLog.cpp

@@ -46,6 +46,9 @@ void StartupLog::dump(Channel& out) {
             }
             line += c;
         }
+        if (!line.empty() && line.back() == ']') {
+            line.pop_back();
+        }
         log_stream(out, line);
     }
 }

FluidNC/src/MyIOStream.h

@@ -23,9 +23,7 @@ inline Print& operator<<(Print& lhs, const char* v) {
 }
 
 inline Print& operator<<(Print& lhs, const std::string_view& v) {
-    for (const char* p = v.cbegin(); p < v.cend(); ++p) {
-        lhs.print(*p);
-    }
+    lhs.write(reinterpret_cast<const uint8_t*>(v.data()), v.length());
     return lhs;
 }
 

FluidNC/src/Pin.cpp

@@ -70,7 +70,7 @@ const char* Pin::parse(std::string_view pin_str, Pins::PinDetail*& pinImplementa
     }
 
     // Build an options parser:
-    Pins::PinOptionsParser parser(pin_str.cbegin(), pin_str.cend());
+    Pins::PinOptionsParser parser(pin_str);
 
     // Build this pin:
     if (string_util::equal_ignore_case(prefix, "gpio")) {

FluidNC/src/Pins/GPIOPinDetail.cpp

@@ -115,7 +115,7 @@ namespace Pins {
             } else if (opt.is("high")) {
                 // Default: Active HIGH.
             } else {
-                Assert(false, "Bad GPIO option passed to pin %d: %s", int(index), opt());
+                Assert(false, "Bad GPIO option passed to pin %d: %.*s", int(index), static_cast<int>(opt().length()), opt().data());
             }
         }
         _claimed[index] = true;
@@ -124,9 +124,13 @@ namespace Pins {
         _readWriteMask = int(_attributes.has(PinAttributes::ActiveLow));
     }
 
-    PinAttributes GPIOPinDetail::getAttr() const { return _attributes; }
+    PinAttributes GPIOPinDetail::getAttr() const {
+        return _attributes;
+    }
 
-    PinCapabilities GPIOPinDetail::capabilities() const { return _capabilities; }
+    PinCapabilities GPIOPinDetail::capabilities() const {
+        return _capabilities;
+    }
 
     void IRAM_ATTR GPIOPinDetail::write(int high) {
         if (high != _lastWrittenValue) {

FluidNC/src/Pins/I2SOPinDetail.cpp

@@ -22,16 +22,18 @@ namespace Pins {
             } else if (opt.is("high")) {
                 // Default: Active HIGH.
             } else {
-                Assert(false, "Unsupported I2SO option '%s'", opt());
+                Assert(false, "Unsupported I2SO option '%.*s'", static_cast<int>(opt().length()), opt().data());
             }
         }
         _claimed[index] = true;
 
         // readWriteMask is xor'ed with the value to invert it if active low
-        _readWriteMask = _attributes.has(PinAttributes::ActiveLow);
+        _readWriteMask = int(_attributes.has(PinAttributes::ActiveLow));
     }
 
-    PinCapabilities I2SOPinDetail::capabilities() const { return PinCapabilities::Output | PinCapabilities::I2S; }
+    PinCapabilities I2SOPinDetail::capabilities() const {
+        return PinCapabilities::Output | PinCapabilities::I2S;
+    }
 
     // The write will not happen immediately; the data is queued for
     // delivery to the serial shift register chain via DMA and a FIFO
@@ -77,7 +79,9 @@ namespace Pins {
         i2s_out_write(_index, value.has(PinAttributes::InitialOn) ^ _readWriteMask);
     }
 
-    PinAttributes I2SOPinDetail::getAttr() const { return _attributes; }
+    PinAttributes I2SOPinDetail::getAttr() const {
+        return _attributes;
+    }
 
     std::string I2SOPinDetail::toString() {
         std::string s("I2SO.");

FluidNC/src/Pins/PinOptionsParser.cpp

@@ -2,113 +2,58 @@
 // Use of this source code is governed by a GPLv3 license that can be found in the LICENSE file.
 
 #include "PinOptionsParser.h"
+#include "../string_util.h"
 
 #include <cstring>
 #include <cctype>
 #include <cstdlib>
+#include <charconv>
 
 namespace Pins {
-    PinOption::PinOption(const char* start, const char* end) : _start(start), _end(end), _key(start), _value(start) { tokenize(); }
-
-    // Copy the value into a null-terminated string, converting to lower case
-    const char* PinOption::value() const {
-        static char str[100];
-
-        int valuelen = _valueend - _value;
-        if (valuelen > 100) {
-            valuelen = 99;
-        }
-        const char* p = _value;
-        int         i;
-        for (i = 0; i < valuelen; i++) {
-            str[i] = ::tolower(*p++);
-        }
-        str[i] = '\0';
-        return str;
+    PinOption::PinOption(const std::string_view options) : _options(options) {
+        tokenize();
     }
 
     void PinOption::tokenize() {
-        if (_start != _end) {
-            _key = _start;
-
-            auto i = _start;
-            for (; i != _end && (*i) != ':' && (*i) != ';' && (*i) != '='; ++i) {}
-
-            if (i == _end) {
-                // [start, end> is a key; value is nul
-                _value    = _end;
-                _valueend = _end;
-                _keyend   = _end;
-                _start    = i;
-            } else if (*i == '=') {
-                // Parsing a key-value pair.
-                //
-                // Mark end of the key, which is now in [start, end>
-                _keyend = i;
-                ++i;
-
-                _value = i;
-
-                // Parse the value:
-                for (; i != _end && (*i) != ':' && (*i) != ';'; ++i) {}
+        if (_options.length() == 0) {
+            _option = _key = _value = {};
+            return;
+        }
+        auto pos = _options.find_first_of(":;");
+        _option  = _options.substr(0, pos);
+        if (pos == std::string_view::npos) {
+            _option = _options;
+            _options.remove_prefix(_options.size());
+        } else {
+            _option = _options.substr(0, pos);
+            _options.remove_prefix(pos + 1);
+        }
 
-                _valueend = i;
-                if (i != _end) {
-                    _start = i + 1;
-                } else {
-                    _start = i;
-                }
-            } else {  // must be ':' or ';'
-                      // [start, i> is a key; value is nul
-                _keyend = _value = _valueend = i;
-                _start                       = i + 1;
-            }
+        pos = _option.find_first_of('=');
+        if (pos == std::string_view::npos) {
+            _key   = _option;
+            _value = {};
         } else {
-            // Both key and value are nul.
-            _key = _value = _end;
-            _keyend = _valueend = _end;
+            _key   = _option.substr(0, pos);
+            _value = _option.substr(pos + 1);
         }
     }
 
     bool PinOption::is(const char* option) const {
-        const char* k = _key;
-        while (*option && k != _keyend) {
-            if (::tolower(*k++) != ::tolower(*option++)) {
-                return false;
-            }
-        }
-        // If we get here, we have reached the end of either option or key
-        // and the initial substrings match ignoring case.
-        // If we are at the end of both, we have a match
-        return !*option && k == _keyend;
+        return string_util::equal_ignore_case(_key, option);
     }
 
     int PinOption::iValue() const {
         // Parse to integer
-        return ::atoi(value());
-    }
-
-    double PinOption::dValue() const {
-        // Parse to integer
-        return ::atof(value());
+        int num;
+        auto [ptr, ec] = std::from_chars(_value.data(), _value.data() + _value.length(), num);
+        return num;
     }
 
     PinOption& PinOption ::operator++() {
         tokenize();
         return *this;
     }
 
-    PinOptionsParser::PinOptionsParser(const char* buffer, const char* endBuffer) : _buffer(buffer), _bufferEnd(endBuffer) {
-        // trim whitespaces:
-        while (buffer != endBuffer && ::isspace(*buffer)) {
-            ++buffer;
-        }
-        if (buffer != endBuffer) {
-            while (buffer - 1 != endBuffer && ::isspace(endBuffer[-1])) {
-                --endBuffer;
-            }
-        }
-        _buffer    = buffer;
-        _bufferEnd = endBuffer;
-    }
+    PinOptionsParser::PinOptionsParser(std::string_view options) : _options(string_util::trim(options)) {}
 }

FluidNC/src/Pins/PinOptionsParser.h

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a GPLv3 license that can be found in the LICENSE file.
 
 #pragma once
+#include <string_view>
 
 namespace Pins {
     // Pin options are passed as PinOption object. This is a simple C++ forward iterator,
@@ -23,45 +24,41 @@ namespace Pins {
     class PinOption {
         friend class PinOptionsParser;
 
-        const char* _start;
-        const char* _end;
+        std::string_view _options;
+        std::string_view _option;
+        std::string_view _key;
+        std::string_view _value;
 
-        const char* _key;
-        const char* _keyend;
-        const char* _value;
-        const char* _valueend;
-
-        PinOption(const char* start, const char* end);
+        PinOption(const std::string_view options);
 
         void tokenize();
 
     public:
-        inline const char* operator()() const { return _key; }
-        bool               is(const char* option) const;
+        bool is(const char* option) const;
 
-        int    iValue() const;
-        double dValue() const;
+        int iValue() const;
 
-        const char* value() const;
+        inline const std::string_view operator()() { return _option; }
+        inline const std::string_view value() { return _value; }
+        inline const std::string_view key() { return _key; }
 
         // Iterator support:
         inline PinOption const* operator->() const { return this; }
         inline PinOption        operator*() const { return *this; }
         PinOption&              operator++();
 
-        bool operator==(const PinOption& o) const { return _key == o._key && _keyend == o._keyend; }
-        bool operator!=(const PinOption& o) const { return _key != o._key || _keyend != o._keyend; }
+        bool operator==(const PinOption& o) const { return _key == o._key; }
+        bool operator!=(const PinOption& o) const { return _key != o._key; }
     };
 
     // This parses the options passed to the Pin class.
     class PinOptionsParser {
-        const char* _buffer;
-        const char* _bufferEnd;
+        std::string_view _options;
 
     public:
-        PinOptionsParser(const char* buffer, const char* endBuffer);
+        PinOptionsParser(std::string_view options);
 
-        inline PinOption begin() const { return PinOption(_buffer, _bufferEnd); }
-        inline PinOption end() const { return PinOption(_bufferEnd, _bufferEnd); }
+        inline PinOption begin() const { return PinOption(_options); }
+        inline PinOption end() const { return PinOption(std::string_view()); }
     };
 }

FluidNC/src/Protocol.cpp

@@ -185,7 +185,8 @@ const uint32_t heapWarnThreshold = 15000;
 uint32_t heapLowWater           = UINT_MAX;
 uint32_t heapLowWaterReported   = UINT_MAX;
 int32_t  heapLowWaterReportTime = 0;
-void     protocol_main_loop() {
+
+void protocol_main_loop() {
     start_polling();
 
     // ---------------------------------------------------------------------------------

FluidNC/src/string_util.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <cstdint>
 #include <string_view>
 
 namespace string_util {