#263 klipper support (#274)

* Fix display E trigger and busy write

* Add bed SoftPWM to "-n" flag

* Fix PINDA triggering in klipper

* Make TMC2130 respect pushpull DIAG config.

* Tweak heater cooling behaviour

* Add a clock skew correction option for external interactions that are time-dependent.

* Add test coverage for TMC DIAG config

Co-authored-by: Alex Voinea <[email protected]>

MK404.cpp

@@ -274,13 +274,16 @@ int main(int argc, char *argv[])
 	ValueArg<int> argVCDRate("","tracerate", "Sets the logging frequency of the VCD trace (default 100uS)",false, 100,"integer",cmd);
 	MultiArg<string> argVCD("t","trace","Enables VCD traces for the specified categories or IRQs. use '-t ?' to get a printout of available traces",false,"string",cmd);
 	SwitchArg argTest("","test","Run it test mode (no graphics, don't auto-exit.", cmd);
-	ValueArg<string> argSD("","sdimage","Use the given SD card .img file instead of the default", false ,"", "filename.img", cmd);
+	SwitchArg argSkew("","skew-correct","Attempt to correct for fast clock skew of the simulated board", cmd);
 	SwitchArg argSerial("s","serial","Connect a printer's serial port to a PTY instead of printing its output to the console.", cmd);
+	ValueArg<string> argSD("","sdimage","Use the given SD card .img file instead of the default", false ,"", "filename.img", cmd);
 	SwitchArg argScriptHelp("","scripthelp", "Prints the available scripting commands for the current printer/context",cmd, false);
 	ValueArg<string> argScript("","script","Execute the given script. Use --scripthelp for syntax.", false ,"", "filename.txt", cmd);
 	SwitchArg argNoHacks("n","no-hacks","Disable any special hackery that might have been implemented for a board to run its manufacturer firmware, e.g. if you want to run stock marlin and have issues. Effects depend on the board and firmware.",cmd);
 	SwitchArg argMute("m","mute","Tell a printer to mute any audio it may produce.", cmd);
+	SwitchArg argMarlin("","marlin","Synonym for --no-hacks",cmd,false);
 	SwitchArg argLoad("l","loadfw","Directs the printer to load the default firmware file. (-f implies -l) If neither -l or -f are provided, the printer executes solely from its persisted flash.", cmd);
+	SwitchArg argKlipper("","klipper","Synonym for --skew-correct and --no-hacks",cmd,false);
 	SwitchArg argKeyHelp("k","keys","Prints the list of available keyboard controls",cmd,false);
 	std::vector<string> vstrSizes = FatImage::GetSizes();
 	ValuesConstraint<string> vcSizes(vstrSizes);
@@ -314,6 +317,11 @@ int main(int argc, char *argv[])
 
 	cmd.parse(argc,argv);
 
+	// Handle the convenience synonyms:
+	// Longer term it'd be neat to have a synonym handler in  TCLAP....
+	bool bArgHacks = argNoHacks.isSet() || argKlipper.isSet() || argMarlin.isSet();
+	bool bArgSkew = argSkew.isSet() || argKlipper.isSet();
+
 	// Make new image.
 	if (argImgSize.isSet())
 	{
@@ -349,11 +357,13 @@ int main(int argc, char *argv[])
 		strFW = argFW.getValue();
 	}
 
-	void *pRawPrinter = PrinterFactory::CreatePrinter(argModel.getValue(),pBoard,printer,argBootloader.isSet(),argNoHacks.isSet(),argSerial.isSet(), argSD.getValue() ,
+	void *pRawPrinter = PrinterFactory::CreatePrinter(argModel.getValue(),pBoard,printer,argBootloader.isSet(),bArgHacks,argSerial.isSet(), argSD.getValue() ,
 		strFW,argSpam.getValue(), argGDB.isSet(), argVCDRate.getValue(),"stk500boot_v2_mega2560.hex"); // this line is the CreateBoard() args.
 
 	pBoard->SetPrimary(true); // This is the primary board, responsible for scripting/dispatch. Blocks contention from sub-boards, e.g. MMU.
 
+	pBoard->SetAdjustSkew(bArgSkew);
+
 	if (!bNoGraphics)
 	{
 		glutInit(&argc, argv);		/* initialize GLUT system */

parts/Board.cpp

@@ -35,6 +35,7 @@
 #include "sim_hex.h"  // for read_ihex_file
 #include "sim_io.h"         // for avr_io_getirq
 #include "sim_regbit.h"     // for avr_regbit_get, avr_regbit_set
+#include "sim_time.h"
 #include "uart_pty.h"       // for uart_pty
 #include <cstdint>
 #include <cstdlib>   // for exit, free
@@ -291,8 +292,21 @@ namespace Boards {
 		switch (key)
 		{
 			case 'z':
+			{
 				m_bPaused ^= true;
 				std::cout <<  "Pause: " << m_bPaused << '\n';
+				auto tWall = avr_get_time_stamp(m_pAVR);
+				auto tSim = avr_cycles_to_nsec(m_pAVR, m_pAVR->cycle);
+				if (tWall<tSim)
+				{
+					std::cout << "Sim is ahead by" << std::to_string((tSim - tWall)/1000) << "us!\n";
+				}
+				else
+				{
+					std::cout << "Sim is behind by" << std::to_string((tWall - tSim)/1000) << "us!\n";
+				}
+
+			}
 			break;
 			case 'q':
 				SetQuitFlag();
@@ -396,7 +410,18 @@ namespace Boards {
 		std::cout << "Starting " << m_wiring.GetMCUName() << " execution...\n";
 		int state = cpu_Running;
 		while ((state != cpu_Done) && (state != cpu_Crashed) && !m_bQuit){
-					// Re init the special workarounds we need after a reset.
+			// Check the timing every 10k cycles, ~10 ms
+			if (m_bCorrectSkew && m_pAVR->cycle%500==0)
+			{
+				auto tWall = avr_get_time_stamp(m_pAVR);
+				auto tSim = avr_cycles_to_nsec(m_pAVR, m_pAVR->cycle);
+				if (tWall<tSim)
+				{
+					auto tDiff = tSim - tWall;
+					if (tDiff>200000) usleep(tDiff/1000);
+					//std::cout << "Sim is ahead by" << std::to_string(tSim - tWall) << "ns!\n";
+				}
+			}
 			if (m_bIsPrimary) // Only one board should be scripting.
 			{
 				ScriptHost::DispatchMenuCB();

parts/Board.h

@@ -87,8 +87,12 @@ namespace Boards
 			inline bool IsStopped(){ return m_pAVR->state == cpu_Stopped;}
 			inline bool IsPaused(){ return m_bPaused;}
 
+			// Is this the primary controlling board?
 			inline void SetPrimary(bool bVal) { m_bIsPrimary = bVal;}
 
+			// Should the board attempt to correct clock skew for fast simulation?
+			inline void SetAdjustSkew(bool bVal) { m_bCorrectSkew = bVal;}
+
 		protected:
 			// Define this method and use it to initialize/attach your hardware to the MCU.
 			virtual void SetupHardware() = 0;
@@ -145,6 +149,9 @@ namespace Boards
 
 			std::atomic_uint8_t m_bPaused = {false};
 
+			// Whether any vendor-specific hackery should be disabled.
+			bool m_bNoHacks = false;
+
 		private:
 			void CreateAVR();
 
@@ -157,8 +164,8 @@ namespace Boards
 			avr_flashaddr_t LoadFirmware(const std::string &strFW);
 
 			std::atomic_bool m_bQuit = {false}, m_bReset = {false};
-			bool m_bIsPrimary = false;
-			bool m_bNoHacks = false;
+			bool m_bIsPrimary = false, m_bCorrectSkew = false;
+
 			pthread_t m_thread = 0;
 			const Wirings::Wiring &m_wiring;
 			std::string m_strBoard = "";

parts/boards/EinsyRambo.cpp

@@ -105,6 +105,11 @@ namespace Boards
 		AddHardware(hBed, nullptr, GetDIRQ(HEATER_BED_PIN));
 		hBed.ConnectTo(Heater::TEMP_OUT, tBed.GetIRQ(Thermistor::TEMP_IN));
 
+		if ( m_bNoHacks )
+		{
+			hBed.SetSoftPWM(false);
+		}
+
 		AddHardware(hExtruder, nullptr, GetDIRQ(HEATER_0_PIN));
 		hExtruder.ConnectTo(Heater::TEMP_OUT, tExtruder.GetIRQ(Thermistor::TEMP_IN));
 

parts/components/HD44780.cpp

@@ -333,6 +333,11 @@ uint32_t HD44780::OnCmdReady()
  */
 uint32_t HD44780::ProcessWrite()
 {
+	if (GetFlag(HD44780_FLAG_BUSY))
+	{
+		std::cout << static_cast<const char*>(__FUNCTION__) << " command " << m_uiDataPins << " write when still BUSY" << '\n';
+		return 0;
+	}
 	uint32_t delay = 0; // uS
 	int four = !GetFlag(HD44780_FLAG_D_L);
 	int comp = four && GetFlag(HD44780_FLAG_LOWNIBBLE);
@@ -360,10 +365,6 @@ uint32_t HD44780::ProcessWrite()
 	// write has 8 bits to process
 	if (write)
 	{
-		if (GetFlag(HD44780_FLAG_BUSY))
-		{
-			std::cout << static_cast<const char*>(__FUNCTION__) << " command " << m_uiDataPins << "write when still BUSY" << '\n';
-		}
 		if (m_uiPinState & (1U << RS))	// write data
 		{
 			delay = OnDataReady();
@@ -488,8 +489,8 @@ void HD44780::OnPinChanged(struct avr_irq_t * irq,uint32_t value)
 	m_uiPinState = (m_uiPinState & ~(1U << irq->irq)) | (value << irq->irq);
 	int eo = old & (1U << E);
 	int e = m_uiPinState & (1U << E);
-	// on the E pin rising edge, do stuff otherwise just exit
-	if (!eo && e)
+	// on the E pin falling edge, do stuff otherwise just exit
+	if (eo && !e)
 	{
 		RegisterTimer(m_fcnEPinChanged,1,this);
 	}
@@ -519,6 +520,10 @@ void HD44780::Init(avr_t *avr)
 	TH.AddTrace(this, E, {TC::Display, TC::OutputPin});
 	TH.AddTrace(this, RS, {TC::Display, TC::OutputPin});
 	TH.AddTrace(this, RW, {TC::Display, TC::OutputPin});
+	TH.AddTrace(this, D4, {TC::Display});
+	TH.AddTrace(this, D5, {TC::Display});
+	TH.AddTrace(this, D6, {TC::Display});
+	TH.AddTrace(this, D7, {TC::Display});
 	TH.AddTrace(this, DATA_IN, {TC::Display},8);
 	TH.AddTrace(this, BRIGHTNESS_IN, {TC::Display, TC::OutputPin});
 	TH.AddTrace(this, BRIGHTNESS_PWM_IN, {TC::Display, TC::PWM});

parts/components/Heater.cpp

@@ -39,14 +39,14 @@
 #endif
 
 
-avr_cycle_count_t Heater::OnTempTick(avr_t *, avr_cycle_count_t)
+avr_cycle_count_t Heater::OnTempTick(avr_t * pAVR, avr_cycle_count_t)
 {
 	if (m_bStopTicking)
 	{
 		return 0;
 	}
 
-    if (m_uiPWM>0)
+    if (m_uiPWM>0 || (pAVR->cycle-m_cntOff)<(pAVR->frequency/100))
     {
         float fDelta = (m_fThermalMass*(static_cast<float>(m_uiPWM)/255.0f))*0.3f;
         m_fCurrentTemp += fDelta;
@@ -85,6 +85,11 @@ void Heater::OnPWMChanged(struct avr_irq_t *,uint32_t value)
 	{
         RegisterTimerUsec(m_fcnTempTick, 100000, this);
 	}
+	else
+	{
+		m_cntOff = m_pAVR->cycle;
+	}
+
     if (GetIRQ(ON_OUT)->value != (m_uiPWM>0))
 	{
         RaiseIRQ(ON_OUT,m_uiPWM>0);

parts/components/Heater.h

@@ -100,4 +100,5 @@ class Heater : public BasePeripheral, public Scriptable
 		bool m_bStopTicking = false;
 	    static constexpr Color3fv m_colColdTemp = {0, 1, 1};
 	    static constexpr Color3fv m_colHotTemp = {1, 0, 0};
+		avr_cycle_count_t m_cntOff = 0;
 };

parts/components/PINDA.cpp

@@ -289,7 +289,7 @@ void PINDA::Init(struct avr_t * avr, avr_irq_t *irqX, avr_irq_t *irqY, avr_irq_t
     RegisterNotify(Y_POS_IN, MAKE_C_CALLBACK(PINDA,OnYChanged),this);
     RegisterNotify(Z_POS_IN, MAKE_C_CALLBACK(PINDA,OnZChanged),this);
 	GetIRQ(TRIGGER_OUT)->flags |= IRQ_FLAG_FILTERED; // No retriggers.
-    RaiseIRQ(TRIGGER_OUT,0);
+    //RaiseIRQ(TRIGGER_OUT,0);
 
 	auto &TH = TelemetryHost::GetHost();
 	TH.AddTrace(this, TRIGGER_OUT, {TC::InputPin, TC::Misc});

parts/components/TMC2130.cpp

@@ -151,10 +151,14 @@ void TMC2130::ProcessCommand()
     {
         gsl::at(m_regs.raw,m_cmdProc.bitsIn.address) = m_cmdProc.bitsIn.data;
         //printf("REG %c %02x set to: %010x\n", m_cAxis, m_cmdIn.bitsIn.address, m_cmdIn.bitsIn.data);
-
-		if(m_cmdProc.bitsIn.address == 0x6C) // CHOPCONF
+		switch (m_cmdProc.bitsIn.address)
 		{
-			m_uiStepIncrement = std::pow(2,m_regs.defs.CHOPCONF.mres);
+			case 0x00: // GCONF
+				RaiseDiag(m_regs.defs.DRV_STATUS.stallGuard); // Adjust DIAG out, it mayhave  been reconfigured.
+				break;
+			case 0x6C: // Chopconf
+				m_uiStepIncrement = std::pow(2,m_regs.defs.CHOPCONF.mres);
+				break;
 		}
     }
     else
@@ -181,16 +185,19 @@ uint8_t TMC2130::OnSPIIn(struct avr_irq_t *, uint32_t value)
     return byte; // SPIPeripheral takes care of the reply.
 }
 
-// TODO (anyone): Fix the diag output so it respects GCONF
-// void TMC2130::CheckDiagOut()
-// {
-//     bool bDiag = m_regs.defs.DRV_STATUS.stallGuard && m_regs.defs.GCONF.diag0_stall;
-//     //printf("Diag: %01x\n",bDiag);
-//     if (bDiag)
-// 	{
-//         RaiseIRQ(DIAG_OUT, bDiag^ m_regs.defs.GCONF.diag0_int_pushpull);
-// 	}
-// }
+
+void TMC2130::RaiseDiag(uint8_t value)
+{
+	// TODO (anyone) - right now these are tied together because the Einsy board does so.
+	// But in the future we may need to separate this out to toggle DIAG0 and DIAG one separately.
+    bool bDiag = m_regs.defs.GCONF.diag0_stall || m_regs.defs.GCONF.diag1_stall;
+    //printf("%c Diag: %01x SG %d PP %01x %01x \n",m_cAxis.load(), bDiag, m_regs.defs.DRV_STATUS.stallGuard, m_regs.defs.GCONF.diag0_int_pushpull, value );
+    if (bDiag)
+	{
+		//printf("Raised: %d\n", value ==  m_regs.defs.GCONF.diag0_int_pushpull);
+        RaiseIRQ(DIAG_OUT, value == m_regs.defs.GCONF.diag0_int_pushpull);
+	}
+}
 
 // Called when CSEL changes.
 void TMC2130::OnCSELIn(struct avr_irq_t *, uint32_t value)
@@ -264,12 +271,12 @@ void TMC2130::OnStepIn(struct avr_irq_t * irq, uint32_t value)
 	bStall |= m_bStall;
     if (bStall)
     {
-        RaiseIRQ(DIAG_OUT, 1);
+        RaiseDiag(1);
         m_regs.defs.DRV_STATUS.SG_RESULT = 0;
     }
     else if (!bStall && m_regs.defs.DRV_STATUS.stallGuard)
     {
-          RaiseIRQ(DIAG_OUT, 0);
+          RaiseDiag(0);
           m_regs.defs.DRV_STATUS.SG_RESULT = 250;
     }
     m_regs.defs.DRV_STATUS.stallGuard = bStall;

parts/components/TMC2130.h

@@ -108,7 +108,7 @@ class TMC2130: public SPIPeripheral, public Scriptable
         void ProcessCommand();
         void CreateReply();
 
-        //void CheckDiagOut();
+        void RaiseDiag(uint8_t value);
 
         bool m_bDir  = false;
         std::atomic_bool m_bEnable {true}, m_bConfigured {false};

scripts/tests/test_TMC2130.c

@@ -109,6 +109,14 @@ int main()
 
 	printf("RP %02x %06lx\n",uiStatus, uiReply);
 
+	TMCTX(0x80,1U<<7U); // DIAG0_stall
+
+	printf("DIAG %02x\n",PINA&0x02);
+
+	TMCTX(0x80,1U<<7U | 1U << 12); // DIAG0_stall && pushpull
+
+	printf("DIAG %02x\n",PINA&0x02);
+
 	for(int i=0; i<160; i++)
 	{
 		step();
@@ -169,6 +177,11 @@ int main()
 	step();
 	printf("DIAGDISABLE %02x\n",PINA&0x02);
 
+	TMCTX(0x80,1U<<7U); // DIAG0_stall, Act low
+
+	printf("DIAG %02x\n",PINA&0x02);
+
+
 	cli();
 
 

scripts/tests/test_TMC2130.txt

@@ -6,6 +6,8 @@ GLHelper::SnapRect(tests/snaps/TMC01,0,164,500,80)
 Board::Resume()
 Serial0::NextLineMustBe(RP 09 000080)
 Serial0::NextLineMustBe(RP 08 000000)
+Serial0::NextLineMustBe(DIAG 02) # Normal diag EN (Active low)
+Serial0::NextLineMustBe(DIAG 00) # PP, active high.
 Serial0::NextLineMustBe(DIAGMAX 02)
 Board::Pause()
 GLHelper::SnapRect(tests/snaps/TMC02,0,164,500,80)
@@ -20,4 +22,5 @@ Board::Pause()
 GLHelper::SnapRect(tests/snaps/TMC04,0,164,500,80)
 Board::Resume()
 Serial0::NextLineMustBe(DIAGDISABLE 02)
+Serial0::NextLineMustBe(DIAG 00)
 Board::Quit()