diff --git a/Grbl_Esp32/Grbl_Esp32.ino b/Grbl_Esp32/Grbl_Esp32.ino index 3a4d2ff6..b3133b51 100644 --- a/Grbl_Esp32/Grbl_Esp32.ino +++ b/Grbl_Esp32/Grbl_Esp32.ino @@ -22,6 +22,7 @@ # include "src/Grbl.h" void setup() { + Serial.begin(115200); grbl_init(); } diff --git a/Grbl_Esp32/src/Motors/Motors.cpp b/Grbl_Esp32/src/Motors/Motors.cpp index 5bb8edf9..0e9de7bd 100644 --- a/Grbl_Esp32/src/Motors/Motors.cpp +++ b/Grbl_Esp32/src/Motors/Motors.cpp @@ -393,14 +393,14 @@ void init_motors() { for (int gang_index = 0; gang_index < 2; gang_index++) { Pin pin = ms3_pins[axis][gang_index]; if (pin != Pin::UNDEFINED) { - pin.setAttr(Pin::Attr::Output | Pin::Attr::InitialHigh); + pin.setAttr(Pin::Attr::Output | Pin::Attr::InitialOn); } } } if (StepperResetPin->get() != Pin::UNDEFINED) { // !RESET pin on steppers (MISO On Schematic) - StepperResetPin->get().setAttr(Pin::Attr::Output | Pin::Attr::InitialHigh); + StepperResetPin->get().setAttr(Pin::Attr::Output | Pin::Attr::InitialOn); } #endif diff --git a/Grbl_Esp32/src/Motors/TrinamicDriver.cpp b/Grbl_Esp32/src/Motors/TrinamicDriver.cpp index b9e17f81..b7fd3bce 100644 --- a/Grbl_Esp32/src/Motors/TrinamicDriver.cpp +++ b/Grbl_Esp32/src/Motors/TrinamicDriver.cpp @@ -73,7 +73,7 @@ namespace Motors { _has_errors = false; init_step_dir_pins(); // from StandardStepper - _cs_pin.setAttr(Pin::Attr::Output | Pin::Attr::InitialHigh); + _cs_pin.setAttr(Pin::Attr::Output | Pin::Attr::InitialOn); // use slower speed if I2S if (_cs_pin.capabilities().has(Pin::Capabilities::I2S)) { diff --git a/Grbl_Esp32/src/Pin.cpp b/Grbl_Esp32/src/Pin.cpp index 2e271b30..771f9c9d 100644 --- a/Grbl_Esp32/src/Pin.cpp +++ b/Grbl_Esp32/src/Pin.cpp @@ -1,22 +1,24 @@ #include "Pin.h" -#include "Grbl.h" // Pins: #include "Pins/PinOptionsParser.h" #include "Pins/VoidPinDetail.h" #include "Pins/GPIOPinDetail.h" #include "Pins/I2SPinDetail.h" -#ifdef PIN_DEBUG + +#if defined PIN_DEBUG && defined ESP32 # include "Pins/DebugPinDetail.h" +# include "Grbl.h" // grbl_sendf #endif -bool Pin::parse(String str, Pins::PinDetail*& pinImplementation, int& pinNumber) { +bool Pin::parse(String str, Pins::PinDetail*& pinImplementation) { // Initialize pinImplementation first! Callers might want to delete it, and we don't want a random pointer. pinImplementation = nullptr; if (str == "") { + // Re-use undefined pins happens in 'create': pinImplementation = new Pins::VoidPinDetail(); - return pinImplementation; + return true; } // Parse the definition: [GPIO].[pinNumber]:[attributes] @@ -31,13 +33,13 @@ bool Pin::parse(String str, Pins::PinDetail*& pinImplementation, int& pinNumber) ++idx; } + int pinNumber = 0; if (prefix != "") { if (idx == str.end()) { // Incorrect pin definition. return false; } - pinNumber = 0; for (; idx != str.end() && *idx >= '0' && *idx <= '9'; ++idx) { pinNumber = pinNumber * 10 + int(*idx - '0'); } @@ -67,46 +69,86 @@ bool Pin::parse(String str, Pins::PinDetail*& pinImplementation, int& pinNumber) Pins::PinOptionsParser parser(options.begin(), options.end()); // Build this pin: - - try { - if (prefix == "gpio") { - pinImplementation = new Pins::GPIOPinDetail(uint8_t(pinNumber), parser); - } else if (prefix == "i2s") { + if (prefix == "gpio") { + pinImplementation = new Pins::GPIOPinDetail(uint8_t(pinNumber), parser); + } else if (prefix == "i2s") { #ifdef ESP_32 - pinImplementation = new Pins::I2SPinDetail(uint8_t(pinNumber), parser); + pinImplementation = new Pins::I2SPinDetail(uint8_t(pinNumber), parser); +#else + return false; // not supported #endif - } else { -#ifdef PIN_DEBUG - pinImplementation = new Pins::DebugPinDetail(pinImplementation); -#endif - } - } catch (const std::exception& e) { - grbl_sendf(CLIENT_SERIAL, "%s\r\n", e.what()); - pinImplementation = new Pins::VoidPinDetail(); - return false; + } else if (prefix == "void") { + // Note: having multiple void pins has its uses for debugging. Note that using + // when doing 'x == Pin::UNDEFINED' will evaluate to 'false' if the pin number + // is not 0. + pinImplementation = new Pins::VoidPinDetail(uint8_t(pinNumber)); } - return pinImplementation; + +#if defined PIN_DEBUG && defined ESP32 + pinImplementation = new Pins::DebugPinDetail(pinImplementation); +#endif + return true; } Pin Pin::create(const String& str) { - Pins::PinDetail* pinImplementation; - int pinNumber; + Pins::PinDetail* pinImplementation = nullptr; + try { +#ifdef PIN_DEBUG +# ifdef ESP32 + grbl_sendf(CLIENT_ALL, "Setting up pin: [%s]\r\n", str.c_str()); +# endif +#endif + if (!parse(str, pinImplementation)) { +#ifdef ESP32 + grbl_sendf(CLIENT_ALL, "Setting up pin: '%s' failed.", str.c_str()); +#endif + return Pin::UNDEFINED; + } else { + // Check if we already have this pin: + auto existingPin = Pins::PinLookup::_instance.FindExisting(pinImplementation); - Assert(parse(str, pinImplementation, pinNumber), "Pin definition is invalid."); + // If we already had it, and we didn't find itself, remove the new instance: + if (existingPin >= 0) { +#ifdef ESP32 + grbl_sendf(CLIENT_ALL, "Reusing previous pin initialization."); +#endif + if (pinImplementation) { + delete pinImplementation; + } - // Register: - Pins::PinLookup::_instance.SetPin(uint8_t(pinNumber), pinImplementation); + return Pin(uint8_t(existingPin)); + } else { + // This is a new pin. So, register, and return the pin object that refers to it. + auto pinNumber = pinImplementation->number(); + auto realIndex = Pins::PinLookup::_instance.SetPin(uint8_t(pinNumber), pinImplementation); + return Pin(realIndex); + } + } - return Pin(uint8_t(pinNumber)); + } catch (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 +#endif + // RAII safety guard. + if (pinImplementation) { + delete pinImplementation; + } + + return Pin::UNDEFINED; + } } bool Pin::validate(const String& str) { Pins::PinDetail* pinImplementation; int pinNumber; - auto valid = parse(str, pinImplementation, pinNumber); + auto valid = parse(str, pinImplementation); + if (pinImplementation) { + delete pinImplementation; + } - delete pinImplementation; return valid; } diff --git a/Grbl_Esp32/src/Pin.h b/Grbl_Esp32/src/Pin.h index 7b4aaf30..8cbde78b 100644 --- a/Grbl_Esp32/src/Pin.h +++ b/Grbl_Esp32/src/Pin.h @@ -9,7 +9,7 @@ #include #include -// #define PIN_DEBUG // Pin debugging. WILL spam you with a lot of data! +#define PIN_DEBUG // Pin debugging. WILL spam you with a lot of data! // Forward declarations: class String; @@ -37,7 +37,7 @@ class Pin { inline Pin(uint8_t index) : _index(index) {} - static bool parse(String str, Pins::PinDetail*& detail, int& pinNumber); + static bool parse(String str, Pins::PinDetail*& detail); public: using Capabilities = Pins::PinCapabilities; @@ -61,8 +61,8 @@ public: inline Pin& operator=(Pin&& o) = default; // Some convenience operators: - inline bool operator==(Pin o) const { return _index == _index; } - inline bool operator!=(Pin o) const { return _index != _index; } + inline bool operator==(const Pin& o) const { return _index == o._index; } + inline bool operator!=(const Pin& o) const { return _index != o._index; } inline uint8_t getNative(Capabilities expectedBehavior) const { auto detail = Pins::PinLookup::_instance.GetPin(_index); diff --git a/Grbl_Esp32/src/Pins/DebugPinDetail.cpp b/Grbl_Esp32/src/Pins/DebugPinDetail.cpp index f43cd66f..a8b1f16d 100644 --- a/Grbl_Esp32/src/Pins/DebugPinDetail.cpp +++ b/Grbl_Esp32/src/Pins/DebugPinDetail.cpp @@ -36,7 +36,7 @@ namespace Pins { if (value.has(PinAttributes::Exclusive)) { buf[n++] = 'X'; } - if (value.has(PinAttributes::InitialHigh)) { + if (value.has(PinAttributes::InitialOn)) { buf[n++] = '+'; } buf[n++] = 0; diff --git a/Grbl_Esp32/src/Pins/DebugPinDetail.h b/Grbl_Esp32/src/Pins/DebugPinDetail.h index d352fa16..f503cd12 100644 --- a/Grbl_Esp32/src/Pins/DebugPinDetail.h +++ b/Grbl_Esp32/src/Pins/DebugPinDetail.h @@ -22,7 +22,9 @@ namespace Pins { bool shouldEvent(); public: - DebugPinDetail(PinDetail* implementation) : _implementation(implementation), _lastEvent(0), _eventCount(0) {} + DebugPinDetail(PinDetail* implementation) : + PinDetail(implementation->number()), _implementation(implementation), _lastEvent(0), _eventCount(0), _isrHandler({ 0 }) { + } PinCapabilities capabilities() const override { return _implementation->capabilities(); } diff --git a/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp b/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp index 62c6e36b..6c4a6739 100644 --- a/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp +++ b/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp @@ -2,9 +2,9 @@ #include "../Assert.h" namespace Pins { - ErrorPinDetail::ErrorPinDetail(const PinOptionsParser& options) {} + ErrorPinDetail::ErrorPinDetail(const PinOptionsParser& options) : PinDetail(0) {} - PinCapabilities ErrorPinDetail::capabilities() const { return PinCapabilities::None; } + PinCapabilities ErrorPinDetail::capabilities() const { return PinCapabilities::Error; } void ErrorPinDetail::write(int high) { Assert(false, "Cannot write to an error pin."); } int ErrorPinDetail::read() { Assert(false, "Cannot read from an error pin."); } diff --git a/Grbl_Esp32/src/Pins/GPIOPinDetail.cpp b/Grbl_Esp32/src/Pins/GPIOPinDetail.cpp index 5a5c579b..7c0d6546 100644 --- a/Grbl_Esp32/src/Pins/GPIOPinDetail.cpp +++ b/Grbl_Esp32/src/Pins/GPIOPinDetail.cpp @@ -77,10 +77,15 @@ namespace Pins { } GPIOPinDetail::GPIOPinDetail(uint8_t index, PinOptionsParser options) : - _index(index), _capabilities(GetDefaultCapabilities(index)), _attributes(Pins::PinAttributes::Undefined), _readWriteMask(0) { - if (_capabilities == PinCapabilities::None) { - throw std::runtime_error("Bad GPIO number"); - } + PinDetail(index), _capabilities(GetDefaultCapabilities(index)), _attributes(Pins::PinAttributes::Undefined), _readWriteMask(0) { + // NOTE: + // + // RAII is very important here! If we throw an exception in the constructor, the resources + // that were allocated by the constructor up to that point _MUST_ be freed! Otherwise, you + // WILL get into trouble. + + Assert(_capabilities != PinCapabilities::None, "Bad GPIO number"); + // User defined pin capabilities for (auto opt : options) { if (opt.is("pu")) { @@ -88,20 +93,16 @@ namespace Pins { } else if (opt.is("pd")) { _attributes = _attributes | PinAttributes::PullDown; } else if (opt.is("low")) { - _attributes = _attributes | PinAttributes::ActiveLow; + _attributes = _attributes | PinAttributes::ActiveLow; } else if (opt.is("high")) { // Default: Active HIGH. } else { - throw std::runtime_error("Bad GPIO option"); + Assert(false, "Bad GPIO option passed to pin: %s", opt()); } } - // XXX This should not be done when the constructor is called for - // validating a setting. // Update the R/W mask for ActiveLow setting if (_attributes.has(PinAttributes::ActiveLow)) { - __pinMode(_index, OUTPUT); - __digitalWrite(_index, HIGH); _readWriteMask = HIGH; } else { _readWriteMask = LOW; @@ -146,9 +147,11 @@ namespace Pins { pinModeValue |= OUTPUT; } - // TODO: If the pin is ActiveLow, should we take this into account or not? - if (value.has(PinAttributes::InitialHigh)) { - __digitalWrite(_index, HIGH); + // If the pin is ActiveLow, we should take that into account here: + if (value.has(PinAttributes::InitialOn)) { + __digitalWrite(_index, HIGH ^ _readWriteMask); + } else { + __digitalWrite(_index, LOW ^ _readWriteMask); } __pinMode(_index, pinModeValue); diff --git a/Grbl_Esp32/src/Pins/GPIOPinDetail.h b/Grbl_Esp32/src/Pins/GPIOPinDetail.h index 204fa791..d217a018 100644 --- a/Grbl_Esp32/src/Pins/GPIOPinDetail.h +++ b/Grbl_Esp32/src/Pins/GPIOPinDetail.h @@ -4,7 +4,6 @@ namespace Pins { class GPIOPinDetail : public PinDetail { - uint8_t _index; PinCapabilities _capabilities; PinAttributes _attributes; int _readWriteMask; diff --git a/Grbl_Esp32/src/Pins/I2SPinDetail.cpp b/Grbl_Esp32/src/Pins/I2SPinDetail.cpp index 32831f79..367e2207 100644 --- a/Grbl_Esp32/src/Pins/I2SPinDetail.cpp +++ b/Grbl_Esp32/src/Pins/I2SPinDetail.cpp @@ -8,7 +8,7 @@ extern "C" void __digitalWrite(uint8_t pin, uint8_t val); namespace Pins { I2SPinDetail::I2SPinDetail(uint8_t index, const PinOptionsParser& options) : - _index(index), _capabilities(PinCapabilities::Output | PinCapabilities::I2S), _attributes(Pins::PinAttributes::Undefined), + PinDetail(index), _capabilities(PinCapabilities::Output | PinCapabilities::I2S), _attributes(Pins::PinAttributes::Undefined), _readWriteMask(0) { // User defined pin capabilities for (auto opt : options) { @@ -24,8 +24,7 @@ namespace Pins { } // Update the R/W mask for ActiveLow setting - if (_attributes.ActiveLow) { - __digitalWrite(_index, HIGH); + if (_attributes.has(PinAttributes::ActiveLow)) { _readWriteMask = 1; } else { _readWriteMask = 0; @@ -54,6 +53,9 @@ namespace Pins { // I2S out pins cannot be configured, hence there // is nothing to do here for them. We basically // just check for conflicts above... + + // If the pin is ActiveLow, we should take that into account here: + write(value.has(PinAttributes::InitialOn) ? HIGH : LOW); } String I2SPinDetail::toString() const { return String("I2S_") + int(_index); } diff --git a/Grbl_Esp32/src/Pins/PinAttributes.cpp b/Grbl_Esp32/src/Pins/PinAttributes.cpp index 25f21eb2..20a2208c 100644 --- a/Grbl_Esp32/src/Pins/PinAttributes.cpp +++ b/Grbl_Esp32/src/Pins/PinAttributes.cpp @@ -18,7 +18,7 @@ namespace Pins { const int capabilityMask = (1 << (__LINE__ - START_LINE)) - 1; // -------- Mask capabilities till here PinAttributes PinAttributes::ActiveLow(1 << (__LINE__ - START_LINE)); PinAttributes PinAttributes::Exclusive(1 << (__LINE__ - START_LINE)); // \/ These are attributes - PinAttributes PinAttributes::InitialHigh(1 << (__LINE__ - START_LINE)); // \/ These are attributes + PinAttributes PinAttributes::InitialOn(1 << (__LINE__ - START_LINE)); // \/ These are attributes bool PinAttributes::validateWith(PinCapabilities caps) { auto capMask = (caps._value & capabilityMask); diff --git a/Grbl_Esp32/src/Pins/PinAttributes.h b/Grbl_Esp32/src/Pins/PinAttributes.h index d270fb3e..e6853f11 100644 --- a/Grbl_Esp32/src/Pins/PinAttributes.h +++ b/Grbl_Esp32/src/Pins/PinAttributes.h @@ -35,7 +35,7 @@ namespace Pins { static PinAttributes ActiveLow; static PinAttributes Exclusive; - static PinAttributes InitialHigh; + static PinAttributes InitialOn; inline PinAttributes operator|(PinAttributes rhs) { return PinAttributes(_value | rhs._value); } inline PinAttributes operator&(PinAttributes rhs) { return PinAttributes(_value & rhs._value); } diff --git a/Grbl_Esp32/src/Pins/PinCapabilities.cpp b/Grbl_Esp32/src/Pins/PinCapabilities.cpp index a07b07f3..70111f91 100644 --- a/Grbl_Esp32/src/Pins/PinCapabilities.cpp +++ b/Grbl_Esp32/src/Pins/PinCapabilities.cpp @@ -17,10 +17,14 @@ namespace Pins { PinCapabilities PinCapabilities::PullUp(1 << (__LINE__ - START_LINE)); // NOTE: Mapped in PinAttributes! PinCapabilities PinCapabilities::PullDown(1 << (__LINE__ - START_LINE)); // NOTE: Mapped in PinAttributes! PinCapabilities PinCapabilities::ISR(1 << (__LINE__ - START_LINE)); // NOTE: Mapped in PinAttributes! - PinCapabilities PinCapabilities::Native(1 << (__LINE__ - START_LINE)); + PinCapabilities PinCapabilities::PWM(1 << (__LINE__ - START_LINE)); PinCapabilities PinCapabilities::UART(1 << (__LINE__ - START_LINE)); PinCapabilities PinCapabilities::ADC(1 << (__LINE__ - START_LINE)); PinCapabilities PinCapabilities::DAC(1 << (__LINE__ - START_LINE)); + + PinCapabilities PinCapabilities::Native(1 << (__LINE__ - START_LINE)); PinCapabilities PinCapabilities::I2S(1 << (__LINE__ - START_LINE)); + PinCapabilities PinCapabilities::Error(1 << (__LINE__ - START_LINE)); + PinCapabilities PinCapabilities::Void(1 << (__LINE__ - START_LINE)); } diff --git a/Grbl_Esp32/src/Pins/PinCapabilities.h b/Grbl_Esp32/src/Pins/PinCapabilities.h index dcb66ab1..e093e7ab 100644 --- a/Grbl_Esp32/src/Pins/PinCapabilities.h +++ b/Grbl_Esp32/src/Pins/PinCapabilities.h @@ -30,12 +30,18 @@ namespace Pins { static PinCapabilities PullDown; // NOTE: Mapped in PinAttributes! static PinCapabilities ISR; // NOTE: Mapped in PinAttributes! - static PinCapabilities Native; + // Other capabilities: static PinCapabilities ADC; static PinCapabilities DAC; static PinCapabilities PWM; static PinCapabilities UART; + + // Each class of pins (e.g. GPIO, etc) should have their own 'capability'. This ensures that we + // can compare classes of pins along with their properties by just looking at the capabilities. + static PinCapabilities Native; static PinCapabilities I2S; + static PinCapabilities Error; + static PinCapabilities Void; inline PinCapabilities operator|(PinCapabilities rhs) { return PinCapabilities(_value | rhs._value); } inline PinCapabilities operator&(PinCapabilities rhs) { return PinCapabilities(_value & rhs._value); } @@ -44,6 +50,6 @@ namespace Pins { inline operator bool() { return _value != 0; } - inline bool has(PinCapabilities t) { return (*this & t).operator bool(); } + inline bool has(PinCapabilities t) { return (*this & t) == t; } }; } diff --git a/Grbl_Esp32/src/Pins/PinDetail.h b/Grbl_Esp32/src/Pins/PinDetail.h index 5e903461..9dbfa85d 100644 --- a/Grbl_Esp32/src/Pins/PinDetail.h +++ b/Grbl_Esp32/src/Pins/PinDetail.h @@ -9,10 +9,14 @@ #include namespace Pins { + // Implementation details of pins. class PinDetail { + protected: + int _index; + public: - PinDetail() = default; + PinDetail(int number) : _index(number) {} PinDetail(const PinDetail& o) = delete; PinDetail(PinDetail&& o) = delete; PinDetail& operator=(const PinDetail& o) = delete; @@ -31,6 +35,8 @@ namespace Pins { virtual String toString() const = 0; + inline int number() const { return _index; } + virtual ~PinDetail() {} }; } diff --git a/Grbl_Esp32/src/Pins/PinLookup.h b/Grbl_Esp32/src/Pins/PinLookup.h index 9d24f82f..a61483a5 100644 --- a/Grbl_Esp32/src/Pins/PinLookup.h +++ b/Grbl_Esp32/src/Pins/PinLookup.h @@ -15,8 +15,8 @@ namespace Pins { // probably do with less, but this is as safe as it gets. PinDetail* _pins[256]; - // Should be plenty for the GPIO _pins: - static const int NumberNativePins = 64; + // According to Arduino.h there are 40 GPIO pins. So, let's start at 41. + static const int NumberNativePins = 41; public: PinLookup(); @@ -53,6 +53,20 @@ namespace Pins { return _pins[index]; } + int FindExisting(PinDetail* instance) const { + // Checks if a pin with this number and capabilities already exists: + for (int i = 0; i < 256; ++i) { + if (_pins[i] != nullptr) { + if (_pins[i]->number() == instance->number() && // check number of pin + _pins[i]->capabilities() == instance->capabilities()) // check pin capabilities + { + return i; + } + } + } + return -1; + } + PinLookup(const PinLookup&) = delete; PinLookup(PinLookup&&) = delete; diff --git a/Grbl_Esp32/src/Pins/VoidPinDetail.cpp b/Grbl_Esp32/src/Pins/VoidPinDetail.cpp index 6098e6bc..3f4b7995 100644 --- a/Grbl_Esp32/src/Pins/VoidPinDetail.cpp +++ b/Grbl_Esp32/src/Pins/VoidPinDetail.cpp @@ -1,10 +1,13 @@ #include "VoidPinDetail.h" namespace Pins { - VoidPinDetail::VoidPinDetail() : _frequency(0), _maxDuty(0) {} + VoidPinDetail::VoidPinDetail(int number) : PinDetail(number) {} VoidPinDetail::VoidPinDetail(const PinOptionsParser& options) : VoidPinDetail() {} - PinCapabilities VoidPinDetail::capabilities() const { return PinCapabilities::None; } // Should we? + PinCapabilities VoidPinDetail::capabilities() const { + // Void pins support basic functionality. It just won't do you any good. + return PinCapabilities::Output | PinCapabilities::Input | PinCapabilities::ISR | PinCapabilities::Void; + } void VoidPinDetail::write(int high) {} int VoidPinDetail::read() { return 0; } diff --git a/Grbl_Esp32/src/Pins/VoidPinDetail.h b/Grbl_Esp32/src/Pins/VoidPinDetail.h index 2d5c6164..6f5eaf60 100644 --- a/Grbl_Esp32/src/Pins/VoidPinDetail.h +++ b/Grbl_Esp32/src/Pins/VoidPinDetail.h @@ -5,11 +5,8 @@ namespace Pins { class VoidPinDetail : public PinDetail { - uint32_t _frequency; - uint32_t _maxDuty; - public: - VoidPinDetail(); + VoidPinDetail(int number = 0); VoidPinDetail(const PinOptionsParser& options); PinCapabilities capabilities() const override; diff --git a/Grbl_Esp32/src/Settings.cpp b/Grbl_Esp32/src/Settings.cpp index 31636544..bde6fc49 100644 --- a/Grbl_Esp32/src/Settings.cpp +++ b/Grbl_Esp32/src/Settings.cpp @@ -450,7 +450,7 @@ void PinSetting::load() { esp_err_t err = nvs_get_str(_handle, _keyName, NULL, &len); if (err) { #ifdef PIN_DEBUG - grbl_msg_sendf(CLIENT_ALL, MsgLevel::Info, "Initializing pin %s with: %s", _keyName, _defaultValue); + grbl_msg_sendf(CLIENT_ALL, MsgLevel::Info, "Initializing pin %s as '%s'", _fullName, _defaultValue); #endif _storedValue = _defaultValue; _currentValue = Pin::create(_defaultValue); @@ -460,7 +460,7 @@ void PinSetting::load() { err = nvs_get_str(_handle, _keyName, buf, &len); if (err) { #ifdef PIN_DEBUG - grbl_msg_sendf(CLIENT_ALL, MsgLevel::Info, "Initializing pin %s with: %s", _keyName, _defaultValue); + grbl_msg_sendf(CLIENT_ALL, MsgLevel::Info, "Initializing pin %s as '%s'", _fullName, _defaultValue); #endif _storedValue = _defaultValue; _currentValue = Pin::create(_defaultValue); @@ -468,7 +468,7 @@ void PinSetting::load() { } #ifdef PIN_DEBUG - grbl_msg_sendf(CLIENT_ALL, MsgLevel::Info, "Initializing pin %s with: %s", _keyName, _storedValue); + grbl_msg_sendf(CLIENT_ALL, MsgLevel::Info, "Initializing pin %s as '%s'", _fullName, _storedValue); #endif _storedValue = String(buf); _currentValue = Pin::create(_storedValue); diff --git a/Grbl_Esp32/src/Settings.h b/Grbl_Esp32/src/Settings.h index 2c54102e..2a804880 100644 --- a/Grbl_Esp32/src/Settings.h +++ b/Grbl_Esp32/src/Settings.h @@ -339,7 +339,6 @@ private: public: PinSetting(const char* description, const char* name, const char* defVal, bool (*checker)(char*)); - PinSetting(const char* name, const char* defVal, bool (*checker)(char*) = NULL) : PinSetting(NULL, name, defVal, checker) {}; void load(); diff --git a/Grbl_Esp32/test/Pins/Error.cpp b/Grbl_Esp32/test/Pins/Error.cpp index 5435dc1a..3eaa38af 100644 --- a/Grbl_Esp32/test/Pins/Error.cpp +++ b/Grbl_Esp32/test/Pins/Error.cpp @@ -19,7 +19,7 @@ namespace Pins { AssertThrow(errorPin.attachInterrupt([](void* arg) {}, CHANGE)); AssertThrow(errorPin.detachInterrupt()); - Assert(errorPin.capabilities() == Pin::Capabilities::None, "Incorrect caps"); + Assert(errorPin.capabilities() == Pin::Capabilities::Error, "Incorrect caps"); Assert(errorPin.name() == "ERROR_PIN"); } diff --git a/Grbl_Esp32/test/Pins/GPIO.cpp b/Grbl_Esp32/test/Pins/GPIO.cpp index c058c93a..54a41600 100644 --- a/Grbl_Esp32/test/Pins/GPIO.cpp +++ b/Grbl_Esp32/test/Pins/GPIO.cpp @@ -255,26 +255,26 @@ namespace Pins { Pin gpio16 = Pin::create("gpio.16:low"); Pin gpio17 = Pin::create("gpio.17"); - gpio16.setAttr(Pin::Attr::Output | Pin::Attr::InitialHigh); + gpio16.setAttr(Pin::Attr::Output); gpio17.setAttr(Pin::Attr::Input); Assert(false == gpio16.read()); Assert(true == gpio17.read()); - Assert(false == GPIONative::read(16)); + Assert(true == GPIONative::read(16)); Assert(true == GPIONative::read(17)); gpio16.on(); Assert(true == gpio16.read()); Assert(false == gpio17.read()); - Assert(true == GPIONative::read(16)); + Assert(false == GPIONative::read(16)); Assert(false == GPIONative::read(17)); gpio16.off(); Assert(false == gpio16.read()); Assert(true == gpio17.read()); - Assert(false == GPIONative::read(16)); + Assert(true == GPIONative::read(16)); Assert(true == GPIONative::read(17)); } } diff --git a/Grbl_Esp32/test/Pins/Undefined.cpp b/Grbl_Esp32/test/Pins/Undefined.cpp index 3a691368..7c105e1b 100644 --- a/Grbl_Esp32/test/Pins/Undefined.cpp +++ b/Grbl_Esp32/test/Pins/Undefined.cpp @@ -24,7 +24,38 @@ namespace Pins { AssertThrow(unassigned.attachInterrupt([](void* arg) {}, CHANGE)); AssertThrow(unassigned.detachInterrupt()); - Assert(unassigned.capabilities() == Pin::Capabilities::None); - Assert(unassigned.name().equals(UNDEFINED_PIN)); + Assert(unassigned.capabilities().has(Pin::Capabilities::Void)); + Assert(unassigned.name().equals("")); + } + + Test(Undefined, MultipleInstances) { + { + Pin unassigned = Pin::UNDEFINED; + Pin unassigned2 = Pin::UNDEFINED; + + Assert(unassigned == unassigned2, "Should evaluate to true."); + } + + { + Pin unassigned = Pin::create(""); + Pin unassigned2 = Pin::UNDEFINED; + + Assert(unassigned == unassigned2, "Should evaluate to true."); + } + + { + Pin unassigned = Pin::create("void.2"); + Pin unassigned2 = Pin::UNDEFINED; + + Assert(unassigned != unassigned2, "Second void pin should match first."); + } + + + { + Pin unassigned = Pin::create("void.2"); + Pin unassigned2 = Pin::create("void.2"); + + Assert(unassigned == unassigned2, "Second void pin should match first."); + } } } diff --git a/X86TestSupport/SoftwareGPIO.h b/X86TestSupport/SoftwareGPIO.h index 6c75fa04..741ee9e6 100644 --- a/X86TestSupport/SoftwareGPIO.h +++ b/X86TestSupport/SoftwareGPIO.h @@ -5,45 +5,44 @@ #include struct SoftwarePin { - SoftwarePin() : callback(), argument(nullptr), mode(0), padValue(false), pinMode(0) {} + SoftwarePin() : callback(), argument(nullptr), mode(0), driverValue(false), padValue(false), pinMode(0) {} void (*callback)(void*); void* argument; int mode; + bool driverValue; bool padValue; int pinMode; void handleISR() { callback(argument); } void reset() { - callback = nullptr; - argument = nullptr; - mode = 0; - padValue = false; - pinMode = 0; + callback = nullptr; + argument = nullptr; + mode = 0; + driverValue = false; + padValue = false; + pinMode = 0; } void handlePadChangeWithHystesis(bool newval) { auto oldval = padValue; - if (oldval != newval) - { - std::default_random_engine generator; + if (oldval != newval) { + std::default_random_engine generator; std::normal_distribution distribution(5, 2); - int count = int(distribution(generator)); + int count = int(distribution(generator)); // Bound it a bit if (count < 0) { count = 0; - } - else if (count > 8) { + } else if (count > 8) { count = 8; } count = count * 2 + 1; // make it odd. auto currentVal = oldval; - for (int i = 0; i < count; ++i) - { + for (int i = 0; i < count; ++i) { currentVal = !currentVal; handlePadChange(currentVal); } @@ -52,24 +51,23 @@ struct SoftwarePin { void handlePadChange(bool newval) { auto oldval = padValue; - if (oldval != newval) - { + if (oldval != newval) { switch (mode) { - case RISING: - if (!oldval && newval) { - handleISR(); - } - break; - case FALLING: - if (oldval && !newval) { - handleISR(); - } - break; - case CHANGE: - if (oldval != newval) { - handleISR(); - } - break; + case RISING: + if (!oldval && newval) { + handleISR(); + } + break; + case FALLING: + if (oldval && !newval) { + handleISR(); + } + break; + case CHANGE: + if (oldval != newval) { + handleISR(); + } + break; } padValue = newval; } @@ -103,8 +101,18 @@ public: } void setMode(int index, int mode) { - auto& pin = pins[index]; - pin.pinMode = mode; + auto& pin = pins[index]; + auto oldModeHasOutput = (pin.pinMode & OUTPUT) == OUTPUT; + pin.pinMode = mode; + auto modeHasOutput = (pin.pinMode & OUTPUT) == OUTPUT; + + if (modeHasOutput && !oldModeHasOutput) { + if (virtualCircuit != nullptr) { + virtualCircuit(pins, index, pin.driverValue); + } else if (circuitHandlesHystesis) { + pins[index].handlePadChange(pin.driverValue); + } + } } void writeOutput(int index, bool value) { @@ -115,10 +123,11 @@ public: virtualCircuit(pins, index, value); } else if (circuitHandlesHystesis) { pins[index].handlePadChange(value); - } - else { + } else { pins[index].handlePadChangeWithHystesis(value); } + } else { + pins[index].driverValue = value; } }