diff --git a/Marlin/src/core/serial.h b/Marlin/src/core/serial.h index f76f0e32f..57e9636be 100644 --- a/Marlin/src/core/serial.h +++ b/Marlin/src/core/serial.h @@ -62,37 +62,36 @@ extern uint8_t marlin_debug_flags; // // Serial redirection // -#define SERIAL_ALL 0xFF #if HAS_MULTI_SERIAL - #define _PORT_REDIRECT(n,p) REMEMBER(n,multiSerial.portMask,p) - #define _PORT_RESTORE(n,p) RESTORE(n) - #define SERIAL_ASSERT(P) if(multiSerial.portMask!=(P)){ debugger(); } + #define _PORT_REDIRECT(n,p) REMEMBER(n,multiSerial.portMask,p) + #define _PORT_RESTORE(n,p) RESTORE(n) + #define SERIAL_ASSERT(P) if(multiSerial.portMask!=(P)){ debugger(); } #ifdef SERIAL_CATCHALL typedef MultiSerial SerialOutputT; #else - typedef MultiSerial, decltype(MYSERIAL1)), 0> SerialOutputT; + typedef MultiSerial, decltype(MYSERIAL1)), 0> SerialOutputT; #endif - extern SerialOutputT multiSerial; - #define _SERIAL_IMPL multiSerial + extern SerialOutputT multiSerial; + #define _SERIAL_IMPL multiSerial #else - #define _PORT_REDIRECT(n,p) NOOP - #define _PORT_RESTORE(n) NOOP - #define SERIAL_ASSERT(P) NOOP - #define _SERIAL_IMPL MYSERIAL0 + #define _PORT_REDIRECT(n,p) NOOP + #define _PORT_RESTORE(n) NOOP + #define SERIAL_ASSERT(P) NOOP + #define _SERIAL_IMPL MYSERIAL0 #endif #if ENABLED(MEATPACK) extern MeatpackSerial mpSerial; - #define SERIAL_IMPL mpSerial + #define SERIAL_IMPL mpSerial #else - #define SERIAL_IMPL _SERIAL_IMPL + #define SERIAL_IMPL _SERIAL_IMPL #endif #define SERIAL_OUT(WHAT, V...) (void)SERIAL_IMPL.WHAT(V) -#define PORT_REDIRECT(p) _PORT_REDIRECT(1,p) -#define PORT_RESTORE() _PORT_RESTORE(1) -#define SERIAL_PORTMASK(P) _BV(P) +#define PORT_REDIRECT(p) _PORT_REDIRECT(1,p) +#define PORT_RESTORE() _PORT_RESTORE(1) +#define SERIAL_PORTMASK(P) SerialMask::from(P) // // SERIAL_CHAR - Print one or more individual chars diff --git a/Marlin/src/core/serial_base.h b/Marlin/src/core/serial_base.h index 418bb557e..f8fe3a181 100644 --- a/Marlin/src/core/serial_base.h +++ b/Marlin/src/core/serial_base.h @@ -22,12 +22,29 @@ #pragma once #include "../inc/MarlinConfigPre.h" -#include "macros.h" #if ENABLED(EMERGENCY_PARSER) #include "../feature/e_parser.h" #endif +// Used in multiple places +// You can build it but not manipulate it. +// There are only few places where it's required to access the underlying member: GCodeQueue, SerialMask and MultiSerial +struct serial_index_t { + // A signed index, where -1 is a special case meaning no action (neither output or input) + int8_t index; + + // Check if the index is within the range [a ... b] + constexpr inline bool within(const int8_t a, const int8_t b) const { return WITHIN(index, a, b); } + constexpr inline bool valid() const { return WITHIN(index, 0, 7); } // At most, 8 bits + + // Construction is either from an index + constexpr serial_index_t(const int8_t index) : index(index) {} + + // Default to "no index" + constexpr serial_index_t() : index(-1) {} +}; + // flushTX is not implemented in all HAL, so use SFINAE to call the method where it is. CALL_IF_EXISTS_IMPL(void, flushTX); CALL_IF_EXISTS_IMPL(bool, connected, true); @@ -79,10 +96,10 @@ struct SerialBase { void end() { static_cast(this)->end(); } /** Check for available data from the port @param index The port index, usually 0 */ - int available(uint8_t index = 0) { return static_cast(this)->available(index); } + int available(serial_index_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); } + int read(serial_index_t index = 0) { return static_cast(this)->read(index); } // Check if the serial port is connected (usually bypassed) bool connected() { return static_cast(this)->connected(); } // Redirect flush diff --git a/Marlin/src/core/serial_hook.h b/Marlin/src/core/serial_hook.h index dc2da1350..5e81e9e0e 100644 --- a/Marlin/src/core/serial_hook.h +++ b/Marlin/src/core/serial_hook.h @@ -21,11 +21,32 @@ */ #pragma once -#include "macros.h" #include "serial_base.h" -// Used in multiple places -typedef int8_t serial_index_t; +// A mask containing a bitmap of the serial port to act upon +// This is written to ensure a serial index is never used as a serial mask +class SerialMask { + uint8_t mask; + + // This constructor is private to ensure you can't convert an index to a mask + // The compiler will stop here if you are mixing index and mask in your code. + // If you need to, you'll have to use the explicit static "from" method here + SerialMask(const serial_index_t); + +public: + inline constexpr bool enabled(const SerialMask PortMask) const { return mask & PortMask.mask; } + inline constexpr SerialMask combine(const SerialMask other) const { return SerialMask(mask | other.mask); } + inline constexpr SerialMask operator<< (const int offset) const { return SerialMask(mask << offset); } + static inline SerialMask from(const serial_index_t index) { + if (index.valid()) return SerialMask(_BV(index.index)); + return SerialMask(0); // A invalid index mean no output + } + + constexpr SerialMask(const uint8_t mask) : mask(mask) {} + constexpr SerialMask(const SerialMask & other) : mask(other.mask) {} // Can't use = default here since not all framework support this + + static constexpr uint8_t All = 0xFF; +}; // 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 @@ -39,10 +60,10 @@ struct BaseSerial : public SerialBase< BaseSerial >, public SerialT { void msgDone() {} // 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); } + int available(serial_index_t) { return (int)SerialT::available(); } + int read(serial_index_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; @@ -77,11 +98,10 @@ struct ConditionalSerial : public SerialBase< ConditionalSerial > { 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(); } - + int available(serial_index_t ) { return (int)out.available(); } + int read(serial_index_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) {} }; @@ -102,8 +122,8 @@ 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); } - int available(uint8_t) { return (int)out.available(); } - int read(uint8_t) { return (int)out.read(); } + int available(serial_index_t) { return (int)out.available(); } + int read(serial_index_t) { return (int)out.read(); } int available() { return (int)out.available(); } int read() { return (int)out.read(); } @@ -130,8 +150,8 @@ struct RuntimeSerial : public SerialBase< RuntimeSerial >, public Seria if (eofHook) eofHook(userPointer); } - int available(uint8_t) { return (int)SerialT::available(); } - int read(uint8_t) { return (int)SerialT::read(); } + int available(serial_index_t) { return (int)SerialT::available(); } + int read(serial_index_t) { return (int)SerialT::read(); } using SerialT::available; using SerialT::read; using SerialT::flush; @@ -170,53 +190,51 @@ template > { typedef SerialBase< MultiSerial > BaseClassT; - uint8_t portMask; + SerialMask portMask; Serial0T & serial0; Serial1T & serial1; - enum Masks { - UsageMask = ((1 << step) - 1), // A bit mask containing as many bits as step - FirstOutputMask = (UsageMask << offset), - SecondOutputMask = (UsageMask << (offset + step)), - AllMask = FirstOutputMask | SecondOutputMask, - }; + static constexpr uint8_t Usage = ((1 << step) - 1); // A bit mask containing as many bits as step + static constexpr uint8_t FirstOutput = (Usage << offset); + static constexpr uint8_t SecondOutput = (Usage << (offset + step)); + static constexpr uint8_t Both = FirstOutput | SecondOutput; NO_INLINE size_t write(uint8_t c) { size_t ret = 0; - if (portMask & FirstOutputMask) ret = serial0.write(c); - if (portMask & SecondOutputMask) ret = serial1.write(c) | ret; + if (portMask.enabled(FirstOutput)) ret = serial0.write(c); + if (portMask.enabled(SecondOutput)) ret = serial1.write(c) | ret; return ret; } NO_INLINE void msgDone() { - if (portMask & FirstOutputMask) serial0.msgDone(); - if (portMask & SecondOutputMask) serial1.msgDone(); + if (portMask.enabled(FirstOutput)) serial0.msgDone(); + if (portMask.enabled(SecondOutput)) serial1.msgDone(); } - int available(uint8_t index) { - if (index >= 0 + offset && index < step + offset) + int available(serial_index_t index) { + if (index.within(0 + offset, step + offset - 1)) return serial0.available(index); - else if (index >= step + offset && index < 2 * step + offset) + else if (index.within(step + offset, 2 * step + offset - 1)) return serial1.available(index); return false; } - int read(uint8_t index) { - if (index >= 0 + offset && index < step + offset) + int read(serial_index_t index) { + if (index.within(0 + offset, step + offset - 1)) return serial0.read(index); - else if (index >= step + offset && index < 2 * step + offset) + else if (index.within(step + offset, 2 * step + offset - 1)) return serial1.read(index); return -1; } void begin(const long br) { - if (portMask & FirstOutputMask) serial0.begin(br); - if (portMask & SecondOutputMask) serial1.begin(br); + if (portMask.enabled(FirstOutput)) serial0.begin(br); + if (portMask.enabled(SecondOutput)) serial1.begin(br); } void end() { - if (portMask & FirstOutputMask) serial0.end(); - if (portMask & SecondOutputMask) serial1.end(); + if (portMask.enabled(FirstOutput)) serial0.end(); + if (portMask.enabled(SecondOutput)) serial1.end(); } bool connected() { bool ret = true; - if (portMask & FirstOutputMask) ret = CALL_IF_EXISTS(bool, &serial0, connected); - if (portMask & SecondOutputMask) ret = ret && CALL_IF_EXISTS(bool, &serial1, connected); + if (portMask.enabled(FirstOutput)) ret = CALL_IF_EXISTS(bool, &serial0, connected); + if (portMask.enabled(SecondOutput)) ret = ret && CALL_IF_EXISTS(bool, &serial1, connected); return ret; } @@ -225,15 +243,15 @@ struct MultiSerial : public SerialBase< MultiSerial> { uint8_t charCount; uint8_t readIndex; - NO_INLINE size_t write(uint8_t c) { return out.write(c); } - void flush() { out.flush(); } - void begin(long br) { out.begin(br); readIndex = 0; } - void end() { out.end(); } + NO_INLINE size_t write(uint8_t c) { return out.write(c); } + void flush() { out.flush(); } + void begin(long br) { out.begin(br); readIndex = 0; } + void end() { out.end(); } - void msgDone() { out.msgDone(); } + void msgDone() { out.msgDone(); } // Existing instances implement Arduino's operator bool, so use that if it's available - bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } - void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } + bool connected() { return Private::HasMember_connected::value ? CALL_IF_EXISTS(bool, &out, connected) : (bool)out; } + void flushTX() { CALL_IF_EXISTS(void, &out, flushTX); } - int available(uint8_t index) { + int available(serial_index_t index) { // There is a potential issue here with multiserial, since it'll return its decoded buffer whatever the serial index here. // So, instead of doing MeatpackSerial> we should do MultiSerial, MeatpackSerial<...>> // TODO, let's fix this later on @@ -160,7 +160,7 @@ struct MeatpackSerial : public SerialBase > { return charCount; } - int readImpl(const uint8_t index) { + int readImpl(const serial_index_t index) { // Not enough char to make progress? if (charCount == 0 && available(index) == 0) return -1; @@ -168,9 +168,9 @@ struct MeatpackSerial : public SerialBase > { return serialBuffer[readIndex++]; } - int read(uint8_t index) { return readImpl(index); } - int available() { return available(0); } - int read() { return readImpl(0); } + int read(serial_index_t index) { return readImpl(index); } + int available() { return available(0); } + int read() { return readImpl(0); } MeatpackSerial(const bool e, SerialT & out) : BaseClassT(e), out(out) {} }; diff --git a/Marlin/src/gcode/gcode.cpp b/Marlin/src/gcode/gcode.cpp index 5f30064b1..4e324e789 100644 --- a/Marlin/src/gcode/gcode.cpp +++ b/Marlin/src/gcode/gcode.cpp @@ -1067,7 +1067,7 @@ void GcodeSuite::process_subcommands_now(char * gcode) { static millis_t next_busy_signal_ms = 0; if (!autoreport_paused && host_keepalive_interval && busy_state != NOT_BUSY) { if (PENDING(ms, next_busy_signal_ms)) return; - PORT_REDIRECT(SERIAL_ALL); + PORT_REDIRECT(SerialMask::All); switch (busy_state) { case IN_HANDLER: case IN_PROCESS: diff --git a/Marlin/src/gcode/host/M118.cpp b/Marlin/src/gcode/host/M118.cpp index 9982492f9..d6e591add 100644 --- a/Marlin/src/gcode/host/M118.cpp +++ b/Marlin/src/gcode/host/M118.cpp @@ -52,7 +52,7 @@ void GcodeSuite::M118() { while (*p == ' ') ++p; } - PORT_REDIRECT(WITHIN(port, 0, NUM_SERIAL) ? (port ? SERIAL_PORTMASK(port - 1) : SERIAL_ALL) : multiSerial.portMask); + PORT_REDIRECT(WITHIN(port, 0, NUM_SERIAL) ? (port ? SERIAL_PORTMASK(port - 1) : SerialMask::All) : multiSerial.portMask); if (hasE) SERIAL_ECHO_START(); if (hasA) SERIAL_ECHOPGM("//"); diff --git a/Marlin/src/gcode/queue.cpp b/Marlin/src/gcode/queue.cpp index 9b69f9f1f..a764d80ec 100644 --- a/Marlin/src/gcode/queue.cpp +++ b/Marlin/src/gcode/queue.cpp @@ -240,7 +240,7 @@ void GCodeQueue::RingBuffer::ok_to_send() { CommandLine &command = commands[index_r]; #if HAS_MULTI_SERIAL const serial_index_t serial_ind = command.port; - if (serial_ind < 0) return; + if (!serial_ind.valid()) return; // Optimization here, skip processing if it's not going anywhere PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command #endif if (command.skip_ok) return; @@ -264,15 +264,15 @@ void GCodeQueue::RingBuffer::ok_to_send() { */ void GCodeQueue::flush_and_request_resend(const serial_index_t serial_ind) { #if HAS_MULTI_SERIAL - if (serial_ind < 0) return; // Never mind. Command came from SD or Flash Drive + if (!serial_ind.valid()) return; // Optimization here, skip if the command came from SD or Flash Drive PORT_REDIRECT(SERIAL_PORTMASK(serial_ind)); // Reply to the serial port that sent the command #endif SERIAL_FLUSH(); SERIAL_ECHOPGM(STR_RESEND); - SERIAL_ECHOLN(serial_state[serial_ind].last_N + 1); + SERIAL_ECHOLN(serial_state[serial_ind.index].last_N + 1); } -inline bool serial_data_available(uint8_t index) { +inline bool serial_data_available(serial_index_t index) { const int a = SERIAL_IMPL.available(index); #if BOTH(RX_BUFFER_MONITOR, RX_BUFFER_SIZE) if (a > RX_BUFFER_SIZE - 2) { @@ -290,15 +290,15 @@ inline bool any_serial_data_available() { return true; } -inline int read_serial(const uint8_t index) { return SERIAL_IMPL.read(index); } +inline int read_serial(const serial_index_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 SERIAL_ERROR_START(); - SERIAL_ECHOLNPAIR_P(err, serial_state[serial_ind].last_N); + SERIAL_ECHOLNPAIR_P(err, serial_state[serial_ind.index].last_N); while (read_serial(serial_ind) != -1) { /* nada */ } // Clear out the RX buffer. Why don't use flush here ? flush_and_request_resend(serial_ind); - serial_state[serial_ind].count = 0; + serial_state[serial_ind.index].count = 0; } FORCE_INLINE bool is_M29(const char * const cmd) { // matches "M29" & "M29 ", but not "M290", etc diff --git a/Marlin/src/gcode/queue.h b/Marlin/src/gcode/queue.h index 4757b8c37..8e87d114e 100644 --- a/Marlin/src/gcode/queue.h +++ b/Marlin/src/gcode/queue.h @@ -79,13 +79,13 @@ public: void commit_command(bool skip_ok #if HAS_MULTI_SERIAL - , serial_index_t serial_ind=-1 + , serial_index_t serial_ind = serial_index_t() #endif ); bool enqueue(const char* cmd, bool skip_ok = true #if HAS_MULTI_SERIAL - , serial_index_t serial_ind=-1 + , serial_index_t serial_ind = serial_index_t() #endif ); @@ -197,7 +197,7 @@ public: /** * (Re)Set the current line number for the last received command */ - static inline void set_current_line_number(long n) { serial_state[ring_buffer.command_port()].last_N = n; } + static inline void set_current_line_number(long n) { serial_state[ring_buffer.command_port().index].last_N = n; } private: diff --git a/Marlin/src/gcode/sd/M1001.cpp b/Marlin/src/gcode/sd/M1001.cpp index 1cf700ae2..415fbb6fa 100644 --- a/Marlin/src/gcode/sd/M1001.cpp +++ b/Marlin/src/gcode/sd/M1001.cpp @@ -82,7 +82,7 @@ void GcodeSuite::M1001() { // Announce SD file completion { - PORT_REDIRECT(SERIAL_ALL); + PORT_REDIRECT(SerialMask::All); SERIAL_ECHOLNPGM(STR_FILE_PRINTED); } diff --git a/Marlin/src/lcd/extui/lib/dgus/DGUSScreenHandler.cpp b/Marlin/src/lcd/extui/lib/dgus/DGUSScreenHandler.cpp index 180c89580..93a7de14a 100644 --- a/Marlin/src/lcd/extui/lib/dgus/DGUSScreenHandler.cpp +++ b/Marlin/src/lcd/extui/lib/dgus/DGUSScreenHandler.cpp @@ -395,21 +395,21 @@ void DGUSScreenHandler::HandleTemperatureChanged(DGUS_VP_Variable &var, void *va default: return; #if HOTENDS >= 1 case VP_T_E0_Set: - NOMORE(newvalue, HEATER_0_MAXTEMP); + NOMORE(newvalue, (uint16_t)HEATER_0_MAXTEMP); thermalManager.setTargetHotend(newvalue, 0); acceptedvalue = thermalManager.degTargetHotend(0); break; #endif #if HOTENDS >= 2 case VP_T_E1_Set: - NOMORE(newvalue, HEATER_1_MAXTEMP); + NOMORE(newvalue, (uint16_t)HEATER_1_MAXTEMP); thermalManager.setTargetHotend(newvalue, 1); acceptedvalue = thermalManager.degTargetHotend(1); break; #endif #if HAS_HEATED_BED case VP_T_Bed_Set: - NOMORE(newvalue, BED_MAXTEMP); + NOMORE(newvalue, (uint16_t)BED_MAXTEMP); thermalManager.setTargetBed(newvalue); acceptedvalue = thermalManager.degTargetBed(); break; diff --git a/Marlin/src/libs/autoreport.h b/Marlin/src/libs/autoreport.h index 232216578..a6bc5adbf 100644 --- a/Marlin/src/libs/autoreport.h +++ b/Marlin/src/libs/autoreport.h @@ -28,8 +28,8 @@ struct AutoReporter { millis_t next_report_ms; uint8_t report_interval; #if HAS_MULTI_SERIAL - serial_index_t report_port_mask; - AutoReporter() : report_port_mask(SERIAL_ALL) {} + SerialMask report_port_mask; + AutoReporter() : report_port_mask(SerialMask::All) {} #endif inline void set_interval(uint8_t seconds, const uint8_t limit=60) { diff --git a/Marlin/src/module/temperature.cpp b/Marlin/src/module/temperature.cpp index a1d5745de..a000a31de 100644 --- a/Marlin/src/module/temperature.cpp +++ b/Marlin/src/module/temperature.cpp @@ -329,7 +329,7 @@ const char str_t_thermal_runaway[] PROGMEM = STR_T_THERMAL_RUNAWAY, */ void Temperature::report_fan_speed(const uint8_t target) { if (target >= FAN_COUNT) return; - PORT_REDIRECT(SERIAL_ALL); + PORT_REDIRECT(SerialMask::All); SERIAL_ECHOLNPAIR("M106 P", target, " S", fan_speed[target]); } #endif diff --git a/Marlin/src/sd/cardreader.cpp b/Marlin/src/sd/cardreader.cpp index c973ba594..fac14e72a 100644 --- a/Marlin/src/sd/cardreader.cpp +++ b/Marlin/src/sd/cardreader.cpp @@ -549,7 +549,7 @@ void openFailed(const char * const fname) { void announceOpen(const uint8_t doing, const char * const path) { if (doing) { - PORT_REDIRECT(SERIAL_ALL); + PORT_REDIRECT(SerialMask::All); SERIAL_ECHO_START(); SERIAL_ECHOPGM("Now "); SERIAL_ECHOPGM_P(doing == 1 ? PSTR("doing") : PSTR("fresh")); @@ -615,7 +615,7 @@ void CardReader::openFileRead(char * const path, const uint8_t subcall_type/*=0* sdpos = 0; { // Don't remove this block, as the PORT_REDIRECT is a RAII - PORT_REDIRECT(SERIAL_ALL); + PORT_REDIRECT(SerialMask::All); SERIAL_ECHOLNPAIR(STR_SD_FILE_OPENED, fname, STR_SD_SIZE, filesize); SERIAL_ECHOLNPGM(STR_SD_FILE_SELECTED); }