Newbie needs pointers.... :)

[RobertD]

Hello friends.

This is my first C program, and I have been plugging at this for a while. I had it running perfect in Basic, but C is more complicated and I'm making slow progress. I would like comments from the pros. This part of the program is to control an LED display, with last position in eeprom, to be reloaded upon startup.


I have problem with the counter tc going below 0 and over 16, despite having put limits on tc. I also have problem with the delay loop, which I should put in a subroutine because I need to reuse it for every position. I call each display updates separately because it was easier to debug. Are there obvious flaws, major mistakes to correct? I'm not looking for a total debug, just a few pointers to guide me in the right direction. I need to complete this project soon, so any help will be greatly rewarded with emoticons and future consideration...

Obviously not a pro job, but as long as it works in the end, I'll be happy.
There's plenty of time for me to learn to be more proficient in C, since this is my first one.

Thanks for your patience.

Robert

#include <EEPROM.h>
#include <ExtraOutputs.h>



const int buttonPin3 = 3;   
const int buttonPin4 = 4;     
const int ledPin11 =  11;      
const int ledPin10 =  10; 

//__________display connections_________________

const int latchPin = 17; 
const int clockPin =15; 
const int dataPin = 16; 
const int numberOfChips = 3; 
ExtraOutputs eo(latchPin, clockPin); 

int button3 = 0; 
int button4 = 0;
word tick1;
int tc;
int oldtc;
int i; 
int j;
int d;



void setup() {
  
  pinMode(ledPin11, OUTPUT);
  pinMode(ledPin10, OUTPUT);   
  pinMode(buttonPin3, INPUT); 
  pinMode(buttonPin4, INPUT); 
  Serial.begin(9600);
  
  //___________read eeprom for last tc position__________

  oldtc = EEPROM.read(10);
  tc = oldtc;
  
 
  Serial.println(tc, DEC);
  Serial.print("tc");

//_________update display__________
  
do 
        {
    if (d == 1)  eo.digitalWrite(8, HIGH);
    if (d == 2)  eo.digitalWrite(9, HIGH);
    if (d == 3) eo.digitalWrite(10, HIGH);
    if (d == 4) eo.digitalWrite(11, HIGH);
    if (d == 5) eo.digitalWrite(12, HIGH);
    if (d == 6) eo.digitalWrite(13, HIGH);
    if (d == 7) eo.digitalWrite(14, HIGH);
    if (d == 8) eo.digitalWrite(15, HIGH);
    if (d == 9)  eo.digitalWrite(0, HIGH);
    if (d == 10) eo.digitalWrite(1, HIGH);
    if (d == 11) eo.digitalWrite(2, HIGH);
    if (d == 12) eo.digitalWrite(3, HIGH);
    if (d == 13) eo.digitalWrite(4, HIGH);
    if (d == 14) eo.digitalWrite(5, HIGH);
    if (d == 15) eo.digitalWrite(6, HIGH);
    if (d == 16) eo.digitalWrite(7, HIGH);
    delay (50); 
    d++;
        }  while (d <= tc);  // here I have problem with integer and pointer conflict


 
   
}
 
void loop()  

  
  {

//  The eeprom works fine, but loops every time tc changes, I would prefer it only  
// updates after the delay. A pointer would be preferable here, to have   the
 //eeprom outside the loop combined with the delay routine.
    
       mem_write:
          {
           oldtc =  EEPROM.read(10);
             if (tc != oldtc)
             {
              EEPROM.write(10, tc);
              Serial.println("eeprom");
             }
              
           }
 
  //___________ read switches______________________
  
  button3 = digitalRead(buttonPin3);
  button4 = digitalRead(buttonPin4);

         
  
  //____________both switches off___________________

//I'm working on this delay so it's not ready yet.
      
           if (button3 == 0 && button4 == 0)
                     {
                        int x,y;
                       for(x = 0; x < 2000; x++)
                            {
                             for(y = 0; y < 2000000; y++)
                             {
                          }

                 
               

 //__________going up display____________________________
//  this works fine
  
  
   if (button3 == HIGH)

       {
         digitalWrite(ledPin11, HIGH); 
         tc = tc + 1;  
         Serial.println(tc, DEC);
          if (tc >= 16) tc == 16; 
     
    if (tc == 1)  eo.digitalWrite(8, HIGH);
    if (tc == 2)  eo.digitalWrite(9, HIGH);
    if (tc == 3) eo.digitalWrite(10, HIGH);
    if (tc == 4) eo.digitalWrite(11, HIGH);
    if (tc == 5) eo.digitalWrite(12, HIGH);
    if (tc == 6) eo.digitalWrite(13, HIGH);
    if (tc == 7) eo.digitalWrite(14, HIGH);
    if (tc == 8) eo.digitalWrite(15, HIGH);
    if (tc == 9)  eo.digitalWrite(0, HIGH);
    if (tc == 10) eo.digitalWrite(1, HIGH);
    if (tc == 11) eo.digitalWrite(2, HIGH);
    if (tc == 12) eo.digitalWrite(3, HIGH);
    if (tc == 13) eo.digitalWrite(4, HIGH);
    if (tc == 14) eo.digitalWrite(5, HIGH);
    if (tc == 15) eo.digitalWrite(6, HIGH);
    if (tc == 16) eo.digitalWrite(7, HIGH);
    delay(300);
    digitalWrite(ledPin11, LOW);    
      
          }
         
     
 //_________going down display__________________    
// this works fine too
   
   
   if (button4 == HIGH) 
          {
          digitalWrite(ledPin10, HIGH);  
          tc = tc - 1;
          Serial.println(tc, DEC);
          if (tc <= 0) tc == 0; 

    if (tc == 15)  eo.digitalWrite(7, LOW); 
    if (tc == 14)  eo.digitalWrite(6, LOW);
    if (tc == 13)  eo.digitalWrite(5, LOW);
    if (tc == 12)  eo.digitalWrite(4, LOW);
    if (tc == 11)  eo.digitalWrite(3, LOW);
    if (tc == 10)  eo.digitalWrite(2, LOW);
    if (tc == 9)   eo.digitalWrite(1, LOW);
    if (tc == 8)   eo.digitalWrite(0, LOW);
    if (tc == 7)  eo.digitalWrite(15, LOW);
    if (tc == 6)  eo.digitalWrite(14, LOW);
    if (tc == 5)  eo.digitalWrite(13, LOW);
    if (tc == 4)  eo.digitalWrite(12, LOW);
    if (tc == 3)  eo.digitalWrite(11, LOW);
    if (tc == 2)  eo.digitalWrite(10, LOW); 
    if (tc == 1)   eo.digitalWrite(9, LOW); 
    if (tc == 0)   eo.digitalWrite(8, LOW);
     delay(300);
     digitalWrite(ledPin10, LOW); 
     }
  }

[quzah]

do 
        {
    if (d == 1)  eo.digitalWrite(8, HIGH);
    if (d == 2)  eo.digitalWrite(9, HIGH);
    if (d == 3) eo.digitalWrite(10, HIGH);
    if (d == 4) eo.digitalWrite(11, HIGH);
    if (d == 5) eo.digitalWrite(12, HIGH);
    if (d == 6) eo.digitalWrite(13, HIGH);
    if (d == 7) eo.digitalWrite(14, HIGH);
    if (d == 8) eo.digitalWrite(15, HIGH);
    if (d == 9)  eo.digitalWrite(0, HIGH);
    if (d == 10) eo.digitalWrite(1, HIGH);
    if (d == 11) eo.digitalWrite(2, HIGH);
    if (d == 12) eo.digitalWrite(3, HIGH);
    if (d == 13) eo.digitalWrite(4, HIGH);
    if (d == 14) eo.digitalWrite(5, HIGH);
    if (d == 15) eo.digitalWrite(6, HIGH);
    if (d == 16) eo.digitalWrite(7, HIGH);
    delay (50); 
    d++;
        }  while (d <= tc);  // here I have problem with integer and pointer conflict

Behold, the power of cheese!

do
{
    eo.digitalWrite( (d + 7) % 16), HIGH );
    delay( 50 );
} while( ++d < tc );

edit - I think that looks right. You really screwed with my head starting at 1 and going through 16, instead of starting at 0. For some reason that jump from 15 to 0 being a difference of nine wrecked my mind for a bit.

What were you thinking going from 1-16 in one spot, and 0-15 in another?


Quzah.

[anduril462]

A few other things. In C, you always want to be explicit. If you don't intend for the funciton to take any parameters, say so:

void loop(void)

Without that second void, the compiler wont check to make sure you call the function correctly.

The following line worries me, considering you're coming from basic:

       mem_write:

That's a label in C, like it is in basic. But in C, you only use labels for goto statements, which are generally considered evil (read why here: Goto Considered Harmful). If you mean to make some comment as to the codes purpose, then use // or /* */ comments.

Noobs seem to love the idea of saving newlines, like they're on some endangered species list or something. Phooey! Don't try to make your successive if statements fully justified either. Put a new line in there and indent the subordinate lines of code:

if (tc <= 0)
    tc == 0;

Not that the compiler cares, but when you come asking others for help, sticking to a widely used indentation style and code format goes a long way to making sure people can read your code and want to help you. I suggest any of the top 3 or 4 methods here: Indent style - Wikipedia, the free encyclopedia.

And I agree very much with what Quzah said. Other than that, I think you're doing alright.

[RobertD]

Thank you friends for your encouragement, I am forever in your debt. I was just reading about arrays and pointers trying to figure out where I went wrong.
But you just made my day. All day today, I was threading water and getting nowhere and frustrated.

[anduril462]

You're very welcome. I accept cash, certified checks, money orders, all major credit cards and first-born sons as payment for my services .

[RobertD]

The check is in the mail...

My array starts at 1 going up and ends at 0 going down.
But you're right I can change that in the array.
BTW, the sequence in the array goes from 8 to 15 first, then 0 to 7.

mem_write is just a label I'll change it. As you can see, I do make beginner's mistakes.
I have learned that "goto" is a no no, use pointers instead.
I'll read the indentation rules, not just go willy nilly.

[anduril462]

Aww...and I was hoping you had a first-born son lying around. Oh well, child services would probably be after me for that anyhow.

I don't think pointers are what you want in place of a goto. Usually, you use conditional statements, like if/else or loops (for/while/do while) to control program flow instead of gotos. Sometimes, with a loop, you use the continue or break keywords to skip to the next iteration or quit the loop early. That covers 99% of the cases where you might otherwise use a goto.

[RobertD]

You can have my girlfriend's first born son, he's a pain in the butt.

[whiteflags]

I have learned that "goto" is a no no, use pointers instead.

The goto keyword is a no no, but pointers don't replace goto: it's a flow control statement, so some sort of a loop prevents spaghetti code.