fucntion call, digitalWrite not responding to integer

Hi all,
I'm having a problem with my step sequencer code.
Basically i have to do multiple digitalWrites again and again through out my code to switch channels on a multiplexer (mux sheild). I decided to try to use function calls, instead of specifying the digitalWrites through the code multiple times. This is theoretically working fine, however, the function i am trying to use just isnt responding to any integers. i need to change the value of "i" for each execution of the function, buts its not working, I have tried loads of combinations of placing the interger here and there, but with no luck.

am i misunderstanding what a function is capable of? or am i just making a silly mistake, i cant really tell from what i have read as none of the examples are close enough to my code to make a comparison, at least for me anyway.

any help would be hugely appreciated.

heres the code:

#define S0 5 
#define S1 4
#define S2 3
#define S3 2
#define Mux0 A0
#define Mux1 A1
#define Mux2 A2

//LED NAMES
#define Led1 Mux0
#define Led2 Mux0
#define Led3 Mux0
#define Led4 Mux0
#define Led5 Mux0
#define Led6 Mux0
#define Led7 Mux0
#define Led8 Mux0

#define Led9 Mux1
#define Led10 Mux1
#define Led11 Mux1
#define Led12 Mux1
#define Led13 Mux1
#define Led14 Mux1
#define Led15 Mux1
#define Led16 Mux1

#define Led17 Mux1
#define Led18 Mux1
#define Led19 Mux1
#define Led20 Mux1
#define Led21 Mux1
#define Led22 Mux1
#define Led23 Mux1
#define Led24 Mux1

#define Led25 Mux0
#define Led26 Mux0
#define Led27 Mux0
#define Led28 Mux0
#define Led29 Mux0
#define Led30 Mux0
#define Led31 Mux0
#define Led32 Mux0

#define Led33 A8
#define Led34 A9
#define Led35 A10
#define Led36 A11
#define Led37 A15
#define Led38 A14
#define Led39 A13
#define Led40 A12


int beatStep = 1;
int counter = 1;
byte midi_start = 0xfa;
byte midi_stop = 0xfc;
byte midi_clock = 0xf8;
byte midi_continue = 0xfb;
int play_flag = 0;
byte data;

int voice1[16];


int i=0;
void muxWrite(){
  digitalWrite(S0, (i&15)>>3); //S3
  digitalWrite(S1, (i&7)>>2);  //S2
  digitalWrite(S2, (i&3)>>1);  //S1
  digitalWrite(S3, (i&1));     //S0
}

void setup()
{



Serial.begin (115200); 
  
  //4 MUX CONTROLLER PINS       (DIGITAL OUTPUTS)
  pinMode(S0, OUTPUT);
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(S3, OUTPUT);
  pinMode(Mux0,OUTPUT);      //16 ANALOGUE INPUT ON MUX 0  (ANALOGUE INPUT)
  pinMode(Mux1,OUTPUT);     //16 LED OUTPUT ON MUX 1      (DIGITAL OUTPUT)
  pinMode(Mux2,INPUT);     //16 LED OUTPUT ON MUX 2      (DIGITAL OUTPUT)



  pinMode(13,OUTPUT);       

}  




void loop()
{
  //#######################################################################################################################################################################################################  


  if(Serial.available() > 0) {
    data = Serial.read();
    if(data == midi_start || data == midi_continue) {
      play_flag = 1;
    }
    else if(data == midi_stop) {
      play_flag = 0;
    }
    else if((data == midi_clock) && (play_flag == 1)) {
      counter ++;
    }

    if (data == midi_start){
      counter = 1;
      beatStep = 1;
    }
    if (counter > 24){
      counter = 1;
      beatStep ++;
    }
    if (beatStep > 4){
      beatStep = 1;
    }
  }

  if (voice1[1] == 0){

    if ((counter/4 == 1) && (beatStep == 1)){
      i = 15;
      muxWrite;

      digitalWrite(Led29,HIGH);
    }
    else digitalWrite(Led29,LOW);
  }

 } //VOID LOOP OUT

You aren't calling the function properly. Change this "muxWrite;" to this "muxWrite();".

It would be better if you wrote the function so that you pass the integer value of the pins as an argument. That's left as an exercise for the reader.

Pete

Hurrah!

thank you, i appreciated that, i changed these 2 things;

void muxWrite(int i=0){
  digitalWrite(S0, (i&15)>>3); //S3
  digitalWrite(S1, (i&7)>>2);  //S2
  digitalWrite(S2, (i&3)>>1);  //S1
  digitalWrite(S3, (i&1));     //S0
}
if (voice1[1] == 0){

    if ((counter/4 == 1) && (beatStep == 1)){

      muxWrite(15);

      digitalWrite(Led29,HIGH);
    }
    else digitalWrite(Led29,LOW);
  }

i kept getting the error "i wasn't declared in this scope"
i didnt realise that the function would automatically assign the (15) to i, i kept trying int i=15; and i=15;

thanks, i tried to wirte it as an argument on one of my attampts beforehand, i just didnt send the 15 to the function correctly.

anyway, thanks given
noxxi

  digitalWrite(S0, (i&15)>>3); //S3

What do you suppose (0 & 15) returns? How can that possibly ever differ from 0?

What is the use of right shifting 0?

the value of i is only zero if the function is called without argument (0 is default) or if the param is explicit zero.

That said the function can be made shiftless, because digitalWrite() interprets all non zero values as HIGH

void muxWrite(int i=0)
{
  digitalWrite(S0, i & 8);  // just test for the bit that would become 1 in the shift version
  digitalWrite(S1,  i & 4);  
  digitalWrite(S2, i & 2); 
  digitalWrite(S3, i & 1);
}

removed the comments as these were in conflict with the code ...

PaulS:

  digitalWrite(S0, (i&15)>>3); //S3

What do you suppose (0 & 15) returns? How can that possibly ever differ from 0?

What is the use of right shifting 0?

to be honest, i'm not really sure, its taken from the Mayhew Labs mux sheild page, i would suppose that 0&15 returns 0? which is fine, i want it to be 0 in some cases, its easier just to keep the code the same all the way through and only change the integer at each call. am i missing something here? because to be honest, i dont really undertsnad that line myself, i just know it works.

robtillaart:
the value of i is only zero if the function is called without argument (0 is default) or if the param is explicit zero.

That said the function can be made shiftless, because digitalWrite() interprets all non zero values as HIGH

void muxWrite(int i=0)

{
  digitalWrite(S0, i & 8);  // just test for the bit that would become 1 in the shift version
  digitalWrite(S1,  i & 4); 
  digitalWrite(S2, i & 2);
  digitalWrite(S3, i & 1);
}



removed the comments as these were in conflict with the code ...

ahh, after reading through that and refering to the arduino reference page, and checking out some binary, its become clear to me that the purpose of (S0, (i&15)>>3) and so on, is to always output either 0 or 1 (low or high) at the end of the operation, i never realised (even though it now seems obvious) that it could be 0 to whatever and still work.

so i'm guessing that the >> is solely to convert the output to either 1 or 0 and nothing else? (i is never > 15)

also could you tell me, is it sloppy to use bitshift? its obviously a tidier solution as far as output goes, but the actual value of the output is totally non importnt in this case, am i going to get undesirable effects from using bitshift?

thanks
noxxi

so i'm guessing that the >> is solely to convert the output to either 1 or 0 and nothing else? (i is never > 15)

also could you tell me, is it sloppy to use bitshift? its obviously a tidier solution as far as output goes, but the actual value of the output is totally non importnt in this case, am i going to get undesirable effects from using bitshift?

the >> moves/shifts the bits so that you are left with a 1 or 0 indeed. The fact that the shiftless code works is because the digitalWrite interprets everything that is not LOW as HIGH. If it had a stricter interface the conversion to 0/1 would be mandatory.

It is not sloppy to use bit shift, on contrary it makes the code more explicit what you mean.

If the Arduino team decides to make digitalWrite() with an additional parameter e.g. LOW, HIGH, and lets say INVERT (inverts the current state) then the shiftless version would become trickier as not all non zero values mean HIGH.

THe only effect you get from the bitshift and the mask is that it costs a bit extra time few micros()...

thanks robtillaart, i thought it might use more processing time, but i dont think its critical to my current project, thanks for the clarification though. i think i will leave it in place, it seems more like the right way to go about it, even though its less legible, as long as i know what it does then its fine :slight_smile: