Improve handling of too-long lines, and up minimum buffers to 512bytes.

Fixes #206

g2core/config.cpp

@@ -178,8 +178,7 @@ stat_t config_test_assertions()
         (BAD_MAGIC(nvl.magic_start)) ||
         (BAD_MAGIC(nvl.magic_end)) ||
         (BAD_MAGIC(nvStr.magic_start)) ||
-        (BAD_MAGIC(nvStr.magic_end)) ||
-        (global_string_buf[GLOBAL_STRING_LEN-1] != NUL)) {
+        (BAD_MAGIC(nvStr.magic_end))) {
         return(cm_panic(STAT_CONFIG_ASSERTION_FAILURE, "config_test_assertions()"));
     }
     return (STAT_OK);

g2core/config.h

@@ -177,7 +177,7 @@ typedef uint16_t index_t;               // use this if there are > 255 indexed o
 #define NV_MESSAGE_LEN 128              // sufficient space to contain end-user messages
 
                                         // pre-allocated defines (take RAM permanently)
-#define NV_SHARED_STRING_LEN 512        // shared string for string values
+#define NV_SHARED_STRING_LEN 1024        // shared string for string values
 #define NV_BODY_LEN 40                  // body elements - allow for 1 parent + N children
 #define NV_EXEC_LEN 10                  // elements reserved for exec, which won't directly respond
 // (each body element takes about 30 bytes of RAM)

g2core/controller.h

@@ -28,9 +28,10 @@
 #ifndef CONTROLLER_H_ONCE
 #define CONTROLLER_H_ONCE
 
+#include "xio.h"
+
 // see also: g2core.h MESSAGE_LEN and config.h NV_ lengths
-#define SAVED_BUFFER_LEN 80             // saved buffer size (for reporting only)
-#define MAXED_BUFFER_LEN 255            // same as streaming RX buffer size as a worst case
+#define SAVED_BUFFER_LEN RX_BUFFER_SIZE // saved buffer size (for reporting only)
 #define OUTPUT_BUFFER_LEN 512           // text buffer size
 
 #define LED_NORMAL_BLINK_RATE 3000      // blink rate for normal operation (in ms)

g2core/error.h

@@ -53,9 +53,6 @@
 typedef uint8_t stat_t;
 extern stat_t status_code;
 
-#define GLOBAL_STRING_LEN 256  // allow sufficient space for JSON responses and message strings
-extern char global_string_buf[];
-
 char *get_status_message(stat_t status);
 
 // ritorno is a handy way to provide exception returns

g2core/gcode_parser.cpp

@@ -249,7 +249,7 @@ stat_t _verify_checksum(char *str)
  *   - block_delete_flag is set true if block delete encountered, false otherwise
  */
 
-char _normalize_scratch[RX_BUFFER_MIN_SIZE];
+char _normalize_scratch[RX_BUFFER_SIZE];
 
 void _normalize_gcode_block(char *str, char **active_comment, uint8_t *block_delete_flag)
 {

g2core/main.cpp

@@ -49,7 +49,6 @@
 /******************** System Globals *************************/
 
 stat_t status_code;						    // allocate a variable for the ritorno macro
-char global_string_buf[GLOBAL_STRING_LEN];	// allocate a string for global message use
 
 /************* System Globals For Diagnostics ****************/
 

g2core/planner.cpp

@@ -72,7 +72,7 @@ mpMotionRuntimeSingleton_t mr;      // context for block runtime
 #define JSON_COMMAND_BUFFER_SIZE 3
 
 struct json_command_buffer_t {
-    char buf[RX_BUFFER_MIN_SIZE];
+    char buf[RX_BUFFER_SIZE];
     json_command_buffer_t *pv;
     json_command_buffer_t *nx;
 };

g2core/report.cpp

@@ -59,9 +59,10 @@ stat_t rpt_exception(stat_t status, const char *msg)
 
         // you cannot send an exception report if the USB has not been set up. Causes a processor exception.
         if (cs.controller_state >= CONTROLLER_READY) {
-           sprintf(global_string_buf, "{\"er\":{\"fb\":%0.2f,\"st\":%d,\"msg\":\"%s - %s\"}}\n",
+            char buffer[128];
+            sprintf(buffer, "{\"er\":{\"fb\":%0.2f,\"st\":%d,\"msg\":\"%s - %s\"}}\n",
                                         G2CORE_FIRMWARE_BUILD, status, get_status_message(status), msg);
-           xio_writeline(global_string_buf);
+            xio_writeline(buffer);
         }
     }
     return (status);            // makes it possible to inline, e.g: return(rpt_exception(status));

g2core/xio.cpp

@@ -49,6 +49,8 @@ using Motate::TXBuffer;
 #include "text_parser.h"
 #endif
 
+// defines for assertions
+
 /**** HIGH LEVEL EXPLANATION OF XIO ****
  *
  * The XIO subsystem serves three purposes:
@@ -102,14 +104,6 @@ struct xioDeviceWrapperBase {                // C++ base class for device primit
     devflags_t flags;                        // bitfield for device state flags (these are not)
     devflags_t next_flags;                   // bitfield for next-state transitions
 
-    // line reader functions
-//    uint16_t read_index;                    // index into line being read
-//    const uint16_t read_buf_size;           // static variable set at init time
-//    char read_buf[USB_LINE_BUFFER_SIZE];    // buffer for reading lines
-
-    // Internal use only:
-//    bool _ready_to_send;
-
     // Checks against class flags variable:
 //  bool canRead() { return caps & DEV_CAN_READ; }
 //  bool canWrite() { return caps & DEV_CAN_WRITE; }
@@ -422,7 +416,7 @@ extern xio_t xio;
 
 // LineRXBuffer takes the Motate RXBuffer (which handles "transfers", usually DMA), and adds G2 line-reading
 // semantics to it.
-template <uint16_t _size, typename owner_type, uint8_t _header_count = 8, uint16_t _line_buffer_size = 255>
+template <uint16_t _size, typename owner_type, uint8_t _header_count = 8, uint16_t _line_buffer_size = RX_BUFFER_SIZE>
 struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
     typedef RXBuffer<_size, owner_type, char> parent_type;
 
@@ -440,15 +434,17 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
     // START OF LineRXBuffer PROPER
     static_assert(((_header_count-1)&_header_count)==0, "_header_count must be 2^N");
 
-    char _line_buffer[_line_buffer_size]; // hold exactly one line to return
+    char _line_buffer[_line_buffer_size+1]; // hold exactly one line to return
     uint32_t _line_end_guard = 0xBEEF;
 
     // General term usage:
     // * "index" indicates it's in to _headers array
     // * "offset" means it's a character in the _data array
 
     uint16_t _scan_offset;          // offset into data of the last character scanned
-    uint16_t _line_start_offset;    // offset into first character of the line
+    uint16_t _line_start_offset;    // offset into first character of the line, or the first char to ignore (too-long lines)
+    uint16_t _last_line_length;     // used for ensuring lines aren't too long
+    bool     _ignore_until_next_line; // if we get a too-long-line, we ignore the rest by setting this flag
     bool     _at_start_of_line;     // true if the last character scanned was the end of a line
 
     uint16_t _lines_found;          // count of complete non-control lines that were found during scanning.
@@ -728,13 +724,30 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
 #endif
             // Look for line endings
             if (c == '\r' || c == '\n') {
-                if (!_at_start_of_line) {   // We only mark ends_line for the first end-line char, and if
+                if (_ignore_until_next_line) {
+                    // we finally ended the line we were ignoring
+                    // add a skip section to jump over the overage
+                    _skip_sections.addSkip(_line_start_offset, _scan_offset);
+                    // move the start of the next skip section to after this skip
+                    _line_start_offset = _scan_offset;
+
+                    // we DON'T want to end it normally (by counting a line)
+                    _at_start_of_line = true;
+                    _ignore_until_next_line = false;
+
+                    _last_line_length = 0;
+                }
+                else if (!_at_start_of_line) {   // We only mark ends_line for the first end-line char, and if
                     ends_line  = true;      // _at_start_of_line is already true, this is not the first.
                 }
             }
+            // prevent going furnther if we are ignoring
+            else if (_ignore_until_next_line)
+            {
+                // don't do anything
+            }
             // Classify the line if it's a single character 
-            else
-            if (_at_start_of_line &&
+            else if (_at_start_of_line &&
                 ((c == '!')         ||
                  (c == '~')         ||
                  (c == ENQ)         ||        // request ENQ/ack
@@ -754,20 +767,22 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
                 if (_at_start_of_line) {
                     // This is the first character at the beginning of the line.
                     _line_start_offset = _scan_offset;
+                    _last_line_length = 0;
                 }
                 _at_start_of_line = false;
             }
 
             // bump the _scan_offset
             _scan_offset = _getNextScanOffset();
+            _last_line_length++;
 
             if (ends_line) {
                 // _scan_offset is now one past the end of the line,
                 // which means it is at the start of a new line
                 _at_start_of_line = true;
 
                 // Here we classify the line.
-                // If we are is_control is already true, it's an already classified
+                // If is_control is already true, it's an already classified
                 // single-character command.
                 if (!is_control) {
                     // TODO --- Call a function to do this
@@ -793,7 +808,25 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
                     _lines_found++;
                 }
             } // if ends_line
+            else if (_last_line_length == (_line_buffer_size - 1)) {
+                // force an end-of-line, splitting this line into two lines
+                _ignore_until_next_line = true;
+                _line_start_offset = _scan_offset;
+                _lines_found++;
+            }
         } //while (_isMoreToScan())
+
+        // special edge case: we ran out of items to scan (buffer full?), but we're ignoring because a line was too long
+        // example: we get a line that it multiple-times the length of the buffer
+        // so we'll dump skip sections to the readline will move the read pointer forward
+        if (_ignore_until_next_line && (_line_start_offset != _scan_offset)) {
+            // add a skip section to jump over the overage
+            _skip_sections.addSkip(_line_start_offset, _scan_offset);
+            // move the start of the next skip section to after this skip
+            _line_start_offset = _scan_offset;
+        }
+
+
         return false; // no control was found
     };
 
@@ -842,11 +875,8 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
                 }
             }
 
-            while ((_scan_offset != _line_start_offset) &&
-                   (line_size < (_line_buffer_size - 2))) {
-//                if (!_canBeRead(read_offset)) { // This test should NEVER fail.
-//                    _debug_trap("readline hit unreadable and shouldn't have!");
-//                }
+            // note that if it's marked as a control, it's guaranteed to fit in the line buffer
+            while (_scan_offset != _line_start_offset) {
 
                 // copy the charater to _line_buffer
                 char c = _data[_line_start_offset];
@@ -866,38 +896,6 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
             if (ctrl_is_at_beginning_of_data) {
                 _read_offset = _scan_offset;
             }
-//            else {
-//                // special case: if the return value is '%'
-//                // then we actually consider everything before it to be read
-//
-//                if ('%' == _line_buffer[0]) {
-//                    // Things that must be managed here:
-//                    // * _read_offset -- we're skipping data
-//                    // * _lines_found -- we shouldn't have any lines "left"
-//                    // * _skip_sections -- there's nothing to skip, we just did
-//
-//                    // Things that won't be changed (further):
-//                    // * _scan_offset -- we're not changing past where it's scanned
-//                    // * _line_start_offset -- we've already adjusted it
-//                    // * _at_start_of_line -- should always be true when we're here
-//
-//                    // move the read buffer up to where we're scanning
-//                    _read_offset = _scan_offset;
-//
-//                    // record that we have 0 lines (of data) in the buffer
-//                    _lines_found = 0;
-//
-//                    // and clear out any skip sections we have
-//                    while (!_skip_sections.isEmpty()) {
-//                        _skip_sections.popSkip();
-//                    }
-//                }
-//            }
-
-//            if (ctrl_is_at_beginning_of_data) {
-//                // attempt to request more data
-//                _restartTransfer();
-//            }
 
             return _line_buffer;
         } // end if (found_control)
@@ -907,23 +905,23 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
             return nullptr;
         }
 
+        // skip sections will always start at the beginning of a line
+        // handle this, even with no line, in case we're ignoring a huge too-long line
+        _skip_sections.skip(_read_offset);
+
         if (_lines_found == 0) {
             // nothing to return
             line_size = 0;
             return nullptr;
         }
 
-
         // By the time we get here, we know we have at least one line in _data.
 
 
         if (_data[_read_offset] == 0) {
             _debug_trap("read ran into NULL");
         }
 
-        // skip sections will always start at the beginning of a line
-        _skip_sections.skip(_read_offset);
-
         // scan past any leftover CR or LF from the previous line
         char c = _data[_read_offset];
         while ((c == '\n') || (c == '\r')) {
@@ -936,11 +934,7 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
             c = _data[_read_offset];
         }
 
-        while (line_size < (_line_buffer_size - 2)) {
-//            if (!_canBeRead(read_offset)) { // This test should NEVER fail.
-//                _debug_trap("readline hit unreadable and shouldn't have!");
-//            }
-
+        while (line_size < (_line_buffer_size - 1)) {
             _read_offset = (_read_offset+1)&(_size-1);
 
             if ( c == '\r' ||
@@ -958,6 +952,10 @@ struct LineRXBuffer : RXBuffer<_size, owner_type, char> {
 
             c = _data[_read_offset];
         }
+        if (line_size == (_line_buffer_size - 1)) {
+            // add a line-ending
+            *dst_ptr++ = '\n';
+        }
 
         --_lines_found;
 
@@ -1221,7 +1219,7 @@ struct xioDeviceWrapper : xioDeviceWrapperBase {    // describes a device for re
 
 
 // Specialization for xio_flash_file -- we don't need most of the structure around a Device for xio_flash_file
-template<uint16_t _line_buffer_size = 255>
+template<uint16_t _line_buffer_size = 512>
 struct xioFlashFileDeviceWrapper : xioDeviceWrapperBase {    // describes a device for reading and writing
     xio_flash_file *_current_file = nullptr;
 

g2core/xio.h

@@ -61,8 +61,6 @@
 #undef  _FDEV_EOF
 #define _FDEV_EOF -2
 
-#define USB_LINE_BUFFER_SIZE    255         // text buffer size
-
 //*** Device flags ***
 typedef uint16_t devflags_t;                // might need to bump to 32 be 16 or 32
 
@@ -111,7 +109,7 @@ enum xioSPIMode {
 
 /**** readline stuff *****/
 
-#define RX_BUFFER_MIN_SIZE       256        // minimum requested buffer size (they are usually larger)
+#define RX_BUFFER_SIZE       512            // maximum length of recieved lines from xio_readline
 
 /**** function prototypes ****/