Combining two sketches, help!

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.

The code snippet I posted should (maybe) be:

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

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

I'm not positive, because I don't know what you want to have happen when.

If that placement of { and } causes the compiler grief, it is because of something that went wrong earlier. You'll need to post modified code and exact error messages.

your correction made the program compile without errors, however now the counting is just not occurring according to the serial monitor. this might be a problem on the physical side since I'm in the process of moving the circuit into its "permanent" home.

To clarify what I want to happen when, I'm using the blink program to run a motor which in turn moves an arm up and down. I'm using the count program to count how many times the arm breaks the photo gates beam. So I need to blink program to run with the count program so that it is counting continuously while the motor runs.

Here is the current code:

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

(and yes I know indenting doesn't do anything for the compiler but it made it easier for me to read)

but it made it easier for me to read)

Using Tools + Auto Format before posting code would make it easier for ME to read.

Consistently placing the { (I prefer on a new line) makes it easier to read the code.

Using names like lastBtnUp and prevBtnUp in the same code REALLY makes it hard to follow. If a variable contains a state, the name should reflect that. If a variable contains a time, the name should reflect that.

I'm using the blink program to run a motor

A motor connected to the ledPin? Being turned on and off by the ledState variable? Why? You don't have a damned LED connected.

Anyway, that code does something. You want it to do something. Presumably, those two somethings are not the same thing, or you wouldn't still be looking for help. What either of those two somethings is, though, is not clear.

I did use tools > auto format as per your earlier request, and that is exactly what I posted and you read.

I'm using the blink program on the motor because with the setup I have, it allows me to tune the motor to the desired rpm. do you have a different suggestion? I would love a better solution as how its set up now is giving me tons of trouble (see previous experience - none)

Like I said though, I believe the problem is now on the physical side of things and I will continue on with testing until I get it.

Thank you very much I appreciate your help greatly and I will use the within advice for all my future posts.

I'm using the blink program on the motor because with the setup I have, it allows me to tune the motor to the desired rpm. do you have a different suggestion?

Yes. I suggest that you call the variable containing the pin the motor is connect to motorPin, not ledPin. I suggest that you call the variable containing the motor state motorState, not ledState.