Use local UART driver not HardwareSerial (#857)

* Use local UART driver not HardwareSerial

The HardwareSerial driver is broken in Arduino framework versions
1.0.5 and 1.0.6 .  espressif/arduino-esp32#5005
Instead of waiting for a fix, I wrote a very simple UART driver that
does exactly what we need with no unnecessary bells and whistles to
cause problems.

* Added missing files, changed method signatures

The methods implemented by the UART class now
have the same signatures as the HardwareSerial
class, so it will be easy to switch back if we
need to.

* Incorporated suggestions from Stefan

* Fixed TX_IDLE_NUM bug reported by mstrens

* Quick test for Bf: problem

This is not the final solution.

* Fixed stupid typo in last commit

* Another test - check for client_buffer space

* Use the esp-idf uart driver

You can revert to the direct driver for testing by
defining DIRECT_UART

* Uart class now supports VFD and TMC

* data bits, stop bits, parity as enum classes

The constants for data bits, stop bits, and parity
were changed to enum classes so the compiler can
check for argument order mismatches.

* Set half duplex after uart init

* Init TMC UART only once

* rx/tx pin order mixup, missing _uart_started

* Test: use Arduino Serial

This reverts to the Arduino serial driver for
UI communication, leaving the VFS comms on the
Uart class on top of the esp_idf UART driver.

You can switch back and forth with the
   define REVERT_TO_SERIAL
line in Serial.cpp

* REVERT_TO_ARDUINO_SERIAL off by default

* Added debug messages

* Update Grbl.h

* Update platformio.ini

Co-authored-by: bdring <[email protected]>

Author: MitchBradley

Diff for: Grbl_Esp32/src/Grbl.cpp

@@ -30,7 +30,7 @@ void grbl_init() {
     WiFi.enableSTA(false);
     WiFi.enableAP(false);
     WiFi.mode(WIFI_OFF);
-    serial_init();  // Setup serial baud rate and interrupts
+    client_init();  // Setup serial baud rate and interrupts
     display_init();
     grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "Grbl_ESP32 Ver %s Date %s", GRBL_VERSION, GRBL_VERSION_BUILD);  // print grbl_esp32 verion info
     grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "Compiled with ESP32 SDK:%s", ESP.getSdkVersion());              // print the SDK version
@@ -40,7 +40,7 @@ void grbl_init() {
 #endif
     settings_init();  // Load Grbl settings from non-volatile storage
     stepper_init();   // Configure stepper pins and interrupt timers
-    system_ini();  // Configure pinout pins and pin-change interrupt (Renamed due to conflict with esp32 files)
+    system_ini();     // Configure pinout pins and pin-change interrupt (Renamed due to conflict with esp32 files)
     init_motors();
     memset(sys_position, 0, sizeof(sys_position));  // Clear machine position.
     machine_init();                                 // weak definition in Grbl.cpp does nothing
@@ -93,8 +93,8 @@ static void reset_variables() {
     sys_rt_s_override                    = SpindleSpeedOverride::Default;
 
     // Reset Grbl primary systems.
-    serial_reset_read_buffer(CLIENT_ALL);  // Clear serial read buffer
-    gc_init();                             // Set g-code parser to default state
+    client_reset_read_buffer(CLIENT_ALL);
+    gc_init();  // Set g-code parser to default state
     spindle->stop();
     coolant_init();
     limits_init();

Diff for: Grbl_Esp32/src/Grbl.h

@@ -22,7 +22,7 @@
 
 // Grbl versioning system
 const char* const GRBL_VERSION       = "1.3a";
-const char* const GRBL_VERSION_BUILD = "20210401";
+const char* const GRBL_VERSION_BUILD = "20210413";
 
 //#include <sdkconfig.h>
 #include <Arduino.h>
@@ -51,8 +51,9 @@ const char* const GRBL_VERSION_BUILD = "20210401";
 #include "Limits.h"
 #include "MotionControl.h"
 #include "Protocol.h"
-#include "Report.h"
+#include "Uart.h"
 #include "Serial.h"
+#include "Report.h"
 #include "Pins.h"
 #include "Spindles/Spindle.h"
 #include "Motors/Motors.h"

Diff for: Grbl_Esp32/src/I2SOut.cpp

@@ -48,6 +48,7 @@
 #include "WebUI/ESPResponse.h"
 #include "Probe.h"
 #include "System.h"
+#include "Serial.h"
 #include "Report.h"
 
 #include <FreeRTOS.h>

Diff for: Grbl_Esp32/src/Limits.cpp

@@ -55,10 +55,12 @@ void IRAM_ATTR isr_limit_switches() {
 #    ifdef HARD_LIMIT_FORCE_STATE_CHECK
             // Check limit pin state.
             if (limits_get_state()) {
+                grbl_msg_sendf(CLIENT_ALL, MsgLevel::Debug, "Hard limits");
                 mc_reset();                                // Initiate system kill.
                 sys_rt_exec_alarm = ExecAlarm::HardLimit;  // Indicate hard limit critical event
             }
 #    else
+            grbl_msg_sendf(CLIENT_ALL, MsgLevel::Debug, "Hard limits");
             mc_reset();                                // Initiate system kill.
             sys_rt_exec_alarm = ExecAlarm::HardLimit;  // Indicate hard limit critical event
 #    endif
@@ -195,7 +197,8 @@ void limits_go_home(uint8_t cycle_mask) {
 
                 if (sys_rt_exec_alarm != ExecAlarm::None) {
                     motors_set_homing_mode(cycle_mask, false);  // tell motors homing is done...failed
-                    mc_reset();                                 // Stop motors, if they are running.
+                    grbl_msg_sendf(CLIENT_ALL, MsgLevel::Debug, "Homing fail");
+                    mc_reset();  // Stop motors, if they are running.
                     protocol_execute_realtime();
                     return;
                 } else {
@@ -351,6 +354,7 @@ void limits_soft_check(float* target) {
                 }
             } while (sys.state != State::Idle);
         }
+        grbl_msg_sendf(CLIENT_ALL, MsgLevel::Debug, "Soft limits");
         mc_reset();                                // Issue system reset and ensure spindle and coolant are shutdown.
         sys_rt_exec_alarm = ExecAlarm::SoftLimit;  // Indicate soft limit critical event
         protocol_execute_realtime();               // Execute to enter critical event loop and system abort
@@ -367,7 +371,7 @@ void limitCheckTask(void* pvParameters) {
         AxisMask switch_state;
         switch_state = limits_get_state();
         if (switch_state) {
-            //grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "Limit Switch State %08d", switch_state);
+            grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Debug, "Limit Switch State %08d", switch_state);
             mc_reset();                                // Initiate system kill.
             sys_rt_exec_alarm = ExecAlarm::HardLimit;  // Indicate hard limit critical event
         }

Diff for: Grbl_Esp32/src/MotionControl.cpp

@@ -499,6 +499,7 @@ void mc_override_ctrl_update(uint8_t override_state) {
 // lost, since there was an abrupt uncontrolled deceleration. Called at an interrupt level by
 // realtime abort command and hard limits. So, keep to a minimum.
 void mc_reset() {
+    grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Debug, "mc_reset()");
     // Only this function can set the system reset. Helps prevent multiple kill calls.
     if (!sys_rt_exec_state.bit.reset) {
         sys_rt_exec_state.bit.reset = true;

Diff for: Grbl_Esp32/src/Motors/TrinamicUartDriver.h

@@ -21,6 +21,7 @@
 
 #include "Motor.h"
 #include "StandardStepper.h"
+#include "../Uart.h"
 
 #include <TMCStepper.h>  // https://github.com/teemuatlut/TMCStepper
 
@@ -63,7 +64,7 @@ const double TRINAMIC_UART_FCLK = 12700000.0;  // Internal clock Approx (Hz) use
 #    define TMC_UART_TX UNDEFINED_PIN
 #endif
 
-extern HardwareSerial tmc_serial;
+extern Uart tmc_serial;
 
 namespace Motors {
 
@@ -75,6 +76,9 @@ namespace Motors {
     };
 
     class TrinamicUartDriver : public StandardStepper {
+    private:
+        static bool _uart_started;
+
     public:
         TrinamicUartDriver(uint8_t  axis_index,
                            uint8_t  step_pin,
@@ -128,4 +132,4 @@ namespace Motors {
         // void config_message() override;
     };
 
-}
+}

Diff for: Grbl_Esp32/src/Motors/TrinamicUartDriverClass.cpp

@@ -26,10 +26,12 @@
 
 #include <TMCStepper.h>
 
-HardwareSerial tmc_serial(TMC_UART);
+Uart tmc_serial(TMC_UART);
 
 namespace Motors {
 
+    bool TrinamicUartDriver::_uart_started = false;
+
     TrinamicUartDriver* TrinamicUartDriver::List = NULL;  // a static ist of all drivers for stallguard reporting
 
     /* HW Serial Constructor. */
@@ -41,9 +43,11 @@ namespace Motors {
         _r_sense            = r_sense;
         this->addr          = addr;
 
-        uart_set_pin(TMC_UART, TMC_UART_TX, TMC_UART_RX, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
-        tmc_serial.begin(115200, SERIAL_8N1, TMC_UART_RX, TMC_UART_TX);
-        tmc_serial.setRxBufferSize(128);
+        if (!_uart_started) {
+            tmc_serial.setPins(TMC_UART_TX, TMC_UART_RX);
+            tmc_serial.begin(115200, Uart::Data::Bits8, Uart::Stop::Bits1, Uart::Parity::None);
+            _uart_started = true;
+        }
         hw_serial_init();
 
         link = List;
@@ -231,7 +235,7 @@ namespace Motors {
                 tmcstepper->en_spreadCycle(false);
                 tmcstepper->pwm_autoscale(false);
                 tmcstepper->TCOOLTHRS(calc_tstep(homing_feed_rate->get(), 150.0));
-                tmcstepper->SGTHRS(constrain(axis_settings[_axis_index]->stallguard->get(),0,255));
+                tmcstepper->SGTHRS(constrain(axis_settings[_axis_index]->stallguard->get(), 0, 255));
                 break;
             default:
                 grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "Unknown Trinamic mode:d", _mode);
@@ -394,4 +398,4 @@ namespace Motors {
 #endif
         }
     }
-}
+}

Diff for: Grbl_Esp32/src/ProcessSettings.cpp

@@ -192,6 +192,7 @@ Error toggle_check_mode(const char* value, WebUI::AuthenticationLevel auth_level
     // is idle and ready, regardless of alarm locks. This is mainly to keep things
     // simple and consistent.
     if (sys.state == State::CheckMode) {
+        grbl_msg_sendf(CLIENT_ALL, MsgLevel::Debug, "Check mode");
         mc_reset();
         report_feedback_message(Message::Disabled);
     } else {

Diff for: Grbl_Esp32/src/Protocol.cpp

@@ -103,7 +103,7 @@ bool can_park() {
   GRBL PRIMARY LOOP:
 */
 void protocol_main_loop() {
-    serial_reset_read_buffer(CLIENT_ALL);
+    client_reset_read_buffer(CLIENT_ALL);
     empty_lines();
     //uint8_t client = CLIENT_SERIAL; // default client
     // Perform some machine checks to make sure everything is good to go.
@@ -135,7 +135,7 @@ void protocol_main_loop() {
     // Primary loop! Upon a system abort, this exits back to main() to reset the system.
     // This is also where Grbl idles while waiting for something to do.
     // ---------------------------------------------------------------------------------
-    uint8_t c;
+    int c;
     for (;;) {
 #ifdef ENABLE_SD_CARD
         if (SD_ready_next) {
@@ -157,7 +157,7 @@ void protocol_main_loop() {
         uint8_t client = CLIENT_SERIAL;
         char*   line;
         for (client = 0; client < CLIENT_COUNT; client++) {
-            while ((c = serial_read(client)) != SERIAL_NO_DATA) {
+            while ((c = client_read(client)) != -1) {
                 Error res = add_char_to_line(c, client);
                 switch (res) {
                     case Error::Ok:

Diff for: Grbl_Esp32/src/Report.cpp

@@ -54,30 +54,8 @@ EspClass esp;
 #endif
 const int DEFAULTBUFFERSIZE = 64;
 
-// this is a generic send function that everything should use, so interfaces could be added (Bluetooth, etc)
 void grbl_send(uint8_t client, const char* text) {
-    if (client == CLIENT_INPUT) {
-        return;
-    }
-#ifdef ENABLE_BLUETOOTH
-    if (WebUI::SerialBT.hasClient() && (client == CLIENT_BT || client == CLIENT_ALL)) {
-        WebUI::SerialBT.print(text);
-        //delay(10); // possible fix for dropped characters
-    }
-#endif
-#if defined(ENABLE_WIFI) && defined(ENABLE_HTTP) && defined(ENABLE_SERIAL2SOCKET_OUT)
-    if (client == CLIENT_WEBUI || client == CLIENT_ALL) {
-        WebUI::Serial2Socket.write((const uint8_t*)text, strlen(text));
-    }
-#endif
-#if defined(ENABLE_WIFI) && defined(ENABLE_TELNET)
-    if (client == CLIENT_TELNET || client == CLIENT_ALL) {
-        WebUI::telnet_server.write((const uint8_t*)text, strlen(text));
-    }
-#endif
-    if (client == CLIENT_SERIAL || client == CLIENT_ALL) {
-        Serial.print(text);
-    }
+    client_write(client, text);
 }
 
 // This is a formating version of the grbl_send(CLIENT_ALL,...) function that work like printf
@@ -658,7 +636,7 @@ void report_realtime_status(uint8_t client) {
         }
 #    endif  //ENABLE_BLUETOOTH
         if (client == CLIENT_SERIAL) {
-            bufsize = serial_get_rx_buffer_available(CLIENT_SERIAL);
+            bufsize = client_get_rx_buffer_available(CLIENT_SERIAL);
         }
         sprintf(temp, "|Bf:%d,%d", plan_get_block_buffer_available(), bufsize);
         strcat(status, temp);