Combining two sketches, help!

Hi there first post so be gentle please,

I'm trying to combine two sketches, the first is supposed to count the number of times a photo interrupters beam is broken, and the other one is the standard blink program. Both have been tested and both work the way I want them to. When I combine them together and redefine the "setup" and "loop" codes for each program together and then upload them, neither program works.

Here is my count program:

const int inPinUp = 6;
const int inPinDown = 7;
int channel = 1;
int buttonUpState = 0;
int buttonDownState = 0;
int prevBtnUp = LOW;
int prevBtnDwn = LOW;
unsigned long lastBtnUp = 0;
unsigned long lastBtnDwn = 0;
int transInt = 50;

void setup()
{
  Serial.begin(9600);
  pinMode(inPinUp, INPUT);
  pinMode(inPinDown, INPUT);
  
}


void loop()
{
  
  buttonUpState = digitalRead(inPinUp);
  buttonDownState = digitalRead(inPinDown);
  
  if (buttonUpState == HIGH && prevBtnUp == LOW)
  {
    if (millis() - lastBtnUp > transInt)
    {
    channel++;

    lastBtnUp = millis();
    Serial.println(channel);
    }
  }
  prevBtnUp = buttonUpState;
  
  
}

here is the blink code:

/*
  Blink
  Turns on an LED on for one second, then off for one second, repeatedly.
 
  This example code is in the public domain.
 */
 
// Pin 13 has an LED connected on most Arduino boards.
// give it a name:
int led = 9;

// the setup routine runs once when you press reset:
void setup() {                
  // initialize the digital pin as an output.
  pinMode(led, OUTPUT);     
}

// the loop routine runs over and over again forever:
void loop() {
  digitalWrite(led, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(1000);               // wait for a second
  digitalWrite(led, LOW);    // turn the LED off by making the voltage LOW
  delay(1000);               // wait for a second
}

and here is my attempt at combining the two codes:

void setup (){
  setupCount();
  setupBlink();
}
void loop (){
  loopCount();
  loopBlink();
}

const int inPinUp = 6;
const int inPinDown = 7;
int channel = 1;
int buttonUpState = 0;
int buttonDownState = 0;
int prevBtnUp = LOW;
int prevBtnDwn = LOW;
unsigned long lastBtnUp = 0;
unsigned long lastBtnDwn = 0;
int transInt = 50;

void setupCount()
{
  Serial.begin(9600);
  pinMode(inPinUp, INPUT);
  pinMode(inPinDown, INPUT);
  
}


void loopCount()
{
  
  buttonUpState = digitalRead(inPinUp);
  buttonDownState = digitalRead(inPinDown);
  
  if (buttonUpState == HIGH && prevBtnUp == LOW)
  {
    if (millis() - lastBtnUp > transInt)
    {
    channel++;

    lastBtnUp = millis();
    Serial.println(channel);
    }
  }
  prevBtnUp = buttonUpState;
  
  
} 


/*
  Blink
  Turns on an LED on for one second, then off for one second, repeatedly.
 
  This example code is in the public domain.
 */
 
// Pin 13 has an LED connected on most Arduino boards.
// give it a name:
int led = 9;

// the setup routine runs once when you press reset:
void setupBlink() {  
  // initialize the digital pin as an output.
  pinMode(led, OUTPUT);     
}

// the loop routine runs over and over again forever:
void loopBlink() {
  digitalWrite(led, HIGH);   // turn the LED on (HIGH is the voltage level)
  delay(1000);               // wait for a second
  digitalWrite(led, LOW);    // turn the LED off by making the voltage LOW
  delay(1000);               // wait for a second
}

Any help would be greatly appreciated!

APieceOfToast:
neither program works.

The code does something. You're expecting it to do something else. Posting those two pieces of information is how you get help. Saying "it doesn't work" isn't how you get help.

My first thought, though is that you shouldn't be using delay() in your blink code and be using code from the Blink Without Delay example.

You are aware that loopCount() will only be called every two seconds, right?

Ditch those delays. Look at the blink with delay example.

Why do you have three setup() type functions? Put all that code in ONE function.

The combine code does something. What that is isn't clear.

Have a look at this, it tells you about some of the things you have to think about when combining code:-
http://www.thebox.myzen.co.uk/Tutorial/Merging_Code.html

Grumpy_Mike:
Have a look at this, it tells you about some of the things you have to think about when combining code:-
Merging Code

Funnily enough that is what I used to get where I am now.

PaulS:
You are aware that loopCount() will only be called every two seconds, right?

Ditch those delays. Look at the blink with delay example.

Why do you have three setup() type functions? Put all that code in ONE function.

The combine code does something. What that is isn't clear.

I will check out the blink without delay example, I din't realize that loopCount() would be called every two seconds.
As to why I have three setup() functions see the above link, I may have interpreted it incorrectly. The tutorial says to redefine setup() and loop() for each program and then define each one under a setup() function.

Arrch:

APieceOfToast:
neither program works.

The code does something. You're expecting it to do something else. Posting those two pieces of information is how you get help. Saying "it doesn't work" isn't how you get help.

My first thought, though is that you shouldn't be using delay() in your blink code and be using code from the Blink Without Delay example.

My bad, by "It doesn't work" I meant that the program does not perform either of the functions intended, as in it does not blink, nor does it count. This may be a result of the delays being in there, I will need to look at the blink without delay.
Your hunch appears to be a solid lead.

Thank you all for the quick replies.

As to why I have three setup() functions see the above link, I may have interpreted it incorrectly. The tutorial says to redefine setup() and loop() for each program and then define each one under a setup() function.

To test that they can play well together, that's fine. Actually merging the code shows more commitment.

So, comment out one of the loopXXXXX() calls in loop. Does that other one work as it did individually? If not, it's a hardware problem.

Ok, so I re-did the code using the blink without delay program. Unfortunately the program still does not count the times the beam is broken, or blink. I removed the multiple utterances of setup() and loop().

Here is what I have now:

const int inPinUp = 6;
const int inPinDown = 7;
int channel = 1;
int buttonUpState = 0;
int buttonDownState = 0;
int prevBtnUp = LOW;
int prevBtnDwn = LOW;
unsigned long lastBtnUp = 0;
unsigned long lastBtnDwn = 0;
int transInt = 50;
// constants won't change. Used here to 
// set pin numbers:
const int ledPin =  9;      // the number of the LED pin

// Variables will change:
int ledState = LOW;             // ledState used to set the LED
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 1000;           // interval at which to blink (milliseconds)

void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);      
}

void loop()
{
    buttonUpState = digitalRead(inPinUp);
  buttonDownState = digitalRead(inPinDown);
    if (buttonUpState == HIGH && prevBtnUp == LOW)
        if (millis() - lastBtnUp > transInt)
    {
    channel++;
  prevBtnUp = buttonUpState;
  // here is where you'd put code that needs to be running all the time.

  Serial.begin(9600);
  pinMode(inPinUp, INPUT);
  pinMode(inPinDown, INPUT);
  
}
  




    lastBtnUp = millis();
    Serial.println(channel);
    


  
  

  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  unsigned long currentMillis = millis();
 
  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

It compiles, but that doesn't mean I did ANYTHING right, and I'm waaaay outside of my comfort zone.

Edit: I have not checked the blink without delay code, but before I started combining both codes functioned exactly as I expected them to.

Why are you calling Serial.begin() and setting pinModes inside your loop function. In the Arduino IDE, press Ctrl+T to fix the atrocious formatting.

Why are you calling Serial.begin() and setting pinModes inside your loop function.

More importantly, why are you doing them only under such specific circumstances? It seems clear that you do not understand what either sketch was doing.

well like I said

APieceOfToast:
I'm waaaay outside of my comfort zone.

anyway I went in to fix the formatting and move my Serial.Begin() and pinModes outside of my loop function, and well suddenly the Arduino IDE wants me to empty my loop function as it refuses to compile and instead progressively tells me that everything from the bottom to top of my code is wrong and I have no idea what happened.

Here's where I'm at now:

const int inPinUp = 6;
const int inPinDown = 7;
int channel = 1;
int buttonUpState = 0;
int buttonDownState = 0;
int prevBtnUp = LOW;
int prevBtnDwn = LOW;
unsigned long lastBtnUp = 0;
unsigned long lastBtnDwn = 0;
int transInt = 50;
// constants won't change. Used here to 
// set pin numbers:
const int ledPin =  9;      // the number of the LED pin

// Variables will change:
int ledState = LOW;             // ledState used to set the LED
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 1000;           // interval at which to blink (milliseconds)

void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);      
  {
    buttonUpState = digitalRead(inPinUp);
    buttonDownState = digitalRead(inPinDown);
    if (buttonUpState == HIGH && prevBtnUp == LOW)
      if (millis() - lastBtnUp > transInt)
      {
        channel++;
        prevBtnUp = buttonUpState;
        lastBtnUp = millis();
        Serial.println(channel);
        // here is where you'd put code that needs to be running all the time.

        Serial.begin(9600);
        pinMode(inPinUp, INPUT);
        pinMode(inPinDown, INPUT);


      }

    void loop()

      // check to see if it's time to blink the LED; that is, if the 
      // difference between the current time and last time you blinked 
      // the LED is bigger than the interval at which you want to 
      // blink the LED.

      unsigned long currentMillis = millis();

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

I'm sorry if I cause any more brain damage through my abuse of the code.

Your loop function is inside your setup function. That's what the compiler doesn't like (among other things).

If you're THAT far outside of your comfort zone, I recommend going through a few more of the examples to understand the basics of the structure and syntax for the programming language. You seem to be blindly moving stuff around hoping that something magical happens and it compiles.

If you make a habit of putting each { on a new line, instead of on a line with other code, and of using Tools + Auto Format periodically, you'll see the extra braces you have, and the missing ones.

void setup()
{
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);      
  { // <-- What is this for?
    buttonUpState = digitalRead(inPinUp);
    if (buttonUpState == HIGH && prevBtnUp == LOW)
      if (millis() - lastBtnUp > transInt)
      {

Every if statement should be followed by { and }. They take very little effort to type, and ones that are not strictly needed cause the compiler no problems. They DO help make it clear what the if statement controls.

        Serial.println(channel);
        // here is where you'd put code that needs to be running all the time.

        Serial.begin(9600);

I want to print something, then I want to get ready to print. Can you not see that this is a problem?

You've been told, more than once, that Serial.begin() and pinMode() belong in setup(), not loop().

Why is loop() now embedded in setup?

Print your code. Take a pencil and play connect the dots with the { and }. Understand what each one introduces and what each one closes out.

Unfortunately I am that far outside of my comfort zone, I started playing with arduino's and coding a couple weeks ago and was barely comfortable blinking LED's on and off, and then my work asked me to work on a project involving arduino and coding. The result is the above code, and I suppose that to some extent that I am "blindly moving stuff around hoping that something magical happens and it compiles" however I do try to move things where I believe they should be according to my extremely limited experience.

Having said my bit I really appreciate the help I'm getting from this forum, I will certainly print my code and connect my brackets. I thought I had that covered but clearly not.

It may be a while before I am able to update my progress and I would greatly appreciate it if you two would keep an eye out for that.

A few approaches:

  1. incorporate the blinking code into your other code, with a ticker (through millies): something like this
  if ((millies() - blinking_millis) > DESIRED_TIME) { //test to see if desired time has passed
    blinking_millis -= DESIRED_TIME; //reset blinking millis
    blink_led(); //blink my led
  }
  1. use an interrupt to blink the led - nothing in the main loop
  2. or to use an interrupt to detect a broken light beam.
    blinking_millis -= DESIRED_TIME; //reset blinking millis

Minus?

PaulS:

    blinking_millis -= DESIRED_TIME; //reset blinking millis

Minus?

Paul have you not met our dear Henry before? He has been giving out hardware advice of the same quality for some months now. There should be a sticky for newcomers to ignore him.

Arrch:
My first thought, though is that you shouldn't be using delay() in your blink code and be using code from the Blink Without Delay example.

^^^ That.

Throw away the 'blink' code and replace it with 'blink without delay'. The code in 'blink without delay' uses a non-blocking technique which means that no part of your sketch needs to wait for something to happen. This technique enables you to combine many piece of functionality in a single sketch without them interfering with each other. Your inquiry is a classic illustration of why this approach is such a good idea. I'd go so far as to say the 'blink' example should be removed from the standard distribution, just because it encourages people to take an approach which is inherently not suitable for more complex projects.

ok, I went and redid the program to clean up the formatting and as many of the errors I could find. here's where it's at now:

//blink without delay
const int ledPin = 9;

int ledState = LOW;
long previousMillis = 0;

long interval = 1000;

// photogate
const int inPinUp = 6;
const int inPinDown = 7;
int channel = 1;
int buttonUpState = 0;
int buttonDownState = 0;
int prevBtnUp = LOW;
int prevBtnDwn = LOW;
unsigned long lastBtnUp = 0;
unsigned long lastBtnDwn = 0;
int transInt = 50;

void setup ()
{
  pinMode (ledPin, OUTPUT);
  Serial.begin(9600);
  pinMode(inPinUp, INPUT);
  pinMode(inPinDown, INPUT);
}

void loop ()
{
  buttonUpState = digitalRead(inPinUp); //begin photogate
  buttonDownState = digitalRead(inPinDown);

  {
    if (buttonUpState == HIGH && prevBtnUp == LOW)
      if (millis() - lastBtnUp > transInt)

        channel++;

    lastBtnUp = millis();
    Serial.println(channel);
  }

  prevBtnUp = buttonUpState;


//blink without delay
  unsigned long currentMillis = millis ();

  if(currentMillis - previousMillis > interval) {
    previousMillis = currentMillis;

    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    digitalWrite(ledPin, ledState);
  }
}

It compiles and uploads without a problem but when I open the serial monitor to monitor the count it's counting the number 1 repeatedly extremely quickly. And I have no idea why. Any thoughts or suggestions?

  {
    if (buttonUpState == HIGH && prevBtnUp == LOW)
      if (millis() - lastBtnUp > transInt)

        channel++;

    lastBtnUp = millis();
    Serial.println(channel);
  }

Curly braces where you don't need them. No curly braces where you should have them. Indenting doesn't mean squat to the compiler. Curly braces do. Use them. Even when you don't think you need to.

PaulS:

  {

if (buttonUpState == HIGH && prevBtnUp == LOW)
      if (millis() - lastBtnUp > transInt)

channel++;

lastBtnUp = millis();
    Serial.println(channel);
  }



Curly braces where you don't need them. No curly braces where you should have them. Indenting doesn't mean squat to the compiler. Curly braces do. Use them. Even when you don't think you need to.

PaulS would you please explain what you mean by that? when I add in the { and } before and after the if statements the compiler fails citing the statement "expected primary-expression before '}' token"
I must be misunderstanding what you mean by that.

In response to the use them even when I don't need to, that just causes more problems with compiling as the error message then reads "too many left curly brackets" despite having double checked to make sure I've closed all the open brackets.