Bug fixes for stack handling

Bug fix: When files were closed and users forgot to pop the stack again using M121, a wrong stack state may be used
Bug fix: Macro invocations from indented blocks could lead to successive instructions being skipped when a macro with a non-zero indent level finished execution

8 files changed, 33 insertions, 23 deletions

diff --git a/src/GCodes/GCodeBuffer/GCodeBuffer.cpp b/src/GCodes/GCodeBuffer/GCodeBuffer.cpp
index 48455163..dd63a541 100644
--- a/src/GCodes/GCodeBuffer/GCodeBuffer.cpp
+++ b/src/GCodes/GCodeBuffer/GCodeBuffer.cpp
@@ -121,7 +121,7 @@ void GCodeBuffer::Reset() noexcept
 	}
 #endif
 
-	while (PopState()) { }
+	while (PopState(false)) { }
 
 #if HAS_SBC_INTERFACE
 	isBinaryBuffer = false;
@@ -833,18 +833,24 @@ bool GCodeBuffer::PushState(bool withinSameFile) noexcept
 }
 
 // Pop state returning true if successful (i.e. no stack underrun)
-bool GCodeBuffer::PopState() noexcept
+bool GCodeBuffer::PopState(bool withinSameFile) noexcept
 {
-	GCodeMachineState * const ms = machineState;
-	if (ms->GetPrevious() == nullptr)
+	bool poppedFileState;
+	do
 	{
-		ms->messageAcknowledged = false;			// avoid getting stuck in a loop trying to pop
-		ms->waitingForAcknowledgement = false;
-		return false;
-	}
+		GCodeMachineState * const ms = machineState;
+		if (ms->GetPrevious() == nullptr)
+		{
+			ms->messageAcknowledged = false;			// avoid getting stuck in a loop trying to pop
+			ms->waitingForAcknowledgement = false;
+			return false;
+		}
 
-	machineState = ms->Pop();						// get the previous state and copy down any error message
-	delete ms;
+		poppedFileState = !ms->localPush;
+		machineState = ms->Pop();						// get the previous state and copy down any error message
+		delete ms;
+	} while (!withinSameFile && !poppedFileState);
+	IF_NOT_BINARY(stringParser.ResetIndentation());
 
 	reprap.InputsUpdated();
 	return true;
@@ -870,7 +876,7 @@ void GCodeBuffer::AbortFile(bool abortAll, bool requestAbort) noexcept
 #endif
 				machineState->CloseFile();
 			}
-		} while (PopState() && (abortAll || !machineState->DoingFile()));
+		} while (PopState(false) && abortAll);
 
 #if HAS_SBC_INTERFACE
 		abortFile = requestAbort;
diff --git a/src/GCodes/GCodeBuffer/GCodeBuffer.h b/src/GCodes/GCodeBuffer/GCodeBuffer.h
index bf0a5132..77afefba 100644
--- a/src/GCodes/GCodeBuffer/GCodeBuffer.h
+++ b/src/GCodes/GCodeBuffer/GCodeBuffer.h
@@ -142,7 +142,7 @@ public:
 	const char *GetDistanceUnits() const noexcept;
 	unsigned int GetStackDepth() const noexcept;
 	bool PushState(bool withinSameFile) noexcept;				// Push state returning true if successful (i.e. stack not overflowed)
-	bool PopState() noexcept;									// Pop state returning true if successful (i.e. no stack underrun)
+	bool PopState(bool withinSameFile) noexcept;				// Pop state returning true if successful (i.e. no stack underrun)
 
 	void AbortFile(bool abortAll, bool requestAbort = true) noexcept;
 	bool IsDoingFile() const noexcept;							// Return true if this source is executing a file
diff --git a/src/GCodes/GCodeBuffer/StringParser.cpp b/src/GCodes/GCodeBuffer/StringParser.cpp
index eab0489c..a4ea34a4 100644
--- a/src/GCodes/GCodeBuffer/StringParser.cpp
+++ b/src/GCodes/GCodeBuffer/StringParser.cpp
@@ -1055,6 +1055,11 @@ void StringParser::PutCommand(const char *str) noexcept
 	} while (c != 0);
 }
 
+void StringParser::ResetIndentation() noexcept
+{
+	indentToSkipTo = (gb.GetBlockIndent() > 0) ? gb.GetBlockIndent() : NoIndentSkip;
+}
+
 void StringParser::SetFinished() noexcept
 {
 	if (commandEnd < gcodeLineEnd)
diff --git a/src/GCodes/GCodeBuffer/StringParser.h b/src/GCodes/GCodeBuffer/StringParser.h
index 00af55d3..9bb6ba60 100644
--- a/src/GCodes/GCodeBuffer/StringParser.h
+++ b/src/GCodes/GCodeBuffer/StringParser.h
@@ -62,6 +62,7 @@ public:
 	void GetUnsignedArray(uint32_t arr[], size_t& length) THROWS(GCodeException);				// Get a :-separated list of unsigned ints after a key letter
 	void GetDriverIdArray(DriverId arr[], size_t& length) THROWS(GCodeException);				// Get a :-separated list of drivers after a key letter
 
+	void ResetIndentation() noexcept;										// Reset the indentation level to the last one
 	void SetFinished() noexcept;											// Set the G Code finished
 	void SetCommsProperties(uint32_t arg) noexcept { checksumRequired = (arg & 1); crcRequired = (arg & 4); }
 
diff --git a/src/GCodes/GCodes.cpp b/src/GCodes/GCodes.cpp
index 32fd1342..301bb4a5 100644
--- a/src/GCodes/GCodes.cpp
+++ b/src/GCodes/GCodes.cpp
@@ -499,7 +499,7 @@ bool GCodes::SpinGCodeBuffer(GCodeBuffer& gb) noexcept
 		if (gb.LatestMachineState().messageAcknowledged)
 		{
 			const bool wasCancelled = gb.LatestMachineState().messageCancelled;
-			gb.PopState();												// this could fail if the current macro has already been aborted
+			gb.PopState(true);											// this could fail if the current macro has already been aborted
 
 			if (wasCancelled)
 			{
@@ -652,7 +652,7 @@ bool GCodes::DoFilePrint(GCodeBuffer& gb, const StringRef& reply) noexcept
 				CheckFinishedRunningConfigFile(gb);
 
 				// Pop the stack and notify the SBC that we have closed the file
-				Pop(gb);
+				Pop(gb, false);
 				gb.Init();
 				gb.LatestMachineState().firstCommandAfterRestart = false;
 
@@ -781,7 +781,7 @@ bool GCodes::DoFilePrint(GCodeBuffer& gb, const StringRef& reply) noexcept
 				gb.GetFileInput()->Reset(fd);
 				fd.Close();
 				CheckFinishedRunningConfigFile(gb);
-				Pop(gb);
+				Pop(gb, false);
 				gb.Init();
 				if (gb.GetState() == GCodeState::normal)
 				{
@@ -1463,11 +1463,9 @@ bool GCodes::Push(GCodeBuffer& gb, bool withinSameFile) noexcept
 }
 
 // Recover a saved state
-void GCodes::Pop(GCodeBuffer& gb) noexcept
+void GCodes::Pop(GCodeBuffer& gb, bool withinSameFile) noexcept
 {
-	// FIXME If withinSameFile is false, we should pop all stack levels that have the same file (ID)
-	// and output a warning message is the stack is popped more than once
-	if (!gb.PopState())
+	if (!gb.PopState(withinSameFile))
 	{
 		platform.Message(ErrorMessage, "Pop(): stack underflow\n");
 	}
@@ -2775,7 +2773,7 @@ void GCodes::FileMacroCyclesReturn(GCodeBuffer& gb) noexcept
 			gb.GetFileInput()->Reset(file);
 			file.Close();
 
-			gb.PopState();
+			gb.PopState(false);
 #endif
 		}
 
diff --git a/src/GCodes/GCodes.h b/src/GCodes/GCodes.h
index 349f3526..49fb067b 100644
--- a/src/GCodes/GCodes.h
+++ b/src/GCodes/GCodes.h
@@ -405,7 +405,7 @@ private:
 	const char *LoadExtrusionAndFeedrateFromGCode(GCodeBuffer& gb, bool isPrintingMove) THROWS(GCodeException);	// Set up the extrusion of a move
 
 	bool Push(GCodeBuffer& gb, bool withinSameFile) noexcept;										// Push feedrate etc on the stack
-	void Pop(GCodeBuffer& gb) noexcept;																// Pop feedrate etc
+	void Pop(GCodeBuffer& gb, bool withinSameFile) noexcept;										// Pop feedrate etc
 	void DisableDrives() noexcept;																	// Turn the motors off
 	bool SendConfigToLine();																		// Deal with M503
 
diff --git a/src/GCodes/GCodes2.cpp b/src/GCodes/GCodes2.cpp
index 0c923c73..491e8b74 100644
--- a/src/GCodes/GCodes2.cpp
+++ b/src/GCodes/GCodes2.cpp
@@ -1912,7 +1912,7 @@ bool GCodes::HandleMcode(GCodeBuffer& gb, const StringRef& reply) THROWS(GCodeEx
 				break;
 
 			case 121:
-				Pop(gb);
+				Pop(gb, true);
 				break;
 
 			case 122:
diff --git a/src/SBC/SbcInterface.cpp b/src/SBC/SbcInterface.cpp
index 0a7c68f3..7513f28c 100644
--- a/src/SBC/SbcInterface.cpp
+++ b/src/SBC/SbcInterface.cpp
@@ -411,7 +411,7 @@ void SbcInterface::ExchangeData() noexcept
 						if (error)
 						{
 							gb->CurrentFileMachineState().CloseFile();
-							gb->PopState();
+							gb->PopState(false);
 							gb->Init();
 						}
 						else