I'll never understand functions

Hi all,
Still very much a newbie attempting the impossible.
I have a program to monitor 8 switches which control the position of 8 points.
All working fine but WAY too crude, doing the same thing 8 times in a row.
So I thought I’d add an FOR statement.
Then do the loop 8 times within the FOR using “x” as the increment.
It would shrink the program by an incredible amount.

Just can’t seem to get that going and the more I fiddle the more mysterious the error messages.
Then tried to call it as a function from within the for loop and that was even worse.

I have just spent 3 days solid reading all about if loops and about functions that there is and am no wiser.
Can someone please point out where I do an apparently stupid thing??

//have 8 switches floating/pulled low to change point position

#include <Servo.h>
#include <EEPROM.h>

Servo myservo1, myservo2, myservo3, myservo4, myservo5, myservo6, myservo7, myservo8;  // create servo object to control a servo

byte t1;                        //just for general use as a temp variable for switch pos.
byte t2;
byte t3;
byte t4;
byte t5;
byte t6;
byte t7;
byte t8;

byte x;
byte y;

byte val1;                  //these represent actual servo positions (either 80 or 100)
byte val2;
byte val3;
byte val4;
byte val5;
byte val6;
byte val7;
byte val8;

byte straight = 88;            //duration of the pulse to be sent if points are straight ahead
byte turn = 97;              //and if turning off

byte switch1 = A0;              //start using some analog pins here for input
byte switch2 = A1;
byte switch3 = A2;
byte switch4 = A3;
byte switch5 = A4;
byte switch6 = A5;
byte switch7 = 10;              //total of 8 switch pins
byte switch8 = 11;
byte led1 = 12;                 //2 LED pins for warning signals
byte led2 = 13;

void setup()
{
  pinMode(switch1, INPUT_PULLUP);    //set input pins to PULL UP so the switch can pull them down.
  pinMode(switch2, INPUT_PULLUP);    //simply earth it for LOW, let it float for HIGH
  pinMode(switch3, INPUT_PULLUP);
  pinMode(switch4, INPUT_PULLUP);
  pinMode(switch5, INPUT_PULLUP);
  pinMode(switch6, INPUT_PULLUP);
  pinMode(switch7, INPUT_PULLUP);
  pinMode(switch8, INPUT_PULLUP);
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);

  myservo1.attach(2);                 //attach all servos
  myservo2.attach(3);
  myservo3.attach(4);
  myservo4.attach(5);
  myservo5.attach(6);
  myservo6.attach(7);
  myservo7.attach(8);
  myservo8.attach(9);

//  pos = EEPROM.read (pos1);         //get the stored values from the eeprom

  val1 = straight;                   //set all points centre for now
  val2 = straight;                   //void loop will read the switches and fix the points position
  val3 = straight;
  val4 = straight;
  val5 = straight;
  val6 = straight;
  val7 = straight;
  val8 = straight;

  myservo1.write(val1);             //attach the servos
  myservo2.write(val2);
  myservo3.write(val3);
  myservo4.write(val4);
  myservo5.write(val5); 
  myservo6.write(val6);
  myservo7.write(val7);
  myservo8.write(val8);
}

void loop()
{
  for (x=0;x<=7;x++)
  {
   t(x)=digitalRead (switch(x); 
  }
}
  //This is where it all goes wrong.
  // I have left 1 iteration of the original loop attached for vetting purposes
  
/*  t1 = digitalRead (switch1);                 //A0 for in D2 for out
  if ((t1 == LOW) && (val1 == straight))
  {
    myservo1.write(turn);
    val1 = turn;                              // if switch doesn't agree with current setting tell servo to go to new position
  }
  if ((t1 == HIGH) && (val1 == turn))
  {
    myservo1.write (straight);
    val1 = straight;                          //and update val
  }
*/

The error I get is: ‘t’ was not declared in this scope

But the verbose code also says: 8_signals_mega:215: error: expected declaration before ‘}’ token

And: 8_signals_mega:210: error: expected unqualified-id before ‘else’

None of which makes sense to me.

Oodles of thanks in advance

Aussiewill

You need to read up on arrays. The numbered variables can each be stored in an array, and indexed by number.

For example,

byte val[8]={ ...8 comma separated values...}; // stores 8 servo positions

"stores 8 servo positions"
0 to 255 at each of val[0] to val[7], if that wasn't explained already.

Agreed, spend a little time learning about arrays. For example, you have a group of byte variables:

byte t1;
byte t2;
etc.

First, try to pick variable names that suggests their purpose, so:

byte tempPosition[8];

creates 8 byte-sized slots for storing your temporary positions. Note: In C, arrays start with element 0, not 1, so you have access to tempPosition[0] through tempPosition[7]. It’s still 8 elements, but the last valid element is tempPosition[7].

You also want 8 switches tied to specific pins. You can define an array, and tie each element to a specific pin using an array initializer list:

byte mySwitches[] = {A0, A1, A2, A3, A4, A5, 10, 11};

If you supply an initializer list like we did here, you do not have to fill in the number of elements in the array. The computer is pretty good at counting and will do it for you. Note that initializer lists start and end with a brace, and each element is separated with a comma after it, except the last one. After this array definition is compiled, mySwitches[5] will be associated with A5 and mySwitches[7] with pin 11.

Your code:

for (x=0;x<=7;x++)
  {
   t(x)=digitalRead (switch(x);
  }

has the idea of an array, but isn’t quite right. First, array elements use brackets, not parentheses, when used. So, t(x) should have used brackets, not parentheses. The same is true for your switch(x); you need to use brackets, not parentheses. Finally, digitalRead() is a function call, and it needs an opening and closing parentheses, you forgot the closing parentheses. So, making the corrections and using our variable names, your code would be rewritten:

for (x=0;x < 8;x++)   // More common to use less than (<) in expression #2
  {
   tempPosition[x] = digitalRead( mySwitches[x] );
  }

Spend a little time learning about arrays and you’ll see how they can simplify your code.

In complete seriousness: google "C++ tutorial", and do a C++ tutorial. You will have no luck trying to learn to program by copying examples and coming onto the board here getting advice in dribs and drabs. A tutorial will run you through the basics.

Keep in mind that the arduino programming environment has some oddities which a regular tutorial will not cover - we avoid C++ Strings and dynamic memory in general, there's setup() and loop() instead of main(). But the bulk of what you will learn applies.

Hi
Thanks to all for those those replies.
Unfortunately, they miss the point.
I HAD arrays to fill the switches, t variables etc. and that worked fine.
It is only when I tried to make a for loop to execute the routine at the end (everything AFTER the for in void loop() that I could not get anywhere.

So THAT was when I went back to my original code, the one that does it all by defining everything one at a time.

And yes, I read a series of articles on arrays and even more articles AND examples on functions including the tutorial on functions (which I found very confusing) till I am now more confused than before.

What I want to do is to call the routine at the end EIGHT times while substituting t(x) for t1,t2,etc and switch(x) for switch1, switch2 etc.

With the compiler giving me THREE different errors to confuse me even further I have tried all kinds of things such as:

calling adjust(); (which contained THAT routine). That was OK. As soon as I tried passing a parameter (x) by calling adjust (x); it declared it "void".

OR eg. in the "for" loop, as soon as I tried to use t(x) it said "t not defined in this context. Etc. etc.

Aussiewill

Apologies to econjack.

What you wrote is probably exactly what I was after and gives the solution, I think.

I missed it in all the talk about reading up on arrays and functions.
And yes, I had read about in arrays but for some reason thought that applied ONLY to defining the array. Quite often I seem to read a different meaning into things than meant.

Apologies again, and I’ll now go back with fresh hopes.

Thanks
Aussiewill

Sometimes you have to think on it before concepts settle in.

There are very good teaching sketches under examples in the IDE you should review.
If there is something in those examples you don’t understand, ask for clarification here, lots of volunteers ready to help.

Hello Aussiewill

aussiewill:
Hi

I HAD arrays to fill the switches, t variables etc. and that worked fine.
It is only when I tried to make a for loop to execute the routine at the end (everything AFTER the for in void loop() that I could not get anywhere.

I think that is ( was ?) false. Econjack is very OK.

Maybe (compiled but not tested) you could use something like :

//have 8 switches floating/pulled low to change point position

#include <Servo.h>
//#include <EEPROM.h>

Servo myservo[2]; // ---->>>  an array of servo

byte t[2];
byte x;
byte y;
byte val[2];                //these represent actual servo positions (either 80 or 100)
byte straight = 88;            //duration of the pulse to be sent if points are straight ahead
byte turn = 97;              //and if turning off


byte mySwitches[2] = {A0, A1};
byte led1 = 12;                 //2 LED pins for warning signals
byte led2 = 13;

void setup()
{
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  
for (x=0;x<=1;x++)
  {
   pinMode(mySwitches[x], INPUT_PULLUP); 
   myservo[x].attach(x-1);
   val[x] = straight;
   myservo[x].write(val[x]);
  }  
}

void loop()
{
  for (x=0;x<=1;x++)
  {
   Position_Servo(x);
  }
}
// *********************************************************************************
void Position_Servo( byte servo ) {
  
  //This is where it all goes wrong.
  // I have left 1 iteration of the original loop attached for vetting purposes
 
  t[servo] = digitalRead( mySwitches[servo] );
  if ((t[servo] == LOW) && (val[servo] == straight))    //A0 for in D2 for out
  {
    myservo[servo].write(turn);
    val[servo] = turn;                              // if switch doesn't agree with current setting tell servo to go to new position
  }
  if ((t[servo] == HIGH) && (val[servo] == turn))
  {
    myservo[servo].write (straight);
    val[servo] = straight;                          //and update val
  }
}

Regards,
bidouilleelec

Hi again,
Econjack, I copied your sample into my code but came up with this error:

expected primary-expression before ‘switch’

void loop()
{
for (x = 0; x < 8; x++)
{
temp = digitalRead (switch );
}
}

which seems to mean incorrect bracket use/location.
But I just can’t see it. What am I doing wrong now??

Aussiewill

@bidouilleelec

  myservo[x].attach(x-1);

You have a pin numbered -1?

for (x=0;x<=1;x++) These would normally, and more naturally, be written for (x=0;x < 2;x++) (though without the magic number 2)

aussiewill:
Hi again,
Econjack, I copied your sample into my code but came up with this error:

expected primary-expression before ‘switch’

void loop()
{
for (x = 0; x < 8; x++)
{
temp = digitalRead (switch );
}
}

which seems to mean incorrect bracket use/location.
But I just can’t see it. What am I doing wrong now??

Aussiewill

switch is a C++ keyword.

Please remember to use code tags when posting code .

And once more: Hi!

I decided to copy bidouilleelec’s code, as the easy (and may I say VERY compact) option.
I made some mods to accommodate some of my terminology but was VERY careful to keep the code intact.

//control 8 servos with 8 switches

//have 8 switches floating/pulled low to change point position

#include <Servo.h>
#include <EEPROM.h>

Servo myservo[8];             //create 8 servo objects

byte t [8];
byte x;                         //general use variables
byte y;

byte val[8];                  //for storing point positions

byte straight = 88;            //duration of the pulse to be sent if points are straight ahead
byte turn = 97;              //and if turning off

byte switch[8] = {A0, A1, A2, A3, A4, A5, 10, 11};   //8 switches


byte led1 = 12;                 //2 LED pins for warning signals
byte led2 = 13;



void setup()
{
  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);

  for (x = 0; x < 8; x++)
  {
    pinMode(switch[x], INPUT_PULLUP);         //already defined as A0 etc
    myservo[x].attach(x + 2);                 //x starts at 0 and servopins start at 2
    val[x] = straight;                        //fill all 8 val with value "straight
    myservo[x].write(val[x]);                 //and write it to all servos
  }
  }


    void loop()
  {
    for (x=0;x<8;x++)
  {
    Position_Servo(x);
  }
  }
//**********************************************

void Position_Servo( byte servo )
{
  t[servo] = digitalRead( switch[servo] );
  if ((t[servo] == LOW) && (val[servo] == straight))    //A0 for in D2 for out
  {
    myservo[servo].write(turn);
    val[servo] = turn;                              // if switch doesn't agree with current setting tell servo to go to new position
  }
  if ((t[servo] == HIGH) && (val[servo] == turn))
  {
    myservo[servo].write (straight);
    val[servo] = straight;                          //and update val
  }
}

HOWEVER: when I compile I get the following (apparently total of 3) errors which make no sense (to me).

8_signals_test.ino:19: error: expected unqualified-id before ‘switch’

byte switch[8] = {A0, A1, A2, A3, A4, A5, 10, 11}; //8 switches

^

C:\Users\wh_vi\AppData\Local\Temp\arduino_modified_sketch_572680\8_signals_test.ino.ino: In function ‘void setup()’:

8_signals_test.ino:34: error: expected primary-expression before ‘switch’

pinMode(switch, INPUT_PULLUP); //already defined as A0 etc

^

C:\Users\wh_vi\AppData\Local\Temp\arduino_modified_sketch_572680\8_signals_test.ino.ino: In function ‘void Position_Servo(byte)’:

8_signals_test.ino:53: error: expected primary-expression before ‘switch’

t[servo] = digitalRead( switch[servo] );

^

exit status 1
expected unqualified-id before ‘switch’

Can I once more cry for help???

Aussiewill

As pointed out previously, switch is a C++ keyword. Change the name of the array

Most of the talk here has been about arrays (all of which seems to have been relevant) but the title is about "functions" and it may be that the demo Several Things at a Time and Planning and Implementing a Program will help with that aspect.

I did not use arrays in either of them simply because I wanted to limit the scope of my tutorials. However the same concepts can easily be used with arrays.

...R

AWOL:
@bidouilleelec

  myservo[x].attach(x-1);

You have a pin numbered -1?

 for (x=0;x<=1;x++)

These would normally, and more naturally, be written

 for (x=0;x < 2;x++)

(though without the magic number 2)

That’s just an example.

bidouilleelec

What everyone else said. ‘switch’ is a C++ language keyword, it has special meaning to the compiler. You cannot name a variable or function ‘switch’.

If you have got arrays figured out, the next thing is structures.

//have 8 switches floating/pulled low to change point position

#include <Servo.h>
#include <EEPROM.h>

const byte straight = 88;            //duration of the pulse to be sent if points are straight ahead
const byte turn = 97;              //and if turning off

const byte led1 = 12;                 //2 LED pins for warning signals
const byte led2 = 13;

struct ServoManager {
  Servo myservo;
  byte switchPin;
  byte servoPin;
  byte t;  //just for general use as a temp variable for switch pos.
  byte val;                  //these represent actual servo positions (either 80 or 100)

  ServoManager(byte switchPinAttach, byte servoPinAttach) {
    switchPin = switchPinAttach;
    servoPin = servoPinAttach;
    val = straight;
  }

  void setup() {
    pinMode(switchPin, INPUT_PULLUP);
    myservo.attach(servoPin);
    myservo.write(val);
  }

  void loop() {
    t = digitalRead (switchPin);
    if ((t == LOW) && (val == straight))
    {
      myservo.write(turn);
      val = turn;                              // if switch doesn't agree with current setting tell servo to go to new position
    }
    if ((t == HIGH) && (val == turn))
    {
      myservo.write (straight);
      val = straight;                          //and update val
    }
  }
};

const int nServos = 8;

ServoManager servo[nServos] = {
  ServoManager(A0, 2),
  ServoManager(A1, 3),
  ServoManager(A2, 4),
  ServoManager(A3, 5),
  ServoManager(A4, 6),
  ServoManager(A5, 7),
  ServoManager(10, 8),
  ServoManager(11, 9)
};

byte x;
byte y;

void setup() {
  for (int i = 0; i < nServos; i++) {
    servo[i].setup();
  }

  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
}

void loop() {
  for (int i = 0; i < nServos; i++) {
    servo[i].loop();
  }
}

Hello AWOL,

Please, read all the thread.
And I wrote :
" you could use something like "
and
"That's just an example"

You wrote :
"These would normally, and more naturally, be written"

That's your mind.

bidouilleelec

Regards,
bidouilleelec

bidouilleelec:
That's your mind.

. . . and very, very many others.

Hi,

OK, final pasting.
Yes, I remember actually using the switch.......case argument somewhere but totally forgot it was a "reserved"word.

One thing that does sort of surprise me is that the compiler doesn't pick up that type of thing.
I would kind of think that if it encounters a command like that in those type of circumstances it should have the ability to bring up an error msg along the lines of: " check for naming conflicts".
After all, if it knows a constant or variable etc. is already defined, it picks THAT up ok and tells you.

Once again guys, loads of thanks and karma.

Aussiewill