josh
/
Teacup_Firmware
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

dda_clock(): modify dda->n only in atomic code 

dda_clock() might be interrupted by dda_step(), and dda_step might
use or modify variables also being used in dda_clock. In particular
dda->c is modified in both functions but it is done atomically in
dda_clock() to prevent dda_step() from interrupting during the
write.  But dda->n is also modified in both places and it is not
protected in dda_clock().

Move updates to dda->n to the atomic section along with dda->c.

Note by Traumflug: good catch! It even makes the binary 14 bytes
smaller, so likely faster.
This commit is contained in:
Phil Hord 2016-05-20 11:39:04 -04:00 committed by Markus Hitter
parent fe398d84ce
commit 1eefb28a19
1 changed files with 8 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
dda.c
View File

@ -760,6 +760,7 @@ void dda_clock() {
uint8_t endstop_trigger = 0;
#ifdef ACCELERATION_RAMPING
uint32_t move_step_no, move_c;
int32_t move_n;
uint8_t recalc_speed;
#endif
@ -874,29 +875,29 @@ void dda_clock() {
recalc_speed = 0;
if (move_step_no < dda->rampup_steps) {
#ifdef LOOKAHEAD
dda->n = dda->start_steps + move_step_no;
move_n = dda->start_steps + move_step_no;
#else
dda->n = move_step_no;
move_n = move_step_no;
#endif
recalc_speed = 1;
}
else if (move_step_no >= dda->rampdown_steps) {
#ifdef LOOKAHEAD
dda->n = dda->total_steps - move_step_no + dda->end_steps;
move_n = dda->total_steps - move_step_no + dda->end_steps;
#else
dda->n = dda->total_steps - move_step_no;
move_n = dda->total_steps - move_step_no;
#endif
recalc_speed = 1;
}
if (recalc_speed) {
if (dda->n == 0)
if (move_n == 0)
move_c = pgm_read_dword(&c0_P[dda->fast_axis]);
else
// Explicit formula: c0 * (sqrt(n + 1) - sqrt(n)),
// approximation here: c0 * (1 / (2 * sqrt(n))).
// This >> 13 looks odd, but is verified with the explicit formula.
move_c = (pgm_read_dword(&c0_P[dda->fast_axis]) *
int_inv_sqrt(dda->n)) >> 13;
int_inv_sqrt(move_n)) >> 13;
// TODO: most likely this whole check is obsolete. It was left as a
// safety margin, only. Rampup steps calculation should be accurate
@ -918,6 +919,7 @@ void dda_clock() {
// Write results.
ATOMIC_START
dda->c = move_c;
dda->n = move_n;
ATOMIC_END
}
#endif