Code not controlling Servo

Hi all,

I have written code to control a servo using an Xbox 360 controller, however it doesn't seem to write the position to the servo to make it move.

I know that the servo is connected correctly to the Arduino Uno as I have been able to control it via the Serial. I know that the Xbox 360 is communicating correctly as it is able to send commands which can be read via the Serial.

My question is at what point does this code not write the inputted value to the servo? How can I correct this?

A potential solution I've seen is to use a refresh() command - this doesn't seem to work for me.

Here is the code:

/*
 Inspiration from:
 
 Example sketch for the Xbox Wireless Reciver library - developed by Kristian Lauszus
 It supports up to four controllers wirelessly
 For more information see the blog post: http://blog.tkjelectronics.dk/2012/12/xbox-360-receiver-added-to-the-usb-host-library/ or
 send me an e-mail:  kristianl@tkjelectronics.com

 Six servo controller(Xbox360 Wireless)
 Barrett Anderies
 May 25, 2014
 Modified by Sergei Dines
 August 3, 2014
 */

#include <XBOXRECV.h>
#include <Servo.h>

// Satisfy the IDE, which needs to see the include statment in the ino too.
#ifdef dobogusinclude
#include <spi4teensy3.h>
#include <SPI.h>
#endif
//================================   Definitions    ================================//

//#define SERVO1_INIT 90 //Define initial servo position

//================================ Global Variables ================================//
USB Usb;
XBOXRECV Xbox(&Usb);

Servo servo1; //Instantiate servo variables

int s1 = 0; //Define variables to store servo positions ant to set to initial position

//================================ Setup ================================//
void setup() {
  Serial.begin(9600);
#if !defined(__MIPSEL__)
  while (!Serial); // Wait for serial port to connect - used on Leonardo, Teensy and other boards with built-in USB CDC serial connection
#endif
  if (Usb.Init() == -1) {
    Serial.print(F("\r\nOSC did not start"));
    while (1); //halt
  }
  Serial.print(F("\r\nXbox Wireless Receiver Library Started"));

  servo1.attach(9); 
  
}

void loop() {
  Usb.Task();
  if (Xbox.XboxReceiverConnected) {
    for (uint8_t i = 0; i < 4; i++) {
      if (Xbox.Xbox360Connected[i]) {
        if (Xbox.getAnalogHat(LeftHatX, i) > 7500 || Xbox.getAnalogHat(LeftHatX, i) < -7500 || Xbox.getAnalogHat(LeftHatY, i) > 7500 || Xbox.getAnalogHat(LeftHatY, i) < -7500 || Xbox.getAnalogHat(RightHatX, i) > 7500 || Xbox.getAnalogHat(RightHatX, i) < -7500 || Xbox.getAnalogHat(RightHatY, i) > 7500 || Xbox.getAnalogHat(RightHatY, i) < -7500) {
         
          if (Xbox.getAnalogHat(LeftHatY, i) > 7500 || Xbox.getAnalogHat(LeftHatY, i) < -7500) {
            Serial.print(F("LeftHatY: "));
            Serial.print(Xbox.getAnalogHat(LeftHatY, i));
            Serial.print("\t");
            
            s1 = map(Xbox.getAnalogHat(LeftHatY, i), -32757, 32757, 0, 179);
            
            Serial.print(F("Output to servo: "));
            Serial.print(s1);
            Serial.print("\t");
          }
          Serial.println();
        }
        servo1.write(s1);
      }
    }
    
  }
  delay(20);
}

Thanks for any suggestions!

        if (Xbox.getAnalogHat(LeftHatX, i) > 7500 || Xbox.getAnalogHat(LeftHatX, i) < -7500 || Xbox.getAnalogHat(LeftHatY, i) > 7500 || Xbox.getAnalogHat(LeftHatY, i) < -7500 || Xbox.getAnalogHat(RightHatX, i) > 7500 || Xbox.getAnalogHat(RightHatX, i) < -7500 || Xbox.getAnalogHat(RightHatY, i) > 7500 || Xbox.getAnalogHat(RightHatY, i) < -7500) {

My finger got tired of scrolling. White space, like carriage returns, means next to nothing to the compiler, but it means a lot to humans. I can't remember what the first part of that statement looked like by the time I get to the end. Put carriage returns in there so scrolling is not needed.

Stop reading the controller so damned many times. Read the left value ONCE. Read the right value ONCE. Print the values. Then, make decisions on the stored values.

Your print statement that says you are outputting something to the servo is nonsense. You may not actually be writing that to the servo. Put the print statement with the write() statement so it can be believed.

How is it possible for a human to make sense of this complex IF statement

if (Xbox.getAnalogHat(LeftHatX, i) > 7500 || Xbox.getAnalogHat(LeftHatX, i) < -7500 || Xbox.getAnalogHat(LeftHatY, i) > 7500 || Xbox.getAnalogHat(LeftHatY, i) < -7500 || Xbox.getAnalogHat(RightHatX, i) > 7500 || Xbox.getAnalogHat(RightHatX, i) < -7500 || Xbox.getAnalogHat(RightHatY, i) > 7500 || Xbox.getAnalogHat(RightHatY, i) < -7500)

I think there are 8 parts to this which would mean 255 possibilities to get it wrong.

And, as @PaulS has said, each piece of the IF statement could be getting a different value from Xbox.getAnalogHat()

Get the values from the controller and save them so you can re-use the same values in every test. Then re-write the complex IF statement as a series of simple cascaded IF statments. That way you can put in print() statements so you can debug the process piece by piece.

...R

So what values are you seeing from this code:

            Serial.print(F("Output to servo: "));
            Serial.print(s1);

johnwasser - I see a series of numbers from 0 to 179 depending on where I'm putting the joystick.

Taking into account your tips, my new code reads as follows;

#include <XBOXRECV.h>
#include <Servo.h>

// Satisfy the IDE, which needs to see the include statment in the ino too.
#ifdef dobogusinclude
#include <spi4teensy3.h>
#include <SPI.h>
#endif
//================================   Definitions    ================================//

int s1 = 0;

//================================ Global Variables ================================//
USB Usb;
XBOXRECV Xbox(&Usb);

Servo servo1; //Instantiate servo variables

//================================ Setup ================================//
void setup() {
  Serial.begin(9600);
#if !defined(__MIPSEL__)
  while (!Serial); // Wait for serial port to connect - used on Leonardo, Teensy and other boards with built-in USB CDC serial connection
#endif
  if (Usb.Init() == -1) {
    Serial.print(F("\r\nOSC did not start"));
    while (1); //halt
  }
  Serial.print(F("\r\nXbox Wireless Receiver Library Started"));
  servo1.attach(9); 
  servo1.write(90);
}

void loop() {
  Usb.Task();
  if (Xbox.XboxReceiverConnected) {
    for (uint8_t i = 0; i < 4; i++) {
      if (Xbox.Xbox360Connected[i]) {
        s1 = map(Xbox.getAnalogHat(LeftHatY, i), -32757, 32757, 0, 179);
      }
//      else
//      {
//        servo1.write(90);
//      }
    }
    
  }
  servo1.write(s1);
  Serial.print(F("Value written to servo: "));
  Serial.print(servo1.read());
  Serial.println();
  delay(20);
}

Again, I am having the same issue where the value isn't being written/sent to the servo. I have been able to control the servo using an analog joystick so I'm confident the circuit is correct!

Could it be that I'm sending it too many positions?

Thanks for your suggestions so far, any more would be great!

Have you tried different pins for the servo? Maybe pin 9 is used by the USB shield.

That's exactly it! Thanks!

I blindly thought it wouldn't matter, but looking at the documentation for the usb shield it was the case!

Perhaps I should have applied Robin2's signature more.