Reversing Mux Pins...

So, I’ve got a basic piano sketch below, and I’ve managed to change it from using an 8 channel mux, to a 16 channel, that’s all good…

When I was building my stripboard version it was going to be a pain to wire the switches correctly due to the orientation of both the stripboard and the breakout boards for my 4067’s, so I wired them in reverse thinking I’ll fix the code later… But I’m lost.

I’ve wandered into bitwise territory and from the bits I can remember of binary from school, and my mediocre match brain, I can’t figure out how to do it.

So, the code is below, when the “buttons” are pressed, they return their number to the key[0]…

Currently what return “16” I want to return “0”, and what returns “15” I want to return “1”, and so on and so forth.

I’m pretty sure I need to change something in void getButtons();, but I’m just not sure how to go about it.

Any help would be HUGELY appreciated, otherwise I’ll have to rebuild my board. PLUS, I’d love to actually understand why it’s going wrong…

This happens in the loop…

  //get the state of the buttons
  getButtons();
  
  //Get up to 4 keys
  k = 0;
  key[0] = key[1] = key[2] = key[3] = 0;
  for (i = 0; i < 16; i++) {            // read through buttons
    if ( !((buttons >> i) & 1) ) {
      key[k] = i + 1;
      //      Serial.println(key[0]);
      k++;
      k &= 0x3;
    }
  }

Calling up this bit of code… which I believe returns bits (stuff like 4294967295 and 4294965247)
and then somehow gets turn back to decimals around key[0] in the section above.

void getButtons(void) {
  buttons = 0;
  for (i = 0; i < 16; i++) {
    digitalWrite(MUX_SEL_A, i & 1);
    digitalWrite(MUX_SEL_B, (i >> 1) & 1);
    digitalWrite(MUX_SEL_C, (i >> 2) & 1);
    digitalWrite(MUX_SEL_D, (i >> 3) & 1);
    buttons |= digitalRead(MUX_OUT_0) << i;
  }
  buttons <<= 16;
  for (i = 0; i < 16; i++) {
    digitalWrite(MUX_SEL_A, i & 1);
    digitalWrite(MUX_SEL_B, (i >> 1) & 1);
    digitalWrite(MUX_SEL_C, (i >> 2) & 1);
    digitalWrite(MUX_SEL_D, (i >> 3) & 1);
    buttons |= digitalRead(MUX_OUT_1) << i;
  }
//  buttons |= 0x1000000;  // for the 25th button
//  buttons &= ~0x1000000;

  //  Serial.println(buttons);
  old25 = pin25;
  pin25 = digitalRead(KEY_25);
}

Currently what return “16” I want to return “0”, and what returns “15” I want to return “2”, and so on and so forth.

“15” returns 1, surely?

Otherwise, if it’s purely a random reassignment, you will need a table to do the mapping.

Edit…

 void getButtons(void) {
  buttons = 0;
  uint16_t mask = 0x8000;
  for (i = 0; i < 16; i++) {
    digitalWrite(MUX_SEL_A, i & 1 ? HIGH : LOW);
    digitalWrite(MUX_SEL_B, (i >> 1) & 1 ? HIGH : LOW);
    digitalWrite(MUX_SEL_C, (i >> 2) & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_D, (i >> 3) & 1  ? HIGH : LOW);
    buttons |= digitalRead(MUX_OUT_0) == HIGH ? mask : 0;
    mask >>= 1;
  }
  buttons <<= 16;
  uint16_t mask = 0x8000;
  for (i = 0; i < 16; i++) {
    digitalWrite(MUX_SEL_A, i & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_B, (i >> 1) & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_C, (i >> 2) & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_D, (i >> 3) & 1  ? HIGH : LOW);
    buttons |= digitalRead(MUX_OUT_1) == HIGH ? mask : 0;
    mask >>= 1;
  }
//  buttons |= 0x1000000;  // for the 25th button
//  buttons &= ~0x1000000;

  //  Serial.println(buttons);
  old25 = pin25;
  pin25 = digitalRead(KEY_25);
}

Don’t know if you want the upper and lower 16 bits swapped or not.
If you do it is easily enough achieved by swapping MUX_OUT_0 / MUX_OUT_1.
Also you could probably read both mud in a single loop if you were so inclined.

pcbbc:
“15” returns 1, surely?

Thanks for that, you’re right, I’ve edited the first post.

pcbbc:
Edit…

 void getButtons(void) {

buttons = 0;
  uint16_t mask = 0x8000;
  for (i = 0; i < 16; i++) {
    digitalWrite(MUX_SEL_A, i & 1 ? HIGH : LOW);
    digitalWrite(MUX_SEL_B, (i >> 1) & 1 ? HIGH : LOW);
    digitalWrite(MUX_SEL_C, (i >> 2) & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_D, (i >> 3) & 1  ? HIGH : LOW);
    buttons |= digitalRead(MUX_OUT_0) == HIGH ? mask : 0;
    mask >>= 1;
  }
  buttons <<= 16;
  uint16_t mask = 0x8000;
  for (i = 0; i < 16; i++) {
    digitalWrite(MUX_SEL_A, i & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_B, (i >> 1) & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_C, (i >> 2) & 1  ? HIGH : LOW);
    digitalWrite(MUX_SEL_D, (i >> 3) & 1  ? HIGH : LOW);
    buttons |= digitalRead(MUX_OUT_1) == HIGH ? mask : 0;
    mask >>= 1;
  }
//  buttons |= 0x1000000;  // for the 25th button
//  buttons &= ~0x1000000;

//  Serial.println(buttons);
  old25 = pin25;
  pin25 = digitalRead(KEY_25);
}

So this code would use a mask to rearrange the bits and give me reversed keys? Or is the “HIGH : LOW” swapping the bits round… (Sorry, not to hot on the Bits/Bytes side of things…) and what is the mask doing?

I tried swapping the pins to MUX_OUT_0 and MUX_OUT_1 round and it just changed the multiplexer I was addressing…

Any suggestions on improving the code would be great! How would I do it in a single loop?

Thanks for the pointers!

Currently what return "16" I want to return "0", and what returns "15" I want to return "1", and so on and so forth.

Why not leave the fancy code that you don't understand alone, and just subtract your value from 16?

aarg:
Why not leave the fancy code that you don't understand alone, and just subtract your value from 16?

Well... I’d like to understand the code? I’ve managed to figure out the rest of it, but my background is more circuits than programming. This is just a fun learning experience.

Also, if I subtract 16, 15 would be -1, etc, it wouldn’t swap my keys round, right?

neo_nmik:
Well... I’d like to understand the code? I’ve managed to figure out the rest of it, but my background is more circuits than programming. This is just a fun learning experience.

Also, if I subtract 16, 15 would be -1, etc, it wouldn’t swap my keys round, right?

No subtract the value from 16. So 16 - 15 = 1. Honestly, if that doesn't click with you, having the rest of it explained to you, may not be the fun experience you seek. Not now, at least...

pcbbc:
Don’t know if you want the upper and lower 16 bits swapped or not.
If you do it is easily enough achieved by swapping MUX_OUT_0 / MUX_OUT_1.
Also you could probably read both mud in a single loop if you were so inclined.

By the way, this worked great!

It returned an error on redeclaring the u_int mask, so I removed the “u_int” and it compiled. Not getting any of the 2nd mux’s keys, but I wasn’t before... don’t think it’s a circuit Issues as I can get it to work by removing the first read section, but I figure it!

Cheers for the help!

aarg:
No subtract the value from 16. So 16 - 15 = 1. Honestly, if that doesn’t click with you, having the rest of it explained to you, may not be the fun experience you seek. Not now, at least…

:-X

Of course! :o . Been a long day, sorry. Perfect solution.

I would like to understand the actual mechanism behind reading the Mux though. I did just copy the code, and I do always like to know what’s there and why it’s there. :slight_smile:

Cheers for the input!

Of course there are multiple ways to achieve the same end result. Did you happen to notice, there is not a single comment line "//" in the entire function, except the useless and redundant " //get the state of the buttons"? Wow, I guess that must be what a function called "getButtons" does. Thanks for the heads up... :slight_smile:

That is a common coding style. It's basically "counting coup" because it's like, "the code speaks for itself" or "if you need to be told what it's doing, you're not as smart as I am", and so on. This psychology was discredited in professional circles a long time ago. It's much more common, sadly to say, with hardware engineer written software, than with code written by professional programmers.

The code syntax should be legible by itself, yes, but it is only really a framework on which to build readable, understandable code. Actually, you have been victimized by this code, because it forces the reader to treat it as a Mensa puzzle to solve. That's fine if the entire sketch is only a few lines, but frequently we see posts here with several thousand undocumented lines of this kind of ---.

It's not that one should not take advantage of certain abbreviations and conveniences offered by the language, but it should be done delicately and selectively. Here this is not the case. Inline documentation would have saved it, but it is absent.

aarg:
Of course there are multiple ways to achieve the same end result. Did you happen to notice, there is not a single comment line "//" in the entire function, except the useless and redundant " //get the state of the buttons"? Wow, I guess that must be what a function called "getButtons" does. Thanks for the heads up... :slight_smile:

Yes, the entire code I used is lacking in comments, but mostly it’s easy to read, even for me (only dabbled in Visual Basic as a young man, and C/C++ for Arduino, and recently taught myself Python).

I think the problem here is, as you say, the code has been assembled by a hardware engineer, and I think that a lot of the parts have been pulled from elsewhere and examples.

And that’s why I’m trying to figure it out, where someone else didn’t!

It's not that mysterious if you know C/C++ fairly well, but the end result is very cluttered. Sometimes the low level stuff like this is hard to code "cleanly". There are abstraction tools in the language that you can use to help with that, which essentially allow you to express the same thing in more readable form (and compile to exactly the same resulting machine code). I wish I had time to demonstrate, but not now.

Quick example

   digitalWrite(MUX_SEL_A, i & 1);
    digitalWrite(MUX_SEL_B, i & 2);
    digitalWrite(MUX_SEL_C, i & 4);
    digitalWrite(MUX_SEL_D, i & 8);

because any non-zero value sent to digitalWrite() sets a HIGH level at the pin. Well, I admit that's not abstraction, but it's easier to read IMHO.

Also, if this is a custom board, you should have aligned the mux select pins with contiguous output bits in the data register of the port you're using. Then you could write all the pins in one line, also in several processor cycle (so 100x faster at least):

PORTB = (PORTB & 0xf0) | i;