diff --git a/docs/Config_Changes.md b/docs/Config_Changes.md index dc054b48..828094b0 100644 --- a/docs/Config_Changes.md +++ b/docs/Config_Changes.md @@ -8,6 +8,11 @@ All dates in this document are approximate. ## Changes +20211104: The "step pulse duration" option in "make menuconfig" has +been removed. A new `step_pulse_duration` setting in the +[stepper config section](Config_Reference.md#stepper) should be set +for all steppers that need a custom pulse duration. + 20211102: Several deprecated features have been removed. The stepper `step_distance` option has been removed (deprecated on 20201222). The `rpi_temperature` sensor alias has been removed (deprecated on diff --git a/docs/Config_Reference.md b/docs/Config_Reference.md index 880a419b..2539def7 100644 --- a/docs/Config_Reference.md +++ b/docs/Config_Reference.md @@ -153,6 +153,11 @@ microsteps: # gear_ratio is specified then rotation_distance specifies the # distance the axis travels for one full rotation of the final gear. # The default is to not use a gear ratio. +#step_pulse_duration: +# The minimum time between the step pulse signal edge and the +# following "unstep" signal edge. This is also used to set the +# minimum time between a step pulse and a direction change signal. +# The default is 0.000002 (which is 2us). endstop_pin: # Endstop switch detection pin. If this endstop pin is on a # different mcu than the stepper motor then it enables "multi-mcu diff --git a/klippy/stepper.py b/klippy/stepper.py index 013d667b..c5f81382 100644 --- a/klippy/stepper.py +++ b/klippy/stepper.py @@ -17,9 +17,10 @@ class error(Exception): # Interface to low-level mcu and chelper code class MCU_stepper: def __init__(self, name, step_pin_params, dir_pin_params, step_dist, - units_in_radians=False): + step_pulse_duration=None, units_in_radians=False): self._name = name self._step_dist = step_dist + self._step_pulse_duration = step_pulse_duration self._units_in_radians = units_in_radians self._mcu = step_pin_params['chip'] self._oid = oid = self._mcu.create_oid() @@ -58,9 +59,13 @@ class MCU_stepper: sk = ffi_main.gc(getattr(ffi_lib, alloc_func)(*params), ffi_lib.free) self.set_stepper_kinematics(sk) def _build_config(self): + if self._step_pulse_duration is None: + self._step_pulse_duration = .000002 + step_pulse_ticks = self._mcu.seconds_to_clock(self._step_pulse_duration) self._mcu.add_config_cmd( "config_stepper oid=%d step_pin=%s dir_pin=%s invert_step=%d" - % (self._oid, self._step_pin, self._dir_pin, self._invert_step)) + " step_pulse_ticks=%u" % (self._oid, self._step_pin, self._dir_pin, + self._invert_step, step_pulse_ticks)) self._mcu.add_config_cmd("reset_step_clock oid=%d clock=0" % (self._oid,), on_restart=True) step_cmd_tag = self._mcu.lookup_command_tag( @@ -73,9 +78,9 @@ class MCU_stepper: "stepper_get_position oid=%c", "stepper_position oid=%c pos=%i", oid=self._oid) max_error = self._mcu.get_max_stepper_error() + max_error_ticks = self._mcu.seconds_to_clock(max_error) ffi_main, ffi_lib = chelper.get_ffi() - ffi_lib.stepcompress_fill(self._stepqueue, - self._mcu.seconds_to_clock(max_error), + ffi_lib.stepcompress_fill(self._stepqueue, max_error_ticks, self._invert_dir, step_cmd_tag, dir_cmd_tag) def get_oid(self): return self._oid @@ -201,8 +206,10 @@ def PrinterStepper(config, units_in_radians=False): dir_pin = config.get('dir_pin') dir_pin_params = ppins.lookup_pin(dir_pin, can_invert=True) step_dist = parse_step_distance(config, units_in_radians, True) + step_pulse_duration = config.getfloat('step_pulse_duration', None, + minval=0., maxval=.001) mcu_stepper = MCU_stepper(name, step_pin_params, dir_pin_params, step_dist, - units_in_radians) + step_pulse_duration, units_in_radians) # Register with helper modules for mname in ['stepper_enable', 'force_move', 'motion_report']: m = printer.load_object(config, mname) diff --git a/src/Kconfig b/src/Kconfig index f1081907..43ab2bc6 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -73,31 +73,6 @@ config USB_SERIAL_NUMBER string "USB serial number" if !USB_SERIAL_NUMBER_CHIPID endmenu -# Step timing customization -config CUSTOM_STEP_DELAY - bool "Specify a custom step pulse duration" - depends on LOW_LEVEL_OPTIONS -config STEP_DELAY - int - default 2 -config STEP_DELAY - int "Step pulse duration (in microseconds)" - depends on CUSTOM_STEP_DELAY - help - Specify the duration of the stepper step pulse time. This - setting applies to all stepper drivers controlled by the - micro-controller. If this value is set to zero (or less) then - the code will "step" and "unstep" in the same C function. - - A setting of zero (or less) on 8-bit AVR micro-controllers - results in a minimum step pulse time a little over 2us. - - A setting of zero on ARM micro-controllers typically results - in a minimum step pulse time of 20 cpu cycles. - - The default for AVR is -1, for all other micro-controllers it - is 2us. - config INITIAL_PINS string "GPIO pins to set at micro-controller startup" depends on LOW_LEVEL_OPTIONS diff --git a/src/avr/Kconfig b/src/avr/Kconfig index 260e5a84..fd24852d 100644 --- a/src/avr/Kconfig +++ b/src/avr/Kconfig @@ -13,9 +13,6 @@ config AVR_SELECT select HAVE_GPIO_BITBANGING if !MACH_atmega168 select HAVE_STRICT_TIMING -config STEP_DELAY - default -1 - config BOARD_DIRECTORY string default "avr" diff --git a/src/stepper.c b/src/stepper.c index e4a12693..5d69d9df 100644 --- a/src/stepper.c +++ b/src/stepper.c @@ -14,12 +14,11 @@ #include "stepper.h" // stepper_event #include "trsync.h" // trsync_add_signal -DECL_CONSTANT("STEP_DELAY", CONFIG_STEP_DELAY); - - -/**************************************************************** - * Steppers - ****************************************************************/ +#if CONFIG_INLINE_STEPPER_HACK && CONFIG_MACH_AVR + #define HAVE_AVR_OPTIMIZATION 1 +#else + #define HAVE_AVR_OPTIMIZATION 0 +#endif struct stepper_move { struct move_node node; @@ -35,13 +34,8 @@ struct stepper { struct timer time; uint32_t interval; int16_t add; -#if CONFIG_STEP_DELAY <= 0 - uint_fast16_t count; -#define next_step_time time.waketime -#else uint32_t count; - uint32_t next_step_time; -#endif + uint32_t next_step_time, step_pulse_ticks; struct gpio_out step_pin, dir_pin; uint32_t position; struct move_queue_head mq; @@ -53,8 +47,8 @@ struct stepper { enum { POSITION_BIAS=0x40000000 }; enum { - SF_LAST_DIR=1<<0, SF_NEXT_DIR=1<<1, SF_INVERT_STEP=1<<2, SF_HAVE_ADD=1<<3, - SF_NEED_RESET=1<<4 + SF_LAST_DIR=1<<0, SF_NEXT_DIR=1<<1, SF_INVERT_STEP=1<<2, SF_NEED_RESET=1<<3, + SF_SINGLE_SCHED=1<<4, SF_HAVE_ADD=1<<5 }; // Setup a stepper for the next move in its queue @@ -70,18 +64,17 @@ stepper_load_next(struct stepper *s, uint32_t min_next_time) // Load next 'struct stepper_move' into 'struct stepper' struct move_node *mn = move_queue_pop(&s->mq); struct stepper_move *m = container_of(mn, struct stepper_move, node); - s->next_step_time += m->interval; s->add = m->add; s->interval = m->interval + m->add; - if (CONFIG_STEP_DELAY <= 0) { - if (CONFIG_MACH_AVR) - // On AVR see if the add can be optimized away - s->flags = m->add ? s->flags|SF_HAVE_ADD : s->flags & ~SF_HAVE_ADD; + if (HAVE_AVR_OPTIMIZATION && s->flags & SF_SINGLE_SCHED) { + s->time.waketime += m->interval; + s->flags = m->add ? s->flags | SF_HAVE_ADD : s->flags & ~SF_HAVE_ADD; s->count = m->count; } else { // On faster mcus, it is necessary to schedule unstep events // and so there are twice as many events. Also check that the // next step event isn't too close to the last unstep. + s->next_step_time += m->interval; if (unlikely(timer_is_before(s->next_step_time, min_next_time))) { if ((int32_t)(s->next_step_time - min_next_time) < (int32_t)(-timer_from_us(1000))) @@ -104,14 +97,17 @@ stepper_load_next(struct stepper *s, uint32_t min_next_time) return SF_RESCHEDULE; } +#define AVR_STEP_INSNS 40 // minimum instructions between step gpio pulses + // AVR optimized step function static uint_fast8_t -stepper_event_avr(struct stepper *s) +stepper_event_avr(struct timer *t) { + struct stepper *s = container_of(t, struct stepper, time); gpio_out_toggle_noirq(s->step_pin); - uint_fast16_t count = s->count - 1; + uint16_t *pcount = (void*)&s->count, count = *pcount - 1; if (likely(count)) { - s->count = count; + *pcount = count; s->time.waketime += s->interval; gpio_out_toggle_noirq(s->step_pin); if (s->flags & SF_HAVE_ADD) @@ -123,42 +119,14 @@ stepper_event_avr(struct stepper *s) return ret; } -// Optimized step function for stepping and unstepping in same function -static uint_fast8_t -stepper_event_nodelay(struct stepper *s) -{ - gpio_out_toggle_noirq(s->step_pin); - uint_fast16_t count = s->count - 1; - if (likely(count)) { - s->count = count; - s->time.waketime += s->interval; - s->interval += s->add; - gpio_out_toggle_noirq(s->step_pin); - return SF_RESCHEDULE; - } - uint_fast8_t ret = stepper_load_next(s, 0); - gpio_out_toggle_noirq(s->step_pin); - return ret; -} - -// Timer callback - step the given stepper. +// Regular "double scheduled" step function uint_fast8_t -stepper_event(struct timer *t) +stepper_event_full(struct timer *t) { struct stepper *s = container_of(t, struct stepper, time); - if (CONFIG_STEP_DELAY <= 0 && CONFIG_MACH_AVR) - return stepper_event_avr(s); - if (CONFIG_STEP_DELAY <= 0) - return stepper_event_nodelay(s); - - // Normal step code - schedule the unstep event - if (!CONFIG_HAVE_STRICT_TIMING) - gpio_out_toggle_noirq(s->step_pin); - uint32_t step_delay = timer_from_us(CONFIG_STEP_DELAY); - uint32_t min_next_time = timer_read_time() + step_delay; - if (CONFIG_HAVE_STRICT_TIMING) - // Toggling gpio after reading the time is a micro-optimization - gpio_out_toggle_noirq(s->step_pin); + gpio_out_toggle_noirq(s->step_pin); + uint32_t curtime = timer_read_time(); + uint32_t min_next_time = curtime + s->step_pulse_ticks; s->count--; if (likely(s->count & 1)) // Schedule unstep event @@ -178,20 +146,36 @@ reschedule_min: return SF_RESCHEDULE; } +// Optimized entry point for step function (may be inlined into sched.c code) +uint_fast8_t +stepper_event(struct timer *t) +{ + if (HAVE_AVR_OPTIMIZATION) + return stepper_event_avr(t); + return stepper_event_full(t); +} + void command_config_stepper(uint32_t *args) { struct stepper *s = oid_alloc(args[0], command_config_stepper, sizeof(*s)); - if (!CONFIG_INLINE_STEPPER_HACK) - s->time.func = stepper_event; s->flags = args[3] ? SF_INVERT_STEP : 0; s->step_pin = gpio_out_setup(args[1], s->flags & SF_INVERT_STEP); s->dir_pin = gpio_out_setup(args[2], 0); s->position = -POSITION_BIAS; + s->step_pulse_ticks = args[4]; move_queue_setup(&s->mq, sizeof(struct stepper_move)); + if (HAVE_AVR_OPTIMIZATION) { + if (s->step_pulse_ticks <= AVR_STEP_INSNS) + s->flags |= SF_SINGLE_SCHED; + else + s->time.func = stepper_event_full; + } else if (!CONFIG_INLINE_STEPPER_HACK) { + s->time.func = stepper_event_full; + } } -DECL_COMMAND(command_config_stepper, - "config_stepper oid=%c step_pin=%c dir_pin=%c invert_step=%c"); +DECL_COMMAND(command_config_stepper, "config_stepper oid=%c step_pin=%c" + " dir_pin=%c invert_step=%c step_pulse_ticks=%u"); // Return the 'struct stepper' for a given stepper oid static struct stepper * @@ -256,7 +240,7 @@ command_reset_step_clock(uint32_t *args) irq_disable(); if (s->count) shutdown("Can't reset time when stepper active"); - s->next_step_time = waketime; + s->next_step_time = s->time.waketime = waketime; s->flags &= ~SF_NEED_RESET; irq_enable(); } @@ -268,7 +252,7 @@ stepper_get_position(struct stepper *s) { uint32_t position = s->position; // If stepper is mid-move, subtract out steps not yet taken - if (CONFIG_STEP_DELAY <= 0) + if (HAVE_AVR_OPTIMIZATION && s->flags & SF_SINGLE_SCHED) position -= s->count; else position -= s->count / 2; @@ -297,10 +281,10 @@ stepper_stop(struct trsync_signal *tss, uint8_t reason) { struct stepper *s = container_of(tss, struct stepper, stop_signal); sched_del_timer(&s->time); - s->next_step_time = 0; + s->next_step_time = s->time.waketime = 0; s->position = -stepper_get_position(s); s->count = 0; - s->flags = (s->flags & SF_INVERT_STEP) | SF_NEED_RESET; + s->flags = (s->flags & (SF_INVERT_STEP|SF_SINGLE_SCHED)) | SF_NEED_RESET; gpio_out_write(s->dir_pin, 0); gpio_out_write(s->step_pin, s->flags & SF_INVERT_STEP); while (!move_queue_empty(&s->mq)) {