Random notes

Hi I’m a newbie in coding, I’m just following tutorial on internet to create a midi keyboard from an old Yamaha PSR-3 with 49 keys board XE534, 6x9 matrix. I’m using a clone pro micro but I don’t no if the coding is ok because when I connect arduino to keyboard I have random notes from Fl studio.

// Here's the CODE:
// works with keyboards with matrix 6x9
#define matrix1 6
#define matrix2 9

#include "MIDIUSB.h"
int octave = 0; // add or subtract 9 for one octave
bool currentState[matrix1 * matrix2];
bool requestState[matrix1 * matrix2];
void setup() 
{
  //----matrix1 number of pins------
  pinMode(A0,INPUT); //1
  pinMode(A1,INPUT); //2
  pinMode(A2,INPUT); //3
  pinMode(A3,INPUT); //4
  pinMode(14,INPUT); //5
  pinMode(15,INPUT); //6
  
  //----matrix2 number of pins------
  //each matrix2 pin must be one above the previous one
  pinMode(2,INPUT); //1
  pinMode(3,INPUT); //2
  pinMode(4,INPUT); //3
  pinMode(5,INPUT); //4
  pinMode(6,INPUT); //5
  pinMode(7,INPUT); //6
  pinMode(8,INPUT); //7
  pinMode(9,INPUT); //8
  pinMode(10,INPUT); //9
  
  
}

void loop() 
{
  readKeys();
  writeKeys();
  delay(1);
}

void readKeys()
{
  for(int i=0; i<matrix2; i++)
  {
    //--------matrix2 pins----------
    pinMode(2,INPUT);
    pinMode(3,INPUT);
    pinMode(4,INPUT);
    pinMode(5,INPUT);
    pinMode(6,INPUT);
    pinMode(7,INPUT);
    pinMode(8,INPUT);
    pinMode(9,INPUT);
    pinMode(10,INPUT);
    pinMode(i+2,OUTPUT); // <--i+2 (this is your first pin number)
    
    digitalWrite(i,LOW);
    delayMicroseconds(1000);

    //you have to repeat this finction matrix1 times
    requestState[i*6+0] = !digitalRead(A3);
    requestState[i*6+1] = !digitalRead(A2);
    requestState[i*6+2] = !digitalRead(A1);
    requestState[i*6+3] = !digitalRead(A0);
    requestState[i*6+4] = !digitalRead(15);
    requestState[i*6+5] = !digitalRead(14);
    
  }
}


void writeKeys()
{
  for(int i=0; i<matrix1 * matrix2; i++)
  {
    if(requestState[i]==true && currentState[i] == false)
    {
      noteOn(0, 36+octave+(i*matrix2)%matrix1 * matrix2+i/matrix1, 64);
      currentState[i] = requestState[i];
    }
    
    if(requestState[i]==false && currentState[i] == true)
    {
      noteOff(0, 36+octave+(i*matrix2)%matrix1 * matrix2+i/matrix1, 64);
      currentState[i] = requestState[i];
    }
  }
  MidiUSB.flush();
}


void noteOn(byte channel, byte pitch, byte velocity) {
  midiEventPacket_t noteOn = {0x09, 0x90 | channel, pitch, velocity};
  MidiUSB.sendMIDI(noteOn);
}

void noteOff(byte channel, byte pitch, byte velocity) {
  midiEventPacket_t noteOff = {0x08, 0x80 | channel, pitch, velocity};
  MidiUSB.sendMIDI(noteOff);
}

sketch_49_keys-2.ino (2.4 KB)

is this correct?

    pinMode(i+2,OUTPUT); // <--i+2 (this is your first pin number)
    digitalWrite(i,LOW);

I would have expected

    pinMode(i+2,OUTPUT); // <--i+2 (this is your first pin number)
    digitalWrite(i+2,LOW); // <--- USE i+2, the pin you set as an output just above

it probably would be easier to read the code if you were storing 2D information in 2D arrays. Instead of

bool currentState[matrix1 * matrix2];
bool requestState[matrix1 * matrix2];

you could do

bool currentState[matrix1][matrix2];
bool requestState[matrix1][matrix2];

and then you really have an array matching your 6x9 matrix. Will make it easier to spot errors in addressing storage for the states

Note: Rather than setting all 6 pins to INPUT each time, I’d do it one at a time:

void readKeys()
{
  for (int i = 0; i < matrix2; i++)
  {
    int pin = i+2; // Pins 2 through 10
    //--------matrix2 pins----------
    pinMode(pin, OUTPUT);
    digitalWrite(pin, LOW);
    //you have to repeat this function matrix1 times
    requestState[i * 6 + 0] = !digitalRead(A3);
    requestState[i * 6 + 1] = !digitalRead(A2);
    requestState[i * 6 + 2] = !digitalRead(A1);
    requestState[i * 6 + 3] = !digitalRead(A0);
    requestState[i * 6 + 4] = !digitalRead(15);
    requestState[i * 6 + 5] = !digitalRead(14);
    pinMode(pin, INPUT);
  }
}

and if the row/col pins were in an array, you could use more for() loops instead of duplicating code

J-M-L:
and if the row/col pins were in an array, you could use more for() loops instead of duplicating code

Like this:

#include "MIDIUSB.h"

// works with keyboards with matrix 6x9
const byte ROWS = 6;
const byte COLUMNS = 9;

const byte RowPins[ROWS] = {A3, A2, A1, A0, 15, 14};  // Arduino Micro
const byte ColumnPins[COLUMNS] = {2, 3, 4, 5, 6, 7, 8, 9, 10};
int Octave = 0; // add or subtract 9 for one octave
bool PreviousState[ROWS * COLUMNS];
bool NewState[ROWS * COLUMNS];

void setup()
{
  for (int row = 0; row < ROWS; row++)
    pinMode(RowPins[row], INPUT);

  for (int column = 0; column < COLUMNS; column++)
    pinMode(ColumnPins[column], INPUT);
}

void loop()
{
  readKeys();
  writeKeys();
  delay(1);
}

void readKeys()
{
  for (int column = 0; column < COLUMNS; column++)
  {
    pinMode(ColumnPins[column], OUTPUT);
    digitalWrite(ColumnPins[column], LOW);
    for (int row = 0; row < ROWS; row++)
      NewState[column * ROWS + row] = !digitalRead(RowPins[row]);
    pinMode(ColumnPins[column], INPUT);   // Added. Thanks for the correction
  }
}

void writeKeys()
{
  for (int i = 0; i < ROWS * COLUMNS; i++)
  {
    if (NewState[i] != PreviousState[i])
    {
      // State Changed
      PreviousState[i] = NewState[i];

      if (NewState[i])
      {
        // Pressed
        noteOn(0, 36 + Octave + i, 64);
      }
      else
      {
        // Released
        noteOff(0, 36 + Octave + i, 64);
      }
    }
  }
  MidiUSB.flush();
}

void noteOn(byte channel, byte pitch, byte velocity)
{
  midiEventPacket_t noteOn = {0x09, 0x90 | channel, pitch, velocity};
  MidiUSB.sendMIDI(noteOn);
}

void noteOff(byte channel, byte pitch, byte velocity)
{
  midiEventPacket_t noteOff = {0x08, 0x80 | channel, pitch, velocity};
  MidiUSB.sendMIDI(noteOff);
}

you forgot to restore the pinMode()

may be something like this (using 2D arrays and gathering the states in a structure as we need only 1 bit to represent each state - so 1 byte for previous and current states is plenty!)

#include <MIDIUSB.h>

// works with keyboards with matrix 6x9 on Arduino Micro
const uint8_t rowPins[]    = {A3, A2, A1, A0, 15, 14};
const uint8_t columnPins[] = { 2,  3,  4,  5,  6,  7,  8,  9, 10};

const uint8_t ROWS    = sizeof rowPins / sizeof rowPins[0];
const uint8_t COLUMNS = sizeof columnPins / sizeof columnPins[0];

const int octave = 0; // add or subtract 9 for one octave

struct {
  uint8_t previous: 1;
  uint8_t current: 1;
} matrixState[COLUMNS][ROWS]; // pack things a bit, no need for two arrays of bool

void readKeys()
{
  for (uint8_t column = 0; column < COLUMNS; column++) {
    pinMode(columnPins[column], OUTPUT);
    digitalWrite(columnPins[column], LOW);
    for (uint8_t row = 0; row < ROWS; row++)
      matrixState[column][row].current = (digitalRead(rowPins[row]) == LOW) ? 1 : 0;
    pinMode(columnPins[column], INPUT);
  }
}

void noteOn(uint8_t pitch, uint8_t velocity)
{
  midiEventPacket_t noteOn = {0x09, 0x90, pitch, velocity};
  MidiUSB.sendMIDI(noteOn);
}

void noteOff(uint8_t pitch, uint8_t velocity)
{
  midiEventPacket_t noteOff = {0x08, 0x80, pitch, velocity};
  MidiUSB.sendMIDI(noteOff);
}

void writeKeys()
{
  for (uint8_t r = 0; r < ROWS ; r++) {
    for (uint8_t c = 0; c < COLUMNS ; c++) {
      if (matrixState[c][r].current != matrixState[c][r].previous) {  // State Changed
        uint8_t pitch = 36 + octave + c * ROWS + r;
        if (matrixState[c][r].current) noteOn(pitch, 64);  // Pressed
        else noteOff(pitch, 64); // Released
        matrixState[c][r].previous = matrixState[c][r].current;
      }
    }
  }
  MidiUSB.flush();
}

void setup()
{
  for (auto & p : rowPins) pinMode(p, INPUT);
  for (auto & p : columnPins) pinMode(p, INPUT);
}

void loop()
{
  readKeys();
  writeKeys();
  delay(1);
}

(totally untested, typed here)

J-M-L: you forgot to restore the pinMode()

Added above. Thanks for the correction.

@johnwasser, Would it be easier to use DDRx and PORTx registers? Thanks

abdelhmimas: Would it be easier to use DDRx and PORTx registers?

Generally, no. Using direct register access is generally harder. There are very few circumstances where the few benefits of direct register access (speed of execution, primarily) are more important than portability and ease of programming.

johnwasser: Generally, no. Using direct register access is generally harder. There are very few circumstances where the few benefits of direct register access (speed of execution, primarily) are more important than portability and ease of programming.

What do you mean by generally harder? on the contrary (from my point of view anyway), the sketch would be more compact (no need for loop) and more legible. can you elaborate? thanks

abdelhmimas:
What do you mean by generally harder? on the contrary (from my point of view anyway), the sketch would be more compact (no need for loop) and more legible. can you elaborate? thanks

Since you have not shown your version of the sketch I can’t see how it became more compact and legible. Please show your version that uses direct register access in place of pinMode(), digitalRead() and digitalWrite().

abdelhmimas: @johnwasser, Would it be easier to use DDRx and PORTx registers? Thanks

For something like a key matrix, only a board like the Mega2560 that has ports with no pins assigned to critical peripheral functions, such as port A, C, and K might be easier. The Arduino pins for port K are also in sequence, so you basically see whatever value you write to port K as a digital value on the pins - e.g. if you set PK0 to 0, ADC8 goes HIGH, PK1 to LOW, ADC9 goes LOW and so on. There are 16 more uncommitted consecutive port pins on port A and C that come out on the double row expansion connector at the end of the board.

johnwasser: Since you have not shown your version of the sketch I can't see how it became more compact and legible. Please show your version that uses direct register access in place of pinMode(), digitalRead() and digitalWrite().

Ok, just an idea, the loop in your setup() can be replaced for example by: DDRx= 0x00 will set all the x port pins to inputs, it is only one line. Regards

All the pins are input by default anyway :)

Since you have 9 pins in the matrix that won’t fit into one register (8 max). That means you’ll have to add some business logic to know which register to use Since the other dimension is only 6 pins, if you don’t want to mess with the 2 other pins possibly represented by the extra 2 bits you’ll need to use bit masking operations

That makes the code difficult to read and maintain - and your code is not really that time sensitive. Staying with the standard IDE functions seems to be the reasonable decision

johnwasser:
Like this:

#include "MIDIUSB.h"

// works with keyboards with matrix 6x9
const byte ROWS = 6;
const byte COLUMNS = 9;

const byte RowPins[ROWS] = {A3, A2, A1, A0, 15, 14};  // Arduino Micro
const byte ColumnPins[COLUMNS] = {2, 3, 4, 5, 6, 7, 8, 9, 10};
int Octave = 0; // add or subtract 9 for one octave
bool PreviousState[ROWS * COLUMNS];
bool NewState[ROWS * COLUMNS];

void setup()
{
 for (int row = 0; row < ROWS; row++)
   pinMode(RowPins[row], INPUT);

for (int column = 0; column < COLUMNS; column++)
   pinMode(ColumnPins[column], INPUT);
}

void loop()
{
 readKeys();
 writeKeys();
 delay(1);
}

void readKeys()
{
 for (int column = 0; column < COLUMNS; column++)
 {
   pinMode(ColumnPins[column], OUTPUT);
   digitalWrite(ColumnPins[column], LOW);
   for (int row = 0; row < ROWS; row++)
     NewState[column * ROWS + row] = !digitalRead(RowPins[row]);
   pinMode(ColumnPins[column], INPUT);   // Added. Thanks for the correction
 }
}

void writeKeys()
{
 for (int i = 0; i < ROWS * COLUMNS; i++)
 {
   if (NewState[i] != PreviousState[i])
   {
     // State Changed
     PreviousState[i] = NewState[i];

if (NewState[i])
     {
       // Pressed
       noteOn(0, 36 + Octave + i, 64);
     }
     else
     {
       // Released
       noteOff(0, 36 + Octave + i, 64);
     }
   }
 }
 MidiUSB.flush();
}

void noteOn(byte channel, byte pitch, byte velocity)
{
 midiEventPacket_t noteOn = {0x09, 0x90 | channel, pitch, velocity};
 MidiUSB.sendMIDI(noteOn);
}

void noteOff(byte channel, byte pitch, byte velocity)
{
 midiEventPacket_t noteOff = {0x08, 0x80 | channel, pitch, velocity};
 MidiUSB.sendMIDI(noteOff);
}

Thanks for all your reply, ok i’ve done the changes to my code but still getting same notes on several keys, some of them are playing 2 notes !! I try to connect matrix1 with matrix2 with a wire pin by pin, Ibut still same problem ! Do i need to change something again in the code ?

J-M-L: All the pins are input by default anyway :)

Since you have 9 pins in the matrix that won’t fit into one register (8 max). That means you’ll have to add some business logic to know which register to use Since the other dimension is only 6 pins, if you don’t want to mess with the 2 other pins possibly represented by the extra 2 bits you’ll need to use bit masking operations

That makes the code difficult to read and maintain - and your code is not really that time sensitive. Staying with the standard IDE functions seems to be the reasonable decision

So will an arduino Nano or Uno instead of the Pro micro a better option for a 49 keys keyboard ? What can be the best setup and code for a 6x9 matrix, (6 Rows & 8 columns with all notes, only 1 column have 1 note) ?

No you really need the Pro micro to be able to use MIDIUSB

can you post a picture of your matrix?

Wouldn’t Keypad.h library take care of a lot of this?

“As of version 3.0 it now supports mulitple keypresses. This library is based upon the Keypad Tutorial. It was created to promote Hardware Abstraction. It improves readability of the code by hiding the pinMode and digitalRead calls for the user.”

J-M-L: No you really need the Pro micro to be able to use MIDIUSB

can you post a picture of your matrix?

Ok my board is a Yamaha XE534 with 15 pins cable, below is the link to view the board.

|500x164 |500x375 |500x375