Pages: [1] 2   Go Down
Author Topic: [Solved] Adjusting shift register outputs to the signal of light barriers  (Read 826 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 12
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi,
I'm having problems with a project I'm working on right now.

First of all I will explain what the project is about and what I'm trying to do:

I have built a plate witch six holes arranged like a pyramid in it. Every hole is surrounded by 4 LEDs of which two are serially connected (so they are just behaving as if it was one LED). Next to every hole there is a reflection light barrier. For a picture see attachment: plate prototype

What is the Goal?
Goal is to light up the 4 LEDs around a hole as soon as a plastic cup was placed in the hole. If the cup is beeing removed the 4 LEDs switch of again. This of course working for all of the six holes.

(This plate is a prototype and in the final version i will be having two pyramids with 10 holes each.)


The hardware part:

I'm using an Arduino Duemilanove.

Since the plate has 6 holes with two LEDs each (2x 2 serially connected) I would need 12 digital outputs to controll them all. And since i will be needing 20x2 outputs in the final version I'm using two 74HC595 shiftregister to multiplex the ouputs (so now I have 16 outputs but just using 12 of them).
I'm also using two ULN2803 to supply the LEDs with electrical power.
The two ICs are connected like this http://www.instructables.com/files/orig/F5U/CQS8/GWF5UFZU/F5UCQS8GWF5UFZU.png
But just with two 74HC595 and two ULN2803.

For checking if there is a cup I'm using reflection light barriers. These give me an analog signal which is very low if there is a cup and very high if there is no cup. For multiplexing the analog inputs I'm using a 74HC4051.

I dont think we need to worry about the hardware part unless of course if you have any genius suggestions for improvement smiley-wink But the hardware part is working very well. Means I'm able to control the LEDs as i want and I'm able to read out every single light barrier separately.

Picture: Attachment Hardware

So whats my problem then? Where am I at?

I have written a program that does the exact thing I have described in "What is the Goal" BUT the LEDs are not as bright as they should be. This is because in the beginning of the loop all the LEDs are being switched off and during the loop while the program is checking where a cup is it switches on the LEDs around that hole. As the program gets to the next hole it switches on the LEDs around that hole if there is a cup but also switches of all the other leds. That means all LEDs are constantly being switched on and off again which reduces their brightness extremely.

So what to do?
One thing I could theoretically do is defining every possible situation that could possibly occur and tell the programm what to do in which case. So: "If cup1 high ; cup2 high ;.....;cup 6 low" switch on that that and that LED. That would certainly work but since there are six holes there are 2^6 possibilities (in the final version 2^10) that would be just stupid.
The other possibility would be to instead of switching on only the leds around the hole the program is checking at the time and switching of all the other LEDs get the shiftregister to adjust its output somehow. But thats exactly my problem.

So lets say the program notices that there is a cup in hole one. It will send the first shift register a "0" and the second shift register a "3" because these are the value that will light up the LEDs around hole one.
Now the program gets to the second hole and notices that there is a cup.
The combonation to light up the LEDs around hole two is ShiftReg1 = 0 and ShiftReg2 = 12. But if I send this to the shiftRegisters it would switch of the LEDs around hole one again and again the LEDs would appear less bright.
So if there would be a way to to just "add" the second LEDs to the first ones without defining 2^6 possibilities that would be the solution. Something like: "oh there is a cup in hole two so we somehow take the ShiftReg1 = 0 and ShiftReg2 = 12 and add that to whatever the shiftRegister where before in this case ShiftReg1 = 0 and ShiftReg2 = 3 and magically we and up with ShiftReg1 = 0 and ShiftReg2 = 15 OK BAD EXAMPLE because in this case that wouldnt be too hard to do but i think that is because the two holes are the first two and they are next to each other. I dont think that would work for: "Light up Cup3 and Cup6"...

For your understanding here are the ShiftRegister values for every hole:

Hole 1: ShiftReg1 = 0 and ShiftReg2 = 3
Hole 2: ShiftReg1 = 0 and ShiftReg2 = 12
Hole 3: ShiftReg1 = 0 and ShiftReg2 = 48
Hole 4: ShiftReg1 = 3 and ShiftReg2 = 0
Hole 5: ShiftReg1 = 12 and ShiftReg2 = 0
Hole 6: ShiftReg1 = 0 and ShiftReg2 = 192

These will light up the 4 LEDs around the hole. (see the order attachment)

So here is the code that I'm using right now (the one that is working but not very bright smiley-grin )
As you can tell from the code I'm very new to programming and arduinos. Keep that in mind while trying to explain something to me smiley-wink  

Code:
//defining pins

int S0 = 8;
int S1 = 9;
int S2 = 10;

int SensorPin = A0;

int dataPin = 2;        
int latchPin = 3;
int clockPin = 4;


//defining variables

int sCup1 ;
int sCup2 ;
int sCup3 ;
int sCup4 ;
int sCup5 ;
int sCup6 ;


void setup()
{
    //Configure each IO Pin
    pinMode(S0, OUTPUT);      
    pinMode(S1, OUTPUT);
    pinMode(S2, OUTPUT);
    pinMode(SensorPin, INPUT);
    
    pinMode(dataPin, OUTPUT);      
    pinMode(latchPin, OUTPUT);
    pinMode(clockPin, OUTPUT);
        
}

void loop() {
  
  //read out sensor cup 1
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  int sCup1 = analogRead (SensorPin);
  
    //read out sensor cup 2
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  int sCup2 = analogRead (SensorPin);
  
    //read out sensor cup 3
  digitalWrite (S0, LOW);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  int sCup3 = analogRead (SensorPin);
  
    //read out sensor cup 4
  digitalWrite (S0, HIGH);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  int sCup4 = analogRead (SensorPin);
  
    //read out sensor cup 5
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  int sCup5 = analogRead (SensorPin);
  
    //read out sensor cup 6
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  int sCup6 = analogRead (SensorPin);
  

   //switch off all LEDs
  
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, 0);        
        shiftOut(dataPin, clockPin, MSBFIRST, 0);
        digitalWrite(latchPin, HIGH);  
  
   //LEDs Cup1
  
     if (sCup1 < 500)
  
   {
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, 0);        
        shiftOut(dataPin, clockPin, MSBFIRST, 3);
        digitalWrite(latchPin, HIGH);          
   }
  
  

   //LEDs Cup2
  
        if (sCup2 < 500)
        
   {
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, 0);        
        shiftOut(dataPin, clockPin, MSBFIRST, 12);
        digitalWrite(latchPin, HIGH);          
   }
  

  
   //LEDs Cup3
  
        if (sCup3 < 500)
  
   {
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, 0);        
        shiftOut(dataPin, clockPin, MSBFIRST, 48);
        digitalWrite(latchPin, HIGH);          
   }
  
 
  
   //LEDs Cup4
  
        if (sCup4 < 500)
  
   {
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, 0);        
        shiftOut(dataPin, clockPin, MSBFIRST, 192);
        digitalWrite(latchPin, HIGH);          
   }
  

  
   //LEDs Cup5
  
        if (sCup5 < 500)
  
   {
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, 3);        
        shiftOut(dataPin, clockPin, MSBFIRST, 0);
        digitalWrite(latchPin, HIGH);          
   }
  

  
     //LEDs Cup6
  
        if (sCup6 < 500)
  
   {
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, 12);        
        shiftOut(dataPin, clockPin, MSBFIRST, 0);
        digitalWrite(latchPin, HIGH);          
   }
  

  
}

Thank you in advance for your time and let me know if you need any more information smiley
 


* hardware.jpg (229.06 KB, 816x612 - viewed 14 times.)

* plate prototype.jpg (174.81 KB, 816x612 - viewed 15 times.)

* order.jpg (42.92 KB, 662x568 - viewed 7 times.)
« Last Edit: February 11, 2013, 10:48:11 am by hlawan » Logged

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

I suggest you declare a variable to hold the state of all the LEDs. The most obvious representation IMO would be to hold that state as a bitmask using the same layout that you're using for the shift register.

Do your input reading stuff and update the required state of LEDs as a result. The Arduino runtime library provides convenience functions to let you set and clear individual bits in a number, ot you can use the conventional C++ bitwise operators.

Once you have updated the required state of the LEDs, write that state out to the shift register.

Impact on your code should be negligible - instead of writing each LED state to the shift register,replace that with code to write the LED state to your variable; at the end of the loop, add a statement to write that variable out to the shift registers.
Logged

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

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

Alright, thank you very much that already helped a lot. As I said I'm very new to all this and had never heard of bitmasks. But I read into the topic and wrote a new program that surprisingly worked after a few tries (pretty much smiley-grin )

So thats how my program looks like right now:

Code:
//defining pins

int S0 = 8;
int S1 = 9;
int S2 = 10;

int SensorPin = A0;

int dataPin = 2;        
int latchPin = 3;
int clockPin = 4;

//defining variables


byte LEDpattern1 = B00000000;
byte LEDpattern2 = B00000000;

byte maskCup1 = B00000011;
byte maskCup2 = B00001100;
byte maskCup3 = B00110000;
byte maskCup4 = B11000000;
byte maskCup5 = B00000011;
byte maskCup6 = B00001100;


int sCup1 ;
int sCup2 ;
int sCup3 ;
int sCup4 ;
int sCup5 ;
int sCup6 ;


void setup()
{
    //Configure each IO Pin
    pinMode(S0, OUTPUT);      
    pinMode(S1, OUTPUT);
    pinMode(S2, OUTPUT);
    pinMode(SensorPin, INPUT);
    
    pinMode(dataPin, OUTPUT);      
    pinMode(latchPin, OUTPUT);
    pinMode(clockPin, OUTPUT);
    
    
}

void loop() {
  
  //read out sensor cup 1
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  int sCup1 = analogRead (SensorPin);
  
    //read out sensor cup 2
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  int sCup2 = analogRead (SensorPin);
  
    //read out sensor cup 3
  digitalWrite (S0, LOW);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  int sCup3 = analogRead (SensorPin);
  
    //read out sensor cup 4
  digitalWrite (S0, HIGH);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  int sCup4 = analogRead (SensorPin);
  
    //read out sensor cup 5
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  int sCup5 = analogRead (SensorPin);
  
    //read out sensor cup 6
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  int sCup6 = analogRead (SensorPin);
  

      
  
   //LEDs Cup1
  
if (sCup1 < 500)
  
   {
       LEDpattern1 = (LEDpattern1 | maskCup1);
   }

   else {
   }
  

   //LEDs Cup2
  
 if (sCup2 < 500)
        
   {
      LEDpattern1 = (LEDpattern1 | maskCup2);        
   }

 else  {
   }

  
   //LEDs Cup3
  
 if (sCup3 < 500)
  
   {
      LEDpattern1 = (LEDpattern1 | maskCup3);          
   }
  
 else {
 }

  
   //LEDs Cup4
  
 if (sCup4 < 500)
  
   {
      LEDpattern1 = (LEDpattern1 | maskCup4);        
   }
  
else {
 }
  

   //LEDs Cup5
  
        if (sCup5 < 500)
  
   {
      LEDpattern2 = (LEDpattern2 | maskCup5);          
   }
  
else {
 }
  

     //LEDs Cup6
  
  if (sCup6 < 500)
  
    {
      LEDpattern2 = (LEDpattern2 | maskCup6);        
    }
  
else {
}        
        
        digitalWrite(latchPin, LOW);            
        shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern2);        
        shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern1);
        digitalWrite(latchPin, HIGH);
  
}

So what that program does is switch on the right LEDs that belog to the hole.
So thats good.
What it obviously doesn't do is switch of the LEDs off if the cup is being removed.

This is because I couldn't think of a way to tell the program "else: just turn those two bits of that certain cup into 0 and leave the rest as it is stored in the variable"

Is there a way to just manipulate certain bits of the byte and leave the rest as it is?
« Last Edit: February 10, 2013, 02:51:05 pm by hlawan » Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3453
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Is that not what you are doing already ?
What does
Code:
B01010101 | B10000000
give you ?
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

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

Well you are right

Code:
B01010101 | B10000000

gives me

Code:
B11010101


Let me try to explain what I think my problem is by simulating a run through the program:

We start of with the LEDpattern being: B00000000 // all LEDs switched off

First Cup is in the hole

Code:
LEDpattern = (LEDpattern | maskCup1); // maskCup1 is B00000011

now the LEDpattern is B00000011 // LEDs for the first cup are switched on

Second Cup is in the hole

Code:
LEDpattern = (LEDpattern | maskCup2); // maskCup2 is B00001100

now the LEDpattern is B00001111 // LEDs for the first and second cup are switched on

This goes on till the last cup:

Code:
LEDpattern = (LEDpattern | maskCup4); // maskCup4 is B11000000

now the LEDpattern is 11111111 // LEDs for all cups are switched on

So thats all working fine, even if the cups are inserted in a different order or not all of them.


Now we get to the problem.

The case is: We take out the first Cup and go through the program again.

If I would say something like:

Code:
LEDpattern = (LEDpattern & maskNoCup1); // maskNoCup1 is B00000000

I would end up with LEDpattern B00000000 which would switch off all the LEDs although I just wanted to switch off the LEDs for the first cup.
I think it isn't possible to just put a mask over the LEDpattern again because then I would say something about bits 1 to 6 as well. But I cannot do that because that would switch off other LEDs although the cups are still there or the other way round it would switch on LEDs although the cups they belong to aren't there anymore.

I just want bits 1 to 6 to say what they are but since they could be 1 or 0 it is not possible to have a mask that always works like during the "switching on process".

I hope I could express myself well enough to explain the problem smiley-wink



« Last Edit: February 10, 2013, 04:58:44 pm by hlawan » Logged

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

Code:
LEDpattern = (LEDpattern | maskCup4); // maskCup4 is B11000000
The code above does a bitwise OR to set bits in a variable, where the bits are defined by a bitmask.

The code to clear them is much less obvious to understand, but surprisingly simple:

Code:
LEDpattern = (LEDpattern & ~maskCup4); // maskCup4 is B11000000

The ~ (tilde) operator gives you the bitwise inverse of the mask ("ones' complement") i.e. it inverts each bit. In this example it would give you B00111111.

The & (bitwise AND) operator then clears the bits which are zero in ~maskCup4. The result in this case is that it clears the top two bits and leaves the other bits unchanged.

Other options open to you are to use the Arduino function bitWrite(), bitSet(), bitClear() which let you read and update individual bits in a value without having to think about bitmasks. In this case where you seem to be dealing with multiple bits at the same time, the bitmasking approach you're using would produce more compact code.
Logged

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

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

Thank you very much smiley

The bitClear() funktion was exactly what i needed. Now the program is running fine and everything works.

Heres the code:

Code:
//defining pins

int S0 = 8;
int S1 = 9;
int S2 = 10;

int SensorPin = A0;

int dataPin = 2;       
int latchPin = 3;
int clockPin = 4;


//defining variables


byte LEDpattern1 = B00000000;
byte LEDpattern2 = B00000000;

byte maskCup1 = B00000011;
byte maskCup2 = B00001100;
byte maskCup3 = B00110000;
byte maskCup4 = B11000000;
byte maskCup5 = B00000011;
byte maskCup6 = B00001100;



int sCup1 ;
int sCup2 ;
int sCup3 ;
int sCup4 ;
int sCup5 ;
int sCup6 ;


void setup()
{
  //Configure each IO Pin
  pinMode(S0, OUTPUT);     
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(SensorPin, INPUT);

  pinMode(dataPin, OUTPUT);       
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
     
}

void loop() {

  //read out sensor cup 1
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  int sCup1 = analogRead (SensorPin);

  //read out sensor cup 2
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  int sCup2 = analogRead (SensorPin);

  //read out sensor cup 3
  digitalWrite (S0, LOW);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  int sCup3 = analogRead (SensorPin);

  //read out sensor cup 4
  digitalWrite (S0, HIGH);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  int sCup4 = analogRead (SensorPin);

  //read out sensor cup 5
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  int sCup5 = analogRead (SensorPin);

  //read out sensor cup 6
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  int sCup6 = analogRead (SensorPin);



  //LEDs Cup1

  if (sCup1 < 500)
  {
    LEDpattern1 = (LEDpattern1 | maskCup1);
  }

  else
  {
    bitClear(LEDpattern1 , 0);
    bitClear(LEDpattern1 , 1);
  }


  //LEDs Cup2

  if (sCup2 < 500)
  {
    LEDpattern1 = (LEDpattern1 | maskCup2);       
  }

  else
  {
    bitClear(LEDpattern1 , 2);
    bitClear(LEDpattern1 , 3);
  }


  //LEDs Cup3

    if (sCup3 < 500)
  {
    LEDpattern1 = (LEDpattern1 | maskCup3);         
  }

  else
  {
    bitClear(LEDpattern1 , 4);
    bitClear(LEDpattern1 , 5);
  }

  //LEDs Cup4

  if (sCup4 < 500)
  {
    LEDpattern1 = (LEDpattern1 | maskCup4); 
  }   

  else
  {
    bitClear(LEDpattern1 , 6);
    bitClear(LEDpattern1 , 7);
  }

  //LEDs Cup5

  if (sCup5 < 500)
  {
    LEDpattern2 = (LEDpattern2 | maskCup5);         
  }

  else
  {
    bitClear(LEDpattern2 , 0);
    bitClear(LEDpattern2 , 1);
  }

  //LEDs Cup6

  if (sCup6 < 500)
  {
    LEDpattern2 = (LEDpattern2 | maskCup6);       
  }

  else
  {
    bitClear(LEDpattern2 , 2);
    bitClear(LEDpattern2 , 3);
  }       


  ´//shifting out the data to the shift register

  digitalWrite(latchPin, LOW);           
  shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern2);         
  shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern1);
  digitalWrite(latchPin, HIGH);

}
Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3453
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Well done on solving your problem.

Looking at your code I see variables like this
Code:
byte maskCup1 = B00000011;
byte maskCup2 = B00001100;
byte maskCup3 = B00110000;
byte maskCup4 = B11000000;
byte maskCup5 = B00000011;
byte maskCup6 = B00001100;
and
Code:
int sCup1 ;
int sCup2 ;
int sCup3 ;
int sCup4 ;
int sCup5 ;
int sCup6 ;
To me that looks like it is a prime candidate for the use of arrays which would mean that you could shorten the code that reads the values  into sCup1, sCup2, sCup3 etc and creates the LEDpattern values.  Changing your code may seem like a bit of a chore but it will pay off when you expand the size of your from 6 holes to 20 holes.
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

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

Alright, I didn't have too much time today to read about arrays but I think I understood what they are about.

Here is the new code:

Code:
//defining pins

int S0 = 8;
int S1 = 9;
int S2 = 10;

int SensorPin = A0;

int dataPin = 2;       
int latchPin = 3;
int clockPin = 4;


//defining variables

int sCup[6] ;

byte LEDpattern[] = {B00000000, B00000000};

byte maskCup[] = {B00000011, B00001100, B00110000, B11000000, B00000011, B00001100};




void setup()
{
  //Configure each IO Pin
  pinMode(S0, OUTPUT);     
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(SensorPin, INPUT);

  pinMode(dataPin, OUTPUT);       
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);

  //initialize serial communication
  Serial.begin(9600);       
}

void loop() {

  //read out sensor cup 1
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  sCup[0] = analogRead (SensorPin);

  //read out sensor cup 2
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  sCup[1] = analogRead (SensorPin);

  //read out sensor cup 3
  digitalWrite (S0, LOW);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  sCup[2] = analogRead (SensorPin);

  //read out sensor cup 4
  digitalWrite (S0, HIGH);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  sCup[3] = analogRead (SensorPin);

  //read out sensor cup 5
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  sCup[4] = analogRead (SensorPin);

  //read out sensor cup 6
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  sCup[5] = analogRead (SensorPin);

 
 
  //LEDs Cup1
 
  if (sCup[0] < 500)
  {
    LEDpattern[0] = (LEDpattern[0] | maskCup[0]);
  }

  else
  {
    bitClear(LEDpattern[0] , 0);
    bitClear(LEDpattern[0] , 1);
  }


  //LEDs Cup2

  if (sCup[1] < 500)
  {
    LEDpattern[0] = (LEDpattern[0] | maskCup[1]);       
  }

  else
  {
    bitClear(LEDpattern[0] , 2);
    bitClear(LEDpattern[0] , 3);
  }


  //LEDs Cup3

    if (sCup[2] < 500)
  {
    LEDpattern[0] = (LEDpattern[0] | maskCup[2]);         
  }

  else
  {
    bitClear(LEDpattern[0] , 4);
    bitClear(LEDpattern[0] , 5);
  }

  //LEDs Cup4

  if (sCup[3] < 500)
  {
    LEDpattern[0] = (LEDpattern[0] | maskCup[3]); 
  }   

  else
  {
    bitClear(LEDpattern[0] , 6);
    bitClear(LEDpattern[0] , 7);
  }


  //LEDs Cup5

  if (sCup[4] < 500)
  {
    LEDpattern[1] = (LEDpattern[1] | maskCup[4]);         
  }

  else
  {
    bitClear(LEDpattern[1] , 0);
    bitClear(LEDpattern[1] , 1);
  }


  //LEDs Cup6

  if (sCup[5] < 500)
  {
    LEDpattern[1] = (LEDpattern[1] | maskCup[5]);       
  }

  else
  {
    bitClear(LEDpattern[1] , 2);
    bitClear(LEDpattern[1] , 3);
  }       


  //shifting out the data to the shift registers

  digitalWrite(latchPin, LOW);           
  shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[1]);         
  shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[0]);
  digitalWrite(latchPin, HIGH);


}


Is this what you meant?
Are there more things about the code that work but are not really nice or could be better?
Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3453
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Well, you now have some data in arrays but are not really making use of them.  Arrays are particularly useful when you can put code in a for loop and use the variable of the for loop as an index to the data in the array.  It is a bit complicated in your case because the actions you take are different depending on the value of the array index. 

I would consider trying a for loop to go through the cup numbers so that the sCup
  • < 500 comparison is in the for loop and the actions to be taken are dealt with using  switch/case based on the array index. 

To avoid more duplication of code how about creating a function that does the actions and passing a parameter to it for the LEDpattern to be used and the array index.  The index needed for maskCup[] is already the same as the array index.

Using arrays and a function will considerably reduce the amount of code you have to enter and will make the program easier to extend.
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

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

I tried to use a for loop and switch/case functions and it kind of works. If I place a cup in a hole the leds are switched on but if there is no cup in the hole the leds blink randomly. There seems to be a logical mistake in my thinking or in my understanding of the switch/case.
Is it possible to put a new switch/case into a case of another switch/case?

Code:

//defining pins

int S0 = 8;
int S1 = 9;
int S2 = 10;

int SensorPin = A0;

int dataPin = 2;       
int latchPin = 3;
int clockPin = 4;


//defining variables

int sCup[6] ;

byte LEDpattern[] = {
  B00000000, B00000000};

byte maskCup[] = {
  B00000011, B00001100, B00110000, B11000000, B00000011, B00001100};



//Circle sequence

int seq1[6] = {
  3,12,48,0,0 ,192};
int seq2[6] = {
  0,0 ,0 ,3,12,0};

void setup()
{
  //Configure each IO Pin
  pinMode(S0, OUTPUT);     
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(SensorPin, INPUT);

  pinMode(dataPin, OUTPUT);       
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);

   
}

void loop() {





  //read out sensor cup 1
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  sCup[0] = analogRead (SensorPin);

  //read out sensor cup 2
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  sCup[1] = analogRead (SensorPin);

  //read out sensor cup 3
  digitalWrite (S0, LOW);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  sCup[2] = analogRead (SensorPin);

  //read out sensor cup 4
  digitalWrite (S0, HIGH);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  sCup[3] = analogRead (SensorPin);

  //read out sensor cup 5
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  sCup[4] = analogRead (SensorPin);

  //read out sensor cup 6
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  sCup[5] = analogRead (SensorPin);


  for (int i = 0 ; i < 4 ; i++)
  {

    int range = map(sCup[i],0,1023,0,1);


    switch (range)
    {

    case  0:
      LEDpattern[0] = (LEDpattern[0] | maskCup[i]);
      break;

    case  1:

      switch (i)
      {
      case 0:
        bitClear(LEDpattern[0] , 0);
        bitClear(LEDpattern[0] , 1);
        break;

      case 1:
        bitClear(LEDpattern[0] , 2);
        bitClear(LEDpattern[0] , 3);
        break;

      case 2:
        bitClear(LEDpattern[0] , 4);
        bitClear(LEDpattern[0] , 5);
        break;

      case 3:
        bitClear(LEDpattern[0] , 6);
        bitClear(LEDpattern[0] , 7);
        break;
      }

    }
  }

  for (int i = 4 ; i < 6 ; i++)
  {
    int range = map(sCup[i],0,1023,0,1);

    switch (range)
    {

    case 0:
      LEDpattern[1] = (LEDpattern[1] | maskCup[i]);
      break;

    case 1:

      switch (i)
      {
      case 4:
        bitClear(LEDpattern[1] , 0);
        bitClear(LEDpattern[1] , 1);
        break;

      case 5:
        bitClear(LEDpattern[1] , 2);
        bitClear(LEDpattern[1] , 3);
        break;
      }
    }
  }



  //shifting out the data to the shift registers

  digitalWrite(latchPin, LOW);           
  shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[1]);         
  shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[0]);
  digitalWrite(latchPin, HIGH);


}


Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3453
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I don't have time to look at your code at the moment, but it is certainly possible to nest switch/case instances.
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

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

Alright I thought I found the mistake because I had forgotten to shift out the data while still being in the for loop.
I didn't change too much smiley-grin I'll keep trying...

Here's the code just in case somebody is extremely bored and wants to read it smiley-wink

Code:

//defining pins

int S0 = 8;
int S1 = 9;
int S2 = 10;

int SensorPin = A0;

int dataPin = 2;       
int latchPin = 3;
int clockPin = 4;


//defining variables

int sCup[6] ;

byte LEDpattern[] = {
  B00000000, B00000000};

byte maskCup[] = {
  B00000011, B00001100, B00110000, B11000000, B00000011, B00001100};



//Circle sequence

int seq1[6] = {
  3,12,48,0,0 ,192};
int seq2[6] = {
  0,0 ,0 ,3,12,0};

void setup()
{
  //Configure each IO Pin
  pinMode(S0, OUTPUT);     
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(SensorPin, INPUT);

  pinMode(dataPin, OUTPUT);       
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);

  //initialize serial communication
  Serial.begin(9600);       
}

void loop() {





  //read out sensor cup 1
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  sCup[0] = analogRead (SensorPin);

  //read out sensor cup 2
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  sCup[1] = analogRead (SensorPin);

  //read out sensor cup 3
  digitalWrite (S0, LOW);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  sCup[2] = analogRead (SensorPin);

  //read out sensor cup 4
  digitalWrite (S0, HIGH);
  digitalWrite (S1, HIGH);
  digitalWrite (S2, LOW);
  sCup[3] = analogRead (SensorPin);

  //read out sensor cup 5
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  sCup[4] = analogRead (SensorPin);

  //read out sensor cup 6
  digitalWrite (S0, HIGH);
  digitalWrite (S1, LOW);
  digitalWrite (S2, HIGH);
  sCup[5] = analogRead (SensorPin);


  for (int i = 0 ; i < 4 ; i++)
  {

    int range = map(sCup[i],0,1023,0,1);


    switch (range)
    {

    case  0:
      LEDpattern[0] = (LEDpattern[0] | maskCup[i]);
      break;

    case  1:

      switch (i)
      {
      case 0:
        bitClear(LEDpattern[0] , 0);
        bitClear(LEDpattern[0] , 1);
        break;

      case 1:
        bitClear(LEDpattern[0] , 2);
        bitClear(LEDpattern[0] , 3);
        break;

      case 2:
        bitClear(LEDpattern[0] , 4);
        bitClear(LEDpattern[0] , 5);
        break;

      case 3:
        bitClear(LEDpattern[0] , 6);
        bitClear(LEDpattern[0] , 7);
        break;
      }

    }

    digitalWrite(latchPin, LOW);           
    shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[1]);         
    shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[0]);
    digitalWrite(latchPin, HIGH);
  }


  for (int i = 4 ; i < 6 ; i++)
  {
    int range = map(sCup[i],0,1023,0,1);

    switch (range)
    {

    case 0:
      LEDpattern[1] = (LEDpattern[1] | maskCup[i]);
      break;

    case 1:

      switch (i)
      {
      case 4:
        bitClear(LEDpattern[1] , 0);
        bitClear(LEDpattern[1] , 1);
        break;

      case 5:
        bitClear(LEDpattern[1] , 2);
        bitClear(LEDpattern[1] , 3);
        break;
      }
    }
    digitalWrite(latchPin, LOW);           
    shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[1]);         
    shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[0]);
    digitalWrite(latchPin, HIGH);
  }


}
Logged

East Anglia (UK)
Offline Offline
Faraday Member
**
Karma: 89
Posts: 3453
May all of your blinks be without delay()
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

What sort of values do you get from reading the sensors
Code:
  sCup[5] = analogRead (SensorPin);
I wonder if using map() to convert the values to 0 or 1 is working.  I think that I would write a function to set range explicitly to 1 or 0 depending whether the value was above or below a threshold.  It feels more controllable than letting map() do its stuff because you are depending on the value being above or below 512 at the moment and I wonder if that is the case.

You can do the comparison and setting of range without a function using a ternary operator like this
Code:
value = (sCup[index] < threshold) ? LOW : HIGH;
If I have got that right (I will soon be told if I haven't) then value will be set to LOW if sCup[index] is less than threshold else it will be set to HIGH.

This may or may not be the cause of your problem but using map() like you are feels wrong somehow
Logged

Please do not send me PMs asking for help.  Post in the forum then everyone will benefit from seeing the questions and answers.

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

That indeed was the problem. I don't really understand why but now it is working again.
Thank you very much.

Next thing I will try is to somehow shorten the code for the
Code:
  //read out sensor cup 1
  digitalWrite (S0, LOW);
  digitalWrite (S1, LOW);
  digitalWrite (S2, LOW);
  sCup[0] = analogRead (SensorPin);
.
.
.
.
.




When you said:
Quote
I would consider trying a for loop to go through the cup numbers so that the sCup
< 500 comparison is in the for loop and the actions to be taken are dealt with using  switch/case based on the array index.

To avoid more duplication of code how about creating a function that does the actions and passing a parameter to it for the LEDpattern to be used and the array index.  The index needed for maskCup[] is already the same as the array index.

Was this what you thought of or does the code still seem too long?

Code:
for (int i = 0 ; i < 4 ; i++)
  {

    int value = (sCup[i] < 500) ? LOW : HIGH; 
   
   
    switch (value)
    {

    case  0:
      LEDpattern[0] = (LEDpattern[0] | maskCup[i]);
      break;

    case  1:

      switch (i)
      {
      case 0:
        bitClear(LEDpattern[0] , 0);
        bitClear(LEDpattern[0] , 1);
        break;

      case 1:
        bitClear(LEDpattern[0] , 2);
        bitClear(LEDpattern[0] , 3);
        break;

      case 2:
        bitClear(LEDpattern[0] , 4);
        bitClear(LEDpattern[0] , 5);
        break;

      case 3:
        bitClear(LEDpattern[0] , 6);
        bitClear(LEDpattern[0] , 7);
        break;
      }

    }

    digitalWrite(latchPin, LOW);           
    shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[1]);         
    shiftOut(dataPin, clockPin, MSBFIRST, LEDpattern[0]);
    digitalWrite(latchPin, HIGH);
  }

Logged

Pages: [1] 2   Go Up
Jump to: