Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix more STM32F1 Serial hangs due to overflows #19464

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
3e3e187
Update Malyan LCD to use MSerial on STM32F1, and try to prevent use o…
sjasonsmith Sep 21, 2020
6a718b2
Add Configuration.h option.
sjasonsmith Sep 21, 2020
d59c3d2
Allow compiling with STM32 HAL (for Malyan M200/M300)
sjasonsmith Sep 22, 2020
2b824e4
Fix minor issues impacting CI
sjasonsmith Sep 22, 2020
3432150
Macros for other serial port defines
thinkyhead Sep 23, 2020
60a7c21
tweak option name
thinkyhead Sep 23, 2020
d7986fa
Misc fixes, adjustments
thinkyhead Sep 23, 2020
9d81ee6
Merge remote-tracking branch 'upstream/bugfix-2.0.x' into pr/19464
thinkyhead Sep 23, 2020
62b4062
Tweak ANY_RX
thinkyhead Sep 23, 2020
997cad7
Update MarlinSerial.h
thinkyhead Sep 23, 2020
4b0aa06
Fix, consolidate serial sanity-checking
thinkyhead Sep 23, 2020
6a6432c
Merge branch 'PR/18358_F1_Serial_Ints' of https://github.com/sjasonsm…
thinkyhead Sep 23, 2020
678799c
Macro patch
thinkyhead Sep 23, 2020
19abe24
prep for redo
thinkyhead Sep 23, 2020
5cc87f9
Define NUM_SERIAL once
thinkyhead Sep 23, 2020
2d4d582
keep feet on ground
thinkyhead Sep 23, 2020
f16388b
etc
thinkyhead Sep 24, 2020
804d1b1
Update Configuration.h
thinkyhead Sep 24, 2020
d6035bc
HAL and serial cleanup
thinkyhead Sep 24, 2020
98a225d
Merge remote-tracking branch 'upstream/bugfix-2.0.x' into pr/19464
thinkyhead Sep 24, 2020
db35a68
Merge branch 'bf2_lcd_serial_fixups_19464' into pr/19464
thinkyhead Sep 24, 2020
3732dd8
typo
thinkyhead Sep 24, 2020
d974f7a
fix test
thinkyhead Sep 24, 2020
b253be5
Merge remote-tracking branch 'upstream/bugfix-2.0.x' into pr/19464
thinkyhead Sep 24, 2020
e24ee2e
Consolidate LCD_SERIAL defines
thinkyhead Sep 24, 2020
a2641dc
lcd baudrate
thinkyhead Sep 24, 2020
69c6c3d
Serial fixes
thinkyhead Sep 24, 2020
a598fc7
Merge remote-tracking branch 'upstream/bugfix-2.0.x' into pr/19464
thinkyhead Sep 24, 2020
824d31a
better
thinkyhead Sep 24, 2020
ab550c8
Shift MarlinSerial left
thinkyhead Sep 24, 2020
1bb005d
Ok to keep JTAG off?
thinkyhead Sep 24, 2020
d3d5f91
Remove duplicate LCD_SERIAL check
sjasonsmith Sep 24, 2020
f49dc6b
Add MSerial::emergency_parser_enabled
thinkyhead Sep 25, 2020
9428396
Add a bunch of logic
thinkyhead Sep 25, 2020
d98d7d8
Apply to other HALs
thinkyhead Sep 25, 2020
f8d538d
Fix ep-enabled
thinkyhead Sep 25, 2020
9bfdbee
Evaluate macro arg
thinkyhead Sep 25, 2020
a830dbd
undo 1
thinkyhead Sep 25, 2020
4c54163
Revert serial logic
thinkyhead Sep 25, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Marlin/Configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,10 @@
// Touch-screen LCD for Malyan M200/M300 printers
//
//#define MALYAN_LCD
#if ENABLED(MALYAN_LCD)
// This will be 1 on a Malyan M200, but may differ on other boards.
#define MALYAN_LCD_SERIAL_PORT 1
#endif
thinkyhead marked this conversation as resolved.
Show resolved Hide resolved

//
// Touch UI for FTDI EVE (FT800/FT810) displays
Expand Down
84 changes: 42 additions & 42 deletions Marlin/src/HAL/STM32F1/HAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,73 +68,73 @@
#endif
#endif

#if SERIAL_PORT == 0
#error "SERIAL_PORT cannot be 0. (Port 0 does not exist.) Please update your configuration."
#elif SERIAL_PORT == -1
#define _MSERIAL(X) MSerial##X
#define MSERIAL(X) _MSERIAL(X)

#if defined(STM32_HIGH_DENSITY) || defined(STM32_XL_DENSITY)
#define NUM_UARTS 5
#else
#define NUM_UARTS 3
#endif

#if SERIAL_PORT == -1
#define MYSERIAL0 UsbSerial
#elif SERIAL_PORT == 1
#define MYSERIAL0 MSerial1
#elif SERIAL_PORT == 2
#define MYSERIAL0 MSerial2
#elif SERIAL_PORT == 3
#define MYSERIAL0 MSerial3
#elif SERIAL_PORT == 4
#define MYSERIAL0 MSerial4
#elif SERIAL_PORT == 5
#define MYSERIAL0 MSerial5
#elif WITHIN(SERIAL_PORT, 1, NUM_UARTS)
#define MYSERIAL0 MSERIAL(SERIAL_PORT)
#elif defined(STM32_HIGH_DENSITY) || defined(STM32_XL_DENSITY)
#error "SERIAL_PORT must be -1 or from 1 to 5. Please update your configuration."
#else
#error "SERIAL_PORT must be from -1 to 5. Please update your configuration."
#error "SERIAL_PORT must be -1 or from 1 to 3. Please update your configuration."
#endif

#ifdef SERIAL_PORT_2
#if SERIAL_PORT_2 == 0
#error "SERIAL_PORT_2 cannot be 0. (Port 0 does not exist.) Please update your configuration."
#elif SERIAL_PORT_2 == SERIAL_PORT
#if SERIAL_PORT_2 == SERIAL_PORT
#error "SERIAL_PORT_2 must be different than SERIAL_PORT. Please update your configuration."
#elif SERIAL_PORT_2 == -1
#define MYSERIAL1 UsbSerial
#elif SERIAL_PORT_2 == 1
#define MYSERIAL1 MSerial1
#elif SERIAL_PORT_2 == 2
#define MYSERIAL1 MSerial2
#elif SERIAL_PORT_2 == 3
#define MYSERIAL1 MSerial3
#elif SERIAL_PORT_2 == 4
#define MYSERIAL1 MSerial4
#elif SERIAL_PORT_2 == 5
#define MYSERIAL1 MSerial5
#elif WITHIN(SERIAL_PORT_2, 1, NUM_UARTS)
#define MYSERIAL1 MSERIAL(SERIAL_PORT_2)
#elif defined(STM32_HIGH_DENSITY) || defined(STM32_XL_DENSITY)
#error "SERIAL_PORT_2 must be -1 or from 1 to 5. Please update your configuration."
#else
#error "SERIAL_PORT_2 must be from -1 to 5. Please update your configuration."
#error "SERIAL_PORT_2 must be -1 or from 1 to 3. Please update your configuration."
#endif
#define NUM_SERIAL 2
#else
#define NUM_SERIAL 1
#endif

#ifdef DGUS_SERIAL
#if DGUS_SERIAL_PORT == 0
#error "DGUS_SERIAL_PORT cannot be 0. (Port 0 does not exist.) Please update your configuration."
#elif DGUS_SERIAL_PORT == SERIAL_PORT
#if DGUS_SERIAL_PORT == SERIAL_PORT
#error "DGUS_SERIAL_PORT must be different than SERIAL_PORT. Please update your configuration."
#elif defined(SERIAL_PORT_2) && DGUS_SERIAL_PORT == SERIAL_PORT_2
#error "DGUS_SERIAL_PORT must be different than SERIAL_PORT_2. Please update your configuration."
#elif DGUS_SERIAL_PORT == -1
#define DGUS_SERIAL UsbSerial
#elif DGUS_SERIAL_PORT == 1
#define DGUS_SERIAL MSerial1
#elif DGUS_SERIAL_PORT == 2
#define DGUS_SERIAL MSerial2
#elif DGUS_SERIAL_PORT == 3
#define DGUS_SERIAL MSerial3
#elif DGUS_SERIAL_PORT == 4
#define DGUS_SERIAL MSerial4
#elif DGUS_SERIAL_PORT == 5
#define DGUS_SERIAL MSerial5
#elif WITHIN(DGUS_SERIAL_PORT, 1, NUM_UARTS)
#define DGUS_SERIAL MSERIAL(DGUS_SERIAL_PORT)
#elif defined(STM32_HIGH_DENSITY) || defined(STM32_XL_DENSITY)
#error "DGUS_SERIAL must be -1 or from 1 to 5. Please update your configuration."
#else
#error "DGUS_SERIAL_PORT must be from -1 to 5. Please update your configuration."
#error "DGUS_SERIAL must be -1 or from 1 to 3. Please update your configuration."
#endif
#endif

#ifdef MALYAN_LCD
#if MALYAN_LCD_SERIAL_PORT == SERIAL_PORT
#error "MALYAN_LCD_SERIAL_PORT must be different than SERIAL_PORT. Please update your configuration."
#elif defined(SERIAL_PORT_2) && MALYAN_LCD_SERIAL_PORT == SERIAL_PORT_2
#error "MALYAN_LCD_SERIAL_PORT must be different than SERIAL_PORT_2. Please update your configuration."
#elif defined(DGUS_SERIAL) && MALYAN_LCD_SERIAL_PORT == DGUS_SERIAL_PORT
#error "MALYAN_LCD_SERIAL_PORT must be different than DGUS_SERIAL_PORT. Please update your configuration."
#elif WITHIN(MALYAN_LCD_SERIAL_PORT, 1, NUM_UARTS)
#define MALYAN_LCD_SERIAL MSERIAL(MALYAN_LCD_SERIAL_PORT)
#elif defined(STM32_HIGH_DENSITY) || defined(STM32_XL_DENSITY)
#error "MALYAN_LCD_SERIAL_PORT must be from 1 to 5. Please update your configuration."
#else
#error "MALYAN_LCD_SERIAL_PORT must be from 1 to 3. Please update your configuration."
#endif
#endif

// Set interrupt grouping for this MCU
void HAL_init();
Expand Down
129 changes: 107 additions & 22 deletions Marlin/src/HAL/STM32F1/MarlinSerial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

#ifdef __STM32F1__

#include "../../inc/MarlinConfigPre.h"
#include "../../inc/MarlinConfig.h"
#include "MarlinSerial.h"
#include <libmaple/usart.h>

Expand Down Expand Up @@ -53,7 +53,8 @@ static inline __always_inline void my_usart_irq(ring_buffer *rb, ring_buffer *wb
rb_push_insert(rb, c);
#endif
#if ENABLED(EMERGENCY_PARSER)
emergency_parser.update(serial.emergency_state, c);
if (serial.emergency_parser_enabled)
emergency_parser.update(serial.emergency_state, c);
#endif
}
}
Expand All @@ -73,40 +74,124 @@ static inline __always_inline void my_usart_irq(ring_buffer *rb, ring_buffer *wb
}
}

// Not every MarlinSerial port should handle emergency parsing.
// It would not make sense to parse GCode from TMC responses, for example.
constexpr bool serial_handles_emergency(int port)
{
return false
#ifdef SERIAL_PORT
|| (SERIAL_PORT) == port
#endif
#ifdef SERIAL_PORT_2
|| (SERIAL_PORT_2) == port
#endif
#ifdef DGUS_SERIAL_PORT
|| (DGUS_SERIAL_PORT) == port
#endif
;
}

#define DEFINE_HWSERIAL_MARLIN(name, n) \
MarlinSerial name(USART##n, \
BOARD_USART##n##_TX_PIN, \
BOARD_USART##n##_RX_PIN); \
BOARD_USART##n##_RX_PIN, \
serial_handles_emergency(n));\
extern "C" void __irq_usart##n(void) { \
my_usart_irq(USART##n->rb, USART##n->wb, USART##n##_BASE, MSerial##n); \
}

#define DEFINE_HWSERIAL_UART_MARLIN(name, n) \
MarlinSerial name(UART##n, \
BOARD_USART##n##_TX_PIN, \
BOARD_USART##n##_RX_PIN); \
extern "C" void __irq_usart##n(void) { \
my_usart_irq(USART##n->rb, USART##n->wb, USART##n##_BASE, MSerial##n); \
}
MarlinSerial name(UART##n, \
BOARD_USART##n##_TX_PIN, \
BOARD_USART##n##_RX_PIN, \
serial_handles_emergency(n)); \
extern "C" void __irq_usart##n(void) { \
my_usart_irq(UART##n->rb, UART##n->wb, UART##n##_BASE, MSerial##n); \
}

#if SERIAL_PORT == 1 || SERIAL_PORT_2 == 1 || DGUS_SERIAL_PORT == 1
DEFINE_HWSERIAL_MARLIN(MSerial1, 1);
// Instantiate all UARTs even if they are not needed
// This avoids a bunch of logic to figure out every serial
// port which may be in use on the system.
DEFINE_HWSERIAL_MARLIN(MSerial1, 1);
DEFINE_HWSERIAL_MARLIN(MSerial2, 2);
DEFINE_HWSERIAL_MARLIN(MSerial3, 3);
#if defined(STM32_HIGH_DENSITY) || defined(STM32_XL_DENSITY)
DEFINE_HWSERIAL_UART_MARLIN(MSerial4, 4);
DEFINE_HWSERIAL_UART_MARLIN(MSerial5, 5);
#endif

#if SERIAL_PORT == 2 || SERIAL_PORT_2 == 2 || DGUS_SERIAL_PORT == 2
DEFINE_HWSERIAL_MARLIN(MSerial2, 2);
#endif
// Check the type of each serial port by passing it to a template function.
// HardwareSerial is known to sometimes hang the controller when an error occurs,
// so this case will fail the static assert. All other classes are assumed to be ok.
template <typename T>
constexpr bool IsSerialClassAllowed(const T&) { return true; }
constexpr bool IsSerialClassAllowed(const HardwareSerial&) { return false; }

#if SERIAL_PORT == 3 || SERIAL_PORT_2 == 3 || DGUS_SERIAL_PORT == 3
DEFINE_HWSERIAL_MARLIN(MSerial3, 3);
#endif
#define CHECK_CFG_SERIAL(A) static_assert(IsSerialClassAllowed(A), STRINGIFY(A) " is defined incorrectly");
#define CHECK_AXIS_SERIAL(A) static_assert(IsSerialClassAllowed(A##_HARDWARE_SERIAL), STRINGIFY(A) "_HARDWARE_SERIAL must be defined in the form MSerial1, rather than Serial1");

#if SERIAL_PORT == 4 || SERIAL_PORT_2 == 4 || DGUS_SERIAL_PORT == 4
DEFINE_HWSERIAL_UART_MARLIN(MSerial4, 4);
#endif
// If you encounter this error, replace SerialX with MSerialX, for example MSerial3.

#if SERIAL_PORT == 5 || SERIAL_PORT_2 == 5 || DGUS_SERIAL_PORT == 5
DEFINE_HWSERIAL_UART_MARLIN(MSerial5, 5);
// Non-TMC ports were already validated in HAL.h, so do not require verbose error messages.
#ifdef MYSERIAL0
CHECK_CFG_SERIAL(MYSERIAL0);
#endif
#ifdef MYSERIAL1
CHECK_CFG_SERIAL(MYSERIAL1);
#endif
#ifdef DGUS_SERIAL
CHECK_CFG_SERIAL(DGUS_SERIAL);
#endif
#ifdef MALYAN_LCD_SERIAL
CHECK_CFG_SERIAL(MALYAN_LCD_SERIAL);
#endif
#if AXIS_HAS_HW_SERIAL(X)
CHECK_AXIS_SERIAL(X);
#endif
#if AXIS_HAS_HW_SERIAL(X2)
CHECK_AXIS_SERIAL(X2);
#endif
#if AXIS_HAS_HW_SERIAL(Y)
CHECK_AXIS_SERIAL(Y);
#endif
#if AXIS_HAS_HW_SERIAL(Y2)
CHECK_AXIS_SERIAL(Y2);
#endif
#if AXIS_HAS_HW_SERIAL(Z)
CHECK_AXIS_SERIAL(Z);
#endif
#if AXIS_HAS_HW_SERIAL(Z2)
CHECK_AXIS_SERIAL(Z2);
#endif
#if AXIS_HAS_HW_SERIAL(Z3)
CHECK_AXIS_SERIAL(Z3);
#endif
#if AXIS_HAS_HW_SERIAL(Z4)
CHECK_AXIS_SERIAL(Z4);
#endif
#if AXIS_HAS_HW_SERIAL(E0)
CHECK_AXIS_SERIAL(E0);
#endif
#if AXIS_HAS_HW_SERIAL(E1)
CHECK_AXIS_SERIAL(E1);
#endif
#if AXIS_HAS_HW_SERIAL(E2)
CHECK_AXIS_SERIAL(E2);
#endif
#if AXIS_HAS_HW_SERIAL(E3)
CHECK_AXIS_SERIAL(E3);
#endif
#if AXIS_HAS_HW_SERIAL(E4)
CHECK_AXIS_SERIAL(E4);
#endif
#if AXIS_HAS_HW_SERIAL(E5)
CHECK_AXIS_SERIAL(E5);
#endif
#if AXIS_HAS_HW_SERIAL(E6)
CHECK_AXIS_SERIAL(E6);
#endif
#if AXIS_HAS_HW_SERIAL(E7)
CHECK_AXIS_SERIAL(E7);
#endif

#endif // __STM32F1__
19 changes: 14 additions & 5 deletions Marlin/src/HAL/STM32F1/MarlinSerial.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,22 @@
#include "../../feature/e_parser.h"
#endif

// Increase priority of serial interrupts, to reduce overflow errors
#define UART_IRQ_PRIO 1

class MarlinSerial : public HardwareSerial {
public:
MarlinSerial(struct usart_dev *usart_device, uint8 tx_pin, uint8 rx_pin) :
MarlinSerial(struct usart_dev *usart_device, uint8 tx_pin, uint8 rx_pin, bool emergency_parser) :
HardwareSerial(usart_device, tx_pin, rx_pin)
#if ENABLED(EMERGENCY_PARSER)
, emergency_state(EmergencyParser::State::EP_RESET)
, emergency_parser_enabled(emergency_parser),
emergency_state(EmergencyParser::State::EP_RESET)
#endif
{ }
{
#if DISABLED(EMERGENCY_PARSER)
UNUSED(emergency_parser);
#endif
}

#ifdef UART_IRQ_PRIO
// shadow the parent methods to set irq priority after the begin
Expand All @@ -54,12 +60,15 @@ class MarlinSerial : public HardwareSerial {
#endif

#if ENABLED(EMERGENCY_PARSER)
bool emergency_parser_enabled;
EmergencyParser::State emergency_state;
#endif
};

extern MarlinSerial MSerial1;
extern MarlinSerial MSerial2;
extern MarlinSerial MSerial3;
extern MarlinSerial MSerial4;
extern MarlinSerial MSerial5;
#if defined(STM32_HIGH_DENSITY) || defined(STM32_XL_DENSITY)
extern MarlinSerial MSerial4;
extern MarlinSerial MSerial5;
#endif
14 changes: 8 additions & 6 deletions Marlin/src/lcd/extui/malyan_lcd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@

// On the Malyan M200, this will be Serial1. On a RAMPS board,
// it might not be.
#define LCD_SERIAL Serial1
#ifndef MALYAN_LCD_SERIAL
#error "Must define MALYAN_LCD_SERIAL to use MALYAN_LCD"
#endif

// This is based on longest sys command + a filename, plus some buffer
// in case we encounter some data we don't recognize
Expand All @@ -84,7 +86,7 @@ void write_to_lcd_P(PGM_P const message) {
LOOP_L_N(i, message_length)
encoded_message[i] = pgm_read_byte(&message[i]) | 0x80;

LCD_SERIAL.Print::write(encoded_message, message_length);
MALYAN_LCD_SERIAL.Print::write(encoded_message, message_length);
}

void write_to_lcd(const char * const message) {
Expand All @@ -94,7 +96,7 @@ void write_to_lcd(const char * const message) {
LOOP_L_N(i, message_length)
encoded_message[i] = message[i] | 0x80;

LCD_SERIAL.Print::write(encoded_message, message_length);
MALYAN_LCD_SERIAL.Print::write(encoded_message, message_length);
}

// {E:<msg>} is for error states.
Expand Down Expand Up @@ -432,7 +434,7 @@ namespace ExtUI {
* it and translate into ExtUI operations where possible.
*/
inbound_count = 0;
LCD_SERIAL.begin(500000);
MALYAN_LCD_SERIAL.begin(500000);

// Signal init
write_to_lcd_P(PSTR("{SYS:STARTED}\r\n"));
Expand All @@ -455,8 +457,8 @@ namespace ExtUI {
update_usb_status(false);

// now drain commands...
while (LCD_SERIAL.available())
parse_lcd_byte((byte)LCD_SERIAL.read());
while (MALYAN_LCD_SERIAL.available())
parse_lcd_byte((byte)MALYAN_LCD_SERIAL.read());

#if ENABLED(SDSUPPORT)
// The way last printing status works is simple:
Expand Down
Loading