From d9dba2c9081fbecba79d82fc5c31aaeeba2cedc6 Mon Sep 17 00:00:00 2001 From: odaki Date: Sat, 10 Oct 2020 06:32:32 +0900 Subject: [PATCH] Fix i2s probing hang (#608) * Fix I2S stepper hung just after the completion of motor moving * Fix recompile issue Fixed a problem with the recompile not being recompiled even if the files under the Custom folder are changed. * More comment for macOS in debug.ini * Fix the timing of calling I2S out's exclusion function and reset sequence The reset sequence did not seem to be correct, so I changed it. According to the ESP-IDF PR, the correct sequence is as follows: 1)TX module 2)DMA 3)FIFO https://github.com/espressif/esp-idf/commit/c7f33524b469e75937f003d4c06336bf4694a043#diff-27688c6b3c29373d2a2b142b8471981c * Changed the message level for I2S swtiching from warning to debug * Add some comments --- Grbl_Esp32/src/GCode.cpp | 14 -------- Grbl_Esp32/src/I2SOut.cpp | 55 ++++++++++++++++++++------------ Grbl_Esp32/src/MotionControl.cpp | 36 +++++++++++++++++++-- Grbl_Esp32/src/Stepper.cpp | 7 ++-- debug.ini | 10 ++++-- platformio.ini | 2 +- 6 files changed, 79 insertions(+), 45 deletions(-) diff --git a/Grbl_Esp32/src/GCode.cpp b/Grbl_Esp32/src/GCode.cpp index c26f4b53..0134952e 100644 --- a/Grbl_Esp32/src/GCode.cpp +++ b/Grbl_Esp32/src/GCode.cpp @@ -1502,9 +1502,6 @@ Error gc_execute_line(char* line, uint8_t client) { if (gc_state.modal.motion != Motion::None) { if (axis_command == AxisCommand::MotionMode) { GCUpdatePos gc_update_pos = GCUpdatePos::Target; -#ifdef USE_I2S_STEPS - stepper_id_t save_stepper = current_stepper; -#endif if (gc_state.modal.motion == Motion::Linear) { //mc_line(gc_block.values.xyz, pl_data); mc_line_kins(gc_block.values.xyz, pl_data, gc_state.position); @@ -1527,12 +1524,6 @@ Error gc_execute_line(char* line, uint8_t client) { // upon a successful probing cycle, the machine position and the returned value should be the same. #ifndef ALLOW_FEED_OVERRIDE_DURING_PROBE_CYCLES pl_data->motion.noFeedOverride = 1; -#endif -#ifdef USE_I2S_STEPS - save_stepper = current_stepper; // remember the stepper - if (save_stepper == ST_I2S_STREAM) { - stepper_switch(ST_I2S_STATIC); // Change the stepper to reduce the delay for accurate probing. - } #endif gc_update_pos = mc_probe_cycle(gc_block.values.xyz, pl_data, gc_parser_flags); } @@ -1544,11 +1535,6 @@ Error gc_execute_line(char* line, uint8_t client) { } else if (gc_update_pos == GCUpdatePos::System) { gc_sync_position(); // gc_state.position[] = sys_position } // == GCUpdatePos::None -#ifdef USE_I2S_STEPS - if (save_stepper == ST_I2S_STREAM && current_stepper != ST_I2S_STREAM) { - stepper_switch(ST_I2S_STREAM); // Put the stepper back on. - } -#endif } } // [21. Program flow ]: diff --git a/Grbl_Esp32/src/I2SOut.cpp b/Grbl_Esp32/src/I2SOut.cpp index 419caf23..47fcea0d 100644 --- a/Grbl_Esp32/src/I2SOut.cpp +++ b/Grbl_Esp32/src/I2SOut.cpp @@ -291,9 +291,27 @@ static int IRAM_ATTR i2s_out_start() { // Attach I2S to specified GPIO pin i2s_out_gpio_attach(i2s_out_ws_pin, i2s_out_bck_pin, i2s_out_data_pin); - //start DMA link + + // reest TX/RX module + I2S0.conf.tx_reset = 1; + I2S0.conf.tx_reset = 0; + I2S0.conf.rx_reset = 1; + I2S0.conf.rx_reset = 0; + +# ifdef USE_I2S_OUT_STREAM_IMPL + // reset DMA + I2S0.lc_conf.in_rst = 1; + I2S0.lc_conf.in_rst = 0; + I2S0.lc_conf.out_rst = 1; + I2S0.lc_conf.out_rst = 0; + + I2S0.out_link.addr = (uint32_t)o_dma.desc[0]; +# endif + + // reset FIFO i2s_out_reset_fifo_without_lock(); + // start DMA link # ifdef USE_I2S_OUT_STREAM_IMPL if (i2s_out_pulser_status == PASSTHROUGH) { I2S0.conf_chan.tx_chan_mod = 3; // 3:right+constant 4:left+constant (when tx_msb_right = 1) @@ -304,21 +322,6 @@ static int IRAM_ATTR i2s_out_start() { } # endif -# ifdef USE_I2S_OUT_STREAM_IMPL - //reset DMA - I2S0.lc_conf.in_rst = 1; - I2S0.lc_conf.in_rst = 0; - I2S0.lc_conf.out_rst = 1; - I2S0.lc_conf.out_rst = 0; - - I2S0.out_link.addr = (uint32_t)o_dma.desc[0]; -# endif - - I2S0.conf.tx_reset = 1; - I2S0.conf.tx_reset = 0; - I2S0.conf.rx_reset = 1; - I2S0.conf.rx_reset = 0; - I2S0.conf1.tx_stop_en = 1; // BCK and WCK are suppressed while FIFO is empty # ifdef USE_I2S_OUT_STREAM_IMPL @@ -339,13 +342,14 @@ static int IRAM_ATTR i2s_out_start() { } # ifdef USE_I2S_OUT_STREAM_IMPL - +// Fill out one DMA buffer +// Call with the I2S_OUT_PULSER lock acquired. +// Note that the lock is temporarily released while calling the callback function. static int IRAM_ATTR i2s_fillout_dma_buffer(lldesc_t* dma_desc) { uint32_t* buf = (uint32_t*)dma_desc->buf; o_dma.rw_pos = 0; // It reuses the oldest (just transferred) buffer with the name "current" // and fills the buffer for later DMA. - I2S_OUT_PULSER_ENTER_CRITICAL(); // Lock pulser status if (i2s_out_pulser_status == STEPPING) { // // Fillout the buffer for pulse @@ -408,7 +412,6 @@ static int IRAM_ATTR i2s_fillout_dma_buffer(lldesc_t* dma_desc) { o_dma.rw_pos = 0; // If someone calls i2s_out_push_sample, make sure there is no buffer overflow i2s_out_remain_time_until_next_pulse = 0; } - I2S_OUT_PULSER_EXIT_CRITICAL(); // Unlock pulser status return 0; } @@ -591,10 +594,18 @@ i2s_out_pulser_status_t IRAM_ATTR i2s_out_get_pulser_status() { int IRAM_ATTR i2s_out_set_passthrough() { I2S_OUT_PULSER_ENTER_CRITICAL(); # ifdef USE_I2S_OUT_STREAM_IMPL + // Triggers a change of mode if it is compiled to use I2S stream. + // The mode is not changed directly by this function. + // Pull the trigger if (i2s_out_pulser_status == STEPPING) { - i2s_out_pulser_status = WAITING; // Start stopping the pulser + i2s_out_pulser_status = WAITING; // Start stopping the pulser (trigger) } + // It is a function that may be called via i2sOutTask(). + // (i2sOutTask() -> stepper_pulse_func() -> st_go_idle() -> Stepper_Timer_Stop() -> this function) + // And only i2sOutTask() can change the state to PASSTHROUGH. + // So, to change the state, you need to return to i2sOutTask() as soon as possible. # else + // If it is compiled to not use I2S streams, change the mode directly. i2s_out_pulser_status = PASSTHROUGH; # endif I2S_OUT_PULSER_EXIT_CRITICAL(); @@ -627,6 +638,7 @@ int IRAM_ATTR i2s_out_set_stepping() { I2S_OUT_PULSER_EXIT_CRITICAL(); return 0; } + // Now, DMA completed. Fallthrough. } // Change I2S state from PASSTHROUGH to STEPPING @@ -772,7 +784,6 @@ int IRAM_ATTR i2s_out_init(i2s_out_init_t& init_param) { // // configure I2S data port interface. - i2s_out_reset_fifo(); //reset i2s I2S0.conf.tx_reset = 1; @@ -786,6 +797,8 @@ int IRAM_ATTR i2s_out_init(i2s_out_init_t& init_param) { I2S0.lc_conf.out_rst = 1; // Set this bit to reset out DMA FSM. (R/W) I2S0.lc_conf.out_rst = 0; + i2s_out_reset_fifo_without_lock(); + //Enable and configure DMA I2S0.lc_conf.check_owner = 0; I2S0.lc_conf.out_loop_test = 0; diff --git a/Grbl_Esp32/src/MotionControl.cpp b/Grbl_Esp32/src/MotionControl.cpp index 9b454be3..a3be9ce9 100644 --- a/Grbl_Esp32/src/MotionControl.cpp +++ b/Grbl_Esp32/src/MotionControl.cpp @@ -260,6 +260,25 @@ static bool axis_is_squared(uint8_t axis_mask) { return mask_is_single_axis(axis_mask) && mask_has_squared_axis(axis_mask); } +#ifdef USE_I2S_STEPS +# define BACKUP_STEPPER(save_stepper) \ + do { \ + if (save_stepper == ST_I2S_STREAM) { \ + stepper_switch(ST_I2S_STATIC); /* Change the stepper to reduce the delay for accurate probing. */ \ + } \ + } while (0) + +# define RESTORE_STEPPER(save_stepper) \ + do { \ + if (save_stepper == ST_I2S_STREAM && current_stepper != ST_I2S_STREAM) { \ + stepper_switch(ST_I2S_STREAM); /* Put the stepper back on. */ \ + } \ + } while (0) +#else +# define BACKUP_STEPPER(save_stepper) +# define RESTORE_STEPPER(save_stepper) +#endif + // Perform homing cycle to locate and set machine zero. Only '$H' executes this command. // NOTE: There should be no motions in the buffer and Grbl must be in an idle state before // executing the homing cycle. This prevents incorrect buffered plans after homing. @@ -371,6 +390,13 @@ GCUpdatePos mc_probe_cycle(float* target, plan_line_data_t* pl_data, uint8_t par if (sys.abort) { return GCUpdatePos::None; // Return if system reset has been issued. } + +#ifdef USE_I2S_STEPS + stepper_id_t save_stepper = current_stepper; /* remember the stepper */ +#endif + // Switch stepper mode to the I2S static (realtime mode) + BACKUP_STEPPER(save_stepper); + // Initialize probing control variables uint8_t is_probe_away = bit_istrue(parser_flags, GCParserProbeIsAway); uint8_t is_no_error = bit_istrue(parser_flags, GCParserProbeIsNoError); @@ -381,7 +407,8 @@ GCUpdatePos mc_probe_cycle(float* target, plan_line_data_t* pl_data, uint8_t par if (probe_get_state() ^ is_probe_away) { // Check probe pin state. sys_rt_exec_alarm = ExecAlarm::ProbeFailInitial; protocol_execute_realtime(); - return GCUpdatePos::None; // Nothing else to do but bail. + RESTORE_STEPPER(save_stepper); // Switch the stepper mode to the previous mode + return GCUpdatePos::None; // Nothing else to do but bail. } // Setup and queue probing motion. Auto cycle-start should not start the cycle. mc_line(target, pl_data); @@ -392,9 +419,14 @@ GCUpdatePos mc_probe_cycle(float* target, plan_line_data_t* pl_data, uint8_t par do { protocol_execute_realtime(); if (sys.abort) { - return GCUpdatePos::None; // Check for system abort + RESTORE_STEPPER(save_stepper); // Switch the stepper mode to the previous mode + return GCUpdatePos::None; // Check for system abort } } while (sys.state != State::Idle); + + // Switch the stepper mode to the previous mode + RESTORE_STEPPER(save_stepper); + // Probing cycle complete! // Set state variables and error out, if the probe failed and cycle with error is enabled. if (sys_probe_state == Probe::Active) { diff --git a/Grbl_Esp32/src/Stepper.cpp b/Grbl_Esp32/src/Stepper.cpp index d78a0d88..469a65a6 100644 --- a/Grbl_Esp32/src/Stepper.cpp +++ b/Grbl_Esp32/src/Stepper.cpp @@ -457,11 +457,10 @@ void stepper_switch(stepper_id_t new_stepper) { #ifdef USE_I2S_STEPS if (current_stepper == ST_I2S_STREAM) { if (i2s_out_get_pulser_status() != PASSTHROUGH) { - // This switching function should not be called during streaming. - // However, if it is called, it will stop streaming. - grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Warning, "Stop the I2S streaming and switch to the passthrough mode."); + // Called during streaming. Stop streaming. + grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Debug, "Stop the I2S streaming and switch to the passthrough mode."); i2s_out_set_passthrough(); - i2s_out_delay(); + i2s_out_delay(); // Wait for a change in mode. } } #endif diff --git a/debug.ini b/debug.ini index 0d148f7f..db39152f 100644 --- a/debug.ini +++ b/debug.ini @@ -6,14 +6,18 @@ upload_port = COM3 monitor_port = COM3 ; macOS -;upload_port = /dev/cu.usbserial-* -;monitor_port = /dev/cu.usbserial-* +;upload_port = /dev/cu.SLAB_USBtoUART +;monitor_port = /dev/cu.SLAB_USBtoUART +; macOS ESP-Prog +;upload_port = /dev/cu.usbserial-14200 +;monitor_port = /dev/cu.usbserial-14201 +;upload_protocol = esp-prog ; Linux ;upload_port = /dev/ttyUSB* ;monitor_port = /dev/ttyUSB* build_flags = ${common.build_flags} - -DMACHINE_FILENAME=test_drive.h + -DMACHINE_FILENAME=test_drive.h [env:debug] ; You can switch between debugging tools using debug_tool option diff --git a/platformio.ini b/platformio.ini index c7f14c90..11ad3c32 100644 --- a/platformio.ini +++ b/platformio.ini @@ -49,7 +49,7 @@ board_build.flash_mode = qio build_flags = ${common.build_flags} src_filter = +<*.h> +<*.s> +<*.S> +<*.cpp> +<*.c> +<*.ino> + - -<.git/> - - - - + -<.git/> - - - [env:release]