Adding MIDI to organ pedals with MCP23S17

Hi,
I'm trying to add MIDI to some organ pedals with 74HC595 shift registers and have hit a brick wall. I feel like it might be something simple as I'm relatively new to Arduino and the electronics I have done has been more Audio based.

I have followed the instructions from Code Tinker Hack. Before I go too far hard wiring stuff onto the pedals.

To emulate the button switching on and off, using a jumper lead (with diode) to connect a red wire to a yellow wire (e.g. The 2nd red wire to 3rd yellow wire will activate Switch 18)

Unfortunately though it doesn't work. I rarely get a note on/off value, except for a constant stream of C-1 Note off Values in MIDI-OX. Occasionally when connecting or disconnecting wires I get another value, but rarely.

Some things I have tried:
Adding a 220 Ohm resistor to TX
Several different Arduino Uno's
Several MIDI-USB Interfaces (They all work with other devices)
Two different computers.

Other things I have tried:
Code/instructions from other people such as Evan Kale. (Most of whom seemed to have started with the Code Tinker Hack code.

I am completely stumped. I have been at this on and off for weeks now. I know I could probably do this more simply with a matrix and a Mega as I only have 32 notes, but I also want to add MIDI to some keyboards and stops on the organ, so will need to know how to use the shift registers.

Any help will be hugely appreciated.

// Pin Definitions
// Rows are connected to
const int row1 = 5;
const int row2 = 6;
const int row3 = 7;
const int row4 = 8;

// The 74HC595 uses a serial communication 
// link which has three pins
const int clock = 9;
const int latch = 10;
const int data = 11;


uint8_t keyToMidiMap[32];

boolean keyPressed[32];

int noteVelocity = 127;


// use prepared bit vectors instead of shifting bit left everytime
int bits[] = { B00000001, B00000010, B00000100, B00001000, B00010000, B00100000, B01000000, B10000000 };


// 74HC595 shift to next column
void scanColumn(int value) {
  digitalWrite(latch, LOW); //Pulls the chips latch low
  shiftOut(data, clock, MSBFIRST, value); //Shifts out the 8 bits to the shift register
  digitalWrite(latch, HIGH); //Pulls the latch high displaying the data
}

void setup() {
  
  // Map scan matrix buttons/keys to actual Midi note number. Lowest num 41 corresponds to F MIDI note.
  keyToMidiMap[0] = 48;
  keyToMidiMap[1] = 41;
  keyToMidiMap[2] = 42;
  keyToMidiMap[3] = 43;
  keyToMidiMap[4] = 44;
  keyToMidiMap[5] = 45;
  keyToMidiMap[6] = 46;
  keyToMidiMap[7] = 47;

  keyToMidiMap[8] = 56;
  keyToMidiMap[1 + 8] = 49;
  keyToMidiMap[2 + 8] = 50;
  keyToMidiMap[3 + 8] = 51;
  keyToMidiMap[4 + 8] = 52;
  keyToMidiMap[5 + 8] = 53;
  keyToMidiMap[6 + 8] = 54;
  keyToMidiMap[7 + 8] = 55;

  keyToMidiMap[16] = 64;
  keyToMidiMap[1 + 16] = 57;
  keyToMidiMap[2 + 16] = 58;
  keyToMidiMap[3 + 16] = 59;
  keyToMidiMap[4 + 16] = 60;
  keyToMidiMap[5 + 16] = 61;
  keyToMidiMap[6 + 16] = 62;
  keyToMidiMap[7 + 16] = 63;

  keyToMidiMap[24] = 72;
  keyToMidiMap[1 + 24] = 65;
  keyToMidiMap[2 + 24] = 66;
  keyToMidiMap[3 + 24] = 67;
  keyToMidiMap[4 + 24] = 68;
  keyToMidiMap[5 + 24] = 69;
  keyToMidiMap[6 + 24] = 70;
  keyToMidiMap[7 + 24] = 71;

  // setup pins output/input mode
  pinMode(data, OUTPUT);
  pinMode(clock, OUTPUT);
  pinMode(latch, OUTPUT);

  pinMode(row1, INPUT);
  pinMode(row2, INPUT);
  pinMode(row3, INPUT);
  pinMode(row4, INPUT);

    Serial.begin(31250);

  delay(1000);

}

void loop() {

  for (int col = 0; col < 8; col++) {
    
    // shift scan matrix to following column
    scanColumn(bits[col]);

    // check if any keys were pressed - rows will have HIGH output in this case corresponding
    int groupValue1 = digitalRead(row1);
    int groupValue2 = digitalRead(row2);
    int groupValue3 = digitalRead(row3);
    int groupValue4 = digitalRead(row4);

    // process if any combination of keys pressed
    if (groupValue1 != 0 || groupValue2 != 0 || groupValue3 != 0
        || groupValue4 != 0) {

      if (groupValue1 != 0 && !keyPressed[col]) {
        keyPressed[col] = true;
        noteOn(0x91, keyToMidiMap[col], noteVelocity);
      }

      if (groupValue2 != 0 && !keyPressed[col + 8]) {
        keyPressed[col + 8] = true;
        noteOn(0x91, keyToMidiMap[col + 8], noteVelocity);
      }

      if (groupValue3 != 0 && !keyPressed[col + 16]) {
        keyPressed[col + 16] = true;
        noteOn(0x91, keyToMidiMap[col + 16], noteVelocity);
      }

      if (groupValue4 != 0 && !keyPressed[col + 24]) {
        keyPressed[col + 24] = true;
        noteOn(0x91, keyToMidiMap[col + 24], noteVelocity);
      }

    }

    //  process if any combination of keys released
    if (groupValue1 == 0 && keyPressed[col]) {
      keyPressed[col] = false;
      noteOn(0x91, keyToMidiMap[col], 0);
    }

    if (groupValue2 == 0 && keyPressed[col + 8]) {
      keyPressed[col + 8] = false;
      noteOn(0x91, keyToMidiMap[col + 8], 0);
    }

    if (groupValue3 == 0 && keyPressed[col + 16]) {
      keyPressed[col + 16] = false;
      noteOn(0x91, keyToMidiMap[col + 16], 0);
    }

    if (groupValue4 == 0 && keyPressed[col + 24]) {
      keyPressed[col + 24] = false;
      noteOn(0x91, keyToMidiMap[col + 24], 0);
    }

  }

}


void noteOn(int cmd, int pitch, int velocity) {
    Serial.write(cmd);
  Serial.write(pitch);
  Serial.write(velocity);
}

The problem looks like the code. It is truly appalling. It makes my heart sink to see what rubbish is out there.

For example

keyToMidiMap[1 + 8] = 49;
  keyToMidiMap[2 + 8] = 50;
  keyToMidiMap[3 + 8] = 51;
  keyToMidiMap[4 + 8] = 52;
  keyToMidiMap[5 + 8] = 53;
  keyToMidiMap[6 + 8] = 54;
  keyToMidiMap[7 + 8] = 55;

Why? 1 + 8 is 9
so why do the adding up in the matrix, it makes little sense. And someone who writes code that does the same thing over and over simply doesn't know how to program.

So sadly you have been lead astray by trying to copy very poor software.

I rarely get a note on/off value, except for a constant stream of C-1 Note off

You do know you are sending everything to channel 2 on the MIDI?
Sending a note on message with a velocity of 0 will result in a note off message.
It is so hard to follow that it is difficult to fix.

Are you interested in actually learning to code? If so we can help, otherwise keep on searching for something written by some one who knows what they are doing.

// use prepared bit vectors instead of shifting bit left everytime
int bits[] = { B00000001, B00000010, B00000100, B00001000, B00010000, B00100000, B01000000, B10000000 };

These are NOT vectors they are just bit pattern / numbers like any other variable, anyone worth his salt would have written these in hex.

// use prepared numbers instead of shifting bit left every time
int bits[] = { 0x1, 0x2, 0x4, 0x8, 0x10, 0x20, 0x40, 0x80 };

Thanks for the reply mate.

I do agree it does look like pretty average code. Unfortunately from what I have seen is everyone doing this project (or a similar one) has taken this code and changed it around a bit for them!

You do know you are sending everything to channel 2 on the MIDI?
Sending a note on message with a velocity of 0 will result in a note off message.

Channel 2 is ok (although I was eventually going to change it to Channel 3). The stream of midi data I'm getting though is on Channel 1.

Are you interested in actually learning to code? If so we can help, otherwise keep on searching for something written by some one who knows what they are doing.

I'm definitely interested in learning to code and would be very appreciative of help. I did a semester of Java about 10 years ago at uni, so while I'm pretty rusty I at least have a ballpark idea of the absolute basics.

So where do you think would be the best place to start, is it worth re-writing the whole code? I'm also watching a few videos on shift registers at the moment to get a better idea of how it all actually works.

On that note as well, do you think this is physically the most effective way to be going about this? As I said earlier eventually I'll be using what I have learned here to hopefully have 2 keyboards, 30 or so switches and a pedal (potentiometer) running through a Mega.

Thanks again :slight_smile:

The stream of midi data I'm getting though is on Channel 1.

You sure of that?

noteOn(0x91, keyToMidiMap[col + 24], noteVelocity);

So that is sending to channel 2, to send to channel 1 you have to use 0x90. The lower 4 bits of a command specify the channel number 0 to 15 ( 0x0 to 0xF ). As musicians can't cope with the idea of channel zero all the channel numbers get 1 automatically added to them for presentation to the user, but we are down here in the actual message. If you see this coming in on channel 1 then there is something wrong.

although I was eventually going to change it to Channel 3

So that would need changing the command to 0x92.

is it worth re-writing the whole code

I would say so.
What needs to happen is that when you scan your keys you need to put the results into a variable ( an array ) of what keys are held down and what are not. You then use this along with the results of the last scan of the keyboard you made. Then depending on what you find, that is if a key has been pressed or released since the last scan, you send the appropriate MIDI message.

Then before you scan again transfer the new scan array into the last scan array, and go again. You can see the idea with only one input in the "state change" code in the examples menu of the Arduino IDE.

do you think this is physically the most effective way to be going about this?

There are two aspects to this.

  1. Do you want to use a scanning matrix to read the keys. While this is popular I often think it is just because people don't appreciate the alternatives. A matrix takes time to read, and you have to read it all to see if anything has change. If it were me I would ( and have ) used a port expander chip, I favor the MCP23S17. It has 16 I/O pins and you can simply fit 8 chips on the SPI bus. The big advantage is that you can connect your keys directly to this chip without needing any external resistors, and it has one pin that outputs a signal if any of the inputs have changed so you just need to look at one chip to see if you need to read the chip further to see exactly what has been changed. The vast bulk of the time nothing will have changed, so you can respond faster to a change.

  2. If you want to stick with a scanning matrix do you want to scan it with a shift register driving the pins? Yes you can do it but there are other ways to drive it like by using a MCP23S17.

I'm definitely interested in learning to code

OK I am willing to walk you through this project. It will involve me making suggestions and you implementing them, reporting back and posting your code so far. Note this is a lot more work for me than just writing it for you but I feel it is a better investment in my time than writing code on request ( which I don't do by the way ).

Quote

The stream of midi data I'm getting though is on Channel 1.

You sure of that?

Yeah in MIDIOX it's showing up like this:

 TIMESTAMP IN PORT STATUS DATA1 DATA2 CHAN NOTE EVENT               
 00000043   2  --     90    00    00    1  C -1 Note Off

Which I guess means I have potentially done something wrong physically.

If it were me I would ( and have ) used a port expander chip, I favor the MCP23S17. It has 16 I/O pins and you can simply fit 8 chips on the SPI bus

If you reckon that is the better route I'm definitely happy to go that way. I've ordered a bunch of them from Digikey so hopefully they turn up by the start of next week.

OK I am willing to walk you through this project. It will involve me making suggestions and you implementing them, reporting back and posting your code so far. Note this is a lot more work for me than just writing it for you but I feel it is a better investment in my time than writing code on request ( which I don't do by the way ).

Thanks so much I really appreciate it. So where do you suggest I start until the port expander chips arrive?

So where do you suggest I start until the port expander chips arrive?

Well I always start with the schematic, so that is allocating what port expander bit gets connected to what switch and how you are going to wire it up to your Arduino's SPI port and what the addrss pins on the expander need to be connected to.
There are two 8 bit expanders in that chip called imaginatively A & B , so work out what you are going to connect to what. Post it and lets review what, if anything is missing.

So after a bit of reading datasheets and looking about I've come up with this. The numbers of the switches are the MIDI note numbers, starting at C2.

I was a bit unsure of how to daisy chain them as I found some conflicting information.

Thanks again.

Good shot, just a few things wrong.
The SO and SI of each chip should be connected together, you have the SO of the top chip connected to the SI of the bottom chip.
The Arduino 5V and GND should be shown connected to the Vcc and GND symbols as well to show them connected together. The switches look like they are interfering with each other, so either increase the Y spacing or stagger every other one so they are not on top of each other.

Finally you should put a 0.1uF ceramic capacitor between Vcc and ground of each chip.

Now the signals you have missed off it the Int A and Int B. These are the lines you can monitor to get an indication that a input pin has changed without having to read all the keys and look at each one in turn to see if one has changed. You have some choices here. Each one could go to a separate Arduino input at one extreme meaning you can see each half of each chip. Or the other extreme where they can be all connected together and connected to one Arduino pin.

I would go to the mid point where you use just two Arduino pins, one for each chip, with A and B for each chip being wired to the same Arduino pin. You can do this because each of these outputs is an open drain output. Just enable the internal pull up resistors when you set the mode on the Arduino pins.

Well done on this so far. Make the changes and post the new diagram.

Just a finale cosmetic thing, make the ground symbol point down and the 5V symbol point up.

I've tried to tidy it up a bit so hopefully it's a bit easier to read now as well. When it comes to the +5V symbols, would I technically be better to have them as Vcc?

Thanks :slight_smile:

No it is better to have 5V because that is what it is.
The ground symbols would look better if they were at the bottom of the line close to the Vss rather than the top.
In that way you also get fewer line crossings.

But you are doing well. The switches are better, maybe they could be labelled so you know which is the lowest note.

So I've had a little bit of a fiddle around in EasyEDA today as I've never done any schematic stuff. Is it ok to go this extreme with tidying it up or is it better to go somewhere a bit more in the middle?

The switches are labelled as the number of the midi note they start on (36 being C2). I realised after labelling all of them I probably should have done it in hex.

Is it ok to go this extreme with tidying it up or is it better to go somewhere a bit more in the middle?

Well I have seen many examples of schematics like this and personally I am not keen on them. When you look at it them its is rather like looking at a join the dots picture before any dots are joined. So I would go for something in between. It is probably OK to do the switches like that, although I would probably go for a bus bar type of displaying the connections. I wouldn't use Hex numbers for labeling the switches.

This is an example of the bus bar type of showing lots of wires going to lots of places.

Thanks. I've tried to implement the bus bar idea and gotten it down to this which I think looks a lot neater than my previous ones.

I think I'll be waiting a little bit for the chips to come yet. Do you have any suggestions on where I could start with the code, or even something I could read up to get my head around it?

Thanks again

Yes that looks a lot better.

Well the thing to get your head round is the data sheet for the port expander, it is a complex chip. I just program it with sending the right things to the correct registers but maybe you might look at a library to help you, and read up on how that can be used.

So I'm probably getting a bit over my head with this stuff but I've come up with this so far (unfortunately I haven't had buckets of time to work on it lately).

#include <MCP23S17.h> //Majenko Library

#ifdef __PIC32MX__

#include <DSPI.h>

DSPI0 SPI; // chipKIT uses the DSPI library instead of the SPI library as it's better

#else

#include <SPI.h> // Everytying else uses the SPI library

#endif

const uint8_t chipSelect = 10;

MCP23S17 Bank1(&SPI, chipSelect, 0);  //Last number is address. 0 is A0,A1,A2 grounded. 1 is A0=5v, A1,A2 Grounded etc.
MCP23S17 Bank2(&SPI, chipSelect, 1); //Why is this an & 


void setup() {

  Serial.begin(31250); //MIDI Serial Rate
  
  Bank1.begin();
  Bank2.begin();
  
  for(uint8_t i = 0; i < 16; i++) {
     Bank1.pinMode(i, INPUT_PULLUP); //Sets all chip pins to Input with pullup
  }
  for(uint8_t i = 0; i < 16; i++) {
     Bank2.pinMode(i, INPUT_PULLUP);
  }

  for(uint8_t i = 0; i < 16; i++) {
     Bank1.enableInterrupt(i, CHANGE); // Should this be RISING or CHANGE? enable Interrupt on all pins. When change is noticed it will be driven to configured level and stay until port is read (readPort, digitalRead or getInterruptValue)
  }
  for(uint8_t i = 0; i < 16; i++) {
     Bank2.enableInterrupt(i, CHANGE);
  }

  Bank1.setMirror(true); //Ties GPA and GPB together for interrupts.
  Bank2.setMirror(true);

  Bank1.setInterruptLevel(HIGH); //Should these go HIGH or LOW?
  Bank2.setInterruptLevel(HIGH); 
  
}


void loop() {

  unsigned int pinValuesBank1 = Bank1.getInterruptValue();
  unsigned int pinValuesBank2 = Bank2.getInterruptValue();


    

}

//Where do I set arduino pin for interrupt?






/* It looks like I could use the following to get more than 6 chips on a board
/* Using this function it is possible to configure the interrupt output pins to be open
 *  drain.  This means that interrupt pins from multiple chips can share the same interrupt
 *  pin on the host MCU.  This causes the level set by setInterruptLevel to be ignored.  A
 *  pullup resistor will be required on the host MCU's interrupt pin.
 *
 *  Example:
 *
 *      myExpander.setInterruptOD(true);

void MCP23S17::setInterruptOD(boolean openDrain) {
    if (openDrain) {
        _reg[IOCONA] |= (1<<2);
        _reg[IOCONB] |= (1<<2);
    } else {
        _reg[IOCONA] &= ~(1<<2);
        _reg[IOCONB] &= ~(1<<2);
    }
    writeRegister(IOCONA);
}
 */

This is using Majenko's Library. I've come up with what I did from a combination of his example code, the .cpp file and some other example code I found on Stack Exchange.

It's a bit long to post so I put it on pastebin if you wanted to look at it. /* * MultiTest_Majenko_02.ino * Library Ref: github.com/MajenkoLibraries/MCP - Pastebin.com

Shift registers are cheap. A few daisy-chained registers would be quite a bit better than this matrix of switches.

PaulMurrayCbr:
Shift registers are cheap. A few daisy-chained registers would be quite a bit better than this matrix of switches.

Yes but to read a key change you have to constantly read the whole shift register and compare it to the last time you read it. We are after something way more sophisticated here.

Well done, it will take me a time to have a look at it in detail, as I haven't used this library before.

Initial comments

//Why is this an &

In C this means not the variable but the address of where that variable is stored. Not sure why they use it.

Things like this:-

for(uint8_t i = 0; i < 16; i++) {
     Bank1.pinMode(i, INPUT_PULLUP); //Sets all chip pins to Input with pullup
  }
  for(uint8_t i = 0; i < 16; i++) {
     Bank2.pinMode(i, INPUT_PULLUP);
  }

Without the library each loop would be just one instruction setting the appropriate register to 0xFF ( or 255 in decimal ).
Even as it stands you can combine the loops like this:-

for(uint8_t i = 0; i < 16; i++) {
     Bank1.pinMode(i, INPUT_PULLUP); //Sets all chip pins to Input with pullup
     Bank2.pinMode(i, INPUT_PULLUP);
  }

You can even put other initialisation stuff in the same loop.

Would it not be a good idea to use the names Bank0 and Bank1 to match the actual address you have used?

Bank1.setInterruptLevel(HIGH); //Should these go HIGH or LOW?

It doesn't matter much, it is just the logic level you will see when there is an interrupt. I would make it low to match the switch levels which give a low when pushed.

//Where do I set arduino pin for interrupt?

In the setup declare the two pins you are going to use and set them to input_pullup.
Then in the loop only read the bank when you see the interrupt pin being low. Then print out the contents so you can see them.

Thanks heaps. I've tidied it up a bit as you suggested, definitely a good point on the naming of the banks.
I'm a bit stuck for how to read the bank when the pin is pulled low and then print that out but I'll have a bit more time tomorrow to look into it a bit better. Also the chips turned up today so I'll be able to rig them up and experiment a bit.

#include <MCP23S17.h> //Majenko Library

#ifdef __PIC32MX__

#include <DSPI.h>

DSPI0 SPI; // chipKIT uses the DSPI library instead of the SPI library as it's better

#else

#include <SPI.h> // Everytying else uses the SPI library

#endif

const uint8_t chipSelect = 10;

MCP23S17 Bank0(&SPI, chipSelect, 0);  //Last number is address. 0 is A0,A1,A2 grounded. 1 is A0=5v, A1,A2 Grounded etc.
MCP23S17 Bank1(&SPI, chipSelect, 1); 


void setup() {

 // Serial.begin(31250); //MIDI Serial Rate
  Serial.begin(9600); //For serial monitor

  pinMode(2, INPUT_PULLUP); //Interrupt Pin Bank0
  pinMode(3, INPUT_PULLUP); //Interrupt Pin Bank1
  
  Bank0.begin();
  Bank1.begin();
  
  for(uint8_t i = 0; i < 16; i++) {
     Bank0.pinMode(i, INPUT_PULLUP); //Sets all chip pins to Input with pullup
     Bank1.pinMode(i, INPUT_PULLUP);
     Bank0.enableInterrupt(i, CHANGE);
     Bank1.enableInterrupt(i, CHANGE);
  }


  Bank0.setMirror(true); //Ties GPA and GPB together for interrupts.
  Bank1.setMirror(true);

  Bank0.setInterruptLevel(LOW);
  Bank1.setInterruptLevel(LOW); 
  
}


void loop() {

  unsigned int pinValuesBank0 = Bank0.getInterruptValue();
  unsigned int pinValuesBank1 = Bank1.getInterruptValue();

  Serial.println(???);
 

}

I'm a bit stuck for how to read the bank when the pin is pulled low

Read the pin, then test it with an if statement. If the pin is low then read the bank.
Once it has been read then print out the int value you got when reading the bank.

 Serial.println(pinValuesBank0, HEX );

You will need to do this for each bank.

Print out the value in hex or binary to see the individual bits change. Alternatively use the serial plotter instead of the serial monitor and print in decimal to better visualise the changes.

The switches that you have, are they arranges like a piano keyboard with sharps and naturals, or is it just a row of keys? Maybe a photo would help.