Servo not respecting code

See @alto777 's post ...

Unless you are interested in programming the use of a button it is even not necessary.

You have a keypad with numbers and characters that you can easily solve your requirements.

If you use

  • 1..6 for the delay in minutes
  • 0 to stop the sketch (idle)
  • # to accept a new delay value
  • * to start the servo sequence

you should be able to handle everything.

Did I miss something?

I'll try using the keyboard altough I don't think that'll work I wanted something like "Oh Im in set mode let's input the time then disconnect the keypad, now I'll press a button and the countdown will begin....."

Wouldn't it be a more practical solution to use a second button to set the time instead of connecting/disconnecting the keyboard? You could also use the Leds to signalize the chosen value (e.g. by blinking in "set mode" and not blinking when the sequence has been started ...

If the sequence is running you can also use the start button to stop the sequence and go to set mode.

It more or less depends on your real intention: Is it to learn about Arduino and programming or (just) to solve the task you mentioned ... Makes a big difference.

Crafted by yours truly. Discover the power of code by exploring various examples and incorporating them into your own projects.

You might be interested to explore this

Sketch
// https://forum.arduino.cc/t/servo-not-respecting-code/1128506/2

#include <Servo.h>
#include <Key.h>
#include <Keypad.h>

// As it is a servo I prefer to call it myServo ... 
Servo myServo;

constexpr byte buttonPin = 11;


const byte ROWS = 4;
const byte COLS = 3;
char keys[ROWS][COLS] = {
  {'1', '2', '3'},
  {'4', '5', '6'},
  {'7', '8', '9'},
  {'*', '0', '#'}
};
byte rowPins[ROWS] = {2, 3, 4, 5};
byte colPins[COLS] = {6, 7, 8};
Keypad keypad = Keypad( makeKeymap(keys), rowPins, colPins, ROWS, COLS );


boolean setSequenceActive = false;
int newDelayMinutes = 0;
int acceptedDelayMinutes = 0;
int countDownMinutes;
unsigned long newDelayTime;
constexpr unsigned long waitTime = 1000;   // Should by 60000 ms but for test purposes just 1 sec

enum myStates  {DONOTHING, GO90, START180, WAITFOR180, WAITTOGOBACK};
myStates actState = GO90;


// To allow to handle the pins in for-loops we put their
// data in an array; this is the moste flexible way
// as the pin number do not necessarily have to be in a numerical sequence 
constexpr byte noOfPins = 6;
byte pins[noOfPins] = {A0, A1, A2, A3, A4, A5};


// As it looks that the first x pins shall be set HIGH, the rest LOW
// we can handle this in a simple for-loop
void setPins(int value) {
  for (int i = 0; i < noOfPins; i++) {
    if (i <= value - 1) {
      digitalWrite(pins[i], HIGH);
    } else {
      digitalWrite(pins[i], LOW);
    }
  }
}

void handleKeypad() {
    char key = keypad.getKey();
    if (key == '*') {
         Serial.println("Start sequence");
         actState = START180;
    }
    if (key >= '1' && key <= '6') {
      newDelayMinutes = key - '0';
      Serial.print("New delay value ");
      Serial.print(newDelayMinutes);
      Serial.println("\ min");
    }
    if (key == '#'){
      if (newDelayMinutes > 0) { // start sequence only if the acceptedDelayMinutes are greater than zero
         acceptedDelayMinutes = newDelayMinutes;
         Serial.print("Accepted delay value ");
         Serial.print(acceptedDelayMinutes);
         Serial.println(" min");
      }
    }
}

void setup() {
  Serial.begin(9600);
  pinMode(buttonPin,INPUT_PULLUP);
  for (int i = 0; i < noOfPins; i++) {
    pinMode(pins[i], OUTPUT);
  }
  myServo.attach(10);
  //myServo.write(90);
  actState = GO90;
}  

void loop() { 
  handleKeypad();
  stateMachine();
}

boolean buttonReleased(){
  static unsigned long lastChangeTime = 0;
  static byte lastBstate = HIGH;
  static byte actBstate = HIGH;
  boolean bReleased = false;
  byte bState = digitalRead(buttonPin);
  if (bState != lastBstate){
    lastChangeTime = millis();
    lastBstate = bState;
  }
  if (actBstate != lastBstate && millis()-lastChangeTime > 30){
    actBstate = lastBstate;
    if (actBstate == HIGH) {
       bReleased = true;     
    }
  }
 return bReleased;
}

void stateMachine(){
  static unsigned long startTime = 0;
  static unsigned long countDownStartTime= 0;
  switch(actState){
     case DONOTHING :
            if (buttonReleased() && acceptedDelayMinutes > 0) {
               actState = START180;
            }
       break;
     case GO90 :
            Serial.println("Going to 90");
            myServo.write(90);
            actState = DONOTHING;
            setPins(0);
       break;
     case START180 :
            Serial.println("Starting to WAITFOR180");
            startTime = millis();
            actState = WAITFOR180;
            countDownMinutes = acceptedDelayMinutes;
            countDownStartTime = startTime;
            setPins(acceptedDelayMinutes);
       break;  
     case WAITFOR180 :
       if (millis()-countDownStartTime >= waitTime){ 
          countDownStartTime = millis();
          countDownMinutes--;
          setPins(countDownMinutes); 
       }  
       if (millis()-startTime >= waitTime*acceptedDelayMinutes){ // Instead of delay()
          startTime = millis();
          myServo.write(180);
          actState = WAITTOGOBACK;
          Serial.println("Starting WAITTOGOBACK");
       }
       break;  
     case WAITTOGOBACK :
       if (millis()-startTime >= waitTime){ // Instead of delay()
          actState = GO90;
       }
      break;
      default:
         actState = GO90;
       break;  
  }
}

To be tested here

Features:

  • Set delay value to 1..6 (multiplied by waitTime = 1000; msec to speed up testing)
  • Accept the delay with #
  • Start sequence with * or a button release
  • Time signalized by LEDs
  • Count down of time by switching off LEDs one by one
  • Restart of sequence with the actual delay time with * or button release

There is still room for improvement (e.g. stopping the sequence, disabling to set a new delay time while a sequence is running).

But it might give you some inspirations ...

@CristyRO9 could put the contacts of the pushbutton across the row and column that '*' connects when presst.

Then either would be seen as a key stroke.

a7

A good idea to save pins!

@CristyRO9: Can you figure out which pins had to be used in parallel by a button to achieve a * press?

(A good task to understand how matrix keypads are evaluated :wink: )

You asked about a :-

That is what I showed you. Yeeees

If you don't understand simple push buttons and how they can be wired then it is time you read this:-
http://www.thebox.myzen.co.uk/Tutorial/Inputs.html
You will see there are two ways to wire up push buttons, your way or the correct way. That link hopefully explains why.

Your ideea sounds good, I tried to make a code to match it(I still have to add the sleep state to save power) here is the code:

#include <Servo.h>
Servo dt;
const int buttonPin = 2;
const int button_Pin2 = 9;
const int ledPin = 4;
int buttonState = 0;
int lastButtonState = 0;
int counter = 0;
int ledState = HIGH;
int buttonState2;            
int lastbuttonState2 = LOW;
unsigned long lastDebounceTime = 0;  
unsigned long debounceDelay = 50;

void setup()
{ 
  Serial.begin(9600);
  pinMode(buttonPin, INPUT_PULLUP); 
  pinMode(button_Pin2, INPUT);
  pinMode(ledPin, OUTPUT);
  digitalWrite(ledPin, ledState);
}

void loop()
{
  dt.write(0);
  buttonState = digitalRead(buttonPin);
  if (buttonState != lastButtonState)
  {
    if (buttonState == HIGH)
    {
      counter++;
      Serial.print("Number of button pushes:  ");
      Serial.println(counter);
    }
    else 
    {
      Serial.println("Button off");
    }
    delay(200);
  }
  lastButtonState = buttonState;
  digitalWrite(ledPin, LOW);
  if(counter = 1) {
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
 }
else if(counter = 2) {
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);                  
}
 else if(counter = 3) {
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);                     
}
else if(counter = 4) {
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
}
else if(counter = 5) {
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);    
}
 else {
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);  
  delay(1000);                   
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);    
  delay(1000);
  digitalWrite(ledPin, HIGH);  
  delay(1000);                     
  digitalWrite(ledPin, LOW);    
}
  
  int reading = digitalRead(button_Pin2);
  if (reading != lastbuttonState2) {
    lastDebounceTime = millis();
  }
  if ((millis() - lastDebounceTime) > debounceDelay) {
    if (reading != buttonState2) {
      buttonState2 = reading;
      if (buttonState2 == HIGH) {
        ledState = !ledState;
      }
    }
  }
  digitalWrite(ledPin, ledState);
  lastbuttonState2 = reading;
  while(buttonState2 == HIGH) {
  delay(counter * 60000);
  dt.write(180);
  }
}

Am I on the right way?
Btw whenever at the beginning I run the program the counter is equal to 1,is it a mistake from the code or is it because my pins are not soldered?

Let's say... partially :slight_smile:

You use too many delays where using the millis() functionality would be much better.

You have a lot of identical lines one after another that could be easier to write and maintain using for loops.

You have put almost all functions into loop(). It is much better to write separate functions with well defined names and interfaces and call them from loop.

Have a look at my examples.

You were told in no uncertian terms in your other thread:-

That the pins NEED to be soldered to your board. With out doing this you are just wasting EVERY BODIES time.

Should be
if(counter == 1)
Along with all your other 'if' tests.

The code is doing exactly what you told it to do.

Before the count variable is ever printed it is incremented. So if you start with count = 0 then it will be equal to 1 before it is printed for the first time.
Quick fix make count = -1 when you declare it.

1 Like

This does it with two buttons ... :slight_smile:

Sketch
// https://forum.arduino.cc/t/servo-not-respecting-code/1128506/2

#include <Servo.h>
#include <Key.h>
#include <Keypad.h>

// As it is a servo I prefer to call it myServo ... 
Servo myServo;

constexpr byte startPin = 2;
constexpr byte countPin = 9;
constexpr byte ledPin   =  4;



int blinkCounter = 0;
int newDelayMinutes = 0;
int acceptedDelayMinutes = 0;
int countDownMinutes;
unsigned long newDelayTime;
constexpr unsigned long waitTime = 1000;   // Should by 60000 ms but for test purposes just 1 sec

enum myStates  {DONOTHING, GO90, START180, WAITFOR180, WAITTOGOBACK};
myStates actState = GO90;


// To allow to handle the pins in for-loops we put their
// data in an array; this is the moste flexible way
// as the pin number do not necessarily have to be in a numerical sequence 
constexpr byte noOfPins = 6;
byte pins[noOfPins] = {A0, A1, A2, A3, A4, A5};


// As it looks that the first x pins shall be set HIGH, the rest LOW
// we can handle this in a simple for-loop
void setPins(int value) {
  for (int i = 0; i < noOfPins; i++) {
    if (i <= value - 1) {
      digitalWrite(pins[i], HIGH);
    } else {
      digitalWrite(pins[i], LOW);
    }
  }
}

void setup() {
  Serial.begin(9600);
  pinMode(startPin,INPUT_PULLUP);
  pinMode(countPin,INPUT_PULLUP);
  pinMode(ledPin,OUTPUT);
  for (int i = 0; i < noOfPins; i++) {
    pinMode(pins[i], OUTPUT);
  }
  myServo.attach(10);
  //myServo.write(90);
  actState = GO90;
}  

void loop() { 
  handleStartButton();
  handleCountButton();
  handleBlinking();
  stateMachine();
}

void handleBlinking(){
  static unsigned long lastBlinking = 0;
  if (blinkCounter > 0) {
     if (millis()-lastBlinking > 300) {
     boolean ledState = !digitalRead(ledPin);
     lastBlinking = millis();
     digitalWrite(ledPin, ledState);
     if (!ledState) blinkCounter--;
     }
  }

}

void handleStartButton(){
  if (startButtonReleased() && newDelayMinutes > 0){
     acceptedDelayMinutes = newDelayMinutes;
     actState = START180;
     Serial.print("Sequence started with delay [min] of ");
     Serial.println(acceptedDelayMinutes);
  }
}

void handleCountButton(){
  if (countButtonReleased() && blinkCounter == 0){
     newDelayMinutes++;
     if (newDelayMinutes > 6) {
       newDelayMinutes = 1;
     }
     Serial.print("Delay value [min] set to ");
     Serial.println(newDelayMinutes);
     blinkCounter = newDelayMinutes;
  }
}

boolean startButtonReleased(){
  static unsigned long lastChangeTime = 0;
  static byte lastBstate = HIGH;
  static byte actBstate = HIGH;
  boolean bReleased = false;
  byte bState = digitalRead(startPin);
  if (bState != lastBstate){
    lastChangeTime = millis();
    lastBstate = bState;
  }
  if (actBstate != lastBstate && millis()-lastChangeTime > 30){
    actBstate = lastBstate;
    if (actBstate == HIGH) {
       bReleased = true;     
    }
  }
 return bReleased;
}

boolean countButtonReleased(){
  static unsigned long lastChangeTime = 0;
  static byte lastBstate = HIGH;
  static byte actBstate = HIGH;
  boolean bReleased = false;
  byte bState = digitalRead(countPin);
  if (bState != lastBstate){
    lastChangeTime = millis();
    lastBstate = bState;
  }
  if (actBstate != lastBstate && millis()-lastChangeTime > 30){
    actBstate = lastBstate;
    if (actBstate == HIGH) {
       bReleased = true;     
    }
  }
 return bReleased;
}



void stateMachine(){
  static unsigned long startTime = 0;
  static unsigned long countDownStartTime= 0;
  switch(actState){
     case DONOTHING :
       break;
     case GO90 :
            Serial.println("Going to 90");
            myServo.write(90);
            actState = DONOTHING;
            setPins(0);
       break;
     case START180 :
            Serial.println("Starting to WAITFOR180");
            startTime = millis();
            actState = WAITFOR180;
            countDownMinutes = acceptedDelayMinutes;
            countDownStartTime = startTime;
            setPins(acceptedDelayMinutes);
       break;  
     case WAITFOR180 :
       if (millis()-countDownStartTime >= waitTime){ 
          countDownStartTime = millis();
          countDownMinutes--;
          setPins(countDownMinutes); 
       }  
       if (millis()-startTime >= waitTime*acceptedDelayMinutes){ // Instead of delay()
          startTime = millis();
          myServo.write(180);
          actState = WAITTOGOBACK;
          Serial.println("Starting WAITTOGOBACK");
       }
       break;  
     case WAITTOGOBACK :
       if (millis()-startTime >= waitTime){ // Instead of delay()
          actState = GO90;
       }
      break;
      default:
         actState = GO90;
       break;  
  }
}

Is it podsible to do it with only one button?

Possibly, but you'd need to code to reliably detect the difference between, for example, a long press and a short press.

That is one possibility!

I think there is another one also:

  • button release = add a minute and display the number chosen with the red leds while the green led is off
  • Next button release = Illuminate the green led and prepare to start the sequence in e.g. 4 seconds if the button is not released another time
  • If required press/release button again

This way you have to press/release the button until the chosen time is displayed by the red leds and the green led is off.

To start the sequence you have to press/release once more. The green led is switched on, after e.g. 4 seconds the sequence starts.

I will try that, since this project is almost working I need a bit of help for another, in order not to open a new tread, I will ask my question here. I have another project that involves a servo pointing at a compass position that I will select using a potentiometer. The thing is that the potentiometer doesn't appear to work in this code I serial print it and nothing comes, I tried it using another code and it worked.

#include <Adafruit_HMC5883_U.h>
#include <Wire.h>
#include <Servo.h>

Servo myservo;

int pot;
int set = 90;
int pos;
int data;
float Pi = 3.14159;

Adafruit_HMC5883_Unified mag = Adafruit_HMC5883_Unified(12345);

void setup() {
Serial.begin(9600);
pinMode(A0, INPUT);
myservo.attach(9);

}
void loop() {
  
  sensors_event_t event;
  mag.getEvent(&event);
  Serial.print("X: "); Serial.print(event.magnetic.x); Serial.print("  ");
  Serial.print("Y: "); Serial.print(event.magnetic.y); Serial.print("  ");
  float heading = (atan2(event.magnetic.y, event.magnetic.x) * 180) / Pi;
  pos = int(heading);
  float declinationAngle = 0.109;
  pos += declinationAngle;
  pot = analogRead(A0);
  pot = map(pot, 0, 1023, 0, 360);
  Serial.println(pot);
  data = pot - pos;
  set = 90 + data;
  constrain(set, 80, 110);
  myservo.write(set);
  delay(20);
}

I think I need to solder wires as you said...Can I solder Male to Male Jumpers wires from the breadboard, I mean I want to cut the metal tip and the plastic and use the pure wire with the rubber isolation to solder it to the Nano and the other components/sensors? Assuming the pins have been soldered on the board.

What I normally do is to solder all the header pins to the Arduino, and solder a strip socket to my strip board. This makes a really solid connection, much better than individual wires. Then I solder ordinary wires from the strip board to where it is going, or even mount the components on the board.
This is a board I fitted into a guitar shaped box. It also had three I2C devices that were not on the board but some way off. You can see I soldered three lots of 4 pins sticking out of the board so I could attach them using socketed ribbon cable.

Notice how some sockets switches and batteries are attached to the board by flying leads.

You can but this is a bit of a waste of money when a reel of wire is so cheap. So you can make them as long as you like.
I use stuff like this, available in all sorts of colours.

https://www.rapidonline.com/unistrand-10-0-1mm-equipment-wire-100m-62319

Would AWG wires work?

This part of your code seems to be crucial for what happens (or not):

  pot = analogRead(A0);
  pot = map(pot, 0, 1023, 0, 360);
  Serial.println(pot);
  data = pot - pos;
  set = 90 + data;
  constrain(set, 80, 110);
  myservo.write(set);

My suggestion:

Separate the part e.g. in a standalone sketch. Write some lines around where you set the variable pos to a fixed value, print the interim variables like data and set. If you cannot immediately identify the reason for the failure start modifying pos.

In this test sketch using delay(500) to reduce the printed lines would be ok.