Pages: 1 [2]   Go Down
Author Topic: Combining two sketches, help!  (Read 900 times)
0 Members and 1 Guest are viewing this topic.
Manchester (England England)
Offline Offline
Brattain Member
*****
Karma: 505
Posts: 31343
Solder is electric glue
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

UK
Offline Offline
Shannon Member
****
Karma: 183
Posts: 11138
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

I only provide help via the forum - please do not contact me for private consultancy.

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

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




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

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

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

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

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

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

The code snippet I posted should (maybe) be:
Code:
    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.
Logged

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

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:
 
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)
Logged

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

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

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

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

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

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

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

Logged

Pages: 1 [2]   Go Up
Jump to: