Cleaner Way?
Spilly
Posts: 26
I am a second semester electrical and computer engineering major. I am fairly new to programming, about six months, and I have had some success getting this code to work. However, I would like to know if there is a neater way to write it. This is taken out of a project that I am working on. The entire project is just under 500 lines of code so I am just posting the section in question.
Please ignore the variables not declared in the function. They are global variables.
This is written using the simpleIDE compiler.
Please ignore the variables not declared in the function. They are global variables.
This is written using the simpleIDE compiler.
int main(){
int time, start, started, previous;
while(1)
{
previous = start;
start = get_state(22);
//start timer on rising edge
if((start == 1) && (start != previous) && (started == 0))
{
mstime_stop();
mstime_reset();
mstime_start();
started = 1;
}
//get time elapsed between previous rising edge and current rising edge
else if((start == 1) && (start != previous) && (started == 1))
{
time = mstime_get();
started = 0;
}
else
{
//convert time for one tooth cycle to revs per minute
//tweleve teeth per rev
float calc = ((60*1000) / (12 * time));
int rpm = (int) calc;
holder = rpm;
//change integrator time constant based on rpm
if( (rpm >= 8203) && (a != 0))
{
dataOut= 0b11000000;
spi();
a = 0;
}
else if( (rpm >= 7292) && (rpm < 8203) && (a != 1))
{
dataOut= 0b11000001;
spi();
a = 1;
}
else if( (rpm >= 6563) && (rpm < 7292) && (a != 2))
{
dataOut= 0b11000010;
spi();
a = 2;
}
else if( (rpm >= 5966) && (rpm < 6563) && (a != 3))
{
dataOut= 0b11000011;
spi();
a = 3;
}
else if( (rpm >= 5469) && (rpm < 5966) && (a != 4))
{
dataOut= 0b11000100;
spi();
a = 4;
}
else if( (rpm >= 5049) && (rpm < 5469) && (a != 5))
{
dataOut= 0b11000101;
spi();
a = 5;
}
else if( (rpm >= 4687) && (rpm < 5049) && (a != 6))
{
dataOut= 0b11000110;
spi();
a = 6;
}
else if( (rpm >= 4375) && (rpm < 4687) && (a != 7))
{
dataOut= 0b11000111;
spi();
a = 7;
}
else if( (rpm >= 4102) && (rpm < 4375) && (a != 8))
{
dataOut= 0b11001000;
spi();
a = 8;
}
else if( (rpm >= 3646) && (rpm < 4102) && (a != 9))
{
dataOut= 0b11001001;
spi();
a = 9;
}
else if( (rpm >= 3285) && (rpm < 3646) && (a != 10))
{
dataOut= 0b11001010;
spi();
a = 10;
}
else if( (rpm >= 2983) && (rpm < 3285) && (a != 11))
{
dataOut= 0b11001011;
spi();
a = 11;
}
else if( (rpm >= 2735) && (rpm < 2983) && (a != 12))
{
dataOut= 0b11001100;
spi();
a = 12;
}
else if( (rpm >= 2524) && (rpm < 2735) && (a != 13))
{
dataOut= 0b11001101;
spi();
a = 13;
}
else if( (rpm >= 2344) && (rpm < 2524) && (a != 14))
{
dataOut= 0b11001110;
spi();
a = 14;
}
else if( (rpm >= 2344) && (rpm < 2344) && (a != 15))
{
dataOut= 0b11001111;
spi();
a = 15;
}
else if( (rpm >= 2051) && (rpm < 2188) && (a != 16))
{
dataOut= 0b11010000;
spi();
a = 16;
}
else if( (rpm >= 1823) && (rpm < 2051) && (a != 17))
{
dataOut= 0b11010001;
spi();
a = 17;
}
else if((rpm < 1823) && (a != 18))
{
dataOut= 0b11010001;
spi();
a = 18;
}
}
}
Comments
I take it you don't want to use the spi function unless the "a" variable is different this loop than the previous loop?
I think I'd add a local variable "newA" which gets set based on the rpm and this newA is then compared with a to see if there has been a change. IMO, this simplifies the code.
if (rpm >= 8203) { newA = 0; } else if (rpm >= 7292) { newA = 1; } else if(rpm >= 6563) { newA = 2; } // more else if else if (rpm >= 1823) { newA = 17; } else { newA = 18; } if (newA != a) { dataOut= 0b11000000 + newA; a = newA; spi(); }Does C not have "elseif"? (Apparently not.) My C is pretty rusty.
else if((rpm < 1823) && (a != 18)) { dataOut= 0b11010001; spi(); a = 18; }Suposed to be:
else if((rpm < 1823) && (a != 18)) { dataOut= 0b110100[COLOR=#ff0000]10[/COLOR]; spi(); a = 18; }If not, then the code I posted in post #2 will need to be changed.
Good catch on the bit pattern as well. I did not see that before.