Turn LEDs on/off via contact reed switches - Issues

I have a project where there are 6 drawers that each have a magnetic reed switch in them (normally closed). Each drawer is linked to an LED light that I want to turn on when it opens, while all the others then turn off, irespective of whether other drawers are already open.

Details

When all drawers are shut, all 6 lights are on, highlighting 6 messages above the drawers.

When a drawer opens, the LED that drawer is linked to stays on, but the others go off. It also sends a digital out to an external media player.

If that drawer gets pushed back in, all LEDs come back on again.

If another drawer is opened but the other drawer is still open, the new one becomes the drawer "of focus", so that light is on, and all other lights go off (including the drawer that was previously opened).

Problem

I have if statements set up on whether the contact is open or not, so if it is open, then the LED stays on but the others go off. However when I'm opening another drawer, the old drawer is still being triggered by the if statement, so it stays on too.

Is there another way I can set up my code so that whatever the latest drawer to be opened is, that LED stays on and all others go off?

I've put my code below, which is currently just working on 3 while I'm testing. I think it is likely the case I need to start from scratch, as it isn't going to work with if statements.

Please let me know if you can give me some pointers. Thanks in advance.

int relay1 = 5;
int relay2 = 6;
int relay3 = 7;


int LED1 = 9;
int LED2 = 14;
int LED3 = 15;


int relayState1;
int relayState2;
int relayState3;



boolean gpio1Sent;
boolean gpio2Sent;
boolean gpio3Sent;

int lastOpened = 0;
int relayCheck = 0;
void setup(){
  
Serial.begin(9600);





gpio1Sent = false;


pinMode(LED1, OUTPUT);
pinMode(LED2, OUTPUT);
pinMode(LED3, OUTPUT);

pinMode(relay1, INPUT_PULLUP);
pinMode(relay2, INPUT_PULLUP);
pinMode(relay3, INPUT_PULLUP);

//digitalWrite(LED1, HIGH);
//digitalWrite(LED2, HIGH);
//digitalWrite(LED3, HIGH);


}

void loop(){

relayState1 = digitalRead(relay1);
relayState2 = digitalRead(relay2);
relayState3 = digitalRead(relay3);

relayCheck = relayState1 + relayState2 + relayState3;

if (relayState1 == 1 && gpio1Sent == false){
digitalWrite(LED1, HIGH);
Serial.println("DRAWER 1 GPIO");

gpio1Sent = true;

}

else if (relayState1 == 1 && gpio1Sent == true){
  digitalWrite(LED1, HIGH);
}
else if (relayState1 == 0){
  digitalWrite(LED1, LOW);
  gpio1Sent = false;
}



if (relayState2 == 1 && gpio2Sent == false){

digitalWrite(LED2, HIGH);
Serial.println("DRAWER 2 GPIO");
 gpio2Sent = true;
}
else if (relayState2 == 1 && gpio2Sent == true){
  digitalWrite(LED2, HIGH);
}
else if (relayState2 == 0){
  digitalWrite(LED2, LOW);
  gpio2Sent = false;
}

if (relayState3 == 1 && gpio3Sent == false){

digitalWrite(LED3, HIGH);
Serial.println("DRAWER 3 GPIO");
 gpio3Sent = true;
}
else if (relayState3 == 1 && gpio3Sent == true){
  digitalWrite(LED3, HIGH);
}
else if (relayState3 == 0){
  digitalWrite(LED3, LOW);
  gpio3Sent = false;
}

if (relayCheck == 0){
  digitalWrite(LED1, HIGH);
  digitalWrite(LED2, HIGH);
  digitalWrite(LED3, HIGH);
}



}

if one or more drawers are open - only the led of the last drawer should be on
If all drawers are closed, all LEDs should be on
correct?

You should not look at the logic level of the input but when the logic level changes. If have wired your switches correctly which is between input and ground with the internal pull up resistor enabled then you need what is called a state change detector. There is an example by that name in the IDE.

Basically you keep track of what the switch was last time you looked at it. You then check if the door switch reads high and the last time you looked at it, it was low, then you have the draw opened. So turn all the LEDs off and then the one you want on. Do this for all three switches.

Note if you use arrays you can do it in only a few lines.

OK, there is a problem here insofar as your problem is not yet fully specified!

So, you open one drawer. It's light goes on, the others off. If in fact, we simply turn all lights off before turning that one on, it does make the code simpler since we presumably are happy to ignore the extremely short dark phase as it will not be noticeable to the human eye.

Open a second, it's light goes on, the others off.

What happens if you now close the second drawer?

Why do I mention this? Well, GM referred to "state change" - you detect when a switch was closed (goes to ground) when it was not before or in this case, the opposite. Well, when you close the second drawer, the state of the first drawer does not change so if you want the light to go back to the first open drawer, you will need to use different logic.

Mike, have we decided after that last discussion whether reed switches bounce or not?

noiasca:
if one or more drawers are open - only the led of the last drawer should be on
If all drawers are closed, all LEDs should be on
correct?

That is correct.

Paul__B:
So, you open one drawer. It's light goes on, the others off. If in fact, we simply turn all lights off before turning that one on, it does make the code simpler since we presumably are happy to ignore the extremely short dark phase as it will not be noticeable to the human eye.

Open a second, it's light goes on, the others off.

What happens if you now close the second drawer?

So effectively the first drawer becomes obsolete once a second is open. If the second drawer is shut then all the lights would come back on. At least that is the way I have it planned out. Whether the end user feels the same way....I'm sure that can change quickly enough.

Grumpy_Mike:
Note if you use arrays you can do it in only a few lines.

How would I do this with arrays?

OK, I trust that you are confirming that if the second drawer is closed, you expect the light for the first drawer to come back on - and when it also is closed, all lights go on.

Or you are saying that if the second drawer is closed but the first is still open, it reverts to "all lights on"? Please be clear about this.

Mike, have we decided after that last discussion whether reed switches bounce or not?

Not sure what the last discussion was but reed switches bounce like F***. But bouncing does not matter here as it is the last transition that will be registered.

How would I do this with arrays?

Instead of treating each switch with individual code, you have and array holding the pin numbers of the inputs, and another one holding the pin numbers of the outputs, and another holding the state of the last time the input was read. Then the logic of the system is simply handled in a loop which addresses each switch in turn. Thus cutting you code down by two thirds.

I can't test without your hardware but I come up with following sketch.

//https://forum.arduino.cc/index.php?topic=662083.msg4459920#new
const byte relay[] {5, 6, 7};
const byte led[] {9, 14, 15};
byte relayState[sizeof(relay)];
//bool gpioSent[sizeof(relay)];

void setup() {
  Serial.begin(115200);
  for (auto &i : relay)
    pinMode(i, INPUT_PULLUP);
  for (auto &i : led)
    pinMode(i, OUTPUT);
}

void loop() {
  static int8_t lastOpend = -1;  // last opend drawer. -1 all are closed

  // check for pin changes
  for (byte i = 0; i < sizeof(relay); i++)
  {
    if (!relayState[i] && digitalRead(relay[i]))
    {
      Serial.print(F("opened relay ")); Serial.println(i);
      lastOpend = i;
      relayState[i] = HIGH;
    }
    else if (relayState[i] && !digitalRead(relay[i]))
    {
      Serial.print(F("closed relay ")); Serial.println(i);
      relayState[i] = LOW;
    }
  }

  // check if all drawers are closed:
  bool reset = true;
  for (auto &i : relay)
  {
    if (digitalRead(i)) reset = false;
  }
  if (reset)
    lastOpend = -1;
  Serial.print(F("lastOpend ")); Serial.println(lastOpend);
  
  // Switch the LEDs
  for (byte i = 0; i < sizeof(led); i++)
  {
    if (i == lastOpend || lastOpend == -1) 
      digitalWrite(led[i], HIGH);
    else
      digitalWrite(led[i], LOW);
  }
}

Love that "Opend". :grinning:

I am waiting until he completely clarifies the behaviour required. :roll_eyes:

Did it just for your enjoyment.

Paul__B:
OK, I trust that you are confirming that if the second drawer is closed, you expect the light for the first drawer to come back on - and when it also is closed, all lights go on.

Or you are saying that if the second drawer is closed but the first is still open, it reverts to "all lights on"? Please be clear about this.

The latter, as far as I'm concerned. However, I am not the end user and there is a chance when I show this to them that they decide the former is what they really want...

I appreciate that isn't totally helpful, I am hoping to get some more clarity on this soon.

Well for the latter case, the logic is fairly straightforward.

Monitor all switches, determine transition from "drawer closed" to "drawer open".

When one is opened, its light on, others off. Mark that switch as "current".

Otherwise check to see if current has gone to "drawer closed" and if so, all lights on.

End of loop, goes back to monitoring all switches.

Each switch has a "last" state variable in order to detect a change, this is updated as well as the action performed, when a change - in either direction - is observed.

Note that as the loop is continuously cycling, the first step of monitoring all switches need only look for the first switch it finds that has gone from "drawer closed" to "drawer open". And it can first look for newly opened drawers and act on one, then look for newly closed drawers and perform the last step if it finds one which happens to be the "current".

Thanks Paul.

I spoke to the end user today, and they have now decided that when there are multiple drawers open, and the last one is closed, it should revert to the previously opened one.

So the last opened always lights up, and when that is closed, the one opened previous to that is now the one that is lit.

I hope that makes sense? Would that alter the logic you described in the last post?

noiasca:
I can't test without your hardware but I come up with following sketch.

//https://forum.arduino.cc/index.php?topic=662083.msg4459920#new

const byte relay[] {5, 6, 7};
const byte led[] {9, 14, 15};
byte relayState[sizeof(relay)];
//bool gpioSent[sizeof(relay)];

void setup() {
  Serial.begin(115200);
  for (auto &i : relay)
    pinMode(i, INPUT_PULLUP);
  for (auto &i : led)
    pinMode(i, OUTPUT);
}

void loop() {
  static int8_t lastOpend = -1;  // last opend drawer. -1 all are closed

// check for pin changes
  for (byte i = 0; i < sizeof(relay); i++)
  {
    if (!relayState[i] && digitalRead(relay[i]))
    {
      Serial.print(F("opened relay ")); Serial.println(i);
      lastOpend = i;
      relayState[i] = HIGH;
    }
    else if (relayState[i] && !digitalRead(relay[i]))
    {
      Serial.print(F("closed relay ")); Serial.println(i);
      relayState[i] = LOW;
    }
  }

// check if all drawers are closed:
  bool reset = true;
  for (auto &i : relay)
  {
    if (digitalRead(i)) reset = false;
  }
  if (reset)
    lastOpend = -1;
  Serial.print(F("lastOpend ")); Serial.println(lastOpend);
 
  // Switch the LEDs
  for (byte i = 0; i < sizeof(led); i++)
  {
    if (i == lastOpend || lastOpend == -1)
      digitalWrite(led[i], HIGH);
    else
      digitalWrite(led[i], LOW);
  }
}

I've just tried this and it works great. THe only thing it doesn't do is return the last opened back to the previously opened drawer when the last opened is shut.

So if I open 0, then 1, then 2, and then I close 2 again, I have to find a way to get it to revert to 1, if that makes sense?

Oopoe:
I hope that makes sense? Would that alter the logic you described in the last post?

Yes, it would. :astonished:

It makes it much more tricky. And completely different! You see, that now implies a "priority", a chain of order of which came when because this implies that any or all of the drawers may be open and the order is - arguably - important. Maybe it isn't but let's assume this is the case.

The answer to this is a linked list. This may sound horrendous, but should not be too hard. As before, we need to go through the drawer inputs looking for one that is now open and was previously closed. For each drawer we have a "predecessor" pointer and to make it easier, a "successor". This makes it a double linked list! :sunglasses:

And we again have a "current" pointer. Now the array indices run from 0 to one less than the number of drawers. To mark a "none", we make it minus one (-1). OK, let's start:

Setup "current" to -1, all lights on, all drawers' "last" are closed (0 presumably).

Look for a drawer that was closed, now open. If so, set its "predecessor" to "current", "successor" to -1 and then, "current" to this drawer. All lights off, this drawer (which is now "current") light on. Look up its "predecessor" and if it is not -1, set that drawer's "successor" to "current".

Now look for a drawer that was open, now closed. Is it "current"? Well, if it is, then it has no "successor" so we can next look at its "predecessor". If that is other than -1, then we turn all lights off, make the "predecessor" to be "current" and turn its light on and (probably unnecessarily) its "successor" to -1.

If the "predecessor" was -1, then there are no open drawers and we turn all lights on, "current" to -1.

If the recently closed drawer was not "current", then it has a "successor". We set the successor's "predecessor" to this drawer's and vice versa, if the "predecessor" is other than -1, then the predecessor's "successor" is set to this drawer's.

This drawer is thus removed from the linked list. Note that its "predecessor" and "successor" are now irrelevant as these will be set anew if and when it is next opened. For this reason it was not necessary to initialise all "predecessor" and "successor" to -1 as we had assumed all drawers closed at the very start.

Incidental note: contact bounce will cause repeated cycling through these steps but will not affect the end result! It's a microprocessor; it doesn't care. :grinning:

if that makes sense?

this is because your requirement was not specific enough for that usecase ^^.

can't test, but add a check on lastOpend and re-initialize it to -1 if the last opened drawer was closed.

  for (byte i = 0; i < sizeof(relay); i++)
  {
    if ((!relayState[i] && digitalRead(relay[i])) || (lastOpend==-1 && digitalRead(relay[i])))
    {
      Serial.print(F("opened relay ")); Serial.println(i);
      lastOpend = i;
      relayState[i] = HIGH;
    }
    else if (relayState[i] && !digitalRead(relay[i]))
    {
      Serial.print(F("closed relay ")); Serial.println(i);
      relayState[i] = LOW;
      if (lastOpend = i) lastOpend==-1; 
    }
  }

Thanks for the info.

Wow, this looks crazy complicated! The premise of what you're saying makes sense, but how it's written in code is a different story. I might have to just manage the clients expectations and draw the line when it comes to some of the more advanced functionality.

I appreciate the help.

It would be a lot simpler if you kept the state of the draws in an array it the order they were opened.

Have the array act like a record of what order they were opened. So the first draw to be opened would have the draw number in the first element of the array, the second draw the second element and so on. Have the array store -1 in the elements that are blank.

Then search the array from the end to the start to find the first element that contains a draw number, that is not a -1 and light up that LED.

When a draw is closed search for that draw number in the array and replace it with a -1, then shuffle all the array elements above that down by one to close up the gap. In that way the last none -1 element will always contain the last draw opened.

I think my way works just as well as all that shuffling! :sunglasses:

(That is the whole purpose of linked lists!)

You still have to do shuffling when you remove an entry from the middle of a linked list.