From f003e52009ba649f95463a115253d6dadcf361e6 Mon Sep 17 00:00:00 2001 From: X-Ryl669 Date: Thu, 25 Feb 2021 08:23:29 +0100 Subject: [PATCH] Combined LPC / Serial fixes (#21178) Co-authored-by: Scott Lahteine --- Marlin/src/HAL/DUE/MarlinSerialUSB.cpp | 11 ++-- Marlin/src/HAL/DUE/MarlinSerialUSB.h | 2 +- Marlin/src/HAL/LPC1768/MarlinSerial.cpp | 42 ++++++++++--- Marlin/src/HAL/LPC1768/MarlinSerial.h | 16 +++-- Marlin/src/HAL/LPC1768/usb_serial.cpp | 4 +- Marlin/src/HAL/shared/Delay.cpp | 4 +- Marlin/src/core/serial.h | 1 - Marlin/src/core/serial_base.h | 2 +- Marlin/src/core/serial_hook.h | 80 +++++++++++++------------ Marlin/src/gcode/queue.cpp | 40 ++++++------- docs/Serial.md | 17 +++++- 11 files changed, 132 insertions(+), 87 deletions(-) diff --git a/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp b/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp index a04993ca35..fca677c798 100644 --- a/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp +++ b/Marlin/src/HAL/DUE/MarlinSerialUSB.cpp @@ -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() { } diff --git a/Marlin/src/HAL/DUE/MarlinSerialUSB.h b/Marlin/src/HAL/DUE/MarlinSerialUSB.h index 5281a006b1..f9cea29869 100644 --- a/Marlin/src/HAL/DUE/MarlinSerialUSB.h +++ b/Marlin/src/HAL/DUE/MarlinSerialUSB.h @@ -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) diff --git a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp index c636a40a12..b0dfc0ae90 100644 --- a/Marlin/src/HAL/LPC1768/MarlinSerial.cpp +++ b/Marlin/src/HAL/LPC1768/MarlinSerial.cpp @@ -25,20 +25,46 @@ #include "MarlinSerial.h" #if ANY_SERIAL_IS(0) - MSerialT MSerial(true, LPC_UART0); - extern "C" void UART0_IRQHandler() { MSerial.IRQHandler(); } + MarlinSerial _MSerial(LPC_UART0); + MSerialT MSerial(true, _MSerial); + extern "C" void UART0_IRQHandler() { _MSerial.IRQHandler(); } #endif #if ANY_SERIAL_IS(1) - MSerialT MSerial1(true, (LPC_UART_TypeDef *) LPC_UART1); - extern "C" void UART1_IRQHandler() { MSerial1.IRQHandler(); } + MarlinSerial _MSerial1((LPC_UART_TypeDef *) LPC_UART1); + MSerialT MSerial1(true, _MSerial1); + extern "C" void UART1_IRQHandler() { _MSerial1.IRQHandler(); } #endif #if ANY_SERIAL_IS(2) - MSerialT MSerial2(true, LPC_UART2); - extern "C" void UART2_IRQHandler() { MSerial2.IRQHandler(); } + MarlinSerial _MSerial2(LPC_UART2); + MSerialT MSerial2(true, _MSerial2); + extern "C" void UART2_IRQHandler() { _MSerial2.IRQHandler(); } #endif #if ANY_SERIAL_IS(3) - MSerialT MSerial3(true, LPC_UART3); - extern "C" void UART3_IRQHandler() { MSerial3.IRQHandler(); } + MarlinSerial _MSerial3(LPC_UART3); + MSerialT MSerial3(true, _MSerial3); + extern "C" void UART3_IRQHandler() { _MSerial3.IRQHandler(); } +#endif + +#if ENABLED(EMERGENCY_PARSER) + + bool MarlinSerial::recv_callback(const char c) { + // Need to figure out which serial port we are and react in consequence (Marlin does not have CONTAINER_OF macro) + if (false) {} + #if ANY_SERIAL_IS(0) + else if (this == &_MSerial) emergency_parser.update(MSerial.emergency_state, c); + #endif + #if ANY_SERIAL_IS(1) + else if (this == &_MSerial1) emergency_parser.update(MSerial1.emergency_state, c); + #endif + #if ANY_SERIAL_IS(2) + else if (this == &_MSerial2) emergency_parser.update(MSerial2.emergency_state, c); + #endif + #if ANY_SERIAL_IS(3) + else if (this == &_MSerial3) emergency_parser.update(MSerial3.emergency_state, c); + #endif + return true; + } + #endif #endif // TARGET_LPC1768 diff --git a/Marlin/src/HAL/LPC1768/MarlinSerial.h b/Marlin/src/HAL/LPC1768/MarlinSerial.h index de0f62f006..35c9362b9f 100644 --- a/Marlin/src/HAL/LPC1768/MarlinSerial.h +++ b/Marlin/src/HAL/LPC1768/MarlinSerial.h @@ -47,15 +47,21 @@ public: void end() {} #if ENABLED(EMERGENCY_PARSER) - bool recv_callback(const char c) override { - emergency_parser.update(static_cast *>(this)->emergency_state, c); - return true; // do not discard character - } + bool recv_callback(const char c) override; #endif }; -typedef Serial0Type MSerialT; +// On LPC176x framework, HardwareSerial does not implement the same interface as Arduino's Serial, so overloads +// of 'available' and 'read' method are not used in this multiple inheritance scenario. +// Instead, use a ForwardSerial here that adapts the interface. +typedef ForwardSerial0Type MSerialT; extern MSerialT MSerial; extern MSerialT MSerial1; extern MSerialT MSerial2; extern MSerialT MSerial3; + +// Consequently, we can't use a RuntimeSerial either. The workaround would be to use a RuntimeSerial> type here +// Right now, let's ignore this until it's actually required. +#if ENABLED(SERIAL_RUNTIME_HOOK) + #error "SERIAL_RUNTIME_HOOK is not yet supported for LPC176x." +#endif diff --git a/Marlin/src/HAL/LPC1768/usb_serial.cpp b/Marlin/src/HAL/LPC1768/usb_serial.cpp index d225ce4188..3c1fce54f9 100644 --- a/Marlin/src/HAL/LPC1768/usb_serial.cpp +++ b/Marlin/src/HAL/LPC1768/usb_serial.cpp @@ -29,8 +29,8 @@ EmergencyParser::State emergency_state; -bool CDC_RecvCallback(const char buffer) { - emergency_parser.update(emergency_state, buffer); +bool CDC_RecvCallback(const char c) { + emergency_parser.update(emergency_state, c); return true; } diff --git a/Marlin/src/HAL/shared/Delay.cpp b/Marlin/src/HAL/shared/Delay.cpp index 8ff5a88c63..8a021a2155 100644 --- a/Marlin/src/HAL/shared/Delay.cpp +++ b/Marlin/src/HAL/shared/Delay.cpp @@ -51,7 +51,7 @@ // Use hardware cycle counter instead, it's much safer void delay_dwt(uint32_t count) { // Reuse the ASM_CYCLES_PER_ITERATION variable to avoid wasting another useless variable - register uint32_t start = HW_REG(_DWT_CYCCNT) - ASM_CYCLES_PER_ITERATION, elapsed; + uint32_t start = HW_REG(_DWT_CYCCNT) - ASM_CYCLES_PER_ITERATION, elapsed; do { elapsed = HW_REG(_DWT_CYCCNT) - start; } while (elapsed < count); @@ -114,7 +114,7 @@ serialprintPGM(unit); SERIAL_ECHOLNPAIR(" took: ", total); serialprintPGM(unit); - if (do_flush) SERIAL_FLUSH(); + if (do_flush) SERIAL_FLUSHTX(); }; uint32_t s, e; diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index 0fe8435789..f5c02f50b6 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -58,7 +58,6 @@ extern uint8_t marlin_debug_flags; // // Serial redirection // -typedef int8_t serial_index_t; #define SERIAL_ALL 0x7F #if HAS_MULTI_SERIAL #define _PORT_REDIRECT(n,p) REMEMBER(n,multiSerial.portMask,p) diff --git a/Marlin/src/core/serial_base.h b/Marlin/src/core/serial_base.h index 83b03cc7b6..418bb557e7 100644 --- a/Marlin/src/core/serial_base.h +++ b/Marlin/src/core/serial_base.h @@ -79,7 +79,7 @@ struct SerialBase { void end() { static_cast(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(this)->available(index); } + int available(uint8_t index = 0) { return static_cast(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(this)->read(index); } diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index ad8ec12b6e..afd43892c7 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -24,6 +24,9 @@ #include "macros.h" #include "serial_base.h" +// Used in multiple places +typedef int8_t serial_index_t; + // The most basic serial class: it dispatch to the base serial class with no hook whatsoever. This will compile to nothing but the base serial class template struct BaseSerial : public SerialBase< BaseSerial >, public SerialT { @@ -35,10 +38,11 @@ struct BaseSerial : public SerialBase< BaseSerial >, public SerialT { void msgDone() {} - bool available(uint8_t index) { return index == 0 && SerialT::available(); } - int read(uint8_t index) { return index == 0 ? SerialT::read() : -1; } - bool connected() { return CALL_IF_EXISTS(bool, static_cast(this), connected);; } - void flushTX() { CALL_IF_EXISTS(void, static_cast(this), flushTX); } + // We don't care about indices here, since if one can call us, it's the right index anyway + 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(this), connected);; } + void flushTX() { CALL_IF_EXISTS(void, static_cast(this), flushTX); } // We have 2 implementation of the same method in both base class, let's say which one we want using SerialT::available; @@ -65,18 +69,19 @@ struct ConditionalSerial : public SerialBase< ConditionalSerial > { bool & condition; SerialT & out; NO_INLINE size_t write(uint8_t c) { if (condition) return out.write(c); return 0; } - void flush() { if (condition) out.flush(); } - void begin(long br) { out.begin(br); } - void end() { out.end(); } + void flush() { if (condition) out.flush(); } + void begin(long br) { out.begin(br); } + 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); } + + 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(); } - bool available(uint8_t index) { return index == 0 && out.available(); } - int read(uint8_t index) { return index == 0 ? out.read() : -1; } - using BaseClassT::available; - using BaseClassT::read; ConditionalSerial(bool & conditionVariable, SerialT & out, const bool e) : BaseClassT(e), condition(conditionVariable), out(out) {} }; @@ -97,10 +102,10 @@ struct ForwardSerial : public SerialBase< ForwardSerial > { bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - bool available(uint8_t index) { return index == 0 && out.available(); } - int read(uint8_t index) { return index == 0 ? out.read() : -1; } - bool available() { return out.available(); } - int read() { return 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(); } ForwardSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} }; @@ -125,8 +130,8 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial >, public Seria if (eofHook) eofHook(userPointer); } - bool available(uint8_t index) { return index == 0 && SerialT::available(); } - int read(uint8_t index) { return index == 0 ? SerialT::read() : -1; } + int available(uint8_t) { return (int)SerialT::available(); } + int read(uint8_t) { return (int)SerialT::read(); } using SerialT::available; using SerialT::read; using SerialT::flush; @@ -157,21 +162,22 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial >, public Seria // Forward constructor template - RuntimeSerial(const bool e, Args... args) : BaseClassT(e), SerialT(args...) {} + RuntimeSerial(const bool e, Args... args) : BaseClassT(e), SerialT(args...), writeHook(0), eofHook(0), userPointer(0) {} }; // A class that's duplicating its output conditionally to 2 serial interface -template -struct MultiSerial : public SerialBase< MultiSerial > { - typedef SerialBase< MultiSerial > BaseClassT; +template +struct MultiSerial : public SerialBase< MultiSerial > { + typedef SerialBase< MultiSerial > BaseClassT; uint8_t portMask; Serial0T & serial0; Serial1T & serial1; enum Masks { - FirstOutputMask = (1 << offset), - SecondOutputMask = (1 << (offset + 1)), + UsageMask = ((1 << step) - 1), // A bit mask containing as many bits as step + FirstOutputMask = (UsageMask << offset), + SecondOutputMask = (UsageMask << (offset + step)), AllMask = FirstOutputMask | SecondOutputMask, }; @@ -185,19 +191,19 @@ struct MultiSerial : public SerialBase< MultiSerial if (portMask & FirstOutputMask) serial0.msgDone(); if (portMask & SecondOutputMask) serial1.msgDone(); } - bool available(uint8_t index) { - switch(index) { - case 0 + offset: return serial0.available(); - case 1 + offset: return serial1.available(); - default: return false; - } + 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) { - switch(index) { - case 0 + offset: return serial0.read(); - case 1 + offset: return serial1.read(); - default: return -1; - } + 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) + return serial1.read(index); + return -1; } void begin(const long br) { if (portMask & FirstOutputMask) serial0.begin(br); diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index c49247912c..8c9e9afdc1 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -63,6 +63,10 @@ GCodeQueue queue; // Frequently used G-code strings PGMSTR(G28_STR, "G28"); +#if NO_TIMEOUTS > 0 + static millis_t last_command_time = 0; +#endif + /** * GCode line number handling. Hosts may opt to include line numbers when * sending commands to Marlin, and lines will be checked for sequentiality. @@ -288,6 +292,10 @@ void GCodeQueue::enqueue_now_P(PGM_P const pgcode) { * B Block queue space remaining */ void GCodeQueue::ok_to_send() { + #if NO_TIMEOUTS > 0 + // Start counting from the last command's execution + last_command_time = millis(); + #endif #if HAS_MULTI_SERIAL const serial_index_t serial_ind = command_port(); if (serial_ind < 0) return; @@ -324,28 +332,19 @@ void GCodeQueue::flush_and_request_resend() { ok_to_send(); } -inline bool serial_data_available() { - byte data_available = 0; - if (MYSERIAL0.available()) data_available++; - #ifdef SERIAL_PORT_2 - const bool port2_open = TERN1(HAS_ETHERNET, ethernet.have_telnet_client); - if (port2_open && MYSERIAL1.available()) data_available++; - #endif - return data_available > 0; + +// Multiserial already handle the dispatch to/from multiple port by itself +inline bool serial_data_available(uint8_t index = SERIAL_ALL) { + if (index == SERIAL_ALL) { + for (index = 0; index < NUM_SERIAL; index++) { + if (SERIAL_IMPL.available(index) > 0) return true; + } + return false; + } + return SERIAL_IMPL.available(index) > 0; } -inline int read_serial(const uint8_t index) { - switch (index) { - case 0: return MYSERIAL0.read(); - case 1: { - #if HAS_MULTI_SERIAL - const bool port2_open = TERN1(HAS_ETHERNET, ethernet.have_telnet_client); - if (port2_open) return MYSERIAL1.read(); - #endif - } - default: return -1; - } -} +inline int read_serial(const uint8_t index) { return SERIAL_IMPL.read(index); } void GCodeQueue::gcode_line_error(PGM_P const err, const serial_index_t serial_ind) { PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command @@ -460,7 +459,6 @@ void GCodeQueue::get_serial_commands() { // If the command buffer is empty for too long, // send "wait" to indicate Marlin is still waiting. #if NO_TIMEOUTS > 0 - static millis_t last_command_time = 0; const millis_t ms = millis(); if (length == 0 && !serial_data_available() && ELAPSED(ms, last_command_time + NO_TIMEOUTS)) { SERIAL_ECHOLNPGM(STR_WAIT); diff --git a/docs/Serial.md b/docs/Serial.md index 0db8175b3d..b4079d765f 100644 --- a/docs/Serial.md +++ b/docs/Serial.md @@ -30,14 +30,25 @@ In the `Marlin/src/core/serial_hook.h` file, the different serial feature are de Since all the types above are using CRTP, it's possible to combine them to get the appropriate functionality. This is easily done via type definition of the feature. -For example, to present a serial interface that's outputting to 2 serial port, the first one being hooked at runtime and the second one connected to a runtime switchable telnet client, you'll declare the type to use as: +For example, to create a single serial interface with 2 serial outputs (one enabled at runtime and the other switchable): ```cpp typedef MultiSerial< RuntimeSerial, ConditionalSerial > Serial0Type; ``` +To send the same output to 4 serial ports you could nest `MultiSerial` like this: +```cpp +typedef MultiSerial< MultiSerial< BaseSerial, BaseSerial >, MultiSerial< BaseSerial, BaseSerial, 2, 1>, 0, 2> Serial0Type; +``` +The magical numbers here are the step and offset for computing the serial port. Simplifying the above monster a bit: +```cpp +MS< A = MS, B=MS, offset=0, step=2> +``` +This means that the underlying multiserial A (with output to `a,b`) is available from offset = 0 to offset + step = 1 (default value). +The multiserial B (with output to `c,d`) is available from offset = 2 (the next step from the root multiserial) to offset + step = 3. +In practice, the root multiserial will redirect any index/mask `offset` to `offset + step - 1` to its first leaf, and any index/mask `offset + step` to `offset + 2*step - 1` to its second leaf. + ## Emergency parser -By default, the serial base interface provide an emergency parser that's only enable for serial classes that support it. -Because of this condition, all underlying type takes a first `bool emergencyParserEnabled` argument to their constructor. You must take into account this parameter when defining the actual type used. +By default, the serial base interface provide an emergency parser that's only enable for serial classes that support it. Because of this condition, all underlying types take a first `bool emergencyParserEnabled` argument to their constructor. You must take into account this parameter when defining the actual type used. ## SERIAL macros The following macros are defined (in `serial.h`) to output data to the serial ports: