1

I am trying to get an Arduino to work with some Adafruit Motor shields. Everything seemed to work fine, but when I tried to create a function, I get the error in the subject. As far as I can tell, everything was declared properly (the array is initialized and is indexed elsewhere with without issue). Only the function seems to cause an issue.

Would anyone have a guess at what's wrong?

    //////// Imports
    #include <Wire.h>
    #include <Adafruit_MotorShield.h>
    #include "utility/Adafruit_PWMServoDriver.h"

    ////////My globals. Times are in milliseconds
    unsigned long currentTime = 0;
    unsigned long elapsedTime = 0;
    unsigned long startTime = 0;
    byte currentMotor = 1;
    byte currentShield = 1;
    byte motorRunning = 0;

    //// Settings

    // How many seconds between motors
    unsigned long endTime = 4000;

    // How many shields and motors per shield
    const int nShields = 1;
    const int nMotorsPerShield = 3;

    /////////MicroManager globals
    unsigned int version_ = 2;
    const unsigned long timeOut_ = 1000;
    byte currentPattern_ = 0;
    unsigned long time;

    /////////Adafruit globals
    Adafruit_MotorShield afms[nShields];
    Adafruit_DCMotor *motor[nShields][nMotorsPerShield];

    ////////////Setup
    void setup() {
      Serial.begin(57600);

      unsigned char address = 96;

      for (int i = 0; i < nShields; i++) {
        afms[i] = Adafruit_MotorShield(address);
        address = ++address;
        afms[i].begin();
      }

      for (int i = 0; i < nShields; i++) {
        for (int j = 0; j < nMotorsPerShield; j++) {
          motor[i][j] = afms[i].getMotor(j + 1);
          motor[i][j]->setSpeed(255);
        }
      }

      pinMode(13, OUTPUT);
    }

    ////////////////////Main
    void loop() {
      currentTime = millis();
      elapsedTime = currentTime - startTime;

      if (elapsedTime > endTime) {
        startTime = currentTime;
        currentMotor = currentMotor + 1;
        if (currentMotor > nMotorsPerShield) {
          currentMotor = 1;
        }
      }

      if (Serial.available() > 0) {
        int inByte = Serial.read();

        switch (inByte) {
          case 1:
            if (waitForSerial(timeOut_)) {
              currentPattern_ = Serial.read();
              if (currentPattern_ == 0) {
                for (int i = 0; i < nShields; i++) {
                  for (int j = 0; j < nMotorsPerShield; j++) {
                    motor[1][j]->run(RELEASE);
                  }
                }
              }
              else {
                switchMotor();
              }
              Serial.write( byte(1));
              digitalWrite(13, HIGH);
            }
            break;

          case 30:
            Serial.println("MM-Ard");
            break;

          case 31:
            Serial.println(version_);
            break;
        }
      }

      digitalWrite(13, LOW);
    }


    ////////////////Function definitions

    bool waitForSerial(unsigned long timeOut)
    {
      unsigned long starTime = millis();
      while (Serial.available() == 0 && (millis() - starTime < timeOut) ) {}
      if (Serial.available() > 0)
        return true;
      return false;
    }

    void switchMotor()
    {
      int shield = currentShield - 1;
      int motor = currentMotor - 1;
      for (int i = 0; i < nShields; i++) {
        for (int j = 0; j < nMotorsPerShield; j++) {
          if (i == shield && j == motor)
            motor[i][j]->run(FORWARD);
          else
            motor[i][j]->run(RELEASE);
        }
      }
    }
1
  • 2
    Please post the full error message from the compiler. And the code where it occurs. Commented Feb 11, 2016 at 20:04

2 Answers 2

2

Take a close look at this line:

motor[1][j]->run(RELEASE);

Looks like it should be:

motor[i][j]->run(RELEASE);

Otherwise it is an illegal index (as there is only one shield).

The next error is in the function switchMotor(). Again, have a close look at the definition of "motor". There is a local variable with that name but you want to also access the global array with the same name.

void switchMotor()
{
  int shield = currentShield - 1;
  int motor = currentMotor - 1;
      ^
  for (int i = 0; i < nShields; i++) {
    for (int j = 0; j < nMotorsPerShield; j++) {
      if (i == shield && j == motor)
        motor[i][j]->run(FORWARD);
        ^
      else
        motor[i][j]->run(RELEASE);
    }
  }
}

My guess is that this is where the compiler give you the error message!

Cheers!

3
  • To be able to help you must add the compiler error output with line, file, and the code. It is too much guessing otherwise. Commented Feb 11, 2016 at 21:00
  • Changing the int motor = and the j == motor to xmotor instead of motor allows it to compile perfectly fine for me. Just get rid of that variable naming conflict and it should be fine.
    – Majenko
    Commented Feb 11, 2016 at 22:56
  • Thanks Mikael, the motor naming conflict was the problem! Commented Feb 15, 2016 at 12:23
0

I get a warning flag in UECIDE for the line:

address = ++address;

‣ operation on 'address' may be undefined [-Wsequence-point]

Increment address, and then assign the new value from address into address. That seems a bit odd to me. I think you just meant to increment:

address++;

Or

address = address + 1;

Also it flags up the 1 instead of i that Mikael found:

‣ array subscript is above array bounds [-Warray-bounds]

And of course there is the (somewhat hard to spot - Kudos to Mikael for finding it - it took me a bit to see it myself) naming conflict:

‣ invalid types 'int[int]' for array subscript

It was only when I used the advanced editing facilities of UECIDE to highlight all occurrences of motor that I found there was extra ones that shouldn't have been there causing the problem.

enter image description here

So just changing the integer variable name from motor to something else fixes it:

void switchMotor()
{
  int shield = currentShield - 1;
  int thismotor = currentMotor - 1;       // RENAMED MOTOR
  for (int i = 0; i < nShields; i++) {
    for (int j = 0; j < nMotorsPerShield; j++) {
      if (i == shield && j == thismotor)  // RENAMED MOTOR
        motor[i][j]->run(FORWARD);
      else
        motor[i][j]->run(RELEASE);
    }
  }
}
1
  • Cheers, that was the issue! Commented Feb 15, 2016 at 12:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.