Pages: [1]   Go Down
Author Topic: Need help writing more efficient code (total newbie)  (Read 1339 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 6
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.



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);



}



Logged

France
Offline Offline
Edison Member
*
Karma: 38
Posts: 1012
Scientia potentia est.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hello and welcome smiley

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 smiley
« Last Edit: October 07, 2012, 01:16:36 am by guix » Logged

Des Moines, WA - USA
Offline Offline
God Member
*****
Karma: 25
Posts: 779
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Possible skeleton code -

Code:
// 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);
    }
}
« Last Edit: October 07, 2012, 11:42:31 am by lloyddean » Logged

Des Moines, WA - USA
Offline Offline
God Member
*****
Karma: 25
Posts: 779
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hello and welcome smiley

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 smiley

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

France
Offline Offline
Edison Member
*
Karma: 38
Posts: 1012
Scientia potentia est.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 495
Posts: 19015
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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

Code:
sketch_oct07c.cpp: In function 'void loop()':
sketch_oct07c:22: error: 'i' was not declared in this scope
Logged


Offline Offline
Newbie
*
Karma: 0
Posts: 6
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Logged

France
Offline Offline
Edison Member
*
Karma: 38
Posts: 1012
Scientia potentia est.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The array is used to indicate the pin number, for each iteration of the loop:
Code:
//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.
« Last Edit: October 07, 2012, 02:53:45 am by guix » Logged

nr Bundaberg, Australia
Offline Offline
Tesla Member
***
Karma: 129
Posts: 8583
Scattered showers my arse -- Noah, 2348BC.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
« Last Edit: October 07, 2012, 08:44:25 am by Graynomad » Logged

Rob Gray aka the GRAYnomad www.robgray.com

UK
Offline Offline
Shannon Member
****
Karma: 223
Posts: 12630
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:

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).
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Des Moines, WA - USA
Offline Offline
God Member
*****
Karma: 25
Posts: 779
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Pages: [1]   Go Up
Jump to: