Version 1.17c+2

Fixed bug in G2 and G3 commands (I and J parameters were not always
being assumed to be relative to current X and Y)
Hard fauld handler now stores the faulting PC and status registers in
the software reset data

7 files changed, 100 insertions, 39 deletions

diff --git a/src/GCodes/GCodes.cpp b/src/GCodes/GCodes.cpp
index 6add9276..bf5f2d95 100644
--- a/src/GCodes/GCodes.cpp
+++ b/src/GCodes/GCodes.cpp
@@ -1359,7 +1359,9 @@ bool GCodes::DoArcMove(GCodeBuffer& gb, bool clockwise)
 		}
 	}
 
-	float initialDx = 0.0;
+	// The I and J parameters are always relative to present position
+	arcCentre[Y_AXIS] = moveBuffer.initialCoords[Y_AXIS] + jParam;
+
 	if (currentTool != nullptr)
 	{
 		// Record which axes behave like an X axis
@@ -1369,30 +1371,25 @@ bool GCodes::DoArcMove(GCodeBuffer& gb, bool clockwise)
 		if (axesRelative)
 		{
 			moveBuffer.coords[Y_AXIS] += yParam;
-			arcCentre[Y_AXIS] = moveBuffer.initialCoords[Y_AXIS] + jParam;
 		}
 		else
 		{
 			moveBuffer.coords[Y_AXIS] = yParam - currentTool->GetOffset()[Y_AXIS];
-			arcCentre[Y_AXIS] = jParam - currentTool->GetOffset()[Y_AXIS];
 		}
 
 		// Deal with the X axes
 		for (size_t axis = 0; axis < numAxes; ++axis)
 		{
+			arcCentre[axis] = moveBuffer.initialCoords[axis] + iParam;
 			if ((arcAxesMoving & (1 << axis)) != 0)
 			{
 				if (axesRelative)
 				{
 					moveBuffer.coords[axis] += xParam;
-					arcCentre[axis] = moveBuffer.initialCoords[axis] + iParam;
-					initialDx = -iParam;
 				}
 				else
 				{
 					moveBuffer.coords[axis] = xParam - currentTool->GetOffset()[axis];
-					arcCentre[axis] = iParam + currentTool->GetOffset()[axis];
-					initialDx = moveBuffer.initialCoords[axis] - arcCentre[axis];
 				}
 			}
 		}
@@ -1400,21 +1397,16 @@ bool GCodes::DoArcMove(GCodeBuffer& gb, bool clockwise)
 	else
 	{
 		arcAxesMoving = (1 << X_AXIS);
+		arcCentre[X_AXIS] = moveBuffer.initialCoords[X_AXIS] + iParam;
 		if (axesRelative)
 		{
 			moveBuffer.coords[X_AXIS] += xParam;
-			arcCentre[X_AXIS] = moveBuffer.initialCoords[X_AXIS] + iParam;
 			moveBuffer.coords[Y_AXIS] += yParam;
-			arcCentre[Y_AXIS] = moveBuffer.initialCoords[Y_AXIS] + jParam;;
-			initialDx = -iParam;
 		}
 		else
 		{
 			moveBuffer.coords[X_AXIS] = xParam;
-			arcCentre[X_AXIS] = iParam;
 			moveBuffer.coords[Y_AXIS] = yParam;
-			arcCentre[Y_AXIS] = jParam;
-			initialDx = moveBuffer.initialCoords[X_AXIS] - arcCentre[X_AXIS];
 		}
 	}
 
@@ -1423,10 +1415,9 @@ bool GCodes::DoArcMove(GCodeBuffer& gb, bool clockwise)
 	moveBuffer.xAxes = reprap.GetCurrentXAxes();
 	if (LoadExtrusionAndFeedrateFromGCode(gb, moveBuffer.moveType))		// this reports an error if necessary, so no need to return true if it fails
 	{
-		const float initialDy = moveBuffer.initialCoords[Y_AXIS] - arcCentre[Y_AXIS];
-		arcRadius = sqrtf(initialDx * initialDx + initialDy * initialDy);
-		arcCurrentAngle = atan2(initialDy, initialDx);
-		const float finalTheta = atan2(yParam - jParam, xParam - iParam);
+		arcRadius = sqrtf(iParam * iParam + jParam * jParam);
+		arcCurrentAngle = atan2(-jParam, -iParam);
+		const float finalTheta = atan2(moveBuffer.coords[Y_AXIS] - arcCentre[Y_AXIS], moveBuffer.coords[X_AXIS] - arcCentre[X_AXIS]);
 
 		// Calculate the total angle moved, which depends on which way round we are going
 		float totalArc = (clockwise) ? arcCurrentAngle - finalTheta : finalTheta - arcCurrentAngle;
diff --git a/src/Movement/DDA.cpp b/src/Movement/DDA.cpp
index 20d98b38..74d614c2 100644
--- a/src/Movement/DDA.cpp
+++ b/src/Movement/DDA.cpp
@@ -554,7 +554,10 @@ pre(state == provisional)
 			// Still going up
 			laDDA = laDDA->prev;
 			++laDepth;
-			if (reprap.Debug(moduleDda)) debugPrintf("Recursion start %u\n", laDepth);
+			if (reprap.Debug(moduleDda))
+			{
+				debugPrintf("Recursion start %u\n", laDepth);
+			}
 		}
 		else
 		{
diff --git a/src/Platform.cpp b/src/Platform.cpp
index 16c460a5..79252b91 100644
--- a/src/Platform.cpp
+++ b/src/Platform.cpp
@@ -84,7 +84,7 @@ void UrgentInit()
 {
 #ifdef DUET_NG
 	// When the reset button is pressed on pre-production Duet WiFi boards, if the TMC2660 drivers were previously enabled then we get
-	// uncommanded motor movements if the STEP lines pick up any noise. Try to reduce that by initialising the drivers early here.
+	// uncommanded motor movements if the STEP lines pick up any noise. Try to reduce that by initialising the pins that control the drivers early here.
 	// On the production boards the ENN line is pulled high and that prevents motor movements.
 	for (size_t drive = 0; drive < DRIVES; ++drive)
 	{
@@ -107,6 +107,16 @@ void setup()
 		*heapend++ = memPattern;
 	}
 
+	// Trap integer divide-by-zero.
+	// We could also trap unaligned memory access, if we change the gcc options to not generate code that uses unaligned memory access.
+	SCB->CCR |= SCB_CCR_DIV_0_TRP_Msk;
+
+	// When doing a software reset, we disable the NRST input (User reset) to prevent the negative-going pulse that gets generated on it
+	// being held in the capacitor and changing the reset reason form Software to User. So enable it again here. We hope that the reset signal
+	// will have gone away by now.
+	RSTC->RSTC_MR = RSTC_MR_KEY_PASSWD | RSTC_MR_URSTEN;	// ignore any signal on the NRST pin for now so that the reset reason will show as Software
+
+	// Go on and do the main initialisation
 	reprap.Init();
 }
 
@@ -124,18 +134,42 @@ extern "C"
 		return 0;
 	}
 
+	// Exception handlers
+	// By default the Usage Fault, Bus Fault and Memory Management fault handlers are not enabled,
+	// so they escalate to a Hard Fault and we don't need to provide separate exception handlers for them.
+	// We can get information on what caused a Hard Fault form the status registers.
+	// Also get the program counter when the exception occurred.
+	void prvGetRegistersFromStack(const uint32_t *pulFaultStackAddress)
+	{
+		const uint32_t pc = pulFaultStackAddress[6];
+	    reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::hardFault, pc);
+	}
+
+	// The fault handler implementation calls a function called prvGetRegistersFromStack()
+	void HardFault_Handler() __attribute__((naked));
+	void HardFault_Handler()
+	{
+	    __asm volatile
+	    (
+	        " tst lr, #4                                                \n"
+	        " ite eq                                                    \n"
+	        " mrseq r0, msp                                             \n"
+	        " mrsne r0, psp                                             \n"
+	        " ldr r1, [r0, #24]                                         \n"
+	        " ldr r2, handler2_address_const                            \n"
+	        " bx r2                                                     \n"
+	        " handler2_address_const: .word prvGetRegistersFromStack    \n"
+	    );
+	}
+
+	// We could set up the following fault handlers to retrieve the program counter in the same way as for a Hard Fault,
+	// however these exceptions are unlikely to occur, so for now we just report the exception type.
 	void NMI_Handler        () { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::NMI); }
-	void HardFault_Handler  () { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::hardFault); }
-	void MemManage_Handler  () { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::memManage); }
-	void BusFault_Handler   () { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::busFault); }
-	void UsageFault_Handler () { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::usageFault); }
 	void SVC_Handler		() { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::otherFault); }
 	void DebugMon_Handler   () { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::otherFault); }
 	void PendSV_Handler		() { reprap.GetPlatform()->SoftwareReset((uint16_t)SoftwareResetReason::otherFault); }
 }
 
-
-
 // ZProbeParameters class
 
 void ZProbeParameters::Init(float h)
@@ -1181,7 +1215,7 @@ void Platform::Spin()
 	ClassReport(longWait);
 }
 
-void Platform::SoftwareReset(uint16_t reason)
+void Platform::SoftwareReset(uint16_t reason, uint32_t pc)
 {
 	wdt_restart(WDT);							// kick the watchdog
 	if (reason == (uint16_t)SoftwareResetReason::erase)
@@ -1240,6 +1274,9 @@ void Platform::SoftwareReset(uint16_t reason)
 		srdBuf[slot].magic = SoftwareResetData::magicValue;
 		srdBuf[slot].resetReason = reason;
 		GetStackUsage(NULL, NULL, &srdBuf[slot].neverUsedRam);
+		srdBuf[slot].hfsr = SCB->HFSR;
+		srdBuf[slot].cfsr = SCB->CFSR;
+		srdBuf[slot].pc = pc;
 
 		// Save diagnostics data to Flash
 #ifdef DUET_NG
@@ -1248,13 +1285,13 @@ void Platform::SoftwareReset(uint16_t reason)
 		DueFlashStorage::write(SoftwareResetData::nvAddress, srdBuf, sizeof(srdBuf));
 #endif
 	}
+
+	RSTC->RSTC_MR = RSTC_MR_KEY_PASSWD;			// ignore any signal on the NRST pin for now so that the reset reason will show as Software
 	Reset();
 	for(;;) {}
 }
 
-
 //*****************************************************************************************************************
-
 // Interrupts
 
 #ifndef DUET_NG
@@ -1401,10 +1438,11 @@ void Platform::Diagnostics(MessageType mtype)
 			}
 		}
 
-		Message(mtype, "Last software reset code: ");
+		Message(mtype, "Last software reset code ");
 		if (slot >= 0 && srdBuf[slot].magic == SoftwareResetData::magicValue)
 		{
-			MessageF(mtype, "0x%04x, available RAM %u bytes (slot %d)\n", srdBuf[slot].resetReason, srdBuf[slot].neverUsedRam, slot);
+			MessageF(mtype, "0x%04x, PC 0x%08x, HFSR 0x%08x, CFSR 0x%08x, available RAM %u bytes (slot %d)\n",
+					srdBuf[slot].resetReason, srdBuf[slot].pc, srdBuf[slot].hfsr, srdBuf[slot].cfsr, srdBuf[slot].neverUsedRam, slot);
 			MessageF(mtype, "Spinning module during software reset: %s\n", moduleName[srdBuf[slot].resetReason & 0x0F]);
 		}
 		else
@@ -1512,6 +1550,8 @@ void Platform::Diagnostics(MessageType mtype)
 
 void Platform::DiagnosticTest(int d)
 {
+	static const uint32_t dummy[2] = { 0, 0 };
+
 	switch (d)
 	{
 	case (int)DiagnosticTestType::TestWatchdog:
@@ -1526,6 +1566,15 @@ void Platform::DiagnosticTest(int d)
 		debugPrintf("Diagnostic Test\n");
 		break;
 
+	case (int)DiagnosticTestType::DivideByZero:			// do an integer divide by zero to test exception handling
+		(void)RepRap::DoDivide(1, 0);					// call function in another module so it can't be optimised away
+		break;
+
+	case (int)DiagnosticTestType::UnalignedMemoryAccess:	// do an unaligned memory access to test exception handling
+		SCB->CCR |= SCB_CCR_UNALIGN_TRP_Msk;			// by default, unaligned memory accesses are allowed, so change that
+		(void)RepRap::ReadDword(reinterpret_cast<const char*>(dummy) + 1);	// call function in another module so it can't be optimised away
+		break;
+
 	case (int)DiagnosticTestType::PrintMoves:
 		DDA::PrintMoves();
 		break;
diff --git a/src/Platform.h b/src/Platform.h
index 220af4b2..f887f907 100644
--- a/src/Platform.h
+++ b/src/Platform.h
@@ -160,9 +160,6 @@ enum class SoftwareResetReason : uint16_t
 	erase = 0x10,					// special M999 command to erase firmware and reset
 	NMI = 0x20,
 	hardFault = 0x30,
-	memManage = 0x40,
-	busFault = 0x50,
-	usageFault = 0x60,
 	otherFault = 0x70,
 	inAuxOutput = 0x0800,			// this bit is or'ed in if we were in aux output at the time
 	stuckInSpin = 0x1000,			// we got stuck in a Spin() function for too long
@@ -176,6 +173,9 @@ enum class DiagnosticTestType : int
 	TestWatchdog = 1001,			// test that we get a watchdog reset if the tick interrupt stops
 	TestSpinLockup = 1002,			// test that we get a software reset if a Spin() function takes too long
 	TestSerialBlock = 1003,			// test what happens when we write a blocking message via debugPrintf()
+	DivideByZero = 1004,			// do an integer divide by zero to test exception handling
+	UnalignedMemoryAccess = 1005,	// do an unaligned memory access to test exception handling
+
 	PrintMoves = 100,				// print summary of recent moves
 #ifdef DUET_NG
 	PrintExpanderStatus = 101,		// print DueXn expander status
@@ -326,7 +326,7 @@ public:
 	void ClassReport(float &lastTime);  			// Called on Spin() return to check everything's live.
 	void LogError(ErrorCode e) { errorCodeBits |= (uint32_t)e; }
 
-	void SoftwareReset(uint16_t reason);
+	void SoftwareReset(uint16_t reason, uint32_t pc = 0);
 	bool AtxPower() const;
 	void SetAtxPower(bool on);
 	void SetBoardType(BoardType bt);
@@ -606,10 +606,10 @@ private:
 	// The SAM4E has a large page erase size (8K). For this reason we store the software reset data in the 512-byte user signature area
 	// instead, which doesn't get cleared when the Erase button is pressed. The SoftareResetData struct must have at least one 32-bit
 	// field to guarantee that values of this type will be 32-bit aligned. It must have no virtual members because it is read/written
-	// directly form/to flash memory.
+	// directly from/to flash memory.
 	struct SoftwareResetData
 	{
-		static const uint16_t versionValue = 2;		// increment this whenever this struct changes
+		static const uint16_t versionValue = 4;		// increment this whenever this struct changes
 		static const uint16_t magicValue = 0x7D00 | versionValue;	// value we use to recognise that all the flash data has been written
 		static const uint32_t nvAddress = 0;		// must be 4-byte aligned
 		static const size_t numberOfSlots = 8;		// number of storage slots used to implement wear levelling
@@ -617,10 +617,14 @@ private:
 		uint16_t magic;								// the magic number, including the version
 		uint16_t resetReason;						// this records why we did a software reset, for diagnostic purposes
 		uint32_t neverUsedRam;						// the amount of never used RAM at the last abnormal software reset
+		uint32_t hfsr;								// hard fault status register
+		uint32_t cfsr;								// configurable fault status register
+		uint32_t pc;								// program counter when the exception occurred
 
 		bool isVacant() const						// return true if this struct can be written without erasing it first
 		{
-			return magic == 0xFFFF && resetReason == 0xFFFF && neverUsedRam == 0xFFFFFFFF;
+			return magic == 0xFFFF && resetReason == 0xFFFF && neverUsedRam == 0xFFFFFFFF
+					&& hfsr == 0xFFFFFFFF && cfsr == 0xFFFFFFFF && pc == 0xFFFFFFFF;
 		}
 	};
 
diff --git a/src/Reprap.cpp b/src/Reprap.cpp
index 5af3de3b..76cb970d 100644
--- a/src/Reprap.cpp
+++ b/src/Reprap.cpp
@@ -1576,4 +1576,16 @@ uint32_t RepRap::GetCurrentXAxes() const
 	return (currentTool == nullptr) ? DefaultXAxisMapping : currentTool->GetXAxisMap();
 }
 
+// Helper function for diagnostic tests in Platform.cpp, to cause a deliberate divide-by-zero
+/*static*/ uint32_t RepRap::DoDivide(uint32_t a, uint32_t b)
+{
+	return a/b;
+}
+
+// Helper function for diagnostic tests in Platform.cpp, to cause a deliberate unaligned memory read
+/*static*/ uint32_t RepRap::ReadDword(const char* p)
+{
+	return *reinterpret_cast<const uint32_t*>(p);
+}
+
 // End
diff --git a/src/Reprap.h b/src/Reprap.h
index 7d63152a..194d869c 100644
--- a/src/Reprap.h
+++ b/src/Reprap.h
@@ -99,6 +99,8 @@ public:
 	void SetMessage(const char *msg);
 
     static void CopyParameterText(const char* src, char *dst, size_t length);
+    static uint32_t DoDivide(uint32_t a, uint32_t b);		// helper function for diagnostic tests
+    static uint32_t ReadDword(const char* p);				// helper function for diagnostic tests
 
 private:
 
diff --git a/src/Version.h b/src/Version.h
index 5064b963..32afea09 100644
--- a/src/Version.h
+++ b/src/Version.h
@@ -9,11 +9,11 @@
 #define SRC_VERSION_H_
 
 #ifndef VERSION
-# define VERSION "1.17c+1"
+# define VERSION "1.17c+2"
 #endif
 
 #ifndef DATE
-# define DATE "2017-01-20"
+# define DATE "2017-01-24"
 #endif
 
 #define AUTHORS "reprappro, dc42, chrishamm, t3p3, dnewman"