Fixed issue calused by Move task using same CAN buffer as Main task

4 files changed, 42 insertions, 13 deletions

diff --git a/src/CAN/CanInterface.cpp b/src/CAN/CanInterface.cpp
index 33056ede..34b1edac 100644
--- a/src/CAN/CanInterface.cpp
+++ b/src/CAN/CanInterface.cpp
@@ -16,6 +16,7 @@
 #include <Movement/DDA.h>
 #include <Movement/DriveMovement.h>
 #include <Movement/StepTimer.h>
+#include <Movement/Move.h>
 #include <RTOSIface/RTOSIface.h>
 #include <Platform/TaskPriorities.h>
 #include <GCodes/GCodeException.h>
@@ -159,7 +160,7 @@ static Task<CanSenderTaskStackWords> canSenderTask;
 constexpr size_t CanReceiverTaskStackWords = 1000;
 static Task<CanReceiverTaskStackWords> canReceiverTask;
 
-constexpr size_t CanClockTaskStackWords = 300;
+constexpr size_t CanClockTaskStackWords = 400;			// used to be 300 but RD had a stack overflow
 static Task<CanSenderTaskStackWords> canClockTask;
 
 static CanMessageBuffer * volatile pendingBuffers;
@@ -293,7 +294,7 @@ CanRequestId CanInterface::AllocateRequestId(CanAddress destination) noexcept
 {
 	static uint16_t rid = 0;
 
-	CanRequestId rslt = rid & 0x07FF;
+	CanRequestId rslt = rid & CanRequestIdMask;
 	++rid;
 	return rslt;
 }
@@ -525,9 +526,14 @@ template<class T> static GCodeResult SetRemoteDriverValues(const CanDriversData<
 	return rslt;
 }
 
+// Set remote drivers to enabled, disabled, or idle
+// This function is called by both the main task and the Move task.
+// As there is no mutual exclusion on putting messages in buffers or receiving replies, when this is called by the Move task
+// we send the commands through the FIFO and we use a special RequestId to say that we do not expect a response
 static GCodeResult SetRemoteDriverStates(const CanDriversList& drivers, const StringRef& reply, DriverStateControl state) noexcept
 {
 	GCodeResult rslt = GCodeResult::ok;
+	const bool fromMoveTask = TaskBase::GetCallerTaskHandle() == Move::GetMoveTaskHandle();
 	size_t start = 0;
 	for (;;)
 	{
@@ -543,7 +549,7 @@ static GCodeResult SetRemoteDriverStates(const CanDriversList& drivers, const St
 			reply.lcat(NoCanBufferMessage);
 			return GCodeResult::error;
 		}
-		const CanRequestId rid = CanInterface::AllocateRequestId(boardAddress);
+		const CanRequestId rid = (fromMoveTask) ? CanRequestIdNoReplyNeeded : CanInterface::AllocateRequestId(boardAddress);
 		const auto msg = buf->SetupRequestMessage<CanMessageMultipleDrivesRequest<DriverStateControl>>(rid, CanInterface::GetCanAddress(), boardAddress, CanMessageType::setDriverStates);
 		msg->driversToUpdate = driverBits.GetRaw();
 		const size_t numDrivers = driverBits.CountSetBits();
@@ -552,7 +558,14 @@ static GCodeResult SetRemoteDriverStates(const CanDriversList& drivers, const St
 			msg->values[i] = state;
 		}
 		buf->dataLength = msg->GetActualDataLength(numDrivers);
-		rslt = max(rslt, CanInterface::SendRequestAndGetStandardReply(buf, rid, reply));
+		if (fromMoveTask)
+		{
+			CanInterface::SendMotion(buf);														// if it's coming from the Move task then we must send the command via the fifo
+		}
+		else
+		{
+			rslt = max(rslt, CanInterface::SendRequestAndGetStandardReply(buf, rid, reply));	// send the command via the usual mechanism
+		}
 	}
 	return rslt;
 }
@@ -590,13 +603,25 @@ GCodeResult CanInterface::SendRequestAndGetCustomReply(CanMessageBuffer *buf, Ca
 	if (can0dev == nullptr)
 	{
 		// Transactions sometimes get requested after we have shut down CAN, e.g. when we destroy filament monitors
+		CanMessageBuffer::Free(buf);
 		return GCodeResult::error;
 	}
+
 	const CanAddress dest = buf->id.Dst();
+	const CanMessageType msgType = buf->id.MsgType();								// save for possible error message
+
+	// This code isn't re-entrant, so check that we are the main task
+	if (TaskBase::GetCallerTaskHandle() != Tasks::GetMainTask())
+	{
+		reply.printf("SendReq call from wrong task type=%u dst=%u",(unsigned int)msgType, (unsigned int)dest);
+		reprap.GetPlatform().MessageF(ErrorMessage, "%s\n", reply.c_str());			// send it directly in case the caller discards the reply
+		CanMessageBuffer::Free(buf);
+		return GCodeResult::error;
+	}
+
 	can0dev->SendMessage(TxBufferIndexRequest, MaxRequestSendWait, buf);
 	const uint32_t whenStartedWaiting = millis();
 	unsigned int fragmentsReceived = 0;
-	const CanMessageType msgType = buf->id.MsgType();								// save for possible error message
 	for (;;)
 	{
 		const uint32_t timeWaiting = millis() - whenStartedWaiting;
@@ -643,7 +668,7 @@ GCodeResult CanInterface::SendRequestAndGetCustomReply(CanMessageBuffer *buf, Ca
 			}
 			++fragmentsReceived;
 		}
-		else if (matchesRequest &&buf->id.MsgType() == replyType && fragmentsReceived == 0)
+		else if (matchesRequest && buf->id.MsgType() == replyType && fragmentsReceived == 0)
 		{
 			callback(buf);
 			CanMessageBuffer::Free(buf);
@@ -718,7 +743,7 @@ GCodeResult CanInterface::EnableRemoteDrivers(const CanDriversList& drivers, con
 	return SetRemoteDriverStates(drivers, reply, DriverStateControl(DriverStateControl::driverActive));
 }
 
-// This one is used by Prepare
+// This one is used by Prepare and by M17
 void CanInterface::EnableRemoteDrivers(const CanDriversList& drivers) noexcept
 {
 	String<1> dummy;
diff --git a/src/Movement/Move.cpp b/src/Movement/Move.cpp
index 2feadeb5..e3f073f9 100644
--- a/src/Movement/Move.cpp
+++ b/src/Movement/Move.cpp
@@ -49,11 +49,7 @@
 # include <CAN/CanMotion.h>
 #endif
 
-// Move task stack size
-// 250 is not enough when Move and DDA debug are enabled
-// deckingman's system (MB6HC with CAN expansion) needs at least 365 in 3.3beta3
-constexpr unsigned int MoveTaskStackWords = 450;
-static Task<MoveTaskStackWords> moveTask;
+Task<Move::MoveTaskStackWords> Move::moveTask;
 
 constexpr uint32_t MoveTimeout = 20;					// normal timeout when the Move process is waiting for a new move
 
diff --git a/src/Movement/Move.h b/src/Movement/Move.h
index 5cfcf29e..940cfe8b 100644
--- a/src/Movement/Move.h
+++ b/src/Movement/Move.h
@@ -194,6 +194,8 @@ public:
 
 	static void WakeMoveTaskFromISR() noexcept;
 
+	static const TaskBase *GetMoveTaskHandle() noexcept { return &moveTask; }
+
 #if SUPPORT_REMOTE_COMMANDS
 	void AddMoveFromRemote(const CanMessageMovementLinear& msg) noexcept					// add a move from the ATE to the movement queue
 	{
@@ -222,6 +224,12 @@ private:
 
 	const char *GetCompensationTypeString() const noexcept;
 
+	// Move task stack size
+	// 250 is not enough when Move and DDA debug are enabled
+	// deckingman's system (MB6HC with CAN expansion) needs at least 365 in 3.3beta3
+	static constexpr unsigned int MoveTaskStackWords = 450;
+	static Task<MoveTaskStackWords> moveTask;
+
 #if SUPPORT_ASYNC_MOVES
 	DDARing rings[2];
 	DDARing& auxDDARing = rings[1];						// the DDA ring used for live babystepping, height following and other asynchronous moves
diff --git a/src/Version.h b/src/Version.h
index 0cd87871..81ac3df7 100644
--- a/src/Version.h
+++ b/src/Version.h
@@ -9,7 +9,7 @@
 #define SRC_VERSION_H_
 
 #ifndef VERSION
-# define MAIN_VERSION	"3.3RC2+1"
+# define MAIN_VERSION	"3.3RC2+2"
 # ifdef USE_CAN0
 #  define VERSION_SUFFIX	" (CAN0)"
 # else