Pages: [1] 2   Go Down
Author Topic: compacting code  (Read 483 times)
0 Members and 1 Guest are viewing this topic.
Liberal, Kansas
Offline Offline
Jr. Member
**
Karma: 0
Posts: 59
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Not sure how to do this, I am not really good with coding at all. I can get things to work most of the time, But it almost always comes out sloppy, and really long winded. Here is my code that I have written, Not sure if it can be made in a more simple manner, Or if I am even on the right track. It does compile and runs exactly as I intended, Just that the code looks real inefficient. Any help would be appreciated. I am using a 74hc595 shift register for this.
 Code is in the next post.
« Last Edit: February 02, 2013, 03:38:34 am by daveydav27 » Logged

Liberal, Kansas
Offline Offline
Jr. Member
**
Karma: 0
Posts: 59
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
int latchPin = 8;             //595 pin 12
int clockPin = 12;          //595 pin 11
int dataPin = 11;           //595 pin 14
//595 pin 16 connected to 5VDC
//595 pin 8 connected to GND

void setup() {
pinMode(latchPin, OUTPUT);
pinMode(clockPin, OUTPUT);
pinMode(dataPin, OUTPUT);
}
void loop() {
  
  
  digitalWrite(latchPin, LOW);      //When the latchPin goes from low to high, the data gets moved from the shift registers to the output pins
  shiftOut(dataPin, clockPin, MSBFIRST, B10001000);     //Using the shiftOut function to send binary 11111111 to light all LEDs.  You can change from 1 to 0 to turn off.
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);      
  shiftOut(dataPin, clockPin, MSBFIRST, B11000100);
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);      
  shiftOut(dataPin, clockPin, MSBFIRST, B11100010);
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);      
  shiftOut(dataPin, clockPin, MSBFIRST, B11110001);
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B01111000);
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00110100);
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00010010);
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000001);
  digitalWrite(latchPin, HIGH);
  delay (1000);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  //
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  //
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
   digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  //
 
Logged

Liberal, Kansas
Offline Offline
Jr. Member
**
Karma: 0
Posts: 59
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
   digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B11110000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
  //
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00001111);
  digitalWrite(latchPin, HIGH);
  delay (50);
 
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, MSBFIRST, B00000000);
  digitalWrite(latchPin, HIGH);
  delay (50);
}
Logged

Liberal, Kansas
Offline Offline
Jr. Member
**
Karma: 0
Posts: 59
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Here is a video of the code working.

http://www.youtube.com/TGf034b4Wwo
« Last Edit: February 02, 2013, 03:39:19 am by daveydav27 » Logged

Finland
Offline Offline
Jr. Member
**
Karma: 1
Posts: 84
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Looks like you are repeating the same sequence and writing out a different byte separated by delays. You could put all those bytes into an array and increase the index to the array each time the loop is run. You can make it return back to index 0 in the end and that requires an if statement within the loop.
Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3464
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I can see what you mean about your code being sloppy but you can see what is going on.

Not only is it long it is repetitive, which gives a clue as to how it might be tidied up.
Basically you are doing this each time
Code:
  digitalWrite(latchPin, LOW);     
  shiftOut(dataPin, clockPin, MSBFIRST, B11000100);
  digitalWrite(latchPin, HIGH);
  delay (1000);
only the byte value changes

First stage tidy up.  Write a function to do the stuff that happens over and over again.
A function like this
Code:
void shiftData(byte value)
{
  digitalWrite(latchPin, LOW);     
  shiftOut(dataPin, clockPin, MSBFIRST, value);
  digitalWrite(latchPin, HIGH);
  delay (1000);
}

called like this
Code:
void loop()
{
  shiftData(B11000100);
  shiftData(B11100010);
  //etc etc
}
much less typing as you can see, but that is not the end.  Now you can see another pattern.  If you could hold the byte patterns in a list and say "show the list one entry after another" it would be even neater.

Well, you can.  Put the byte data in an array, read the array entry by entry and call the function.  I suggest you get the function working first and then move on to the array.
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

Liberal, Kansas
Offline Offline
Jr. Member
**
Karma: 0
Posts: 59
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thank you for the help, I will take a look at what you are purposing I do, and see if i can figure it out. Like I said, Programming is not my strong suit at all, But I can figure out, and wire most anything.
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 548
Posts: 46029
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Once you get UKHeliBob's suggestion working, then look into Chaul's suggestion. Put all the data in an array, and call the function in a for loop.
Logged

Malaysia
Offline Offline
Sr. Member
****
Karma: 7
Posts: 393
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

wow your program sure have alot of repetition, i tried the copy and paste way, the pattern stick out like a sore thumb so heres how to do it
Code:
int latchPin = 8;         
int clockPin = 12;       
int dataPin = 11;   

int Pattern1[8]=
{
  B10001000,
  B11000100,
  B11100010,
  B11110001,
  B01111000,
  B00110100,
  B00010010,
  B00000001,
};
int Pattern2[2]=
{
  B11110000,
  B00001111,
}
int Pattern3[2]=
{
  B11110000,
  B00000000,
};
int Pattern4[2]=
{
  B00001111;
  B00000000;
};
void setup()
{
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
}
void loop()
{
  for (int a=0;a<8;a++)
  {
    shiftData(Pattern1[a]);
  }
  for (int b=0;b<=7;b++)
  {
    for (int c=0;c<2;c++);
    {
      shiftData(Pattern2[c]);
    }
  }
  for ( int d=0;d<=3;d++)
  {
    for (int e=0;e<=3;e++)
    {
      for (int f=0;f<2;f++);
      {
        shiftData(Pattern3[f]);
      }
    }
    for (int g=0;g<=3;g++)
    {
      for (int h=0;h<2;h++);
      {
        shiftData(Pattern4[h]);
      }
    }
  }
}

void shiftData(byte value)
{
  digitalWrite(latchPin, LOW);     
  shiftOut(dataPin, clockPin, MSBFIRST, value);
  digitalWrite(latchPin, HIGH);
  delay (50);
}




Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3464
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Why so many for loops, particularly the nested ones  ?  Looks to me as though one array and one for loop would do, or have I missed something
Code:
int latchPin = 8;         
int clockPin = 12;       
int dataPin = 11;   

byte Pattern[]=
{
  B11000100,
  B11100010,
  B11110001,
  B01111000,
  B00110100,
  B00010010,
  B00000001
};  //etc etc

void setup()
{
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
}

void loop()
{
  for (int i = 0;i <= 7;i++)
  {
    shiftData(pattern[i]);
  }
}

void shiftData(byte value)
{
  digitalWrite(latchPin, LOW);     
  shiftOut(dataPin, clockPin, MSBFIRST, value);
  digitalWrite(latchPin, HIGH);
  delay (50);
}
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

Malaysia
Offline Offline
Sr. Member
****
Karma: 7
Posts: 393
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

actually UKHeliBob,
if you look closely at the op's posted code, he/she has 4 distinct pattern thus the many for loop
Logged

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

Code:
#define ENTRIES(ARRAY)  (sizeof(ARRAY) / sizeof(ARRAY[0]))

const uint8_t   pinLATCH    =  8;   // 595 pin 12
const uint8_t   pinCLOCK    = 12;   // 595 pin 11
const uint8_t   pinDATA     = 11;   // 595 pin 14

// 595 pin 16 connected to 5VDC
// 595 pin 8 connected to GND

struct pattern_delay_t
{
    uint8_t     _pattern;
    uint8_t     _delay;
};

pattern_delay_t data[] =
{
      { B10001000, 100 }
    , { B11000100, 100 }
    , { B11100010, 100 }
    , { B11110001, 100 }
    , { B01111000, 100 }
    , { B00110100, 100 }
    , { B00010010, 100 }
    , { B00000001, 100 }
    , { B11110000,   5 }
    , { B00001111,   5 }
    , { B11110000,   5 }
    , { B00001111,   5 }
    , { B11110000,   5 }
    , { B00001111,   5 }
    , { B11110000,   5 }
    , { B00001111,   5 }
    , { B11110000,   5 }
    , { B00001111,   5 }
    , { B11110000,   5 }
    , { B00001111,   5 }
    , { B11110000,   5 }
    , { B00001111,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B11110000,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
    , { B00001111,   5 }
    , { B00000000,   5 }
};

void loop()
{
    for ( int i = 0; i < ENTRIES(data); i++ )
    {
        digitalWrite(pinLATCH, LOW);

            shiftOut(pinDATA, pinCLOCK, MSBFIRST, data[i]._pattern);

        digitalWrite(pinLATCH, HIGH);

        delay((unsigned long)data[i]._delay * 10);
    }
}

void setup()
{
    pinMode(pinLATCH, OUTPUT);
    pinMode(pinCLOCK, OUTPUT);
    pinMode(pinDATA, OUTPUT);
}
« Last Edit: February 03, 2013, 12:36:03 am by lloyddean » Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3464
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I was going to suggest passing 2 parameters to the function.  I would still be in favour of using a function to output the data as to me it makes it more obvious what is going on (meaningful names, easy to find in code, reusable and all that) but reading around it seems that passing a struct to a function is quite complicated, which is a shame.
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 548
Posts: 46029
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
but reading around it seems that passing a struct to a function is quite complicated, which is a shame.
It isn't, if you define both the function and the function prototype. Passing a struct is not a good idea, though. Passing a reference to, or a pointer to, a struct is fine. The reason is that C (and C++) is a call by value language. When variables are used, the contents are copied for the function to use/manipulate/alter. Copying a structure can use a lot of memory.

Passing a pointer means that only the pointer is copied, which is a small/fast copy. With pass by reference, nothing is copied.
Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3464
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Thanks for the background Paul.  Passing a ponter makes sense.  I have never wanted/needed to pass a struct to a function but will do some more reading.
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

Pages: [1] 2   Go Up
Jump to: