Fixed a few bugs in Pin.cpp. Added key-value pair support in PinOptionsParser.

Grbl_Esp32/src/Pin.cpp

@@ -75,7 +75,7 @@ bool Pin::parse(String str, Pins::PinDetail*& pinImplementation) {
     if (prefix == "gpio") {
         pinImplementation = new Pins::GPIOPinDetail(uint8_t(pinNumber), parser);
     } else if (prefix == "i2s") {
-#ifdef ESP_32
+#ifdef ESP32
         pinImplementation = new Pins::I2SPinDetail(uint8_t(pinNumber), parser);
 #else
         return false;  // not supported
@@ -85,6 +85,11 @@ bool Pin::parse(String str, Pins::PinDetail*& pinImplementation) {
         // when doing 'x == Pin::UNDEFINED' will evaluate to 'false' if the pin number
         // is not 0.
         pinImplementation = new Pins::VoidPinDetail(uint8_t(pinNumber));
+    } else {
+#ifdef ESP32
+        grbl_sendf(CLIENT_ALL, "Unknown prefix: " % s "\r\n", prefix.c_str());
+#endif
+        return false;
     }
 
 #if defined PIN_DEBUG && defined ESP32
@@ -96,13 +101,11 @@ bool Pin::parse(String str, Pins::PinDetail*& pinImplementation) {
 Pin Pin::create(const String& str) {
     Pins::PinDetail* pinImplementation = nullptr;
     try {
-#ifdef PIN_DEBUG
-#    ifdef ESP32
+#if defined PIN_DEBUG && defined ESP32
         grbl_sendf(CLIENT_ALL, "Setting up pin: [%s]\r\n", str.c_str());
-#    endif
 #endif
         if (!parse(str, pinImplementation)) {
-#ifdef ESP32
+#if defined PIN_DEBUG && defined ESP32
             grbl_sendf(CLIENT_ALL, "Setting up pin: '%s' failed.", str.c_str());
 #endif
             return Pin::UNDEFINED;
@@ -112,7 +115,7 @@ Pin Pin::create(const String& str) {
 
             // If we already had it, and we didn't find itself, remove the new instance:
             if (existingPin >= 0) {
-#ifdef ESP32
+#if defined PIN_DEBUG && defined ESP32
                 grbl_sendf(CLIENT_ALL, "Reusing previous pin initialization.");
 #endif
                 if (pinImplementation) {
@@ -129,11 +132,10 @@ Pin Pin::create(const String& str) {
         }
 
     } catch (const AssertionFailed& ex) {  // We shouldn't get here under normal circumstances.
-#ifdef PIN_DEBUG
-#    ifdef ESP32
-        grbl_sendf(CLIENT_ALL, "Failed. Details: %s\r\n", ex.stackTrace.c_str());
-#    endif
+#if defined ESP32
+        grbl_sendf(CLIENT_ALL, "Setting up pin failed. Details: %s\r\n", ex.stackTrace.c_str());
 #endif
+
         // RAII safety guard.
         if (pinImplementation) {
             delete pinImplementation;
@@ -145,7 +147,6 @@ Pin Pin::create(const String& str) {
 
 bool Pin::validate(const String& str) {
     Pins::PinDetail* pinImplementation;
-    int              pinNumber;
 
     auto valid = parse(str, pinImplementation);
     if (pinImplementation) {

Grbl_Esp32/src/Pins/PinOptionsParser.cpp

@@ -1,31 +1,86 @@
 #include "PinOptionsParser.h"
 
 #include <cstring>
+#include <cctype>
+#include <cstdlib>
 
 namespace Pins {
-    PinOption::PinOption(char* start, const char* end) : _start(start), _end(end) {}
+    PinOption::PinOption(char* start, const char* end) : _start(start), _end(end), _key(start), _value(start) { tokenize(); }
 
-    bool PinOption::is(const char* option) const { return !::strcmp(option, _start); }
+    void PinOption::tokenize() {
+        if (_start != _end) {
+            _key = _start;
+
+            auto i = _start;
+            for (; i != _end && (*i) != ':' && (*i) != ';' && (*i) != '='; ++i) {
+                *i = ::tolower(*i);
+            }
+
+            if (i == _end) {
+                // [start, end> is a key; value is nul
+                _value = _end;
+                _start = i;
+            } else if (*i == '=') {
+                // Parsing a key-value pair.
+                //
+                // Mark end of the key, which is now in [start, end>
+                *i = '\0';
+                ++i;
+
+                _value = i;
+
+                // Parse the value:
+                for (; i != _end && (*i) != ':' && (*i) != ';'; ++i) {
+                    *i = ::tolower(*i);
+                }
+
+                if (i != _end) {
+                    *i     = '\0';
+                    _start = i + 1;
+                } else {
+                    _start = i;
+                }
+            } else {  // must be ':' or ';'
+                // [start, i> is a key; value is nul
+                _value = i;
+                *i     = '\0';
+                _start = i + 1;
+            }
+        } else {
+            // Both key and value are nul.
+            _key = _value = _end;
+        }
+    }
+
+    bool PinOption::is(const char* option) const { return !::strcmp(option, _key); }
+
+    int PinOption::iValue() const {
+        // Parse to integer
+        return ::atoi(_value);
+    }
+
+    double PinOption::dValue() const {
+        // Parse to integer
+        return ::atof(_value);
+    }
 
     PinOption& PinOption ::operator++() {
-        if (_start != _end) {
-            auto newStart = _start + ::strlen(_start);  // to the \0
-            if (newStart != _end) {                     // and 1 past it if we're not at the end
-                ++newStart;
-            }
-            _start = newStart;
-        }
+        tokenize();
         return *this;
     }
 
-    PinOptionsParser::PinOptionsParser(char* buffer, char* bufferEnd) : _buffer(buffer), _bufferEnd(bufferEnd) {
-        // Do the actual parsing:
-        for (auto i = buffer; i != bufferEnd; ++i) {
-            if (*i == ':' || *i == ';') {
-                *i = '\0';
-            } else if (*i >= 'A' && *i <= 'Z') {  // where did cstring->tolower go? Anyways, here goes:
-                *i = char(*i + 32);
-            }
+    PinOptionsParser::PinOptionsParser(char* buffer, 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;
+            }
+            *endBuffer = '\0';
+        }
+        _buffer    = buffer;
+        _bufferEnd = endBuffer;
     }
 }

Grbl_Esp32/src/Pins/PinOptionsParser.h

@@ -25,20 +25,28 @@ namespace Pins {
         char*       _start;
         const char* _end;
 
+        const char* _key;
+        const char* _value;
+
         PinOption(char* start, const char* end);
 
-    public:
-        inline const char* operator()() const { return _start; }
+        void tokenize();
 
-        bool is(const char* option) const;
+    public:
+        inline const char* operator()() const { return _key; }
+        bool               is(const char* option) const;
+
+        int                iValue() const;
+        double             dValue() const;
+        inline const char* value() const { return _value; }
 
         // Iterator support:
         inline PinOption const* operator->() const { return this; }
         inline PinOption        operator*() const { return *this; }
         PinOption&              operator++();
 
-        bool operator==(const PinOption& o) const { return _start == o._start; }
-        bool operator!=(const PinOption& o) const { return _start != o._start; }
+        bool operator==(const PinOption& o) const { return _key == o._key; }
+        bool operator!=(const PinOption& o) const { return _key != o._key; }
     };
 
     // Options parser. This basically parses the options passed to the Pin class. Destroys

Grbl_Esp32/test/Pins/PinOptionParsing.cpp

@@ -54,6 +54,64 @@ namespace Pins {
         }
     }
 
+    Test(PinOptionParsing, SingleArgWithWS) {
+        const char* input = "  first";
+        char        tmp[20];
+        int         n = snprintf(tmp, 20, "%s", input);
+
+        Pins::PinOptionsParser parser(tmp, tmp + n);
+
+        {
+            auto opt    = parser.begin();
+            auto endopt = parser.end();
+            Assert(opt != endopt, "Expected an argument");
+            Assert(opt->is("first"), "Expected 'first'");
+
+            ++opt;
+            Assert(opt == endopt, "Expected one argument");
+        }
+
+        // Typical use is a for loop. Let's test the two ways to use it:
+        int ctr = 0;
+        for (auto it : parser) {
+            if (ctr == 0) {
+                Assert(it.is("first"), "Expected 'first'");
+            } else {
+                Assert(false, "Didn't expect to get here");
+            }
+            ++ctr;
+        }
+    }
+
+    Test(PinOptionParsing, SingleArgWithWS2) {
+        const char* input = "  first  ";
+        char        tmp[20];
+        int         n = snprintf(tmp, 20, "%s", input);
+
+        Pins::PinOptionsParser parser(tmp, tmp + n);
+
+        {
+            auto opt    = parser.begin();
+            auto endopt = parser.end();
+            Assert(opt != endopt, "Expected an argument");
+            Assert(opt->is("first"), "Expected 'first'");
+
+            ++opt;
+            Assert(opt == endopt, "Expected one argument");
+        }
+
+        // Typical use is a for loop. Let's test the two ways to use it:
+        int ctr = 0;
+        for (auto it : parser) {
+            if (ctr == 0) {
+                Assert(it.is("first"), "Expected 'first'");
+            } else {
+                Assert(false, "Didn't expect to get here");
+            }
+            ++ctr;
+        }
+    }
+
     Test(PinOptionParsing, TwoArg1) {
         const char* input = "first;second";
         char        tmp[20];
@@ -97,7 +155,7 @@ namespace Pins {
         Pins::PinOptionsParser parser(tmp, tmp + n);
 
         {
-            auto opt = parser.begin();
+            auto opt    = parser.begin();
             auto endopt = parser.end();
             Assert(opt != endopt, "Expected an argument");
             Assert(opt->is("first"), "Expected 'first'");
@@ -115,11 +173,50 @@ namespace Pins {
         for (auto it : parser) {
             if (ctr == 0) {
                 Assert(it.is("first"), "Expected 'first'");
-            }
-            else if (ctr == 1) {
+            } else if (ctr == 1) {
                 Assert(it.is("second"), "Expected 'second'");
+            } else {
+                Assert(false, "Didn't expect to get here");
             }
-            else {
+            ++ctr;
+        }
+    }
+
+    Test(PinOptionParsing, TwoArgWithValues) {
+        const char* input = "first=12;second=13";
+        char        tmp[20];
+        int         n = snprintf(tmp, 20, "%s", input);
+
+        Pins::PinOptionsParser parser(tmp, tmp + n);
+
+        {
+            auto opt    = parser.begin();
+            auto endopt = parser.end();
+            Assert(opt != endopt, "Expected an argument");
+            Assert(opt->is("first"), "Expected 'first'");
+            Assert(strcmp("12", opt->value()) == 0);
+            Assert(12 == opt->iValue());
+            Assert(12 == opt->dValue());
+
+            ++opt;
+            Assert(opt != endopt, "Expected second argument");
+            Assert(opt->is("second"), "Expected 'second'");
+            Assert(strcmp("13", opt->value()) == 0);
+            Assert(13 == opt->iValue());
+            Assert(13 == opt->dValue());
+
+            ++opt;
+            Assert(opt == endopt, "Expected one argument");
+        }
+
+        // Typical use is a for loop. Let's test the two ways to use it:
+        int ctr = 0;
+        for (auto it : parser) {
+            if (ctr == 0) {
+                Assert(it.is("first"), "Expected 'first'");
+            } else if (ctr == 1) {
+                Assert(it.is("second"), "Expected 'second'");
+            } else {
                 Assert(false, "Didn't expect to get here");
             }
             ++ctr;