Go Down

Topic: Need help writing more efficient code (total newbie) (Read 1 time) previous topic - next topic

odimachkie

after going through a bunch of tutorials, i decided to try and do something on my own.
apparently i haven't learned much...
i am trying to get effect without using such terrible code.
i am fully aware there are loops i should be using but i just cant get the same effect as just hard coding it this way.

http://www.youtube.com/watch?v=jaoIRQfFII4&feature=plcp

what kind of tricks can i use to get this more slick?
any input would be useful.
thanks

Quote


const int PWM3 = 3;
const int PWM5 = 5;
const int PWM6 = 6;
const int PWM9 = 9;
const int PWM10 = 10;
const int PWM11 = 11;
const int PWM13 = 13;


void setup() {
pinMode(PWM3, OUTPUT);
pinMode(PWM5, OUTPUT);
pinMode(PWM6, OUTPUT);
pinMode(PWM9, OUTPUT);
pinMode(PWM10, OUTPUT);
pinMode(PWM11, OUTPUT);
pinMode(PWM13, OUTPUT);

}

void loop() {

  analogWrite(PWM3,0*i);
  analogWrite(PWM5,2*i);
  analogWrite(PWM6,4+i);
  analogWrite(PWM9,6+i);
  analogWrite(PWM10,8+i);
  analogWrite(PWM11,10+i);
  analogWrite(PWM13,12+i);
   delay(100);
  analogWrite(PWM3,12+i);
  analogWrite(PWM5,0+i);
  analogWrite(PWM6,2+i);
  analogWrite(PWM9,4+i);
  analogWrite(PWM10,6+i);
  analogWrite(PWM11,8+i);
  analogWrite(PWM13,10+i);
   delay(100);
  analogWrite(PWM3,10+i);
  analogWrite(PWM5,12+i);
  analogWrite(PWM6,0+i);
  analogWrite(PWM9,2+i);
  analogWrite(PWM10,4+i);
  analogWrite(PWM11,6+i);
  analogWrite(PWM13,8+i);
   delay(100);
  analogWrite(PWM3,8+i);
  analogWrite(PWM5,10+i);
  analogWrite(PWM6,12+i);
  analogWrite(PWM9,0+i);
  analogWrite(PWM10,2+i);
  analogWrite(PWM11,4+i);
  analogWrite(PWM13,6+i);
   delay(100);
  analogWrite(PWM3,6+i);
  analogWrite(PWM5,8+i);
  analogWrite(PWM6,10+i);
  analogWrite(PWM9,12+i);
  analogWrite(PWM10,0+i);
  analogWrite(PWM11,2+i);
  analogWrite(PWM13,4+i);
   delay(100);
  analogWrite(PWM3,4+i);
  analogWrite(PWM5,6+i);
  analogWrite(PWM6,8+i);
  analogWrite(PWM9,10+i);
  analogWrite(PWM10,12+i);
  analogWrite(PWM11,0+i);
  analogWrite(PWM13,2+i);
   delay(100);
  analogWrite(PWM3,2+i);
  analogWrite(PWM5,4+i);
  analogWrite(PWM6,6+i);
  analogWrite(PWM9,8+i);
  analogWrite(PWM10,10+i);
  analogWrite(PWM11,12+i);
  analogWrite(PWM13,0+i);
   delay(100);



}





guix

#1
Oct 07, 2012, 08:12 am Last Edit: Oct 07, 2012, 08:16 am by guix Reason: 1
Hello and welcome :)

Where is i declared?

And, it may be ugly, yet it's most efficient this way, than using for loops.

You can still use #define instead of const int, to save little program memory :)

lloyddean

#2
Oct 07, 2012, 08:18 am Last Edit: Oct 07, 2012, 06:42 pm by lloyddean Reason: 1
Possible skeleton code -

Code: [Select]

// BUG CORRECTED

#define ARRAY_SIZEOF(ARRAY) (sizeof(ARRAY) / sizeof(ARRAY[0]))

const uint8_t   pinLED0     =  3;
const uint8_t   pinLED1     =  5;
const uint8_t   pinLED2     =  6;
const uint8_t   pinLED3     =  9;
const uint8_t   pinLED4     = 10;
const uint8_t   pinLED5     = 11;
const uint8_t   pinLED6     = 13;

const uint8_t   pinsLED[]   = { pinLED0, pinLED1, pinLED2, pinLED3, pinLED4, pinLED5, pinLED6 };

void loop()
{
   for ( int i = 0; i < ARRAY_SIZEOF(pinsLED); i++ )
   {
       int value = <some calculations to derive value>;

       analogWrite(pinsLED[i], value);
       delay(100);
   }
}


void setup()
{
   for ( int i = ARRAY_SIZEOF(pinsLED); i--; )
   {
       pinMode(pinsLED[i], OUTPUT);
   }
}

lloyddean


Hello and welcome :)

Where is i declared?

And, it may be ugly, yet it's most efficient this way, than using for loops.

You can still use #define instead of const int, to save little program memory :)


'const int' occupies no memory so it won't save a thing!

guix

'const int' occupies no memory so it won't save a thing!
Oh, well sorry, I'm kind of new to C++ :)

Nick Gammon

"const int" is the preferred way, for various subtle reasons.

@OP: It is preferable to post code that compiles:

Code: [Select]

sketch_oct07c.cpp: In function 'void loop()':
sketch_oct07c:22: error: 'i' was not declared in this scope
http://www.gammon.com.au/electronics

odimachkie

its not...
my mistake...
was trying to use a loop to make this work... but i abandoned it and came here...
the video was captured with all of the +i's taken out.

im really trying to follow this skeleton code, but I'm lost...are we dividing arrays in that first line...
sigh...i have a long way to go


guix

#7
Oct 07, 2012, 09:28 am Last Edit: Oct 07, 2012, 09:53 am by guix Reason: 1
The array is used to indicate the pin number, for each iteration of the loop:
Code: [Select]

//Example when loop's index (variable "i") is 0 (the first iteration)
analogWrite(pinsLED[i], value);

// is equal to:
analogWrite(pinsLED[0], value);

// which finally is equal to:
analogWrite(3, value);


EDIT: Oh, I didn't see the very first line...it's the sizeof preprocessor directive, that will be evaluated, and replaced at compile time with the size of the array, 7 in your case. But, still in your case, you can just use sizeof(pinsLED) it will be the same thing because each item in the array is only one byte big.

Graynomad

#8
Oct 07, 2012, 03:40 pm Last Edit: Oct 07, 2012, 03:44 pm by Graynomad Reason: 1
This is the standard way to arrive at the number of elements in an array

#define ARRAY_SIZEOF(ARRAY) (sizeof(pinsLED) / sizeof(pinsLED[0]))

sizeof(pinsLED)  => size of the array in bytes,
sizeof(pinsLED[0]))  => size of the first element in bytes

(sizeof(pinsLED) / sizeof(pinsLED[0])) => size of the array in bytes / size of the first element in bytes => number of elements in the array

The parameter (ARRAY) in this case serves no purpose that I can see.

Personally I would use a name like N_PINS

#define N_PINS (sizeof(pinsLED) / sizeof(pinsLED[0]))

Because the word sizeof has special meaning and if your elements were ints or longs the constant ARRAY_SIZEOF would IMO give the wrong impression about exactly what it refers to.

______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

PeterH


i am fully aware there are loops i should be using but i just cant get the same effect as just hard coding it this way.


When posting code, it's best to use [ code ] [ /code ] tags rather than quote tags.

There is a clear pattern to the sequence of values you are writing it out. You could reduce the amount of code enormously by putting your pin numbers in an array and then processing the array in a for loop to do the same thing to all pins.

The most general approach would be to write a function that outputs the next value to each pin in the array and call that repeatedly with an incrementing argument. But in this particular case the values you're writing out follow a very simple pattern and you could achieve the same effect with even simpler code similar to this (uncompiled):

Code: [Select]


void loop()
{
    analogWrite(pins[pinNumber], value);
    pinNumber++;
    if(pinNumber < pinCount)
    {
        value = (value + 2) % 14;
    }
    else
    {
        pinNumber = 0;
        delay(100);
    }
}


The idea is that each time through loop() you increment the pin number by one and increment the value by two, and write that value to that pin.
When you get to the end of the array you reset the pin number to 0 and do not increment the value (the value you write to the first pin in each sequence is the same value you wrote to the last pin in the previous sequence).
I only provide help via the forum - please do not contact me for private consultancy.

lloyddean


This is the standard way to arrive at the number of elements in an array

#define ARRAY_SIZEOF(ARRAY) (sizeof(pinsLED) / sizeof(pinsLED[0]))

sizeof(pinsLED)  => size of the array in bytes,
sizeof(pinsLED[0]))  => size of the first element in bytes

(sizeof(pinsLED) / sizeof(pinsLED[0])) => size of the array in bytes / size of the first element in bytes => number of elements in the array

The parameter (ARRAY) in this case serves no purpose that I can see.

Personally I would use a name like N_PINS

#define N_PINS (sizeof(pinsLED) / sizeof(pinsLED[0]))

Because the word sizeof has special meaning and if your elements were ints or longs the constant ARRAY_SIZEOF would IMO give the wrong impression about exactly what it refers to.

______
Rob



Sorry, was distracted before code was finished and post - fixed above.

Go Up