Long/Short Switch Closure

Working on a sketch that does one thing for short switch closure (less than 2 seconds) and another thing for long closure. I wrote this sketch to test my approach. It uses two LEDs to indicate long/short closure.

const int SW1 = 0;
const int LED1 = 1;
const int LED2 = 2;
unsigned long LongSW = 2000;     // any press longer than 2 sec = long
unsigned long SW1on = 0;            // time when SW1 pressed
unsigned long SW1off = 0;            // time when SW1 released
pinMode(LED1, OUTPUT);              // long press indication
pinMode(LED2, OUTPUT);              // short press indication

void setup() {
  pinMode(SW1, INPUT_PULLUP);
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  digitalWrite (LED1, HIGH);         //initialize LED1 off
  digitalWrite (LED2, HIGH);         //initialize LED2 off
}
void loop() {
  while (digitalRead(SW1) == HIGH) {}   // wait until SW1 closes
  SW1on = millis();                                // save time SW1 closed
  while (digitalRead(SW1) == LOW) {}    // wait until SW1 opens
  SW1off = millis();                                // save time SW1 opened
  if ((SW1off - SW1on) > LongSW) {       // long switch press ?
    digitalWrite(LED1, LOW);                   // yes, turn on LED1
    else digitalWrite(LED2, LOW);            // no, turn on LED2
  }                                     
  delay 1000;
  digitalWrite(LED1, HIGH);                   // turn off LED1
  digitalWrite(LED2, HIGH);                   // turn off LED2
}

I cannot get past a compile error "expected constructor, destructor, or type conversion before '(' token" on the very last line of code.

I also suspect that long/short detection is probably a very widely used feature and there are probably much simpler, better ways to do it.

Tommy Tyler

From what I can immediately see:

  1. pinMode() is being duplicated in the main body and setup(). Try leaving that in setup() only.

  2. Your while loops are empty! I'm sure you meant

while (digitalRead(SW1) == HIGH) {SW1on  = millis();}    // wait until SW1 closes
while (digitalRead(SW1) == LOW)  {SW1off = millis();}    // wait until SW1 opens
  1. Your else digitalWrite(LED2, LOW); is within the "if" logic, the correct way of using else is AFTER if (condition) {do this}.

  2. You forgot to put delay(1000) in brackets.

  3. I'm assuming you connect your LED directly to pins 1 & 2, in which case digitalWrite(LED1/2, HIGH) turns the LED on, not off. You may have accidentally mistaken HIGH for off. Also, if you're using a 5V board like the UNO, I recommend putting a resistor in series with your LED or you risk blowing it.

  4. You have pin0 declared as INPUT_PULLUP, and I assume the other end of the switch is connected to ground. So why do you think a closed switch will give you a HIGH reading?

The following code compiles properly, but I won't say its the best way to achieve what you intend to do. Currently your code runs freely regardless of any button press, which means if you press it while delay(1000) is running, your threshold of 2000ms between long/short press will not be adhered to. I recommend using an interrupt to tell the board when a button press has started instead of continuously polling and running the entire loop.

const int SW1 = 0;
const int LED1 = 1;
const int LED2 = 2;
unsigned long LongSW = 2000;     // any press longer than 2 sec = long
unsigned long SW1on = 0;            // time when SW1 pressed
unsigned long SW1off = 0;            // time when SW1 released

void setup() {
  pinMode(SW1, INPUT_PULLUP);
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  digitalWrite (LED1, HIGH);         //initialize LED1 off
  digitalWrite (LED2, HIGH);         //initialize LED2 off
}
void loop() {
  while (digitalRead(SW1) == HIGH) {SW1on = millis();}   // wait until SW1 closes
  while (digitalRead(SW1) == LOW) {SW1off = millis();}   // wait until SW1 opens
                                  // save time SW1 opened
  if ((SW1off - SW1on) > LongSW) {       // long switch press ?
    digitalWrite(LED1, LOW);                   // yes, turn on LED1
  } 
  else digitalWrite(LED2, LOW);            // no, turn on LED2                                    
  delay (1000);
  digitalWrite(LED1, HIGH);                   // turn off LED1
  digitalWrite(LED2, HIGH);                   // turn off LED2
}

Please post the entire error code

Wangmaster:

  1. You're right. I will leave pin mode declarations in setup() only. (Incidentally, the picky compile monitor did not seem to mind that duplication. I guess stuff that's redundant, unnecessary, or inefficient, but not serious enough to prevent compiling is not important.

  2. My while loops are empty because I completely (and stupidly) misunderstood the Arduino Reference on "while". I thought it would loop completely through the parenthesis AND curly brackets before doing nothing if the condition within parenthesis was not true, and I wanted to avoid reading millis() on every loop. You've showed me how to do that with just one line of code instead of two.

  3. and 4) Right again. I will fix those.

  4. I connect LED anodes to V+ through resistors so digitalWrite (LED1/2 LOW) turns them on. Thought that was the most common way.

  5. I know SW1 open is HIGH and closed is LOW, but I think I see why you ask. It's another of my clumsy uses of "while". For example, the first two lines in loop() are:
    while (digitalRead(SW1) == HIGH) { } That's my way of saying "do nothing as long as SW1 is open (HIGH)"
    SW1on = millis();

Following your advice in 2), this could be just one line:
while (digitalRead(SW1)==LOW) { SW1on = millis() }

On your final comment, the LED stuff isn't something I expect to use in my application. That's just a way to see if the long/short part of the sketch works. I'm using the smallest, cheapest Arduino ever made (a Trinket) and there's no serial com support for trouble shooting.

fnb111: The line I quoted in my first post is all the error code I got.

Thanks, guys. My sketch compiles now and I can get on with it. I would still like to know what that error message meant. You know, there must be a predetermined list of compile error messages. Why haven't one of you experts written a book explaining them all. That should be a best seller.