Rename all these new PROGMEM variables to end in _P.

Should be done for temptable in ThermistorTable.h, too, but this
would mess up an existing users' configuration.

This tries to put emphasis on the fact that you have to read
these values with pgm_read_*() instead of just using the variable.
Unfortunately, gcc compiler neither inserts PROGMEM reading
instructions automatically when reading data stored in flash,
nor does it complain or warn about the missing read instructions.

As such it's very easy to accidently handle data stored in flash
just like normal data. It'll compile and work ... you just read
arbitrary data (often, but not always zeros) instead of what you
intend.
This commit is contained in:
Markus Hitter 2014-06-13 18:35:18 +02:00
parent 74808610c7
commit 9a08675576
8 changed files with 25 additions and 25 deletions

14
dda.c
View File

@ -52,18 +52,18 @@ TARGET BSS current_position;
/// \brief numbers for tracking the current state of movement /// \brief numbers for tracking the current state of movement
MOVE_STATE BSS move_state; MOVE_STATE BSS move_state;
/// \var steps_per_m /// \var steps_per_m_P
/// \brief motor steps required to advance one meter on each axis /// \brief motor steps required to advance one meter on each axis
static const axes_uint32_t PROGMEM steps_per_m = { static const axes_uint32_t PROGMEM steps_per_m_P = {
STEPS_PER_M_X, STEPS_PER_M_X,
STEPS_PER_M_Y, STEPS_PER_M_Y,
STEPS_PER_M_Z, STEPS_PER_M_Z,
STEPS_PER_M_E STEPS_PER_M_E
}; };
/// \var maximum_feedrate /// \var maximum_feedrate_P
/// \brief maximum allowed feedrate on each axis /// \brief maximum allowed feedrate on each axis
static const axes_uint32_t PROGMEM maximum_feedrate = { static const axes_uint32_t PROGMEM maximum_feedrate_P = {
MAXIMUM_FEEDRATE_X, MAXIMUM_FEEDRATE_X,
MAXIMUM_FEEDRATE_Y, MAXIMUM_FEEDRATE_Y,
MAXIMUM_FEEDRATE_Z, MAXIMUM_FEEDRATE_Z,
@ -202,7 +202,7 @@ void dda_create(DDA *dda, TARGET *target) {
if (i == X || dda->delta[i] > dda->total_steps) { if (i == X || dda->delta[i] > dda->total_steps) {
dda->total_steps = dda->delta[i]; dda->total_steps = dda->delta[i];
dda->fast_um = delta_um[i]; dda->fast_um = delta_um[i];
dda->fast_spm = pgm_read_dword(&steps_per_m[i]); dda->fast_spm = pgm_read_dword(&steps_per_m_P[i]);
} }
} }
@ -243,7 +243,7 @@ void dda_create(DDA *dda, TARGET *target) {
move_duration = distance * ((60 * F_CPU) / (target->F * 1000UL)); move_duration = distance * ((60 * F_CPU) / (target->F * 1000UL));
for (i = X; i < AXIS_COUNT; i++) { for (i = X; i < AXIS_COUNT; i++) {
md_candidate = dda->delta[i] * ((60 * F_CPU) / md_candidate = dda->delta[i] * ((60 * F_CPU) /
(pgm_read_dword(&maximum_feedrate[i]) * 1000UL)); (pgm_read_dword(&maximum_feedrate_P[i]) * 1000UL));
if (md_candidate > move_duration) if (md_candidate > move_duration)
move_duration = md_candidate; move_duration = md_candidate;
} }
@ -276,7 +276,7 @@ void dda_create(DDA *dda, TARGET *target) {
for (i = X; i < AXIS_COUNT; i++) { for (i = X; i < AXIS_COUNT; i++) {
c_limit_calc = ((delta_um[i] * 2400L) / c_limit_calc = ((delta_um[i] * 2400L) /
dda->total_steps * (F_CPU / 40000) / dda->total_steps * (F_CPU / 40000) /
pgm_read_dword(&maximum_feedrate[i])) << 8; pgm_read_dword(&maximum_feedrate_P[i])) << 8;
if (c_limit_calc > c_limit) if (c_limit_calc > c_limit)
c_limit = c_limit_calc; c_limit = c_limit_calc;
} }

View File

@ -136,11 +136,11 @@ int dda_jerk_size_2d(int32_t x1, int32_t y1, uint32_t F1, int32_t x2, int32_t y2
* we shut down the entire machine. * we shut down the entire machine.
* @param msg The reason why the machine did an emergency stop * @param msg The reason why the machine did an emergency stop
*/ */
void dda_emergency_shutdown(PGM_P msg) { void dda_emergency_shutdown(PGM_P msg_P) {
// Todo: is it smart to enable all interrupts again? e.g. can we create concurrent executions? // Todo: is it smart to enable all interrupts again? e.g. can we create concurrent executions?
sei(); // Enable interrupts to print the message sei(); // Enable interrupts to print the message
serial_writestr_P(PSTR("error: emergency stop:")); serial_writestr_P(PSTR("error: emergency stop:"));
if(msg!=NULL) serial_writestr_P(msg); if (msg_P != NULL) serial_writestr_P(msg_P);
serial_writestr_P(PSTR("\r\n")); serial_writestr_P(PSTR("\r\n"));
delay_ms(20); // Delay so the buffer can be flushed - otherwise the message is never sent delay_ms(20); // Delay so the buffer can be flushed - otherwise the message is never sent
timer_stop(); timer_stop();

View File

@ -14,14 +14,14 @@
These should be calculated at run-time once in dda_init() if the These should be calculated at run-time once in dda_init() if the
STEPS_PER_M_* constants are ever replaced with run-time options. STEPS_PER_M_* constants are ever replaced with run-time options.
*/ */
const axes_uint32_t PROGMEM axis_qn = { const axes_uint32_t PROGMEM axis_qn_P = {
(uint32_t)STEPS_PER_M_X / UM_PER_METER, (uint32_t)STEPS_PER_M_X / UM_PER_METER,
(uint32_t)STEPS_PER_M_Y / UM_PER_METER, (uint32_t)STEPS_PER_M_Y / UM_PER_METER,
(uint32_t)STEPS_PER_M_Z / UM_PER_METER, (uint32_t)STEPS_PER_M_Z / UM_PER_METER,
(uint32_t)STEPS_PER_M_E / UM_PER_METER (uint32_t)STEPS_PER_M_E / UM_PER_METER
}; };
const axes_uint32_t PROGMEM axis_qr = { const axes_uint32_t PROGMEM axis_qr_P = {
(uint32_t)STEPS_PER_M_X % UM_PER_METER, (uint32_t)STEPS_PER_M_X % UM_PER_METER,
(uint32_t)STEPS_PER_M_Y % UM_PER_METER, (uint32_t)STEPS_PER_M_Y % UM_PER_METER,
(uint32_t)STEPS_PER_M_Z % UM_PER_METER, (uint32_t)STEPS_PER_M_Z % UM_PER_METER,

View File

@ -25,13 +25,13 @@ inline int32_t muldiv(int32_t multiplicand, uint32_t multiplier,
#define UM_PER_METER (1000000UL) #define UM_PER_METER (1000000UL)
extern const axes_uint32_t PROGMEM axis_qn; extern const axes_uint32_t PROGMEM axis_qn_P;
extern const axes_uint32_t PROGMEM axis_qr; extern const axes_uint32_t PROGMEM axis_qr_P;
static int32_t um_to_steps(int32_t, enum axis_e) __attribute__ ((always_inline)); static int32_t um_to_steps(int32_t, enum axis_e) __attribute__ ((always_inline));
inline int32_t um_to_steps(int32_t distance, enum axis_e a) { inline int32_t um_to_steps(int32_t distance, enum axis_e a) {
return muldivQR(distance, pgm_read_dword(&axis_qn[a]), return muldivQR(distance, pgm_read_dword(&axis_qn_P[a]),
pgm_read_dword(&axis_qr[a]), UM_PER_METER); pgm_read_dword(&axis_qr_P[a]), UM_PER_METER);
} }
// approximate 2D distance // approximate 2D distance

View File

@ -245,19 +245,19 @@ void serial_writestr(uint8_t *data)
For single character writes (i.e. '\n' instead of "\n"), using For single character writes (i.e. '\n' instead of "\n"), using
serial_writechar() directly is the better choice. serial_writechar() directly is the better choice.
*/ */
void serial_writeblock_P(PGM_P data, int datalen) void serial_writeblock_P(PGM_P data_P, int datalen)
{ {
int i; int i;
for (i = 0; i < datalen; i++) for (i = 0; i < datalen; i++)
serial_writechar(pgm_read_byte(&data[i])); serial_writechar(pgm_read_byte(&data_P[i]));
} }
/// Write string from FLASH /// Write string from FLASH
void serial_writestr_P(PGM_P data) void serial_writestr_P(PGM_P data_P)
{ {
uint8_t r, i = 0; uint8_t r, i = 0;
// yes, this is *supposed* to be assignment rather than comparison, so we break when r is assigned zero // yes, this is *supposed* to be assignment rather than comparison, so we break when r is assigned zero
while ((r = pgm_read_byte(&data[i++]))) while ((r = pgm_read_byte(&data_P[i++])))
serial_writechar(r); serial_writechar(r);
} }

View File

@ -37,7 +37,7 @@ void serial_writeblock(void *data, int datalen);
void serial_writestr(uint8_t *data); void serial_writestr(uint8_t *data);
// write from flash // write from flash
void serial_writeblock_P(PGM_P data, int datalen); void serial_writeblock_P(PGM_P data_P, int datalen);
void serial_writestr_P(PGM_P data); void serial_writestr_P(PGM_P data_P);
#endif /* _SERIAL_H */ #endif /* _SERIAL_H */

View File

@ -103,13 +103,13 @@
#define GET_ARG(T) (va_arg(args, T)) #define GET_ARG(T) (va_arg(args, T))
#endif #endif
void sersendf_P(PGM_P format, ...) { void sersendf_P(PGM_P format_P, ...) {
va_list args; va_list args;
va_start(args, format); va_start(args, format_P);
uint16_t i = 0; uint16_t i = 0;
uint8_t c = 1, j = 0; uint8_t c = 1, j = 0;
while ((c = pgm_read_byte(&format[i++]))) { while ((c = pgm_read_byte(&format_P[i++]))) {
if (j) { if (j) {
switch(c) { switch(c) {
case 's': case 's':

View File

@ -7,6 +7,6 @@
#include "simulator.h" #include "simulator.h"
void sersendf(char *format, ...) __attribute__ ((format (printf, 1, 2))); void sersendf(char *format, ...) __attribute__ ((format (printf, 1, 2)));
void sersendf_P(PGM_P format, ...) __attribute__ ((format (printf, 1, 2))); void sersendf_P(PGM_P format_P, ...) __attribute__ ((format (printf, 1, 2)));
#endif /* _SERSENDF_H */ #endif /* _SERSENDF_H */