From 9899b5b712df3f93d24b5a8dd629c5785ddf88dc Mon Sep 17 00:00:00 2001 From: AndyEveritt <38423143+AndyEveritt@users.noreply.github.com> Date: Thu, 12 Sep 2024 10:24:49 +0100 Subject: [PATCH 01/13] Fix typo in M569.7 causing error when using mainboard as expansion board --- src/Platform/Platform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Platform/Platform.cpp b/src/Platform/Platform.cpp index c5b3bf7ee..43b6a3f21 100644 --- a/src/Platform/Platform.cpp +++ b/src/Platform/Platform.cpp @@ -5326,7 +5326,7 @@ GCodeResult Platform::EutProcessM569Point7(const CanMessageGeneric& msg, const S parser.GetStringParam('C', portName.GetRef()); //TODO use the following instead when we track the enable state of each driver individually //if (!brakePorts[drive].AssignPort(portName.c_str(), reply, PinUsedBy::gpout, (driverDisabled[drive]) ? PinAccess::write0 : PinAccess::write1)) ... - if (brakePorts[drive].AssignPort(portName.c_str(), reply, PinUsedBy::gpout, PinAccess::write0)) + if (!brakePorts[drive].AssignPort(portName.c_str(), reply, PinUsedBy::gpout, PinAccess::write0)) { return GCodeResult::error; } From 35ef7be384a24b35dddb89dab314d2c004795275 Mon Sep 17 00:00:00 2001 From: AndyEveritt <38423143+AndyEveritt@users.noreply.github.com> Date: Thu, 12 Sep 2024 15:47:53 +0100 Subject: [PATCH 02/13] process G17-19 & G68-69 in simulation mode --- src/GCodes/GCodes2.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/GCodes/GCodes2.cpp b/src/GCodes/GCodes2.cpp index b9c7c7432..a54d2a148 100644 --- a/src/GCodes/GCodes2.cpp +++ b/src/GCodes/GCodes2.cpp @@ -164,7 +164,13 @@ bool GCodes::HandleGcode(GCodeBuffer& gb, const StringRef& reply) THROWS(GCodeEx } GCodeResult result = GCodeResult::ok; - if (IsSimulating() && code > 4 && code != 10 && code != 11 && code != 20 && code != 21 && (code < 53 || code > 59) && (code < 90 || code > 94)) + if (IsSimulating() && code > 4 // move & dwell + && code != 10 && code != 11 // (un)retract + && code != 17 && code != 18 && code != 19 // selected plane for arc moves + && code != 68 && code != 69 // coordinate rotation + && code != 20 && code != 21 // change units + && (code < 53 || code > 59) // coordinate system + && (code < 90 || code > 94)) // positioning & feedrate modes { HandleReply(gb, result, ""); return true; // we only simulate some gcodes From 0d061414ae5e645babcce78ef99267878e766c7e Mon Sep 17 00:00:00 2001 From: AndyEveritt <38423143+AndyEveritt@users.noreply.github.com> Date: Fri, 13 Sep 2024 18:07:44 +0100 Subject: [PATCH 03/13] `M584 S` defaults to `R` if not provided --- src/GCodes/GCodes3.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GCodes/GCodes3.cpp b/src/GCodes/GCodes3.cpp index f4c4fa716..881997662 100644 --- a/src/GCodes/GCodes3.cpp +++ b/src/GCodes/GCodes3.cpp @@ -505,7 +505,7 @@ GCodeResult GCodes::DoDriveMapping(GCodeBuffer& gb, const StringRef& reply) THRO const AxisWrapType wrapType = (newAxesType != AxisWrapType::undefined) ? newAxesType : (c >= 'A' && c <= 'D') ? AxisWrapType::wrapAt360 // default A thru D to rotational but not continuous : AxisWrapType::noWrap; // default other axes to linear - const bool isNistRotational = (seenS) ? newAxesAreNistRotational : (c >= 'A' && c <= 'D'); + const bool isNistRotational = (seenS) ? newAxesAreNistRotational : wrapType != AxisWrapType::noWrap; platform.SetAxisType(drive, wrapType, isNistRotational); ++numTotalAxes; if (numTotalAxes + numExtruders > MaxAxesPlusExtruders) From 3261dd97a9346f60a8d09c7cffd6b4d657400001 Mon Sep 17 00:00:00 2001 From: Christian Hammacher Date: Fri, 13 Sep 2024 00:51:44 +0200 Subject: [PATCH 04/13] Hopefully fixed race condition at the end of macros --- src/GCodes/GCodeBuffer/BinaryParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GCodes/GCodeBuffer/BinaryParser.cpp b/src/GCodes/GCodeBuffer/BinaryParser.cpp index 5381b427d..fd9240529 100644 --- a/src/GCodes/GCodeBuffer/BinaryParser.cpp +++ b/src/GCodes/GCodeBuffer/BinaryParser.cpp @@ -597,7 +597,7 @@ void BinaryParser::SetFinished() noexcept FilePosition BinaryParser::GetFilePosition() const noexcept { - return ((header->flags & CodeFlags::HasFilePosition) != 0) ? header->filePosition : noFilePosition; + return ((header->flags & CodeFlags::HasFilePosition) != 0 && !gb.IsMacroFileClosed() && !gb.macroJustFinished) ? header->filePosition : noFilePosition; } const char* BinaryParser::DataStart() const noexcept From fa5018b0215031a86ab4d5ce071afe0332b106ad Mon Sep 17 00:00:00 2001 From: Christian Hammacher Date: Fri, 13 Sep 2024 11:08:53 +0200 Subject: [PATCH 05/13] Code cleanup Improved previous fix --- src/GCodes/GCodeBuffer/BinaryParser.cpp | 17 +++++++++++++++-- src/GCodes/GCodeBuffer/BinaryParser.h | 3 ++- src/GCodes/GCodeBuffer/GCodeBuffer.cpp | 22 +++++++++++++--------- src/GCodes/GCodeBuffer/GCodeBuffer.h | 5 ++--- src/GCodes/GCodes.cpp | 2 +- 5 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/GCodes/GCodeBuffer/BinaryParser.cpp b/src/GCodes/GCodeBuffer/BinaryParser.cpp index fd9240529..cc35964ab 100644 --- a/src/GCodes/GCodeBuffer/BinaryParser.cpp +++ b/src/GCodes/GCodeBuffer/BinaryParser.cpp @@ -18,7 +18,7 @@ BinaryParser::BinaryParser(GCodeBuffer& gcodeBuffer) noexcept : gb(gcodeBuffer) { - header = reinterpret_cast(gcodeBuffer.buffer); + header = reinterpret_cast(gcodeBuffer.buffer); } void BinaryParser::Init() noexcept @@ -597,7 +597,20 @@ void BinaryParser::SetFinished() noexcept FilePosition BinaryParser::GetFilePosition() const noexcept { - return ((header->flags & CodeFlags::HasFilePosition) != 0 && !gb.IsMacroFileClosed() && !gb.macroJustFinished) ? header->filePosition : noFilePosition; + return ((header->flags & CodeFlags::HasFilePosition) != 0) ? header->filePosition : noFilePosition; +} + +void BinaryParser::SetFilePosition(FilePosition fpos) noexcept +{ + if (fpos == noFilePosition) + { + header->flags = (CodeFlags)(header->flags & ~CodeFlags::HasFilePosition); + } + else + { + header->filePosition = fpos; + header->flags = (CodeFlags)(header->flags | CodeFlags::HasFilePosition); + } } const char* BinaryParser::DataStart() const noexcept diff --git a/src/GCodes/GCodeBuffer/BinaryParser.h b/src/GCodes/GCodeBuffer/BinaryParser.h index b465dbe3c..ec5edde01 100644 --- a/src/GCodes/GCodeBuffer/BinaryParser.h +++ b/src/GCodes/GCodeBuffer/BinaryParser.h @@ -55,6 +55,7 @@ class BinaryParser void SetFinished() noexcept; // Set the G Code finished FilePosition GetFilePosition() const noexcept; // Get the file position at the start of the current command + void SetFilePosition(FilePosition fpos) noexcept; // Overwrite the file position, e.g. when a macro finishes const char* DataStart() const noexcept; // Get the start of the current command size_t DataLength() const noexcept; // Get the length of the current command @@ -78,7 +79,7 @@ class BinaryParser void WriteParameters(const StringRef& s, bool quoteStrings) const noexcept; size_t bufferLength; - const CodeHeader *header; + CodeHeader *header; int reducedBytesRead; ParameterLettersBitmap parametersPresent; diff --git a/src/GCodes/GCodeBuffer/GCodeBuffer.cpp b/src/GCodes/GCodeBuffer/GCodeBuffer.cpp index c1bed7a2c..2d48267d2 100644 --- a/src/GCodes/GCodeBuffer/GCodeBuffer.cpp +++ b/src/GCodes/GCodeBuffer/GCodeBuffer.cpp @@ -142,7 +142,7 @@ void GCodeBuffer::Reset() noexcept isBinaryBuffer = false; requestedMacroFile.Clear(); isWaitingForMacro = macroFileClosed = false; - macroJustStarted = macroJustFinished = macroFileError = macroFileEmpty = abortFile = abortAllFiles = sendToSbc = messagePromptPending = messageAcknowledged = false; + macroJustStarted = macroFileError = macroFileEmpty = abortFile = abortAllFiles = sendToSbc = messagePromptPending = messageAcknowledged = false; machineState->lastCodeFromSbc = machineState->macroStartedByCode = false; #endif cancelWait = false; @@ -298,7 +298,7 @@ void GCodeBuffer::PutBinary(const uint32_t *data, size_t len) noexcept { machineState->lastCodeFromSbc = true; isBinaryBuffer = true; - macroJustStarted = macroJustFinished = false; + macroJustStarted = false; binaryParser.Put(data, len); } @@ -1187,8 +1187,15 @@ void GCodeBuffer::MacroFileClosed() noexcept { machineState->CloseFile(); macroJustStarted = false; - macroJustFinished = macroFileClosed = true; + macroFileClosed = true; +#if HAS_SBC_INTERFACE + if (IsBinary()) + { + // File position of the last code is no longer valid when we get here, reset it + binaryParser.SetFilePosition(IsFileChannel() && OriginalMachineState().DoingFile() ? printFilePositionAtMacroStart : noFilePosition); + } reprap.GetSbcInterface().EventOccurred(); +#endif } #endif @@ -1251,12 +1258,9 @@ FilePosition GCodeBuffer::GetPrintingFilePosition(bool allowNoFilePos) const noe } #if HAS_MASS_STORAGE || HAS_EMBEDDED_FILES || HAS_SBC_INTERFACE - const FilePosition pos = (IsDoingFileMacro() -# if HAS_SBC_INTERFACE - || IsMacroFileClosed() || macroJustFinished // wait for the next code from the SBC to update the job file position -# endif - ) ? printFilePositionAtMacroStart // the position before we started executing the macro - : GetJobFilePosition(); // the actual position, allowing for bytes cached but not yet processed + const FilePosition pos = IsDoingFileMacro() + ? printFilePositionAtMacroStart // the position before we started executing the macro + : GetJobFilePosition(); // the actual position return (pos != noFilePosition || allowNoFilePos) ? pos : 0; #else return allowNoFilePos ? noFilePosition : 0; diff --git a/src/GCodes/GCodeBuffer/GCodeBuffer.h b/src/GCodes/GCodeBuffer/GCodeBuffer.h index beab6003d..2edac5de9 100644 --- a/src/GCodes/GCodeBuffer/GCodeBuffer.h +++ b/src/GCodes/GCodeBuffer/GCodeBuffer.h @@ -372,9 +372,8 @@ class GCodeBuffer INHERIT_OBJECT_MODEL // Accessed only when the GB mutex is acquired String requestedMacroFile; bool isBinaryBuffer; - uint16_t + uint8_t macroJustStarted : 1, // Whether the GB has just started a macro file - macroJustFinished : 1, // Whether the GB has just finished a macro file macroFileError : 1, // Whether the macro file could be opened or if an error occurred macroFileEmpty : 1, // Whether the macro file is actually empty abortFile : 1, // Whether to abort the last file on the stack @@ -391,7 +390,7 @@ class GCodeBuffer INHERIT_OBJECT_MODEL inline bool GCodeBuffer::IsDoingFileMacro() const noexcept { #if HAS_SBC_INTERFACE - return machineState->doingFileMacro || IsMacroRequestPending(); + return machineState->doingFileMacro || IsMacroRequestPending() || macroFileClosed; #else return machineState->doingFileMacro; #endif diff --git a/src/GCodes/GCodes.cpp b/src/GCodes/GCodes.cpp index 8b4fca681..33e147c9d 100644 --- a/src/GCodes/GCodes.cpp +++ b/src/GCodes/GCodes.cpp @@ -987,7 +987,7 @@ bool GCodes::DoAsynchronousPause(GCodeBuffer& gb, PrintPausedReason reason, GCod // TODO: when using RTOS there is a possible race condition in the following, // because we might try to pause when a waiting move has just been added but before the gcode buffer has been re-initialised ready for the next command ms.GetPauseRestorePoint().filePos = fgb.GetPrintingFilePosition(true); - while (fgb.IsDoingFileMacro()) // must call this after GetFilePosition because this changes IsDoingFileMacro + while (fgb.LatestMachineState().doingFileMacro) // must call this after GetFilePosition because this changes IsDoingFileMacro { ms.pausedInMacro = true; fgb.PopState(false); From d56654b3defe4d702ec940407a6e2215b89f5f62 Mon Sep 17 00:00:00 2001 From: Christian Hammacher Date: Tue, 10 Sep 2024 11:31:59 +0200 Subject: [PATCH 06/13] Fixed query of global vars assigned to OM arrays --- src/ObjectModel/ObjectModel.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ObjectModel/ObjectModel.cpp b/src/ObjectModel/ObjectModel.cpp index 5d2ae944d..064f3f4d4 100644 --- a/src/ObjectModel/ObjectModel.cpp +++ b/src/ObjectModel/ObjectModel.cpp @@ -838,7 +838,10 @@ void ObjectModel::ReportItemAsJsonFull(OutputBuffer *buf, ObjectExplorationConte { buf->cat('['); } + + context.AddIndex(index); ReportItemAsJson(buf, context, classDescriptor, element, endptr + 1); + context.RemoveIndex(); if (*filter == 0) { @@ -1130,7 +1133,10 @@ void ObjectModel::ReportHeapArrayAsJson(OutputBuffer *buf, ObjectExplorationCont } ExpressionValue element; ah.GetElement(i, element); + + context.AddIndex(i); ReportItemAsJson(buf, context, classDescriptor, element, filter); + context.RemoveIndex(); } if (isRootArray && context.GetNextElement() < 0) { From 647584be956fd2b8a30fd3bb8c06485b5922d3b8 Mon Sep 17 00:00:00 2001 From: David Crocker Date: Tue, 17 Sep 2024 13:34:10 +0100 Subject: [PATCH 07/13] Increased version to 3.5.3 --- src/Version.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Version.h b/src/Version.h index dc0bce2e2..ca68244b6 100644 --- a/src/Version.h +++ b/src/Version.h @@ -10,7 +10,7 @@ #ifndef VERSION // Note: the complete VERSION string must be in standard version number format and must not contain spaces! This is so that DWC can parse it. -# define MAIN_VERSION "3.5.3-rc.1" +# define MAIN_VERSION "3.5.3" # ifdef USE_CAN0 # define VERSION_SUFFIX "(CAN0)" # else From ecef5fdc3018973c451c0953979682fe51506b99 Mon Sep 17 00:00:00 2001 From: David Crocker Date: Tue, 17 Sep 2024 20:50:10 +0100 Subject: [PATCH 08/13] Refactoring prior to fixing issue 1044 --- src/Networking/FtpResponder.cpp | 2 +- src/Networking/FtpResponder.h | 2 +- src/Networking/HttpResponder.cpp | 2 +- src/Networking/HttpResponder.h | 2 +- src/Networking/Network.cpp | 47 +++++++++++++++++------------- src/Networking/Network.h | 1 + src/Networking/NetworkClient.cpp | 2 +- src/Networking/NetworkClient.h | 2 +- src/Networking/NetworkResponder.h | 2 +- src/Networking/TelnetResponder.cpp | 2 +- src/Networking/TelnetResponder.h | 2 +- 11 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/Networking/FtpResponder.cpp b/src/Networking/FtpResponder.cpp index 760e442e2..6095a77b4 100644 --- a/src/Networking/FtpResponder.cpp +++ b/src/Networking/FtpResponder.cpp @@ -55,7 +55,7 @@ bool FtpResponder::Accept(Socket *s, NetworkProtocol protocol) noexcept } // This is called to force termination if we implement the specified protocol -void FtpResponder::Terminate(NetworkProtocol protocol, NetworkInterface *interface) noexcept +void FtpResponder::Terminate(NetworkProtocol protocol, const NetworkInterface *interface) noexcept { if (responderState != ResponderState::free && (protocol == FtpProtocol || protocol == AnyProtocol) && skt != nullptr && skt->GetInterface() == interface) { diff --git a/src/Networking/FtpResponder.h b/src/Networking/FtpResponder.h index 0d14c4b0f..a6d978f5e 100644 --- a/src/Networking/FtpResponder.h +++ b/src/Networking/FtpResponder.h @@ -17,7 +17,7 @@ class FtpResponder : public UploadingNetworkResponder bool Spin() noexcept override; // do some work, returning true if we did anything significant bool Accept(Socket *s, NetworkProtocol protocol) noexcept override; // ask the responder to accept this connection, returns true if it did - void Terminate(NetworkProtocol protocol, NetworkInterface *interface) noexcept override; // terminate the responder if it is serving the specified protocol on the specified interface + void Terminate(NetworkProtocol protocol, const NetworkInterface *interface) noexcept override; // terminate the responder if it is serving the specified protocol on the specified interface void Diagnostics(MessageType mtype) const noexcept override; diff --git a/src/Networking/HttpResponder.cpp b/src/Networking/HttpResponder.cpp index de99e1937..b1d7d0bdd 100644 --- a/src/Networking/HttpResponder.cpp +++ b/src/Networking/HttpResponder.cpp @@ -1417,7 +1417,7 @@ void HttpResponder::DoUpload() noexcept #endif // This is called to force termination if we implement the specified protocol -void HttpResponder::Terminate(NetworkProtocol protocol, NetworkInterface *interface) noexcept +void HttpResponder::Terminate(NetworkProtocol protocol, const NetworkInterface *interface) noexcept { if (responderState != ResponderState::free && (protocol == HttpProtocol || protocol == AnyProtocol) && skt != nullptr && skt->GetInterface() == interface) { diff --git a/src/Networking/HttpResponder.h b/src/Networking/HttpResponder.h index 4e92d0c3b..47ddd2657 100644 --- a/src/Networking/HttpResponder.h +++ b/src/Networking/HttpResponder.h @@ -18,7 +18,7 @@ class HttpResponder : public UploadingNetworkResponder HttpResponder(NetworkResponder *n) noexcept; bool Spin() noexcept override; // do some work, returning true if we did anything significant bool Accept(Socket *s, NetworkProtocol protocol) noexcept override; // ask the responder to accept this connection, returns true if it did - void Terminate(NetworkProtocol protocol, NetworkInterface *interface) noexcept override; // terminate the responder if it is serving the specified protocol on the specified interface + void Terminate(NetworkProtocol protocol, const NetworkInterface *interface) noexcept override; // terminate the responder if it is serving the specified protocol on the specified interface void Diagnostics(MessageType mtype) const noexcept override; static void InitStatic() noexcept; diff --git a/src/Networking/Network.cpp b/src/Networking/Network.cpp index 5facca11f..d86c6d73e 100644 --- a/src/Networking/Network.cpp +++ b/src/Networking/Network.cpp @@ -221,6 +221,15 @@ void Network::CreateAdditionalInterface() noexcept #endif +// Terminate all responders that handle a specified protocol (unless AnyProtocol is passed) on a specified interface +void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept +{ + for (NetworkResponder *r = responders; r != nullptr; r = r->GetNext()) + { + r->Terminate(protocol, iface); + } +} + GCodeResult Network::EnableProtocol(unsigned int interface, NetworkProtocol protocol, int port, uint32_t ip, int secure, const StringRef& reply) noexcept { #if HAS_NETWORKING @@ -287,41 +296,40 @@ GCodeResult Network::DisableProtocol(unsigned int interface, NetworkProtocol pro #if HAS_RESPONDERS if (!client) { - for (NetworkResponder *r = responders; r != nullptr; r = r->GetNext()) - { - r->Terminate(protocol, iface); - } + TerminateResponders(iface, protocol); } - // The following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. - // However, the only supported hardware with more than one network interface is the early Duet 3 prototype, so we'll leave this be. switch (protocol) { #if SUPPORT_HTTP case HttpProtocol: - HttpResponder::Disable(); // free up output buffers etc. + HttpResponder::DisableInterface(iface); // free up output buffers etc. break; #endif #if SUPPORT_FTP case FtpProtocol: + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. FtpResponder::Disable(); break; #endif #if SUPPORT_TELNET case TelnetProtocol: + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. TelnetResponder::Disable(); break; #endif #if SUPPORT_MULTICAST_DISCOVERY + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. case MulticastDiscoveryProtocol: break; #endif #if SUPPORT_MQTT case MqttProtocol: + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. MqttClient::Disable(); break; #endif @@ -371,23 +379,20 @@ GCodeResult Network::EnableInterface(unsigned int interface, int mode, const Str if (ret == GCodeResult::ok && mode < 1) // if disabling the interface { #if HAS_RESPONDERS - for (NetworkResponder *r = responders; r != nullptr; r = r->GetNext()) - { - r->Terminate(AnyProtocol, iface); - } + TerminateResponders(iface, AnyProtocol); -#if SUPPORT_HTTP +# if SUPPORT_HTTP HttpResponder::DisableInterface(iface); // remove sessions that use this interface -#endif -#if SUPPORT_FTP - FtpResponder::Disable(); -#endif -#if SUPPORT_TELNET - TelnetResponder::Disable(); // ideally here we would leave any Telnet session using a different interface alone -#endif -#if SUPPORT_MQTT +# endif +# if SUPPORT_FTP + FtpResponder::Disable(); // TODO leave any Telnet session using a different interface alone +# endif +# if SUPPORT_TELNET + TelnetResponder::Disable(); // TODO leave any Telnet session using a different interface alone +# endif +# if SUPPORT_MQTT MqttClient::Disable(); -#endif +# endif #endif // HAS_RESPONDERS } return ret; diff --git a/src/Networking/Network.h b/src/Networking/Network.h index 27223f5d5..414837ad7 100644 --- a/src/Networking/Network.h +++ b/src/Networking/Network.h @@ -115,6 +115,7 @@ class Network INHERIT_OBJECT_MODEL bool FindResponder(Socket *skt, NetworkProtocol protocol) noexcept; bool StartClient(NetworkInterface *interface, NetworkProtocol protocol) noexcept; void StopClient(NetworkInterface *interface, NetworkProtocol protocol) noexcept; + void TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept; void HandleHttpGCodeReply(const char *msg) noexcept; void HandleTelnetGCodeReply(const char *msg) noexcept; diff --git a/src/Networking/NetworkClient.cpp b/src/Networking/NetworkClient.cpp index 1e8cb11a8..de09ec3ee 100644 --- a/src/Networking/NetworkClient.cpp +++ b/src/Networking/NetworkClient.cpp @@ -83,7 +83,7 @@ bool NetworkClient::Accept(Socket *s, NetworkProtocol protocol) noexcept return false; } -void NetworkClient::Terminate(NetworkProtocol protocol, NetworkInterface *iface) noexcept +void NetworkClient::Terminate(NetworkProtocol protocol, const NetworkInterface *iface) noexcept { if ((HandlesProtocol(protocol) || protocol == AnyProtocol) && interface == iface) { diff --git a/src/Networking/NetworkClient.h b/src/Networking/NetworkClient.h index d653c0273..7331a8e4b 100644 --- a/src/Networking/NetworkClient.h +++ b/src/Networking/NetworkClient.h @@ -27,7 +27,7 @@ class NetworkClient : public NetworkResponder bool Start(NetworkProtocol protocol, NetworkInterface *iface) noexcept; void Stop(NetworkProtocol protocol, NetworkInterface *iface) noexcept; bool Accept(Socket *s, NetworkProtocol protocol) noexcept override; - void Terminate(NetworkProtocol protocol, NetworkInterface *iface) noexcept override; + void Terminate(NetworkProtocol protocol, const NetworkInterface *iface) noexcept override; NetworkInterface *GetInterface() { return interface; } NetworkClient *GetNext() const noexcept { return next; } diff --git a/src/Networking/NetworkResponder.h b/src/Networking/NetworkResponder.h index 77f48e961..64ed57d4d 100644 --- a/src/Networking/NetworkResponder.h +++ b/src/Networking/NetworkResponder.h @@ -29,7 +29,7 @@ class NetworkResponder NetworkResponder *GetNext() const noexcept { return next; } virtual bool Spin() noexcept = 0; // do some work, returning true if we did anything significant virtual bool Accept(Socket *s, NetworkProtocol protocol) noexcept = 0; // ask the responder to accept this connection, returns true if it did - virtual void Terminate(NetworkProtocol protocol, NetworkInterface *interface) noexcept = 0; // terminate the responder if it is serving the specified protocol on the specified interface + virtual void Terminate(NetworkProtocol protocol, const NetworkInterface *interface) noexcept = 0; // terminate the responder if it is serving the specified protocol on the specified interface virtual void Diagnostics(MessageType mtype) const noexcept = 0; protected: diff --git a/src/Networking/TelnetResponder.cpp b/src/Networking/TelnetResponder.cpp index 53c995367..a0e9a4992 100644 --- a/src/Networking/TelnetResponder.cpp +++ b/src/Networking/TelnetResponder.cpp @@ -42,7 +42,7 @@ bool TelnetResponder::Accept(Socket *s, NetworkProtocol protocol) noexcept } // This is called to force termination if we implement the specified protocol -void TelnetResponder::Terminate(NetworkProtocol protocol, NetworkInterface *interface) noexcept +void TelnetResponder::Terminate(NetworkProtocol protocol, const NetworkInterface *interface) noexcept { if (responderState != ResponderState::free && (protocol == TelnetProtocol || protocol == AnyProtocol) && skt != nullptr && skt->GetInterface() == interface) { diff --git a/src/Networking/TelnetResponder.h b/src/Networking/TelnetResponder.h index 4776a867c..79ec0f955 100644 --- a/src/Networking/TelnetResponder.h +++ b/src/Networking/TelnetResponder.h @@ -16,7 +16,7 @@ class TelnetResponder : public NetworkResponder TelnetResponder(NetworkResponder *n) noexcept; bool Spin() noexcept override; // do some work, returning true if we did anything significant bool Accept(Socket *s, NetworkProtocol protocol) noexcept override; // ask the responder to accept this connection, returns true if it did - void Terminate(NetworkProtocol protocol, NetworkInterface *interface) noexcept override; // terminate the responder if it is serving the specified protocol on the specified interface + void Terminate(NetworkProtocol protocol, const NetworkInterface *interface) noexcept override; // terminate the responder if it is serving the specified protocol on the specified interface static void InitStatic() noexcept; static void Disable() noexcept; From a3f558f2552799fbaa373dc6e78a1e5c1b23940f Mon Sep 17 00:00:00 2001 From: David Crocker Date: Tue, 17 Sep 2024 21:12:37 +0100 Subject: [PATCH 09/13] More refactoring in preparation for fixing issue 1044 --- src/Networking/Network.cpp | 55 ++++++++++++++++---------------------- src/Networking/Network.h | 13 ++++++--- src/Platform/Platform.cpp | 4 +++ 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/Networking/Network.cpp b/src/Networking/Network.cpp index d86c6d73e..df7940b92 100644 --- a/src/Networking/Network.cpp +++ b/src/Networking/Network.cpp @@ -221,6 +221,8 @@ void Network::CreateAdditionalInterface() noexcept #endif +#if HAS_NETWORKING + // Terminate all responders that handle a specified protocol (unless AnyProtocol is passed) on a specified interface void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept { @@ -230,6 +232,8 @@ void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol } } +#endif + GCodeResult Network::EnableProtocol(unsigned int interface, NetworkProtocol protocol, int port, uint32_t ip, int secure, const StringRef& reply) noexcept { #if HAS_NETWORKING @@ -590,7 +594,7 @@ bool Network::IsWiFiInterface(unsigned int interface) const noexcept #if HAS_NETWORKING -// Main spin loop +// Main spin loop, called by the Network task void Network::Spin() noexcept { for (;;) @@ -649,6 +653,7 @@ void Network::Spin() noexcept } #endif +// Get network diagnostics void Network::Diagnostics(MessageType mtype) noexcept { #if HAS_NETWORKING @@ -685,20 +690,28 @@ void Network::Diagnostics(MessageType mtype) noexcept #endif } -int Network::EnableState(unsigned int interface) const noexcept +IPAddress Network::GetIPAddress(unsigned int interface) const noexcept { + return +#if HAS_NETWORKING + (interface < GetNumNetworkInterfaces()) ? interfaces[interface]->GetIPAddress() : +#endif + IPAddress(); +} + #if HAS_NETWORKING + +int Network::EnableState(unsigned int interface) const noexcept +{ if (interface < GetNumNetworkInterfaces()) { return interfaces[interface]->EnableState(); } -#endif return -1; } void Network::SetEthernetIPAddress(IPAddress p_ipAddress, IPAddress p_netmask, IPAddress p_gateway) noexcept { -#if HAS_NETWORKING for (NetworkInterface *iface : interfaces) { if (iface != nullptr && !iface->IsWiFiInterface()) @@ -706,45 +719,29 @@ void Network::SetEthernetIPAddress(IPAddress p_ipAddress, IPAddress p_netmask, I iface->SetIPAddress(p_ipAddress, p_netmask, p_gateway); } } -#endif -} - -IPAddress Network::GetIPAddress(unsigned int interface) const noexcept -{ - return -#if HAS_NETWORKING - (interface < GetNumNetworkInterfaces()) ? interfaces[interface]->GetIPAddress() : -#endif - IPAddress(); } IPAddress Network::GetNetmask(unsigned int interface) const noexcept { return -#if HAS_NETWORKING (interface < GetNumNetworkInterfaces()) ? interfaces[interface]->GetNetmask() : -#endif IPAddress(); } IPAddress Network::GetGateway(unsigned int interface) const noexcept { return -#if HAS_NETWORKING (interface < GetNumNetworkInterfaces()) ? interfaces[interface]->GetGateway() : -#endif IPAddress(); } bool Network::UsingDhcp(unsigned int interface) const noexcept { -#if HAS_NETWORKING return interface < GetNumNetworkInterfaces() && interfaces[interface]->UsingDhcp(); -#else - return false; -#endif } +#endif + void Network::SetHostname(const char *name) noexcept { #if HAS_NETWORKING @@ -782,36 +779,30 @@ void Network::SetHostname(const char *name) noexcept #endif } +#if HAS_NETWORKING + // Net the MAC address. Pass -1 as the interface number to set the default MAC address for interfaces that don't have one. GCodeResult Network::SetMacAddress(unsigned int interface, const MacAddress& mac, const StringRef& reply) noexcept { -#if HAS_NETWORKING if (interface < GetNumNetworkInterfaces()) { return interfaces[interface]->SetMacAddress(mac, reply); } reply.copy("unknown interface "); return GCodeResult::error; -#else - reply.copy(notSupportedText); - return GCodeResult::error; -#endif } const MacAddress& Network::GetMacAddress(unsigned int interface) const noexcept { -#if HAS_NETWORKING if (interface >= GetNumNetworkInterfaces()) { interface = 0; } return interfaces[interface]->GetMacAddress(); -#else - // TODO: Is this initialized? - return platform.GetDefaultMacAddress(); -#endif } +#endif + // Find a responder to process a new connection bool Network::FindResponder(Socket *skt, NetworkProtocol protocol) noexcept { diff --git a/src/Networking/Network.h b/src/Networking/Network.h index 414837ad7..c38bbb399 100644 --- a/src/Networking/Network.h +++ b/src/Networking/Network.h @@ -97,16 +97,22 @@ class Network INHERIT_OBJECT_MODEL GCodeResult GetNetworkState(unsigned int interface, const StringRef& reply) noexcept; int EnableState(unsigned int interface) const noexcept; - void SetEthernetIPAddress(IPAddress p_ipAddress, IPAddress p_netmask, IPAddress p_gateway) noexcept; IPAddress GetIPAddress(unsigned int interface) const noexcept; + +#if HAS_NETWORKING + void SetEthernetIPAddress(IPAddress p_ipAddress, IPAddress p_netmask, IPAddress p_gateway) noexcept; IPAddress GetNetmask(unsigned int interface) const noexcept; IPAddress GetGateway(unsigned int interface) const noexcept; bool UsingDhcp(unsigned int interface) const noexcept; - const char *GetHostname() const noexcept { return hostname; } - void SetHostname(const char *name) noexcept; GCodeResult SetMacAddress(unsigned int interface, const MacAddress& mac, const StringRef& reply) noexcept; const MacAddress& GetMacAddress(unsigned int interface) const noexcept; + void TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept; +#endif + + const char *GetHostname() const noexcept { return hostname; } + void SetHostname(const char *name) noexcept; + #if SUPPORT_HTTP const char *GetCorsSite() const noexcept { return corsSite.IsEmpty() ? nullptr : corsSite.c_str(); } void SetCorsSite(const char *site) noexcept { corsSite.copy(site); } @@ -115,7 +121,6 @@ class Network INHERIT_OBJECT_MODEL bool FindResponder(Socket *skt, NetworkProtocol protocol) noexcept; bool StartClient(NetworkInterface *interface, NetworkProtocol protocol) noexcept; void StopClient(NetworkInterface *interface, NetworkProtocol protocol) noexcept; - void TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept; void HandleHttpGCodeReply(const char *msg) noexcept; void HandleTelnetGCodeReply(const char *msg) noexcept; diff --git a/src/Platform/Platform.cpp b/src/Platform/Platform.cpp index 43b6a3f21..dd2f51f38 100644 --- a/src/Platform/Platform.cpp +++ b/src/Platform/Platform.cpp @@ -985,6 +985,8 @@ void Platform::Exit() noexcept #endif } +#if HAS_NETWORKING + void Platform::SetIPAddress(IPAddress ip) noexcept { ipAddress = ip; @@ -1003,6 +1005,8 @@ void Platform::SetNetMask(IPAddress nm) noexcept reprap.GetNetwork().SetEthernetIPAddress(ipAddress, netMask, gateWay); } +#endif + // Flush messages to USB and aux, returning true if there is more to send bool Platform::FlushMessages() noexcept { From 2eca53a5cb6bde2f2cc033f8d71081695abdba12 Mon Sep 17 00:00:00 2001 From: David Crocker Date: Wed, 18 Sep 2024 10:15:31 +0100 Subject: [PATCH 10/13] More refactoring --- src/Networking/Network.cpp | 108 +++++++++++++------------------------ src/Networking/Network.h | 7 ++- src/Platform/RepRap.cpp | 2 + 3 files changed, 42 insertions(+), 75 deletions(-) diff --git a/src/Networking/Network.cpp b/src/Networking/Network.cpp index df7940b92..439cae031 100644 --- a/src/Networking/Network.cpp +++ b/src/Networking/Network.cpp @@ -224,12 +224,44 @@ void Network::CreateAdditionalInterface() noexcept #if HAS_NETWORKING // Terminate all responders that handle a specified protocol (unless AnyProtocol is passed) on a specified interface -void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept +void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol, bool client) noexcept { - for (NetworkResponder *r = responders; r != nullptr; r = r->GetNext()) +# if HAS_RESPONDERS + if (!client) + { + for (NetworkResponder *r = responders; r != nullptr; r = r->GetNext()) + { + r->Terminate(protocol, iface); + } + } + +# if SUPPORT_HTTP + if (protocol == HttpProtocol || protocol == AnyProtocol) { - r->Terminate(protocol, iface); + HttpResponder::DisableInterface(iface); // remove HTTP sessions that use this interface } +# endif +# if SUPPORT_FTP + if (protocol == FtpProtocol || protocol == AnyProtocol) + { + FtpResponder::Disable(); // TODO leave any FTP session using a different interface alone + } +# endif +# if SUPPORT_TELNET + if (protocol == TelnetProtocol || protocol == AnyProtocol) + { + TelnetResponder::Disable(); // TODO leave any Telnet session using a different interface alone + } +# endif +# if SUPPORT_MQTT + if (protocol == MqttProtocol || protocol == AnyProtocol) + { + MqttClient::Disable(); // TODO leave any Mqtt clients using a different interface alone + } +# endif + + // Nothing needed here for Multicast Discovery protocol +# endif } #endif @@ -294,54 +326,9 @@ GCodeResult Network::DisableProtocol(unsigned int interface, NetworkProtocol pro NetworkInterface * const iface = interfaces[interface]; const GCodeResult ret = iface->DisableProtocol(protocol, reply, !client); - if (ret == GCodeResult::ok) { -#if HAS_RESPONDERS - if (!client) - { - TerminateResponders(iface, protocol); - } - - switch (protocol) - { -#if SUPPORT_HTTP - case HttpProtocol: - HttpResponder::DisableInterface(iface); // free up output buffers etc. - break; -#endif - -#if SUPPORT_FTP - case FtpProtocol: - // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. - FtpResponder::Disable(); - break; -#endif - -#if SUPPORT_TELNET - case TelnetProtocol: - // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. - TelnetResponder::Disable(); - break; -#endif - -#if SUPPORT_MULTICAST_DISCOVERY - // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. - case MulticastDiscoveryProtocol: - break; -#endif - -#if SUPPORT_MQTT - case MqttProtocol: - // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. - MqttClient::Disable(); - break; -#endif - - default: - break; - } -#endif // HAS_RESPONDERS + TerminateResponders(iface, protocol, client); } return ret; } @@ -382,22 +369,7 @@ GCodeResult Network::EnableInterface(unsigned int interface, int mode, const Str const GCodeResult ret = iface->EnableInterface(mode, ssid, reply); if (ret == GCodeResult::ok && mode < 1) // if disabling the interface { -#if HAS_RESPONDERS - TerminateResponders(iface, AnyProtocol); - -# if SUPPORT_HTTP - HttpResponder::DisableInterface(iface); // remove sessions that use this interface -# endif -# if SUPPORT_FTP - FtpResponder::Disable(); // TODO leave any Telnet session using a different interface alone -# endif -# if SUPPORT_TELNET - TelnetResponder::Disable(); // TODO leave any Telnet session using a different interface alone -# endif -# if SUPPORT_MQTT - MqttClient::Disable(); -# endif -#endif // HAS_RESPONDERS + TerminateResponders(iface, AnyProtocol, false); } return ret; } @@ -740,11 +712,8 @@ bool Network::UsingDhcp(unsigned int interface) const noexcept return interface < GetNumNetworkInterfaces() && interfaces[interface]->UsingDhcp(); } -#endif - void Network::SetHostname(const char *name) noexcept { -#if HAS_NETWORKING size_t i = 0; while (*name && i < ARRAY_UPB(hostname)) { @@ -776,11 +745,8 @@ void Network::SetHostname(const char *name) noexcept iface->UpdateHostname(hostname); } } -#endif } -#if HAS_NETWORKING - // Net the MAC address. Pass -1 as the interface number to set the default MAC address for interfaces that don't have one. GCodeResult Network::SetMacAddress(unsigned int interface, const MacAddress& mac, const StringRef& reply) noexcept { diff --git a/src/Networking/Network.h b/src/Networking/Network.h index c38bbb399..d9a406ee7 100644 --- a/src/Networking/Network.h +++ b/src/Networking/Network.h @@ -106,13 +106,12 @@ class Network INHERIT_OBJECT_MODEL bool UsingDhcp(unsigned int interface) const noexcept; GCodeResult SetMacAddress(unsigned int interface, const MacAddress& mac, const StringRef& reply) noexcept; const MacAddress& GetMacAddress(unsigned int interface) const noexcept; - - void TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept; -#endif - const char *GetHostname() const noexcept { return hostname; } void SetHostname(const char *name) noexcept; + void TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol, bool client) noexcept; +#endif + #if SUPPORT_HTTP const char *GetCorsSite() const noexcept { return corsSite.IsEmpty() ? nullptr : corsSite.c_str(); } void SetCorsSite(const char *site) noexcept { corsSite.copy(site); } diff --git a/src/Platform/RepRap.cpp b/src/Platform/RepRap.cpp index d0a1794db..2d5cef56e 100644 --- a/src/Platform/RepRap.cpp +++ b/src/Platform/RepRap.cpp @@ -2461,9 +2461,11 @@ void RepRap::SetName(const char* nm) noexcept { myName.copy(nm); +#if HAS_NETWORKING // Set new DHCP hostname network->SetHostname(myName.c_str()); NetworkUpdated(); +#endif } // Firmware update operations From d8daa10d64f0bbeb514e5901140446ad2a4a26fe Mon Sep 17 00:00:00 2001 From: David Crocker Date: Wed, 18 Sep 2024 10:16:23 +0100 Subject: [PATCH 11/13] Attempt fix for issue 1044 (but Duet 2 build overflows flash) --- src/Networking/FtpResponder.cpp | 11 +++++++++-- src/Networking/HttpResponder.cpp | 9 ++++++++- src/Networking/Network.cpp | 1 + src/Networking/NetworkResponder.cpp | 8 ++++---- src/Networking/NetworkResponder.h | 1 + src/Networking/TelnetResponder.cpp | 9 ++++++++- 6 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/Networking/FtpResponder.cpp b/src/Networking/FtpResponder.cpp index 6095a77b4..d56fd40f0 100644 --- a/src/Networking/FtpResponder.cpp +++ b/src/Networking/FtpResponder.cpp @@ -59,13 +59,20 @@ void FtpResponder::Terminate(NetworkProtocol protocol, const NetworkInterface *i { if (responderState != ResponderState::free && (protocol == FtpProtocol || protocol == AnyProtocol) && skt != nullptr && skt->GetInterface() == interface) { - ConnectionLost(); + // Don't call ConnectionLost here because that releases outbuf, which may be in use by the Network task, and this is called from the Main task + terminateResponder = true; // tell the responder to terminate } } // Do some work, returning true if we did anything significant bool FtpResponder::Spin() noexcept { + if (terminateResponder) + { + ConnectionLost(); + terminateResponder = false; + } + switch (responderState) { case ResponderState::free: @@ -165,7 +172,7 @@ void FtpResponder::ConnectionLost() noexcept void FtpResponder::SendData() noexcept { // Send our output buffer and output stack - for(;;) + while (!terminateResponder) { if (outBuf == nullptr) { diff --git a/src/Networking/HttpResponder.cpp b/src/Networking/HttpResponder.cpp index b1d7d0bdd..e45e39672 100644 --- a/src/Networking/HttpResponder.cpp +++ b/src/Networking/HttpResponder.cpp @@ -71,6 +71,12 @@ bool HttpResponder::Accept(Socket *s, NetworkProtocol protocol) noexcept // Do some work, returning true if we did anything significant bool HttpResponder::Spin() noexcept { + if (terminateResponder) + { + ConnectionLost(); + terminateResponder = false; + } + switch (responderState) { case ResponderState::free: @@ -1421,7 +1427,8 @@ void HttpResponder::Terminate(NetworkProtocol protocol, const NetworkInterface * { if (responderState != ResponderState::free && (protocol == HttpProtocol || protocol == AnyProtocol) && skt != nullptr && skt->GetInterface() == interface) { - ConnectionLost(); + // Don't call ConnectionLost here because that releases outbuf, which may be in use by the Network task, and this is called from the Main task + terminateResponder = true; // tell the responder to terminate } } diff --git a/src/Networking/Network.cpp b/src/Networking/Network.cpp index 439cae031..99b26c98e 100644 --- a/src/Networking/Network.cpp +++ b/src/Networking/Network.cpp @@ -224,6 +224,7 @@ void Network::CreateAdditionalInterface() noexcept #if HAS_NETWORKING // Terminate all responders that handle a specified protocol (unless AnyProtocol is passed) on a specified interface +// CAUTION: this must only be called by the Network task (not the Main task) because in terminating the responders it releases output buffers that the Network task uses to send data. void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol, bool client) noexcept { # if HAS_RESPONDERS diff --git a/src/Networking/NetworkResponder.cpp b/src/Networking/NetworkResponder.cpp index 4eaaede50..3b286c864 100644 --- a/src/Networking/NetworkResponder.cpp +++ b/src/Networking/NetworkResponder.cpp @@ -17,7 +17,7 @@ NetworkResponder::NetworkResponder(NetworkResponder *n) noexcept #if HAS_MASS_STORAGE fileBeingSent(nullptr), #endif - fileBuffer(nullptr) + fileBuffer(nullptr), terminateResponder(false) { } @@ -41,7 +41,7 @@ void NetworkResponder::Commit(ResponderState nextState, bool report) noexcept void NetworkResponder::SendData() noexcept { // Send our output buffer and output stack - for(;;) + while (!terminateResponder) { if (outBuf == nullptr) { @@ -97,7 +97,7 @@ void NetworkResponder::SendData() noexcept } // If we have a file buffer here, we must be in the process of sending a file - while (fileBuffer != nullptr) + while (fileBuffer != nullptr && !terminateResponder) { if (fileBuffer->IsEmpty() && fileBeingSent != nullptr) { @@ -147,7 +147,7 @@ void NetworkResponder::SendData() noexcept #endif // If we get here then there is nothing left to send - skt->Send(); // tell the socket there is no more data + skt->Send(); // tell the socket there is no more data // If we are going to free up this responder after sending, then we must close the connection if (stateAfterSending == ResponderState::free) diff --git a/src/Networking/NetworkResponder.h b/src/Networking/NetworkResponder.h index 64ed57d4d..b949af78b 100644 --- a/src/Networking/NetworkResponder.h +++ b/src/Networking/NetworkResponder.h @@ -88,6 +88,7 @@ class NetworkResponder FileStore *fileBeingSent; #endif NetworkBuffer *fileBuffer; + bool terminateResponder; }; #endif /* SRC_NETWORKING_NETWORKRESPONDER_H_ */ diff --git a/src/Networking/TelnetResponder.cpp b/src/Networking/TelnetResponder.cpp index a0e9a4992..de2f465bf 100644 --- a/src/Networking/TelnetResponder.cpp +++ b/src/Networking/TelnetResponder.cpp @@ -46,7 +46,8 @@ void TelnetResponder::Terminate(NetworkProtocol protocol, const NetworkInterface { if (responderState != ResponderState::free && (protocol == TelnetProtocol || protocol == AnyProtocol) && skt != nullptr && skt->GetInterface() == interface) { - ConnectionLost(); + // Don't call ConnectionLost here because that releases outbuf, which may be in use by the Network task, and this is called from the Main task + terminateResponder = true; // tell the responder to terminate } } @@ -112,6 +113,12 @@ bool TelnetResponder::SendGCodeReply() noexcept // Do some work, returning true if we did anything significant bool TelnetResponder::Spin() noexcept { + if (terminateResponder) + { + ConnectionLost(); + terminateResponder = false; + } + switch (responderState) { case ResponderState::free: From bc564001e1d594d94b298414f287d348ae2f7f25 Mon Sep 17 00:00:00 2001 From: David Crocker Date: Wed, 18 Sep 2024 11:01:02 +0100 Subject: [PATCH 12/13] Partially reverted changes in commit 2eca53a5cb6bde2f2cc033f8d71081695abdba12 --- src/Networking/Network.cpp | 109 ++++++++++++++++++++++++------------- src/Networking/Network.h | 2 +- 2 files changed, 72 insertions(+), 39 deletions(-) diff --git a/src/Networking/Network.cpp b/src/Networking/Network.cpp index 99b26c98e..df7940b92 100644 --- a/src/Networking/Network.cpp +++ b/src/Networking/Network.cpp @@ -224,45 +224,12 @@ void Network::CreateAdditionalInterface() noexcept #if HAS_NETWORKING // Terminate all responders that handle a specified protocol (unless AnyProtocol is passed) on a specified interface -// CAUTION: this must only be called by the Network task (not the Main task) because in terminating the responders it releases output buffers that the Network task uses to send data. -void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol, bool client) noexcept +void Network::TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept { -# if HAS_RESPONDERS - if (!client) - { - for (NetworkResponder *r = responders; r != nullptr; r = r->GetNext()) - { - r->Terminate(protocol, iface); - } - } - -# if SUPPORT_HTTP - if (protocol == HttpProtocol || protocol == AnyProtocol) - { - HttpResponder::DisableInterface(iface); // remove HTTP sessions that use this interface - } -# endif -# if SUPPORT_FTP - if (protocol == FtpProtocol || protocol == AnyProtocol) - { - FtpResponder::Disable(); // TODO leave any FTP session using a different interface alone - } -# endif -# if SUPPORT_TELNET - if (protocol == TelnetProtocol || protocol == AnyProtocol) - { - TelnetResponder::Disable(); // TODO leave any Telnet session using a different interface alone - } -# endif -# if SUPPORT_MQTT - if (protocol == MqttProtocol || protocol == AnyProtocol) + for (NetworkResponder *r = responders; r != nullptr; r = r->GetNext()) { - MqttClient::Disable(); // TODO leave any Mqtt clients using a different interface alone + r->Terminate(protocol, iface); } -# endif - - // Nothing needed here for Multicast Discovery protocol -# endif } #endif @@ -327,9 +294,54 @@ GCodeResult Network::DisableProtocol(unsigned int interface, NetworkProtocol pro NetworkInterface * const iface = interfaces[interface]; const GCodeResult ret = iface->DisableProtocol(protocol, reply, !client); + if (ret == GCodeResult::ok) { - TerminateResponders(iface, protocol, client); +#if HAS_RESPONDERS + if (!client) + { + TerminateResponders(iface, protocol); + } + + switch (protocol) + { +#if SUPPORT_HTTP + case HttpProtocol: + HttpResponder::DisableInterface(iface); // free up output buffers etc. + break; +#endif + +#if SUPPORT_FTP + case FtpProtocol: + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. + FtpResponder::Disable(); + break; +#endif + +#if SUPPORT_TELNET + case TelnetProtocol: + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. + TelnetResponder::Disable(); + break; +#endif + +#if SUPPORT_MULTICAST_DISCOVERY + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. + case MulticastDiscoveryProtocol: + break; +#endif + +#if SUPPORT_MQTT + case MqttProtocol: + // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. + MqttClient::Disable(); + break; +#endif + + default: + break; + } +#endif // HAS_RESPONDERS } return ret; } @@ -370,7 +382,22 @@ GCodeResult Network::EnableInterface(unsigned int interface, int mode, const Str const GCodeResult ret = iface->EnableInterface(mode, ssid, reply); if (ret == GCodeResult::ok && mode < 1) // if disabling the interface { - TerminateResponders(iface, AnyProtocol, false); +#if HAS_RESPONDERS + TerminateResponders(iface, AnyProtocol); + +# if SUPPORT_HTTP + HttpResponder::DisableInterface(iface); // remove sessions that use this interface +# endif +# if SUPPORT_FTP + FtpResponder::Disable(); // TODO leave any Telnet session using a different interface alone +# endif +# if SUPPORT_TELNET + TelnetResponder::Disable(); // TODO leave any Telnet session using a different interface alone +# endif +# if SUPPORT_MQTT + MqttClient::Disable(); +# endif +#endif // HAS_RESPONDERS } return ret; } @@ -713,8 +740,11 @@ bool Network::UsingDhcp(unsigned int interface) const noexcept return interface < GetNumNetworkInterfaces() && interfaces[interface]->UsingDhcp(); } +#endif + void Network::SetHostname(const char *name) noexcept { +#if HAS_NETWORKING size_t i = 0; while (*name && i < ARRAY_UPB(hostname)) { @@ -746,8 +776,11 @@ void Network::SetHostname(const char *name) noexcept iface->UpdateHostname(hostname); } } +#endif } +#if HAS_NETWORKING + // Net the MAC address. Pass -1 as the interface number to set the default MAC address for interfaces that don't have one. GCodeResult Network::SetMacAddress(unsigned int interface, const MacAddress& mac, const StringRef& reply) noexcept { diff --git a/src/Networking/Network.h b/src/Networking/Network.h index d9a406ee7..150f494fd 100644 --- a/src/Networking/Network.h +++ b/src/Networking/Network.h @@ -109,7 +109,7 @@ class Network INHERIT_OBJECT_MODEL const char *GetHostname() const noexcept { return hostname; } void SetHostname(const char *name) noexcept; - void TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol, bool client) noexcept; + void TerminateResponders(const NetworkInterface *iface, NetworkProtocol protocol) noexcept; #endif #if SUPPORT_HTTP From e7085e439c0edf9ba3d80c8920d726166ff68de3 Mon Sep 17 00:00:00 2001 From: David Crocker Date: Wed, 18 Sep 2024 13:22:58 +0100 Subject: [PATCH 13/13] Removed unused client code from Duet 2 builds to save flash space --- src/Networking/Network.cpp | 44 ++++++++++++++++---------------- src/Networking/Network.h | 7 ++--- src/Networking/NetworkClient.cpp | 7 +++++ src/Networking/NetworkClient.h | 4 +++ src/Networking/NetworkDefs.h | 3 +++ 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/Networking/Network.cpp b/src/Networking/Network.cpp index df7940b92..7fbf14432 100644 --- a/src/Networking/Network.cpp +++ b/src/Networking/Network.cpp @@ -86,7 +86,10 @@ void MacAddress::SetFromBytes(const uint8_t mb[6]) noexcept // Network members Network::Network(Platform& p) noexcept : platform(p) #if HAS_RESPONDERS - , responders(nullptr), clients(nullptr), nextResponderToPoll(nullptr) + , responders(nullptr), nextResponderToPoll(nullptr) +#endif +#if HAS_CLIENTS + , clients(nullptr) #endif { #if HAS_NETWORKING @@ -239,6 +242,7 @@ GCodeResult Network::EnableProtocol(unsigned int interface, NetworkProtocol prot #if HAS_NETWORKING if (interface < GetNumNetworkInterfaces()) { +# if HAS_CLIENTS bool hasFree = false; bool client = false; // Check if there are enough clients to accomodate enabling the protocol. Check if @@ -262,7 +266,7 @@ GCodeResult Network::EnableProtocol(unsigned int interface, NetworkProtocol prot reply.printf("No more instances for client protocol.\n"); return GCodeResult::error; } - +# endif return interfaces[interface]->EnableProtocol(protocol, port, ip, secure, reply); } @@ -281,6 +285,7 @@ GCodeResult Network::DisableProtocol(unsigned int interface, NetworkProtocol pro { bool client = false; +# if HAS_CLIENTS // Check if a client handles the protocol. If so, termination is handled // by the client itself, after attempting to disconnect gracefully. for (NetworkClient *c = clients; c != nullptr; c = c->GetNext()) @@ -291,13 +296,14 @@ GCodeResult Network::DisableProtocol(unsigned int interface, NetworkProtocol pro break; } } +# endif NetworkInterface * const iface = interfaces[interface]; const GCodeResult ret = iface->DisableProtocol(protocol, reply, !client); if (ret == GCodeResult::ok) { -#if HAS_RESPONDERS +# if HAS_RESPONDERS if (!client) { TerminateResponders(iface, protocol); @@ -305,43 +311,43 @@ GCodeResult Network::DisableProtocol(unsigned int interface, NetworkProtocol pro switch (protocol) { -#if SUPPORT_HTTP +# if SUPPORT_HTTP case HttpProtocol: HttpResponder::DisableInterface(iface); // free up output buffers etc. break; -#endif +# endif -#if SUPPORT_FTP +# if SUPPORT_FTP case FtpProtocol: // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. FtpResponder::Disable(); break; -#endif +# endif -#if SUPPORT_TELNET +# if SUPPORT_TELNET case TelnetProtocol: // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. TelnetResponder::Disable(); break; -#endif +# endif -#if SUPPORT_MULTICAST_DISCOVERY +# if SUPPORT_MULTICAST_DISCOVERY // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. case MulticastDiscoveryProtocol: break; -#endif +# endif -#if SUPPORT_MQTT +# if SUPPORT_MQTT case MqttProtocol: // TODO the following isn't quite right, because we shouldn't free up output buffers if another network interface is still serving this protocol. MqttClient::Disable(); break; -#endif +# endif default: break; } -#endif // HAS_RESPONDERS +# endif // HAS_RESPONDERS } return ret; } @@ -740,11 +746,8 @@ bool Network::UsingDhcp(unsigned int interface) const noexcept return interface < GetNumNetworkInterfaces() && interfaces[interface]->UsingDhcp(); } -#endif - void Network::SetHostname(const char *name) noexcept { -#if HAS_NETWORKING size_t i = 0; while (*name && i < ARRAY_UPB(hostname)) { @@ -776,11 +779,8 @@ void Network::SetHostname(const char *name) noexcept iface->UpdateHostname(hostname); } } -#endif } -#if HAS_NETWORKING - // Net the MAC address. Pass -1 as the interface number to set the default MAC address for interfaces that don't have one. GCodeResult Network::SetMacAddress(unsigned int interface, const MacAddress& mac, const StringRef& reply) noexcept { @@ -820,7 +820,7 @@ bool Network::FindResponder(Socket *skt, NetworkProtocol protocol) noexcept bool Network::StartClient(NetworkInterface *interface, NetworkProtocol protocol) noexcept { -#if HAS_RESPONDERS +#if HAS_CLIENTS for (NetworkClient *c = clients; c != nullptr; c = c->GetNext()) { if (c->Start(protocol, interface)) @@ -834,7 +834,7 @@ bool Network::StartClient(NetworkInterface *interface, NetworkProtocol protocol) void Network::StopClient(NetworkInterface *interface, NetworkProtocol protocol) noexcept { -#if HAS_RESPONDERS +#if HAS_CLIENTS for (NetworkClient *c = clients; c != nullptr; c = c->GetNext()) { c->Stop(protocol, interface); diff --git a/src/Networking/Network.h b/src/Networking/Network.h index 150f494fd..7a815928c 100644 --- a/src/Networking/Network.h +++ b/src/Networking/Network.h @@ -38,8 +38,6 @@ const size_t NumTelnetResponders = 1; // the number of concurrent Telnet session const size_t NumFtpResponders = 1; // the number of concurrent FTP sessions we support -#define HAS_RESPONDERS (SUPPORT_HTTP || SUPPORT_FTP || SUPPORT_TELNET) - // Forward declarations class NetworkResponder; class NetworkClient; @@ -146,10 +144,13 @@ class Network INHERIT_OBJECT_MODEL #if HAS_RESPONDERS NetworkResponder *responders; - NetworkClient *clients; NetworkResponder *nextResponderToPoll; #endif +#if HAS_CLIENTS + NetworkClient *clients; +#endif + #if SUPPORT_HTTP Mutex httpMutex; #endif diff --git a/src/Networking/NetworkClient.cpp b/src/Networking/NetworkClient.cpp index de09ec3ee..7108ee808 100644 --- a/src/Networking/NetworkClient.cpp +++ b/src/Networking/NetworkClient.cpp @@ -6,6 +6,9 @@ */ #include "NetworkClient.h" + +#if HAS_CLIENTS + #include "Socket.h" #include @@ -106,3 +109,7 @@ void NetworkClient::ConnectionLost() noexcept } NetworkClient *NetworkClient::clients = nullptr; + +#endif + +// End diff --git a/src/Networking/NetworkClient.h b/src/Networking/NetworkClient.h index 7331a8e4b..d3c5d9713 100644 --- a/src/Networking/NetworkClient.h +++ b/src/Networking/NetworkClient.h @@ -11,6 +11,8 @@ #include #include "NetworkResponder.h" +#if HAS_CLIENTS + // Forward declarations class NetworkClient; class NetworkInterface; @@ -51,4 +53,6 @@ class NetworkClient : public NetworkResponder static NetworkClient *clients; // Head of the list of all network clients }; +#endif + #endif /* SRC_NETWORKING_NETWORKCLIENT_H_ */ diff --git a/src/Networking/NetworkDefs.h b/src/Networking/NetworkDefs.h index 788bfd05b..240965b53 100644 --- a/src/Networking/NetworkDefs.h +++ b/src/Networking/NetworkDefs.h @@ -10,6 +10,9 @@ #include +#define HAS_RESPONDERS (SUPPORT_HTTP || SUPPORT_FTP || SUPPORT_TELNET) +#define HAS_CLIENTS (SUPPORT_MQTT) + class NetworkBuffer; typedef uint8_t SocketNumber;