1
\$\begingroup\$

I'm having this Arduino Sketch:

#define maxDigitalPin 7 //musn't be greater than 127

boolean state[] = { false, false, false, false, false, false };
unsigned long nextTime[] = { 0, 0, 0, 0, 0, 0 };

void setup(){
  Serial.begin(9600);

  for(int i = 2; i <= maxDigitalPin; i++){
    pinMode(i, INPUT_PULLUP);
  }
}

void loop(){
  for(int i = 2; i <= maxDigitalPin; i++){
    int arrayIndex = i - 2;
    if(digitalRead(i) == LOW && !state[arrayIndex] && millis() > nextTime[arrayIndex]){
      Serial.write(i ^ 128);
      state[arrayIndex] = true;
    }
    else if(state[arrayIndex] && digitalRead(i) == HIGH){
      Serial.write(i);
      state[arrayIndex] = false;
      nextTime[arrayIndex] = millis() + 500;
    }
  }
}

It will send a byte to a computer when a sensor goes on and when it goes back off.

The byte looks like:

SPPP PPPP

S is 1 when the sensor has gone on and 0 when it has gone off. P is the pin.

This code prevents sending the on message for a pin for 500ms after it has gone off. This is intended and really necessary.

My concerns:

  • Is it possible to reduce the code to only use one array?
  • I fear this sketch will be slow once it's using more than 7 pins because it's really important that the computer gets informed pretty instantly.

Of course if there is anything else to improve I would like to hear it too.

\$\endgroup\$
5
  • \$\begingroup\$ "Also this code prevents sending a byte about the same pin more than one time in 500ms." This is not true. If your pin is switching fast (ie: 5 ms) from LOW to HIGH you will send a byte for LOW (ok) and a byte for HIGH because on the else branch: -> state[arrayIndex] is true from previous loop -> digitalRead(i) == HIGH is also true \$\endgroup\$
    – Memleak
    Commented Sep 8, 2014 at 19:27
  • \$\begingroup\$ And I think preventing to send info about a pin for 500ms is a really bad idea. You can potentially keep printing only HIGH HIGH HIGH HIGH... and completely miss the LOW switches or vice-versa. Is this the intended behavior? \$\endgroup\$
    – Memleak
    Commented Sep 8, 2014 at 19:29
  • \$\begingroup\$ @Memleak the behavior you described in the first comment is intended. I corrected that in the question. I'm still not sure about you second comment. I don't believe it's possible to miss a byte \$\endgroup\$
    – idmean
    Commented Sep 8, 2014 at 19:55
  • \$\begingroup\$ I mean there is always a off byte following a on byte \$\endgroup\$
    – idmean
    Commented Sep 8, 2014 at 19:56
  • \$\begingroup\$ Yes, actually the way flag is used prevents printing multiple times. Although it is hard to read, it does its job. \$\endgroup\$
    – Memleak
    Commented Sep 8, 2014 at 20:07

1 Answer 1

1
\$\begingroup\$

If I understand correctly, your goal is to report all state changes; a 500 ms blackout is undesirable. If I am correct, read on.

You don't need the nextTime array at all. Change the state array from false/true to LOW/HIGH:

level = digitalRead(i);
if (level != state[arrayIndex]) {
    state[arrayIndex] = level;
    serialWrite(i | (level << 8));
}

(I am assuming that LOW == 0, and HIGH == 1).

Regarding performance: the code runs a busy loop, which is usually undesirable, especially in the embedded world. I seriously recommend to connect your pins to an interrupt, and rewrite the code into a reactive model.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your response. You didn't understand me correct, the 500ms blackout for each pin after it has gone off is intended. As to my information the arduino uno which I'm using has only two interrupts but I'm using over 10 sensors. Am I wrong? Is there some software solution probably? \$\endgroup\$
    – idmean
    Commented Sep 8, 2014 at 21:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.