Pages: [1] 2   Go Down
Author Topic: Combining two sketches, help!  (Read 919 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 9
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
 
Code:
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:
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:
Code:
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!
Logged

California
Offline Offline
Faraday Member
**
Karma: 82
Posts: 3123
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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

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

Manchester (England England)
Online Online
Brattain Member
*****
Karma: 514
Posts: 31554
Solder is electric glue
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

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

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
Funnily enough that is what I used to get where I am now.

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.

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

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

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

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

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

California
Offline Offline
Faraday Member
**
Karma: 82
Posts: 3123
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

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

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


Logged

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

well like I said
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:
Code:
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.
Logged

California
Offline Offline
Faraday Member
**
Karma: 82
Posts: 3123
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
« Last Edit: December 07, 2012, 07:00:08 pm by Arrch » Logged

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

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.

Code:
void setup()
{
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);     
  { // <-- What is this for?
    buttonUpState = digitalRead(inPinUp);

Code:
    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.

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

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

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

Offline Offline
Edison Member
*
Karma: 116
Posts: 2205
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

A few approaches:

1) incorporate the blinking code into your other code, with a ticker (through millies): something like this
Code:
  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
  }

2) use an interrupt to blink the led - nothing in the main loop
3) or to use an interrupt to detect a broken light beam.
Logged

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

Code:
    blinking_millis -= DESIRED_TIME; //reset blinking millis
Minus?
Logged

Pages: [1] 2   Go Up
Jump to: