switches within a counter sketch

Hi folks, Firstly thank you to everyone that has helped me so far - as previously stated in other posts I am a Technology teacher and it has recently been decided by those above we are moving to arduino do to the poor state our PBasic Stamp boards are in. Therefore I am trying to create a scheme of work for my pupils with very little knowledge myself (I only have a month to 6 weeks experience)

One task Im trying to get my pupils to do is designing a toy. Within this toy they will push a master switch, which will switch on an LED and "shake" a toy 5 times. My idea is that this shaking will involve a turntable that will rotate, hit a limit switch, change direction of the motor, until it hits another limit switch. This is to repeat 5 times, then switch off.

Ive tried to create a sketch to do this, but its not working and I cant see why. Here is my sketch.

 /*
toy System - 
*/

int MSSwitchPin = 13;        //master switch
int CWSwitchPin = 12;        //switch to make motor go clockwise
int ACWSwitchPin = 8;        //switch to make motor go Anti Clockwise
int MSLEDPin = 2;            //LED 
int Motor1APin = 6; 	     // H-bridge leg 1
int Motor2APin = 7; 	     // H-bridge leg 2

boolean MSlastButton = LOW;                 
boolean MScurrentButton = LOW;      
boolean MSLEDon = false;

boolean CWlastButton = LOW;                 
boolean CWcurrentButton = LOW;      
boolean CWmotoron = false;

boolean ACWlastButton = LOW;                 
boolean ACWcurrentButton = LOW;      
boolean ACWmotoron = false;

void setup()
{
  pinMode(MSSwitchPin, INPUT);
  pinMode(CWSwitchPin, INPUT);
  pinMode(ACWSwitchPin, INPUT);
  pinMode(MSLEDPin, OUTPUT);
  pinMode(Motor1APin, OUTPUT);
  pinMode(Motor2APin, OUTPUT);
}
boolean MSdebounce(boolean last)            
{
        boolean current = digitalRead(MSSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(MSSwitchPin);      
}
return current;        
}

boolean CWdebounce(boolean last)            
{
        boolean current = digitalRead(CWSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(CWSwitchPin);      
}
return current;        
}

boolean ACWdebounce(boolean last)            
{
        boolean current = digitalRead(ACWSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(ACWSwitchPin);      
}
return current;        
}




void loop()
{
  MScurrentButton = MSdebounce(MSlastButton); 
  CWcurrentButton = CWdebounce(CWlastButton); 
  ACWcurrentButton = ACWdebounce(ACWlastButton); 
  
if (MSlastButton == LOW && MScurrentButton == HIGH)       
 for (int counter = 1 ; counter <= 5; counter = counter +1)  
   {  
      MSLEDon = !MSLEDon;
      CWmotoron = !CWmotoron;
      ACWmotoron = !ACWmotoron;
      
      digitalWrite(MSLEDPin, HIGH);
      digitalWrite(Motor1APin, LOW); // set leg 1 of the H-bridge low
      digitalWrite(Motor2APin, HIGH); // set leg 2 of the H-bridge high

      if (digitalRead(ACWSwitchPin) == HIGH);
      digitalWrite(Motor1APin, HIGH); // set leg 1 of the H-bridge high
      digitalWrite(Motor2APin, LOW); // set leg 2 of the H-bridge low
  
      if (digitalRead(CWSwitchPin) == HIGH);
      digitalWrite(Motor1APin, LOW); // set leg 1 of the H-bridge low
      digitalWrite(Motor2APin, HIGH); // set leg 2 of the H-bridge high    
    }
}

Ive tried to consider keeping the LED on this as well as debouncing. When the sketch is uploaded though the LED stays on like I wanted and the motor runs, but the other 2 switches do not change the direction, which makes me think it is not recognising my if statements.

Can i also say know there are easier ways to write this program, but I'm writing my scheme of work to continue on from previous lessons - I will come to simplifying sketches later on.

Thanks in advance for any help you can give me!!

but its not working

Grrrr.

(And I don't just mean the missing apostrophe and code tags)

You've got three virtually identical functions differing only by a pin number.
This is BASIC thinking. I posted a way around this earlier.

the other 2 switches do not change the direction, which makes me think it is not recognising my if statements.

Put some Serial.prints in to see which parts of the code are being run and the value of variables at critical points such as just before they are tested by the if statements.

if (digitalRead(ACWSwitchPin) == HIGH);

Oops

AWOL:

if (digitalRead(ACWSwitchPin) == HIGH);

Oops

Oops? What have I done wrong with this line?

Look at it.
Does it look like any other "if" you've ever seen?

AWOL:
Look at it.
Does it look like any other "if" you've ever seen?

Do you mean the ; at the end of the line? if so ive now removed this but still not working.

If not then I dont know what your talking about as my code has all been canalised from other codes to create this.
=(

if so ive now removed this

Good

but still not working.

Perhaps you need some braces.
Perhaps you need to define what "work" means.

Ive changed it to add more braces and try and break up aspects, but what is happening is the LED and motor goes on and THAT IS IT.

/*
toy System - 
*/

int MSSwitchPin = 13;        //master switch
int CWSwitchPin = 12;        //switch to make motor go clockwise
int ACWSwitchPin = 8;        //switch to make motor go Anti Clockwise
int MSLEDPin = 2;            //LED 
int Motor1APin = 6; 	     // H-bridge leg 1
int Motor2APin = 7; 	     // H-bridge leg 2

boolean MSlastButton = LOW;                 
boolean MScurrentButton = LOW;      
boolean MSLEDon = false;

boolean CWlastButton = LOW;                 
boolean CWcurrentButton = LOW;      
boolean CWmotoron = false;

boolean ACWlastButton = LOW;                 
boolean ACWcurrentButton = LOW;      
boolean ACWmotoron = false;

void setup()
{
  pinMode(MSSwitchPin, INPUT);
  pinMode(CWSwitchPin, INPUT);
  pinMode(ACWSwitchPin, INPUT);
  pinMode(MSLEDPin, OUTPUT);
  pinMode(Motor1APin, OUTPUT);
  pinMode(Motor2APin, OUTPUT);
}
boolean MSdebounce(boolean last)            
{
        boolean current = digitalRead(MSSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(MSSwitchPin);      
}
return current;        
}

boolean CWdebounce(boolean last)            
{
        boolean current = digitalRead(CWSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(CWSwitchPin);      
}
return current;        
}

boolean ACWdebounce(boolean last)            
{
        boolean current = digitalRead(ACWSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(ACWSwitchPin);      
}
return current;        
}




void loop()
{
  MScurrentButton = MSdebounce(MSlastButton); 
  CWcurrentButton = CWdebounce(CWlastButton); 
  ACWcurrentButton = ACWdebounce(ACWlastButton); 
  
if (MSlastButton == LOW && MScurrentButton == HIGH)       
 { for (int counter = 1 ; counter <= 5; counter = counter +1)  
      MSLEDon = !MSLEDon;
      CWmotoron = !CWmotoron;
      ACWmotoron = !ACWmotoron;
      
      digitalWrite(MSLEDPin, HIGH);
      digitalWrite(Motor1APin, LOW); // set leg 1 of the H-bridge low
      digitalWrite(Motor2APin, HIGH); // set leg 2 of the H-bridge high

      if (digitalRead(ACWSwitchPin) == HIGH)
      {
         digitalWrite(Motor1APin, HIGH); // set leg 1 of the H-bridge high
         digitalWrite(Motor2APin, LOW); // set leg 2 of the H-bridge low
      }
      if (digitalRead(CWSwitchPin) == HIGH)
      {  
        digitalWrite(Motor1APin, LOW); // set leg 1 of the H-bridge low
        digitalWrite(Motor2APin, HIGH); // set leg 2 of the H-bridge high    
      }
   }
}

what I want to happen is for the led and motor to turn like it is when the first master switch is pressed, but once it touches a limit switch(CW) it changes the motor direction to turn clockwise, until it hits the other limit switch(ACW), which will change direction again to change it back to anti-clockwise.

This pressing of each limit switch should happen 5 times, then switch the motor and LED off.
I had an extra line of code at the end switching the motor and LED off but this just stopped the whole sketch from working (Im assuming it read the initial code, passed by the counter, then switched it back off so quickly i wouldnt notice it. :~

All that stuff in the for loop is going to happen pretty quickly
Can your motor react that fast?

AWOL:
All that stuff in the for loop is going to happen pretty quickly
Can your motor react that fast?

surely though it will only happen as fast as I press the switch? My pupils are going to design a gearing system to control the output of the motor which will change the speed my CW and ACW switches are pressed? I am assuming though this is not what your talking about when you talk about the motor reacting fast enough.

Ive got another program that works I based this sketch on, but what happens there is the motor switches off as soon as the button is depressed. Im basically trying to combine this program with debouncing and keeping the output staying on

...is my problem maybe with this section?

MSLEDon = !MSLEDon;
CWmotoron = !CWmotoron;
ACWmotoron = !ACWmotoron;

ive played about and cant figure out what to do

It's all very well to say your code isn't working. But, for my money, the even bigger issue is how are you going to use this to teach your students?

What exactly are you expecting them to learn from this? Giving them a finished piece of code is little use.

I don't get the impression from the code that you have already posted that they are going to see an example of
good programming practice, OR
good analytical designe, OR
how to use a microprocessor (except in a very narrow sense).

I'm sorry if this sounds blunt, but you did say you have a tight deadline so I thought it best not to waffle.

I think my biggest concern is the lack of evidence of analytical design. To my eye the project looks to be cobbled together from several bits picked up here and there. And I would have no objection to that if it was only being used for your own personal satisfaction.

There seems to be plenty of opportunity to simplify the code by using arrays.

I have no idea whether you have actually determined that debouncing is needed. I don't think I have ever needed it in any of my projects, and believe me, I have used some pretty dodgy switches - including just touching 2 pieces of wire together. If it is not needed leave it out as it just confuses things for beginners.

Apart from the debounce functions I see no sign of organizing the code into functions. Even if the total code is very short it would be an excellent idea to get your students using functions from day one. Separating code into short functions each with a very narrowly defined task and a meaningful name makes debugging very much easier.

Finally, I believe strongly that any teaching exercise should include teaching about debugging. Learning the logical and methodical approach necessary to figure out problems in a program is essential.

The code in the demo several things at a time illustrates a lot of what I have been saying.

...R

Robin2:
It's all very well to say your code isn't working. But, for my money, the even bigger issue is how are you going to use this to teach your students?

What exactly are you expecting them to learn from this? Giving them a finished piece of code is little use.

I don't get the impression from the code that you have already posted that they are going to see an example of
good programming practice, OR
good analytical design, OR
how to use a microprocessor (except in a very narrow sense).

I'm sorry if this sounds blunt, but you did say you have a tight deadline so I thought it best not to waffle.

I think my biggest concern is the lack of evidence of analytical design. To my eye the project looks to be cobbled together from several bits picked up here and there. And I would have no objection to that if it was only being used for your own personal satisfaction.

There seems to be plenty of opportunity to simplify the code by using arrays.

I have no idea whether you have actually determined that debouncing is needed. I don't think I have ever needed it in any of my projects, and believe me, I have used some pretty dodgy switches - including just touching 2 pieces of wire together. If it is not needed leave it out as it just confuses things for beginners.

Apart from the debounce functions I see no sign of organizing the code into functions. Even if the total code is very short it would be an excellent idea to get your students using functions from day one. Separating code into short functions each with a very narrowly defined task and a meaningful name makes debugging very much easier.

Finally, I believe strongly that any teaching exercise should include teaching about debugging. Learning the logical and methodical approach necessary to figure out problems in a program is essential.

The code in the demo several things at a time illustrates a lot of what I have been saying.

...R

what Im trying to do is get an answer so I can then go back and reverse engineer it - its the way my mind works, but im trying at the same time to create a flow within the workbook where they are using previous learning experiences to strengthen their knowledge.

Before they have come to this task (the final task of the year) they will have learned about flowcharts, moving into basic programming, progressing into switches using the if command, moving onto counters, then motor control. After this they learn about boolean and how to keep an output on and steady, finally going onto debouncing as they have previously discovered the push switches aren't always true - doesnt always give the desired input they want.

I am willing to show you my work if you don't think I'm approaching it write- send me your email in a private message and I will send it to you (I cannot post it on here as because I'm a teacher any workbooks I create become property of the council and I am not allowed to post it online)

It may not be following good programming practice but as I have said I am a learner myself and I'm trying to teach it in a logical way and showing how things progress - Im teaching this to 14 and 15 year olds in which is essentially an 8th of the Engineering Science course - giving the pupils brand new commands for every new sketch will not help them at this stage in their learning or understanding and will definitely not help when it comes to assignments or exams. The good programming will follow as the progress into the Higher course (I will also be writing that and will try and add in the new commands then to tidy things up.) I will also be encouraging them to buy their own Arduino set (or similar product) to do this at home if it is something that they are genuinely interested in.

Can you explain the point of the five iterations of the "for" loop?
This would go a long way to explaining what working/not working means.

MSlastButton is only ever read, it is never written. That's at least one problem, because the name doesn't suggest that it should be constant.

AWOL:
Can you explain the point of the five iterations of the "for" loop?
This would go a long way to explaining what working/not working means.

What im trying (in both terms of the word I know) is to get the pupils to create a small toy.They will have hooked a motor up to a small garing system with turntable that has a ballet dancer on top (my feeble attempt to make engineering more approachable to girls). When the Master switch is pressed(MSPin) it switches the toy on which will switch on an ultrabrite LED and the motor will start to turn, causing the dancer to rotate. It will then stop when it hits a limit switch.

Once it hits this switch I want it to repeat these next step 5 times.

  • The motor will change direction (Clockwise to Anti-clockwise) to make the dancer turn the other way
  • It will hit another limit switch at the other side
  • The motor will change direction one more time (Anti-clockwise to clockwise)

After this the motor will stop and the LED will go off.

As Im at home instead of in school im using normal push switches instead of limit switches, and do not have any mechanisms hooked to my motor, but it will still simulate it the same way

Once it hits this switch I want it to repeat these next step 5 times

It probably would do all those things, but that "for" loop probably completes in about one millisecond.

AWOL:

Once it hits this switch I want it to repeat these next step 5 times

It probably would do all those things, but that "for" loop probably completes in about one millisecond.

thats what i was scared of - how can the sketch be changed then to fix this? I added a brace with the 2 if commands inside it hoping it would have to complete these before it moved onto the next loop, but im guessing it would skip right past these. I cant even add in a large delay at the end of each if to get around it that way as the final command wouldnt complete.

would an else command work? By putting this to go back to the original command if the button is off? Do I have to set up sub-procedures to do this?

..away to tinker with this option now

..OK - Ive tried coming at this from a different angle and using sub procedures to try and simplify it, but i still cant get it to work.

This is a flowchart of what Im wanting my sketch do

basically it is designed to be a dancing ballerina toy - when a master switch is pressed, and LED goes on to light up the dancer, the dancing ballerina spins clockwise, until the mechanism it is on hits a limit switch, which should then make it dance anticlockwise until it hits a limit switch at the other side. It will then repeat this clockwise and anticlockwise spinning 5 times. after this the LED and motor switches off.

this is how Ive got it wired up to the arduino Uno board

and this is my code:

/*
toy System - 
*/

int MSSwitchPin = 13;        //master switch
int CWSwitchPin = 12;        //switch to make motor go clockwise
int ACWSwitchPin = 8;        //switch to make motor go Anti Clockwise
int MSLEDPin = 2;            //LED 
int Motor1APin = 6; 	     // H-bridge leg 1
int Motor2APin = 7; 	     // H-bridge leg 2

boolean MSlastButton = LOW;                 
boolean MScurrentButton = LOW;      
boolean MSLEDon = false;

boolean CWlastButton = LOW;                 
boolean CWcurrentButton = LOW;      
boolean CWmotoron = false;

boolean ACWlastButton = LOW;                 
boolean ACWcurrentButton = LOW;      
boolean ACWmotoron = false;

void setup()
{
  pinMode(MSSwitchPin, INPUT);
  pinMode(CWSwitchPin, INPUT);
  pinMode(ACWSwitchPin, INPUT);
  pinMode(MSLEDPin, OUTPUT);
  pinMode(Motor1APin, OUTPUT);
  pinMode(Motor2APin, OUTPUT);
}
boolean MSdebounce(boolean last)            
{
        boolean current = digitalRead(MSSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(MSSwitchPin);      
}
return current;        
}

boolean CWdebounce(boolean last)            
{
        boolean current = digitalRead(CWSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(CWSwitchPin);      
}
return current;        
}

boolean ACWdebounce(boolean last)            
{
        boolean current = digitalRead(ACWSwitchPin); 
         if (last != current)                    
{
delay(5);              
current = digitalRead(ACWSwitchPin);      
}
return current;        
}




void loop()
{
  MScurrentButton = MSdebounce(MSlastButton); 
  CWcurrentButton = CWdebounce(CWlastButton); 
  ACWcurrentButton = ACWdebounce(ACWlastButton); 
  
if (MSlastButton == LOW && MScurrentButton == HIGH);  
   MSLEDon = !MSLEDon;
   digitalWrite(MSLEDPin, HIGH);
  for (int counter = 1 ; counter <= 5; counter = counter +1)
   {
      clockwise();
      anticlockwise();
   }
 MSLEDon = MSLEDon;
   digitalWrite(MSLEDPin, LOW);  
   digitalWrite(Motor1APin, LOW); 
   digitalWrite(Motor2APin, LOW);
 }
 


void clockwise()
{
  CWmotoron = !CWmotoron;
  digitalWrite(MSLEDPin, HIGH);
  if (digitalRead(CWSwitchPin) == HIGH)
      { 
       digitalWrite(Motor1APin, HIGH); 
       digitalWrite(Motor2APin, LOW);
      }
  else if (digitalRead(CWSwitchPin) == LOW)
    return;     //return to beginning of sub routine  
}


void anticlockwise()
{
  ACWmotoron = !ACWmotoron;
  digitalWrite(MSLEDPin, HIGH);
  if (digitalRead(CWSwitchPin) == HIGH)
  {
      digitalWrite(Motor1APin, LOW); 
      digitalWrite(Motor2APin, HIGH); 
    }
  else if (digitalRead(ACWSwitchPin) == LOW)
  return;      //return to beginning of subroutine
}

There are a few things Im almost certain are wrong, but I cant figure out how to fix it and cant seem to find the answers anywhere. Firstly Im not sure Im using the return command properly. Im trying to use my knowledge of basic and transfer it here, but its not working. I've also tried using the goto command (I know it seems to be hated with C++) and trying to link the sub routine back to the beginning of said subroutine if it doesn't follow the path I want - doesn't work for some reason. I need to do something like this to make sure it completes the rotation before it moves onto the next section.

I've also make it look more complicated due to the need to debounce my switches as well as trying to ensure that once the switch is pressed the motor stays on instead of switching back off straight away.

When i run the sketch there are no problems and it uploads OK, only problem is that when i push the master switch, the LED goes on (although there is a few seconds delay), but when the other buttons are pressed the LED dims, but no movement in the motor.

Using Arduino is something I've found extremely fun, and I want to be able to pass this onto my pupils, but at the same time it makes me want to cry(something I definitely DO NOT want to do or pass on this feeling of frustration to pupils!) can ANYONE help me please?

     for (int counter = 1 ; counter <= 5; counter = counter +1)
     {
       clockwise();
       anticlockwise();
     }

You're still rushing through these reversals in a few milliseconds at most - the motor simply cannot do this.

I'm going to get shouted-down, but I suggest to get you and the students started, that you put some delay()s in there.

You'll at least see some progress, and then discover why we normally suggest that you don't use delay ().

Also, the IDE has an auto-format feature that will help you sort out your indentation and make your code easier to read.

Did this one already get pointed out?   if (MSlastButton == LOW && MScurrentButton == HIGH); 
The semicolon MUST NOT go there.