Embedded conditional code compaction

I'm porting some AVR code from PROGMEM/PGM_P to __flash, and I want to reduce the amount of conditional compilation I need to do in the code.

Here's all the code (but keep in mind that only the parts in the conditionals should need changing):

```
#define F_CPU 12000000

#include
//#undef __FLASH
#ifndef __FLASH
#include
#define FLASH(x) const x PROGMEM
#else
#define FLASH(x) const __flash x
#endif
#include
#include

#define M_CPORT PORTC
#define M_CDIR DDRC
#define M_C1 PC7
#define M_C2 PC6
#define M_C3 PC5
#define M_C4 PC4
#define M_C5 PC3
#define numCols 5

#define M_RPORT PORTA
#define M_RDIR DDRA
#define M_R1 PA0
#define M_R2 PA1
#define M_R3 PA2
#define M_R4 PA3
#define M_R5 PA4
#define M_R6 PA5
#define M_R7 PA6
#define numRows 7

// bit values for each row
FLASH(unsigned char) rows[] = {_BV(M_R1), _BV(M_R2), _BV(M_R3), _BV(M_R4), _BV(M_R5),
_BV(M_R6), _BV(M_R7)};

// byte values for each row of each frame
FLASH(unsigned char) letterI[7] = {0x1f, 0x4, 0x4, 0x4, 0x4, 0x4, 0x1f};
...

#ifdef __FLASH
const __flash unsigned char const * const __flash sequence[] = {
#else
unsigned PGM_P const PROGMEM sequence[] = {
#endif
letterI, ...
};

// how long to sustain each row fow, based on the number of dots in the row
const unsigned int rowDur[6] = {0, 140, 160, 180, 200, 220};

unsigned char currFrame, currSlice;

unsigned char frameBuffer[7];

// optimized routine to count number of 1s in a frame row
unsigned char count5Bits(unsigned char v)
{
asm volatile("clr __tmp_reg__\n\t"
"ror %[val]\n\t"
"adc __tmp_reg__,__zero_reg__\n\t"
"ror %[val]\n\t"
"adc __tmp_reg__,__zero_reg__\n\t"
"ror %[val]\n\t"
"adc __tmp_reg__,__zero_reg__\n\t"
"ror %[val]\n\t"
"adc __tmp_reg__,__zero_reg__\n\t"
"ror %[val]\n\t"
"adc __tmp_reg__,__zero_reg__\n\t"
"mov %[val], __tmp_reg__"
: [val] "=&r

A few notes:

-
Your included libraries and definitions at the beginning of your code is not very organized.

#include 
//#undef __FLASH
#ifndef __FLASH
#include 
#define FLASH(x) const x PROGMEM
#define FLASH_P(x) const x * const PROGMEM
#define FLASH_PR(x, y) (x *)pgm_read_word(&(y))
#else
#define FLASH(x) const __flash x
#define FLASH_P(x) const __flash x * const __flash
#define FLASH_PR(x, y) (y)
#define pgm_read_byte(x) *(x)
#endif
#include 
#include 

I would sort it where all of the #includes are at the beginning, then a space, and then all of your preprocessor definitions. Also, I don't like to #include stuff in an #ifndef, just include it. Your compiler will make the proper optimizations if it's not used anyways.

#include 
#include 
#include 
#include 

#ifndef __FLASH
#define FLASH(x) const x PROGMEM
#define FLASH_P(x) const x * const PROGMEM
#define FLASH_PR(x, y) (x *)pgm_read_word(&(y))
#else
#define FLASH(x) const __flash x
#define FLASH_P(x) const __flash x * const __flash
#define FLASH_PR(x, y) (y)
#define pgm_read_byte(x) *(x)
#endif

-
You have a lot of preprocessor conditionals spread throughout your program.

#ifdef __FLASH
const __flash unsigned char const * const __flash sequence[] = {
#else
unsigned PGM_P const PROGMEM sequence[] = {
#endif

Another option to this is to have two separate files for the different implementations. This would mean having version-specific implementations of some classes, and switch entire implementations rather than just a few lines here and there. It would clean up your code of all these preprocessor conditionals.

-
You have some preprocessor definitions that aren't capitalized.

#define numRows 7

You should always capitalize all preprocessor definitions.

#define NUMROWS 7

-
You use some magic numbers here and there.

const unsigned int rowDur[6] = {0, 140, 160, 180, 200, 220};

You already defined NUMROWS, so you should use it here.

const unsigned int rowDur[NUMROWS - 1] = {0, 140, 160, 180, 200, 220};

-
Define variables in your for loops.(C99)

for (int r = numRows; r > -1; --r)

-
Use more comments. If another developer was to take a look at your code, they would have to use a lot of reason and logic to derive what you are trying to do in some places. For longer explanations, put your comments above the tricky statement.

M_CPORT |= ~(frameRow 

StackExchange Code Review Q#33883, answer score: 11