From 90d519af5fa8c134d8a6793f6452e2b3deff412a Mon Sep 17 00:00:00 2001 From: Phil Hord Date: Fri, 20 May 2016 11:41:07 -0400 Subject: [PATCH] dda_clock(): guard against *dda changes in interrupts dda_clock() might be interrupted by dda_step(), and dda_step might use or modify variables also being used in dda_clock(). It is possible for dda to be modified when a new dda becomes live during our dda_clock(). Check the dda->id to ensure it has not changed on us before we actually write new calculated values into the dda. Note by Traumflug: copied some of the explanation in the commit message directly into the code. --- dda.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/dda.c b/dda.c index b864b41..572078d 100644 --- a/dda.c +++ b/dda.c @@ -763,6 +763,7 @@ void dda_clock() { int32_t move_n; uint8_t recalc_speed; #endif + uint8_t current_id ; dda = queue_current_movement(); if (dda != last_dda) { @@ -867,6 +868,7 @@ void dda_clock() { // http://www.embedded.com/design/mcus-processors-and-socs/4006438/Generate-stepper-motor-speed-profiles-in-real-time // and http://www.atmel.com/images/doc8017.pdf (Atmel app note AVR446) ATOMIC_START + current_id = dda->id; move_step_no = move_state.step_no; // All other variables are read-only or unused in dda_step(), // so no need for atomic operations. @@ -918,8 +920,18 @@ void dda_clock() { // Write results. ATOMIC_START - dda->c = move_c; - dda->n = move_n; + /** + Apply new n & c values only if dda didn't change underneath us. It + is possible for dda to be modified since fetching values in the + ATOMIC above, e.g. when a new dda becomes live. + + In case such a change happened, values in the new dda are more + recent than our calculation here, anyways. + */ + if (current_id == dda->id) { + dda->c = move_c; + dda->n = move_n; + } ATOMIC_END } #endif