Skip to content

Commit

Permalink
Rewrite the ring buffer code in GCodeQueue so it's not spread over so…
Browse files Browse the repository at this point in the history
… many variables and instead use OOP where it's required

Add serial rx buffer monitor feature to warn the user when the internal serial's buffer is too close to its limits
Add BUG_ON macro so it's possible to stop the printer upon a bug condition
  • Loading branch information
X-Ryl669 committed Feb 22, 2021
1 parent af4461e commit 08065f4
Show file tree
Hide file tree
Showing 13 changed files with 284 additions and 173 deletions.
7 changes: 7 additions & 0 deletions Marlin/Configuration_adv.h
Original file line number Diff line number Diff line change
Expand Up @@ -2012,6 +2012,13 @@
//#define SERIAL_STATS_DROPPED_RX
#endif

// Monitor RX buffer usage
// This will dump an error to the serial port in case of overflow of the serial receive buffer
// If it happens, increase the RX_BUFFER_SIZE value
// Not supported on all platform.
#define RX_BUFFER_MONITOR


/**
* Emergency Command Parser
*
Expand Down
11 changes: 5 additions & 6 deletions Marlin/src/HAL/DUE/MarlinSerialUSB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,11 @@ int MarlinSerialUSB::read() {
return c;
}

bool MarlinSerialUSB::available() {
/* If Pending chars */
return pending_char >= 0 ||
/* or USB CDC enumerated and configured on the PC side and some
bytes where sent to us */
(usb_task_cdc_isenabled() && udi_cdc_is_rx_ready());
int MarlinSerialUSB::available() {
if (pending_char > 0) return pending_char;
return pending_char == 0 ||
// or USB CDC enumerated and configured on the PC side and some bytes where sent to us */
(usb_task_cdc_isenabled() && udi_cdc_is_rx_ready());
}

void MarlinSerialUSB::flush() { }
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/HAL/DUE/MarlinSerialUSB.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct MarlinSerialUSB {
int peek();
int read();
void flush();
bool available();
int available();
size_t write(const uint8_t c);

#if ENABLED(SERIAL_STATS_DROPPED_RX)
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/MarlinCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ void startOrResumeJob() {
*/
inline void manage_inactivity(const bool ignore_stepper_queue=false) {

if (queue.length < BUFSIZE) queue.get_available_commands();
queue.get_available_commands();

const millis_t ms = millis();

Expand Down
37 changes: 37 additions & 0 deletions Marlin/src/core/bug_on.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Marlin 3D Printer Firmware
* Copyright (c) 2021 MarlinFirmware [https://github.com/MarlinFirmware/Marlin]
*
* Copyright (c) 2021 X-Ryl669 [https://blog.cyril.by]
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
*/
#pragma once

// We need SERIAL_ECHOPAIR and macros.h
#include "serial.h"

#if ENABLED(POSTMORTEM_DEBUGGING)
// Useful macro for stopping the CPU on an unexpected condition
// This is used like SERIAL_ECHOPAIR, that is: a key-value call of the local variables you want
// to dump to the serial port before stopping the CPU.
#define BUG_ON(V...) do { SERIAL_ECHOPAIR(ONLY_FILENAME, __LINE__, ": "); SERIAL_ECHOLNPAIR(V); SERIAL_FLUSHTX(); *(char*)0 = 42; } while(0)
#elif ENABLED(MARLIN_DEV_MODE)
// Don't stop the CPU here, but at least dump the bug on the serial port
#define BUG_ON(V...) do { SERIAL_ECHOPAIR(ONLY_FILENAME, __LINE__, ": BUG!\n"); SERIAL_ECHOLNPAIR(V); SERIAL_FLUSHTX(); } while(0)
#else
// Release mode, let's ignore the bug
#define BUG_ON(V...) NOOP
#endif
26 changes: 26 additions & 0 deletions Marlin/src/core/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,32 @@
#define CALL_IF_EXISTS(Return, That, Method, ...) \
static_cast<Return>(Private::Call_ ## Method(That, ##__VA_ARGS__))


// Compile time string manipulation
namespace CompileTimeString {
// Simple compile-time parser to find the position of the end of a string
constexpr const char* findStringEnd(const char *str) {
return *str ? findStringEnd(str + 1) : str;
}

// Check whether a string contains a slash
constexpr bool containsSlash(const char *str) {
return *str == '/' ? true : (*str ? containsSlash(str + 1) : false);
}
// Find the last position of the slash
constexpr const char* findLastSlashPos(const char* str) {
return *str == '/' ? (str + 1) : findLastSlashPos(str - 1);
}
// Compile time evaluation of the last part of a file path
// Typically used to shorten the path to file in compiled strings
// CompileTimeString::fileName(__FILE__) returns "macros.h" and not /path/to/Marlin/src/core/macros.h
constexpr const char* fileName(const char* str) {
return containsSlash(str) ? findLastSlashPos(findStringEnd(str)) : str;
}
}

#define ONLY_FILENAME CompileTimeString::fileName(__FILE__)

#else

#define MIN_2(a,b) ((a)<(b)?(a):(b))
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/core/serial_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ struct SerialBase {
void end() { static_cast<Child*>(this)->end(); }
/** Check for available data from the port
@param index The port index, usually 0 */
bool available(uint8_t index = 0) { return static_cast<Child*>(this)->available(index); }
int available(uint8_t index = 0) { return static_cast<Child*>(this)->available(index); }
/** Read a value from the port
@param index The port index, usually 0 */
int read(uint8_t index = 0) { return static_cast<Child*>(this)->read(index); }
Expand Down
24 changes: 12 additions & 12 deletions Marlin/src/core/serial_hook.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct BaseSerial : public SerialBase< BaseSerial<SerialT> >, public SerialT {
void msgDone() {}

// We don't care about indices here, since if one can call us, it's the right index anyway
bool available(uint8_t) { return (bool)SerialT::available(); }
int available(uint8_t) { return (int)SerialT::available(); }
int read(uint8_t) { return (int)SerialT::read(); }
bool connected() { return CALL_IF_EXISTS(bool, static_cast<SerialT*>(this), connected);; }
void flushTX() { CALL_IF_EXISTS(void, static_cast<SerialT*>(this), flushTX); }
Expand Down Expand Up @@ -74,13 +74,13 @@ struct ConditionalSerial : public SerialBase< ConditionalSerial<SerialT> > {
void end() { out.end(); }

void msgDone() {}
bool connected() { return CALL_IF_EXISTS(bool, &out, connected); }
void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); }
bool connected() { return CALL_IF_EXISTS(bool, &out, connected); }
void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); }

bool available(uint8_t ) { return (bool)out.available(); }
int read(uint8_t ) { return (int)out.read(); }
bool available() { return (bool)out.available(); }
int read() { return (int)out.read(); }
int available(uint8_t ) { return (int)out.available(); }
int read(uint8_t ) { return (int)out.read(); }
int available() { return (int)out.available(); }
int read() { return (int)out.read(); }


ConditionalSerial(bool & conditionVariable, SerialT & out, const bool e) : BaseClassT(e), condition(conditionVariable), out(out) {}
Expand All @@ -102,9 +102,9 @@ struct ForwardSerial : public SerialBase< ForwardSerial<SerialT> > {
bool connected() { return Private::HasMember_connected<SerialT>::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; }
void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); }

bool available(uint8_t) { return (bool)out.available(); }
int available(uint8_t) { return (int)out.available(); }
int read(uint8_t) { return (int)out.read(); }
bool available() { return (bool)out.available(); }
int available() { return (int)out.available(); }
int read() { return (int)out.read(); }

ForwardSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {}
Expand All @@ -130,7 +130,7 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial<SerialT> >, public Seria
if (eofHook) eofHook(userPointer);
}

bool available(uint8_t) { return (bool)SerialT::available(); }
int available(uint8_t) { return (int)SerialT::available(); }
int read(uint8_t) { return (int)SerialT::read(); }
using SerialT::available;
using SerialT::read;
Expand Down Expand Up @@ -191,14 +191,14 @@ struct MultiSerial : public SerialBase< MultiSerial<Serial0T, Serial1T, offset,
if (portMask & FirstOutputMask) serial0.msgDone();
if (portMask & SecondOutputMask) serial1.msgDone();
}
bool available(uint8_t index) {
int available(uint8_t index) {
if (index >= 0 + offset && index < step + offset)
return serial0.available(index);
else if (index >= step + offset && index < 2 * step + offset)
return serial1.available(index);
return false;
}
NO_INLINE int read(uint8_t index) {
int read(uint8_t index) {
if (index >= 0 + offset && index < step + offset)
return serial0.read(index);
else if (index >= step + offset && index < 2 * step + offset)
Expand Down
14 changes: 6 additions & 8 deletions Marlin/src/gcode/calibrate/M100.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,16 +182,14 @@ inline int32_t count_test_bytes(const char * const start_free_memory) {
}
}

void M100_dump_routine(PGM_P const title, const char * const start, const char * const end) {
void M100_dump_routine(PGM_P const title, const GCodeQueue::CommandQueue * start, const GCodeQueue::CommandQueue * end) {
serialprintPGM(title);
SERIAL_EOL();
//
// Round the start and end locations to produce full lines of output
//
dump_free_memory(
(char*)(uintptr_t(uint32_t(start) & ~0xFUL)), // Align to 16-byte boundary
(char*)(uintptr_t(uint32_t(end) | 0xFUL)) // Align end_free_memory to the 15th byte (at or above end_free_memory)
);
while (start < end)
{
SERIAL_ECHO(start->buffer);
++start;
}
}

#endif // M100_FREE_MEMORY_DUMPER
Expand Down
18 changes: 10 additions & 8 deletions Marlin/src/gcode/gcode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ void GcodeSuite::dwell(millis_t time) {
// Placeholders for non-migrated codes
//
#if ENABLED(M100_FREE_MEMORY_WATCHER)
extern void M100_dump_routine(PGM_P const title, const char * const start, const char * const end);
extern void M100_dump_routine(PGM_P const title, const GCodeQueue::CommandQueue * start, const GCodeQueue::CommandQueue * end);
#endif

/**
Expand Down Expand Up @@ -995,25 +995,27 @@ void GcodeSuite::process_parsed_command(const bool no_ok/*=false*/) {
* This is called from the main loop()
*/
void GcodeSuite::process_next_command() {
char * const current_command = queue.command_buffer[queue.index_r];
GCodeQueue::CommandQueue & command = queue.peek_next_command();

PORT_REDIRECT(SERIAL_PORTMASK(queue.port[queue.index_r]));
#if HAS_MULTI_SERIAL
PORT_REDIRECT(SERIAL_PORTMASK(command.port));
#endif

#if ENABLED(POWER_LOSS_RECOVERY)
recovery.queue_index_r = queue.index_r;
recovery.queue_index_r = queue.ring_buffer.index_r;
#endif

if (DEBUGGING(ECHO)) {
SERIAL_ECHO_START();
SERIAL_ECHOLN(current_command);
SERIAL_ECHOLN(command.buffer);
#if ENABLED(M100_FREE_MEMORY_DUMPER)
SERIAL_ECHOPAIR("slot:", queue.index_r);
M100_dump_routine(PSTR(" Command Queue:"), &queue.command_buffer[0][0], &queue.command_buffer[BUFSIZE - 1][MAX_CMD_SIZE - 1]);
SERIAL_ECHOPAIR("slot:", queue.ring_buffer.index_r);
M100_dump_routine(PSTR(" Command Queue:"), &queue.ring_buffer.commands[0], &queue.ring_buffer.commands[BUFSIZE]);
#endif
}

// Parse the next command in the queue
parser.parse(current_command);
parser.parse(command.buffer);
process_parsed_command();
}

Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/gcode/host/M110.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@
void GcodeSuite::M110() {

if (parser.seenval('N'))
queue.last_N[queue.command_port()] = parser.value_long();
queue.set_current_line_number(parser.value_long());

}
Loading

0 comments on commit 08065f4

Please sign in to comment.