Merge pull request #1345 from bdring/dymk/uart-flow-control

Allow rewinding of commands sent via UART buffer

FluidNC/src/FixedCircularBuffer.h

@@ -0,0 +1,61 @@
+// Copyright (c) 2024 - Dylan Knutson <[email protected]>
+// Use of this source code is governed by a GPLv3 license that can be found in the LICENSE file.
+
+#pragma once
+
+#include <vector>
+#include <optional>
+
+/**
+ * A fixed-size circular buffer that stores elements of type T.
+ * Keeps track of how many elements have been pushed onto it, and allows
+ * for indexing as if it was an infinite sized array. If indexing into
+ * the buffer would result in an out-of-bounds access, returns std::nullopt.
+ *
+ * This is useful for implementing "scrollback" of a buffer of e.g. user
+ * provided commands, without using an unbounded amount of memory.
+ */
+template <typename T>
+class FixedCircularBuffer {
+public:
+    std::vector<T> storage;
+    std::size_t    head_idx, tail_idx;
+
+public:
+    FixedCircularBuffer() : FixedCircularBuffer(0) {}
+    FixedCircularBuffer(size_t size) : storage(size), head_idx(0), tail_idx(0) {}
+
+    /**
+     * Push an element onto the end of the buffer.
+     */
+    void push(T&& elem) {
+        storage[tail_idx % storage.size()] = std::move(elem);
+        tail_idx += 1;
+        if (tail_idx - head_idx > storage.size()) {
+            head_idx += 1;
+        }
+    }
+
+    /**
+     * Get the element at the given index, or std::nullopt if the index is out of bounds.
+     */
+    std::optional<T> at(std::size_t idx) const {
+        if (idx >= tail_idx) {
+            return std::nullopt;
+        }
+        if (idx < head_idx) {
+            return std::nullopt;
+        }
+        return storage[idx % storage.size()];
+    }
+
+    /**
+     * Is the buffer empty?
+     */
+    bool is_empty() const { return head_idx == tail_idx; }
+
+    /**
+     * Get the index of the last element pushed onto the buffer.
+     */
+    std::size_t position() const { return tail_idx; }
+};

FluidNC/src/Flowcontrol.cpp

@@ -233,8 +233,10 @@ Error flowcontrol(uint32_t o_label, char* line, size_t& pos, bool& skip) {
         case Op_Repeat:
             if (Job::active()) {
                 if (!skipping && (status = expression(line, pos, value)) == Error::Ok) {
-                    stack_push(o_label, operation, !value);
-                    if (value) {
+                    // TODO - return an error if value < 0
+                    // For now, just guard against negative values
+                    stack_push(o_label, operation, !(value > 0.0));
+                    if (value > 0.0) {
                         context.top().file     = Job::source();
                         context.top().file_pos = context.top().file->position();
                         context.top().repeats  = (uint32_t)value;
@@ -249,7 +251,7 @@ Error flowcontrol(uint32_t o_label, char* line, size_t& pos, bool& skip) {
             if (Job::active()) {
                 if (last_op == Op_Repeat) {
                     if (o_label == context.top().o_label) {
-                        if (context.top().repeats && --context.top().repeats) {
+                        if (context.top().repeats && --context.top().repeats > 0.0) {
                             context.top().file->set_position(context.top().file_pos);
                         } else {
                             stack_pull();

FluidNC/src/Job.cpp

@@ -4,17 +4,29 @@
 #include "Job.h"
 #include <map>
 #include <stack>
+#include "Protocol.h"
 
 std::stack<JobSource*> job;
 
 Channel* Job::leader = nullptr;
 
 bool Job::active() {
-    return !job.empty();
+    return !job.empty() || activeChannel;
 }
 
+JobSource activeChannelJobSource(nullptr);
+
 JobSource* Job::source() {
-    return job.empty() ? nullptr : job.top();
+    if (job.empty()) {
+        if (activeChannel) {
+            activeChannelJobSource.set_channel(activeChannel);
+            return &activeChannelJobSource;
+        } else {
+            return nullptr;
+        }
+    } else {
+        return job.top();
+    }
 }
 
 // save() and restore() are use to close/reopen an SD file atop the job stack
@@ -38,9 +50,11 @@ void Job::nest(Channel* in_channel, Channel* out_channel) {
     job.push(source);
 }
 void Job::pop() {
-    auto source = job.top();
-    job.pop();
-    delete source;
+    if (!job.empty()) {
+        auto source = job.top();
+        job.pop();
+        delete source;
+    }
     if (!active()) {
         leader = nullptr;
     }
@@ -60,14 +74,14 @@ void Job::abort() {
 }
 
 bool Job::get_param(const std::string& name, float& value) {
-    return job.top()->get_param(name, value);
+    return source()->get_param(name, value);
 }
 bool Job::set_param(const std::string& name, float value) {
-    return job.top()->set_param(name, value);
+    return source()->set_param(name, value);
 }
 bool Job::param_exists(const std::string& name) {
-    return job.top()->param_exists(name);
+    return source()->param_exists(name);
 }
 Channel* Job::channel() {
-    return job.top()->channel();
+    return source()->channel();
 }

FluidNC/src/Job.h

@@ -31,6 +31,7 @@ class JobSource {
     void   set_position(size_t pos) { _channel->set_position(pos); }
 
     Channel* channel() { return _channel; }
+    void     set_channel(Channel* channel) { _channel = channel; }
 
     ~JobSource() { delete _channel; }
 };

FluidNC/src/Protocol.h

@@ -50,6 +50,8 @@ extern volatile bool rtCycleStop;
 
 extern volatile bool runLimitLoop;
 
+extern Channel* activeChannel;
+
 // Alarm codes.
 enum class ExecAlarm : uint8_t {
     None                  = 0,

FluidNC/src/UartChannel.cpp

@@ -6,8 +6,10 @@
 #include "Serial.h"                 // allChannels
 
 UartChannel::UartChannel(int num, bool addCR) : Channel("uart_channel", num, addCR) {
-    _lineedit = new Lineedit(this, _line, Channel::maxLine - 1);
-    _active   = false;
+    _lineedit           = new Lineedit(this, _line, Channel::maxLine - 1);
+    _active             = false;
+    _history_buffer     = FixedCircularBuffer<char>(512);
+    _history_buffer_pos = 0;
 }
 
 void UartChannel::init() {
@@ -63,10 +65,13 @@ size_t UartChannel::write(const uint8_t* buffer, size_t length) {
 }
 
 int UartChannel::available() {
-    return _uart->available();
+    return (_history_buffer_pos < _history_buffer.position()) || _uart->available();
 }
 
 int UartChannel::peek() {
+    if (_history_buffer_pos < _history_buffer.position()) {
+        return _history_buffer.at(_history_buffer_pos).value();
+    }
     return _uart->peek();
 }
 
@@ -90,12 +95,22 @@ bool UartChannel::lineComplete(char* line, char c) {
 }
 
 int UartChannel::read() {
+    if (_history_buffer_pos < _history_buffer.position()) {
+        int c = _history_buffer.at(_history_buffer_pos).value();
+        _history_buffer_pos += 1;
+        return c;
+    }
+
     int c = _uart->read();
     if (c == 0x11) {
         // 0x11 is XON.  If we receive that, it is a request to use software flow control
         _uart->setSwFlowControl(true, -1, -1);
         return -1;
     }
+    if (c != -1) {
+        _history_buffer.push((char)c);
+        _history_buffer_pos += 1;
+    }
     return c;
 }
 

FluidNC/src/UartChannel.h

@@ -6,11 +6,14 @@
 #include "Uart.h"
 #include "Channel.h"
 #include "lineedit.h"
+#include "FixedCircularBuffer.h"
 
 class UartChannel : public Channel, public Configuration::Configurable {
 private:
-    Lineedit* _lineedit;
-    Uart*     _uart;
+    Lineedit*                 _lineedit;
+    Uart*                     _uart;
+    FixedCircularBuffer<char> _history_buffer;
+    std::size_t               _history_buffer_pos;
 
     int _uart_num           = 0;
     int _report_interval_ms = 0;
@@ -49,6 +52,9 @@ class UartChannel : public Channel, public Configuration::Configurable {
         handler.item("uart_num", _uart_num);
         handler.item("message_level", _message_level, messageLevels2);
     }
+
+    size_t position() override { return _history_buffer_pos; }
+    void   set_position(size_t pos) override { _history_buffer_pos = pos; }
 };
 
 extern UartChannel Uart0;

FluidNC/tests/FixedCircularBufferTest.cpp

@@ -0,0 +1,42 @@
+// Copyright (c) 2024 - Dylan Knutson <[email protected]>
+// Use of this source code is governed by a GPLv3 license that can be found in the LICENSE file.
+
+#include "gtest/gtest.h"
+#include "src/FixedCircularBuffer.h"
+
+TEST(FixedCircularBuffer, Empty) {
+    FixedCircularBuffer<int> buffer(0);
+
+    ASSERT_TRUE(buffer.is_empty());
+    ASSERT_EQ(buffer.position(), 0);
+    ASSERT_EQ(buffer.at(0), std::nullopt);
+    ASSERT_EQ(buffer.at(1), std::nullopt);
+    ASSERT_EQ(buffer.at(2), std::nullopt);
+}
+
+TEST(FixedCircularBuffer, OneElement) {
+    FixedCircularBuffer<int> buffer(1);
+
+    buffer.push(42);
+
+    ASSERT_FALSE(buffer.is_empty());
+    ASSERT_EQ(buffer.position(), 1);
+    ASSERT_EQ(buffer.at(0), 42);
+    ASSERT_EQ(buffer.at(1), std::nullopt);
+    ASSERT_EQ(buffer.at(2), std::nullopt);
+}
+
+TEST(FixedCircularBuffer, FrontElementsPopped) {
+    FixedCircularBuffer<int> buffer(2);
+
+    buffer.push(1);
+    buffer.push(2);
+    buffer.push(3);
+
+    ASSERT_FALSE(buffer.is_empty());
+    ASSERT_EQ(buffer.position(), 3);
+    ASSERT_EQ(buffer.at(0), std::nullopt);
+    ASSERT_EQ(buffer.at(1), 2);
+    ASSERT_EQ(buffer.at(2), 3);
+    ASSERT_EQ(buffer.at(3), std::nullopt);
+}

fixture_tests/fixtures/flow_control_repeat.nc

@@ -0,0 +1,45 @@
+ignore ok
+
+# test repeat zero times (should not print anything)
+-> o100 repeat [0]
+-> (print, fail lit 0)
+-> o100 endrepeat
+-> (print, pass lit 0)
+<- [MSG:INFO: PRINT, pass lit 0]
+
+# test when using a variable
+-> #<count> = 0
+-> o100 repeat [#<count>]
+-> (print, fail var 0)
+-> o100 endrepeat
+-> (print, pass var 0)
+<- [MSG:INFO: PRINT, pass var 0]
+
+# test negative repeat (should not print anything)
+# todo - negative repeat should probably set an error
+-> o100 repeat [-1]
+-> (print, fail lit -1)
+-> o100 endrepeat
+-> (print, pass lit -1)
+<- [MSG:INFO: PRINT, pass lit -1]
+
+# test repeat a fixed number of times
+-> #<count> = 0
+-> o100 repeat [3]
+-> #<count> = [#<count> + 1]
+-> (print, count=%d#<count>)
+<- [MSG:INFO: PRINT, count=1]
+-> o100 endrepeat
+<- [MSG:INFO: PRINT, count=2]
+<- [MSG:INFO: PRINT, count=3]
+
+# test repeating a variable number of times
+-> #<count> = 0
+-> #<i> = 3
+-> o100 repeat [#<i>]
+-> #<count> = [#<count> + 1]
+-> (print, count=%d#<count>)
+<- [MSG:INFO: PRINT, count=1]
+-> o100 endrepeat
+<- [MSG:INFO: PRINT, count=2]
+<- [MSG:INFO: PRINT, count=3]

fixture_tests/run_fixture

@@ -28,6 +28,12 @@ else:
 def run_fixture(fixture_path, controller):
     op_entries_parsed = op_entries.parse_file(fixture_path)
 
+    print(
+        colored(f"--- Run fixture ", "blue")
+        + colored(fixture_path, "blue", attrs=["bold"])
+        + colored(" ---", "blue")
+    )
+
     try:
         for op_entry in op_entries_parsed:
             if not op_entry.execute(controller):
@@ -39,7 +45,8 @@ def run_fixture(fixture_path, controller):
                 exit(1)
 
     except KeyboardInterrupt:
-        print("Interrupt")
+        print("Interrupted by user")
+        exit(1)
 
     except TimeoutError as e:
         print("Timeout waiting for response, line: " + e.args[0])