i2c SID emulator with teensy 2++ MIDI device

I am posting this here, because I am unsure of where else to get help, as the original forum for the i2c SID thing appears to be closed to new registration. Anybody having any ideas would be most appreciated.

I have a teensy 2++, and I want it to run as a standard midi device that triggers the SID emulator over i2c.

I have a ATMEGA328P chip + 16MHz resonator, a teensy 2++, and a standard Duemilanove w/ 328P.

I burned the i2c SID emulator code onto the ATMEGA328P chip using ArduinoISP on my Duemilanove. Before doing this, I burned Arduino bootloader and other things on the chip, to test that I was doing it right. All went well. It blinks with Arduino firmware, and cycles through the SID emulation gate LEDs, when it starts up. For SID firmware, pin 11 needs to be hooked to pin 17 for timing, so I did this.

I hooked pin 27 (SDA) of 328P to my teensy SDA (D1) and pin 28 (SCL) of 328P to teensy SCL (D0.)

I set device to teensy 2++ as MIDI device in Arduino IDE, and loaded a simple "when you receive noteon, turn on LED, when you receive noteoff, turn off LED." This worked fine when I sent the teensy MIDI device notes, happily blinking the LED like crazy.

Next, looking at the example interface code that comes with the i2c-SID, I tried to just port it straight, which would make it never get to my noteon/off functions (I left LED blinking at end of functions.)

Deciding that all the i2c stuff was a bit over-complicated (I couldn't really follow the code, I am a simple man) I came up with this:

uint8_t i2c_sendbytes(uint8_t address, uint8_t data1, uint8_t data2) {
  Wire.beginTransmission(address);
  Wire.send(data1);
  Wire.send(data2);
  Wire.endTransmission();
  return 0;
}

to replace the original function. In my setup(), I am calling this, to initialize:

Wire.begin();
Wire.beginTransmission(I2C_SLAVE_ADDR);
Wire.send(0);
Wire.endTransmission();

All the other functions that were in i2c SID original demo code are unchanged:
set_adsr, set_frequency, set_wave, set_pulsewidth, reset_siduino

All this in place, I made my noteon/off callbacks look like this:

void NoteOn(byte channel, byte note, byte velocity) {
  uint16_t envelope = 0x41F4;
  uint8_t gate = 1;
  uint8_t program = 0;
  
  if (velocity == 0){
    gate = 0;
  }
  envelope &= 0xFF0F;  // set Sustain Bits to 0
  envelope |= ((velocity >> 3) << 4) & 0x00F0;	// set sustain bits to velocity
  
  if (gate){
    for(i=0;i<3;i++) {
      if(voices[i] == 0) {
        set_adsr(envelope,i*7);
	play_tone(1, note, i*7, programs[program]);
	voices[i] = note;
	break;
      }
    }
  }
  
  // LED on!
  digitalWrite(6, HIGH);
}

void NoteOff(byte channel, byte note, byte velocity) {
  uint8_t program = 0;
  
  for(i=0;i<3;i++) {
    if(voices[i] == note) {
      play_tone(0, note, i*7, programs[program]);
      voices[i] = 0;
      break;
    }
  }
  
  // LED off!
  digitalWrite(6, LOW);
}

The LEDs now blink, but the SID still doesn't seem to be getting the i2c messages properly. After this, I took my arduino and set it up as a slave_receiver and set the teensy as master_writer (both from Wire example.) I wanted to make sure that I was doing the i2c stuff correct, at least. It worked, no prob, and in serial monitor for arduino, it said "x is ..." and counted.

At this point, I was going a bit batty, and tried the only other test I could think of:
teensy as midi i2c master_sender -> arduino i2c slave_receiver.

Here is the full code for that:

#include <Wire.h>

#define LEDPIN 6

void NoteOn(byte channel, byte note, byte velocity) {
  Wire.beginTransmission(4); // transmit to device #4
  Wire.send("1");
  Wire.send(channel);
  Wire.send(note);
  Wire.send(velocity);
  Wire.endTransmission();
  digitalWrite(LEDPIN, HIGH);
}

void NoteOff(byte channel, byte note, byte velocity) {
  Wire.beginTransmission(4); // transmit to device #4
  Wire.send("0");
  Wire.send(channel);
  Wire.send(note);
  Wire.send(velocity);
  Wire.endTransmission();
  digitalWrite(LEDPIN, LOW);
}

void setup() {
  usbMIDI.setHandleNoteOff(NoteOff);
  usbMIDI.setHandleNoteOn(NoteOn);
  pinMode(LEDPIN, OUTPUT);
  Wire.begin();
}

void loop() {
  usbMIDI.read();
}

On my arduino, I just ran the slave_receiver Wire example. I get lots of byte data (starting with correct 1/0) in serial console when I send the teensy midi messages, so it appears to be working.

The only other thing I can think of is that I am sending incorrectly formatted noteon/off messages for the SID chip. Anybody have experience with this, or see some other issue?

I am not sure what else to test here. As far as I can tell, everything is hooked up right, and all the individual parts are working correctly. I have no experience with the SID i2c, so it may be not working or something, but it seems happy when it starts (flashing all the LEDs.)

Wire.send("1");

I'm absolutely no expert on MIDI, but should this be:

Wire.send(1);

That is a totally different thing.

That was just my demo code to make sure I was doing i2c and midi at the same time. I used "1" and "0" to see noteon/off in serial console. The SID uses a different format (SID registers.)

From what I can understand, the i2c messages look like this:

register
register value

so, to make a note, if my limited understanding is correct, I need to set the adsr envelope register/value, then freq register/value, then wave register/value. They have convenience functions that do this (set_adsr, and play_tone,) but in i2c it needs to call ATTACKDECAY, SUSTAINRELEASE registers, then 2 registers for frequency, then choose waveform with another register.

So, I think maybe I am getting closer to how it is supposed to work, even if it doesn't actually work.

I noticed that I should be doing the actual triggering in the loop, and queuing up events in my callbacks (so it's more like the original code.)

Here is my current full code:

#include <Wire.h>

// SID Registers
#define VOICE1 0
#define VOICE2 7
#define VOICE3 14
#define PWLO 2
#define PWHI 3
#define CONTROLREG 4
#define ATTACKDECAY 5
#define SUSTAINRELEASE 6

// SID Control Register Bits
#define GATE (1<<0)
#define GATEOFF 0
#define SYNC (1<<1)  // not implemented
#define RINGMOD (1<<2) 
#define TEST (1<<3)  // not implemented
#define TRI (1<<4)
#define SAW (1<<5)
#define REC (1<<6)
#define NOI (1<<7)


#define FALSE (1!=1)
#define PLAY 1
#define END 0

/**************************************
I2C Constants
**************************************/
#define I2C_SLAVE_ADDR 0x02


/*****************************************
 Notes to Frequency Table
******************************************/
uint16_t pitch_table[108] PROGMEM = {137, 145, 154, 163, 173, 183, 194, 205, 218, 231, 244, 259, 274, 291, 308, 326, 345, 366, 388, 411, 435, 461, 489, 518, 548, 581, 616, 652, 691, 732, 776, 822, 871, 922, 977, 1035, 1097, 1162, 1231, 1304, 1382, 1464, 1551, 1643, 1741, 1845, 1954, 2071, 2194, 2324, 2462, 2609, 2764, 2928, 3102, 3287, 3482, 3689, 3909, 4141, 4387, 4648, 4925, 5218, 5528, 5857, 6205, 6574, 6965, 7379, 7818, 8282, 8775, 9297, 9850, 10435, 11056, 11713, 12410, 13148, 13929, 14758, 15635, 16565, 17550, 18593, 19699, 20870, 22111, 23426, 24819, 26295, 27859, 29515, 31270, 33130, 35100, 37187, 39398, 41741, 44223, 46852, 49638, 52590, 55717, 59030, 62541 };
uint8_t programs[5] = {REC,TRI,SAW,NOI,RINGMOD};

uint8_t voices[3] = {0,0,0};

struct midi {
  uint8_t note, program, gate;
  uint16_t envelope;
  uint8_t control, new_action, start_received;
};

/****************************************************
i2c_sendbytes
  send 2 bytes over i2c bus. returns 0 if successful, returns 1 if communication failed
  ALL communication between arduino and the SID-shield is handled by i2c_sendbytes
*****************************************************/
uint8_t i2c_sendbytes(uint8_t address, uint8_t data1, uint8_t data2) {
  Wire.beginTransmission(address);
  Wire.send(data1);
  Wire.send(data2);
  Wire.endTransmission();
  return 0;
}

/*********************************

SIDuino wave generation control

*********************************/

void set_adsr ( uint16_t adsr, uint8_t adsr_voice) {
  while(i2c_sendbytes(I2C_SLAVE_ADDR,adsr_voice + ATTACKDECAY, (adsr & ~0x00ff)>>8 ));
  while(i2c_sendbytes(I2C_SLAVE_ADDR,adsr_voice + SUSTAINRELEASE, adsr & ~0xff00 ));
}

// pitch=16.77*frequency
void set_frequency(uint16_t pitch, uint8_t freq_voice) {
  while(i2c_sendbytes(I2C_SLAVE_ADDR,freq_voice, pitch&0xFF)); // low register adress
  while(i2c_sendbytes(I2C_SLAVE_ADDR,freq_voice+1, pitch>>8)); // high register adress
}

void set_wave(uint8_t waveform, uint8_t gate, uint8_t wave_voice) {
  while(i2c_sendbytes(I2C_SLAVE_ADDR,wave_voice + CONTROLREG, waveform | gate));
}

void set_pulsewidth(uint16_t pulsewidth, uint8_t pulse_voice) {
  while(i2c_sendbytes(I2C_SLAVE_ADDR,pulse_voice + PWLO, (pulsewidth & 0xFF)));
  while(i2c_sendbytes(I2C_SLAVE_ADDR,pulse_voice + PWHI, (pulsewidth >> 8)));
}

void reset_siduino() {
  uint8_t i;
  for (i=0; i<3; i++) {  // select all Voices
    set_wave(programs[0], 0,i*7); // gate off (tone off)
    set_frequency(0, i*7); // frequency 0
    set_adsr(0x41F4, i*7); // adsr reset
    set_pulsewidth(0x0B00, i*7);
  }
}

void play_tone(uint8_t mastergate, uint8_t note, uint8_t soundvoice,uint8_t waveform) {
  set_frequency(pgm_read_word(&(pitch_table[note])), soundvoice);
  set_wave(waveform, mastergate, soundvoice);
}


/******************************************************************/

uint8_t i;
midi midi_struct;

void NoteOn(byte channel, byte note, byte velocity) {
  midi_struct.new_action = 1;  // note change action
  
  if (velocity == 0){
    midi_struct.gate = 0;
  }else{
    midi_struct.gate = 1;
    
    midi_struct.envelope &= 0xFF0F;  // set Sustain Bits to 0
    midi_struct.envelope |= ((velocity >> 3) << 4) & 0x00F0;	// set sustain bits to velocity
  }
  midi_struct.note = note;
  
  digitalWrite(6, HIGH);
}

void NoteOff(byte channel, byte note, byte velocity) {
  midi_struct.new_action = 1;  // note change action
  
  midi_struct.gate = 0;
  midi_struct.note = note;
  
  digitalWrite(6, LOW);
}

void setup() {
  usbMIDI.setHandleNoteOff(NoteOff);
  usbMIDI.setHandleNoteOn(NoteOn);
  
  // generic note indicator
  pinMode(6, OUTPUT);
  
  delay(2000);  // wait for SID
  
  Wire.begin();
  
  reset_siduino();
  midi_struct.program = 0;
  midi_struct.envelope = 0x41F4;
  midi_struct.start_received = 0;
  
  digitalWrite(6, HIGH);  
}

void loop() {
  usbMIDI.read();
  
  switch(midi_struct.new_action) {
    case 0:
      break;
    case 1:  // note change
      if(midi_struct.gate) {   //choose voice for note on
        for(i=0;i<3;i++) {
          if(voices[i] == 0) {
            set_adsr(midi_struct.envelope, i*7);
            play_tone(midi_struct.gate, midi_struct.note ,i*7, programs[midi_struct.program]);
            voices[i] = midi_struct.note;
            break;
          }
        }
      } else {  // choose voice for note off
        for(i=0;i<3;i++) {
          if(voices[i] == midi_struct.note) {
            play_tone(midi_struct.gate, midi_struct.note, i*7, programs[midi_struct.program]);
            voices[i] = 0;
            break;
          }
        }
      }
    
    midi_struct.new_action = 0;
    break;
  }
}

Hmm. I am starting to think it is a timing issue with atmega328 vs atmega168. i can't get the chip to do stuff, no matter what I try. I will order a 168, and see how it goes.

The differences between the 168 and the 328 are the amount of SRAM and the amount of flash memory. The clock speeds are identical. If you think you are having a timing issue related to clock speed, changing to a 168 won't help.

That totally makes sense, and is why I thought I could just use the 328P. I guess the reason I thought it might be different is that there is stuff in the siduino code to play with clock speed for the atmega8/168, which I thought was also identical, like this:

#ifdef ATMEGA168
	// TIMER2 used as clock devider FCPU/2
	// TIMER2: CTC Mode
	//TCCR2 = (1 << WGM21);
	TCCR2A = (1 << WGM21);
	// TIMER2: clock no prescaling
	//TCCR2 |= (1 << CS20);
	TCCR2B |= (1 << CS20);
	// TIMER2: toogle OC2 on compare match
	TCCR2A |= (1 << COM2A0);
	// TIMER2: set compare value
	OCR2A=0;

	TIMSK1 |= (1 << TOIE1);  // Timer1 Overflow Interrupt Enable

	//initialize I2C
	TWBR = 12;// 0x00;  // set TWI bit rate register calculation: CPU frequency / (16 + TWBR * prescaler Value)
	TWSR = ~((1 << TWPS1) | (1 << TWPS0));  // set prescaler value = 1 in TWI status register
	TWAR = I2C_OWN_ADDR;  // set slave address
	TWCR = ~((1<< TWSTA) | (1<< TWSTO));  // set TWSTA and TWSTO to 0
	TWCR = (1<<TWEA)|(1<<TWEN)|(1<<TWIE);  // initialize slave mode, TWIE enables Interrupt
#endif

Basically, I left ATMEGA168 defined in the siduino code, but set my CPU to atmega328p in my makefile. I may be doing something wrong here, but as far as I can tell this is the correct way to make this work.

I am a bit confused by this sort of stuff:

void set_frequency(uint16_t pitch, uint8_t freq_voice) {
  while(i2c_sendbytes(I2C_SLAVE_ADDR,freq_voice, pitch&0xFF)); // low register adress
  while(i2c_sendbytes(I2C_SLAVE_ADDR,freq_voice+1, pitch>>8)); // high register adress
}

That will indefinitely call i2c_sendbytes until it returns a zero. However i2c_sendbytes always returns a zero. So why confuse things with the while?

This would be less confusing:

void set_frequency(uint16_t pitch, uint8_t freq_voice) {
  i2c_sendbytes(I2C_SLAVE_ADDR,freq_voice, pitch&0xFF); // low register adress
  i2c_sendbytes(I2C_SLAVE_ADDR,freq_voice+1, pitch>>8); // high register adress
}

Or you could leave that and similar functions alone and change i2c_sendbytes to do what the comments say it will do:

/****************************************************
i2c_sendbytes
  send 2 bytes over i2c bus. returns 0 if successful, returns 1 if communication failed
  ALL communication between arduino and the SID-shield is handled by i2c_sendbytes
*****************************************************/
uint8_t i2c_sendbytes(uint8_t address, uint8_t data1, uint8_t data2) {
  Wire.beginTransmission(address);
  Wire.send(data1);
  Wire.send(data2);
  return Wire.endTransmission();
}

Now at least, it only returns zero on success. So if the device happens to be busy, it won't return zero, and you retry whatever it was you were sending.

I was replacing the original code (that doesn't seem to work) that returns 0/1 for status. Since there is no way to do this with Wire (I think it already does the loop stuff) I just made it return 0 (meaning it did it until it was a success.)

Ah, but now I see in your example, and indeed in the docs, endTransmission() does do that. I will update the code, now.

konsumer:
Since there is no way to do this with Wire (I think it already does the loop stuff) ...

It doesn't loop. If it has returned non-zero it has not succeeded (perhaps the device is not connected, has a different address, is wired the wrong way around, is busy, etc.).

Yep, as I said, I looked it up in the docs, and you are indeed correct. I updated my code to do this:

uint8_t i2c_sendbytes(uint8_t address, uint8_t data1, uint8_t data2) {
  Wire.beginTransmission(address);
  Wire.send(data1);
  Wire.send(data2);
  uint8_t ret = Wire.endTransmission();
  if (ret == 1){
    Serial.println("data too long to fit in transmit buffer");
  }else if (ret == 2){
    Serial.println("received NACK on transmit of address");
  }else if (ret == 3){
    Serial.println("received NACK on transmit of data");
  }else if (ret==4){
    Serial.println("some other bad i2c thing happened.");
  }
  return ret;
}

It still doesn't work (I think this is siduino, not my interface code), but is more correct now.

konsumer:
Hmm. I am starting to think it is a timing issue with atmega328 vs atmega168. i can't get the chip to do stuff, no matter what I try. I will order a 168, and see how it goes.

Looking at the code in SIDuino_i2c.c (and assuming this is what you have on your Siduino) it is designed for a ATMEGA168. Maybe you are right and some subtle difference between them is causing this. Perhaps try the standalone SID app I saw on one of the web sites and see if you can get sound out of it?

Yep. it seems really low-level (all pinmodes and interrupts.) I tried all the variants I could find, and for the most part they are all the same code, with tiny differences (i2c here, serial stuff there, etc). Bummed 'cause I gotta wait for a chip in the mail. I will try to fix it, though, if I can figure out what is different.

Now, I am more confused. I am getting this message for every request: "received NACK on transmit of address"

Here is my current debug code for the interface, running on a regular duemilanove:

#include <Wire.h>

#define I2C_SLAVE_ADDR 0x02
#define DEBUGMODE 1

/****************************************************
i2c_sendbytes
  send 2 bytes over i2c bus. returns 0 if successful, returns 1 if communication failed
  ALL communication between arduino and the SID-shield is handled by i2c_sendbytes
*****************************************************/
uint8_t i2c_sendbytes(uint8_t address, uint8_t data1, uint8_t data2) {
  Wire.beginTransmission(address);
  Wire.send(data1);
  Wire.send(data2);
  uint8_t ret = Wire.endTransmission();
  
#ifdef DEBUGMODE
  switch(ret){
    case 0:
      Serial.println(address + data1 + data2);
      break;
    case 1:
      Serial.println("data too long to fit in transmit buffer");
      break;
    case 2:
      Serial.println("received NACK on transmit of address");
      break;
    case 3:
      Serial.println("received NACK on transmit of data");
      break;
    case 4:
      Serial.println("some other bad i2c thing happened.");
      break;
  }
#endif

  return ret;
}

void setup(){
  pinMode(13, OUTPUT);
#ifdef DEBUGMODE
Serial.begin(9600);
#endif
  Wire.begin();
}

void loop(){
  digitalWrite(13, HIGH);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x0,0xAA);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x1,0x29);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x2,0xFC);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x3,0x6);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x4,0x11);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x5,0x6);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x6,0xC8);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x7,0x7F);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x8,0x2B);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x9,0xA1);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xA,0x1);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xB,0x10);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xC,0x7);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xD,0xA8);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xE,0xA4);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xF,0x50);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x10,0xE5);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x11,0x2);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x12,0x10);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x13,0x7);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x14,0x19);
  delay(1000);
  
  digitalWrite(13, LOW);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x0,0xAA);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x1,0x29);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x2,0xFC);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x3,0x6);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x4,0x1);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x5,0x6);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x6,0xC8);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x7,0x7F);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x8,0x2B);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x9,0xA1);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xA,0x1);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xB,0x10);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xC,0x7);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xD,0xA8);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xE,0xA4);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0xF,0x50);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x10,0xE5);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x11,0x2);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x12,0x10);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x13,0x7);
  i2c_sendbytes(I2C_SLAVE_ADDR, 0x14,0x19);
  delay(1000);
}

I got the registers from the labview program, so I am fairly sure they are correct.

Well, this is progress. If you get NAK on address, that means the other device isn't responding. It pulls the SDA line low after the address arrives, if it is connected, wired the right way, has the correct address, has configured I2C, etc.

If not, it just ignores the address, which leaves the line high, which is a NAK.

So you need to check all that stuff.

For some reason the source I am looking at (SIDuino_i2c.c) doesn't even use the Wire library. I don't know why. Maybe it was written before Wire was written.

But maybe since it has this:

#define ATMEGA168

... there is stuff in there (like registers) that are not identical on the 328 chip.

For some reason the source I am looking at (SIDuino_i2c.c) doesn't even use the Wire library.

I added the wire stuff, because for the client code, I wanted to be sure it was setup correctly, and it was alot easier to read, for me.

there is stuff in there (like registers) that are not identical on the 328 chip

I am starting to think that, although it is supposed to be interchangeable.

After many tries and attempts got it working -> Arduino Playground - SIDuinoI2c

But only with the i2c solution in the example script of the i2c SIDuino.

I'm a total programming noob, and I really don't understand the code they used. Has anyone gotten this thing to work with the Wire library (which I understand a little more clearly)? Or could someone give me some pointers what to do?

So, after about a year, I decided to revisit this project.

I used the code at Arduino Playground - SIDuinoI2c.

To ensure that it wasn't a chip problem, or a problem with my compiler or something, I used the supplied hex file and the avrdude command to burn it on an ATMEGA168 with a ArduinoISP, and the same avrdude command-line (accept for the -c & -P flags for my programmer) I have tested this setup with other code, and burned blink program fine to ATMEGA168 using ISP, so I think my dev setup is ok, and the ATMEGA is running with a 16MHz resonator on 9&10, which seems to work fine for Arduino code, etc.

I hooked pin 11 to 17 for timing, as per schematic. I hooked amp up to pin 15, also per instructions.

When I run a teensy 2++ as controller (receive MIDI commands, send i2c using Wire library) I can make the LED channel 1-3 indicator light when sending MIDI notes, but it just makes the same high pitch click sound. it seems to turn light on and off to noteon/off correctly. It's always the same sound, regardless of note. Since the lights on the SIDuino chip are correct, it seems like it must be something weird.

I burned the "stand alone" version onto an ATMEGA8, with the same exact circuit (used a ZIF socket so it's easy to swap out) and I get the correct pattern (but can't control with teensy or arduino, because it takes no input.)

I set my Teensy in MIDI mode (so it acts like a MIDI USB device) and put this code on it:

/**
 * Teensy-based MIDI controller for i2c SIDuino
 */

#include <Wire.h>

// SID Registers
#define VOICE1	0
#define VOICE2	7
#define VOICE3	14
#define PWLO	2
#define PWHI	3
#define CONTROLREG 4
#define ATTACKDECAY 5
#define SUSTAINRELEASE 6

// SID Control Register Bits
#define GATE (1<<0)
#define GATEOFF 0
#define RINGMOD (1<<2)
#define TRI (1<<4)
#define SAW (1<<5)
#define REC (1<<6)
#define NOI (1<<7)

uint8_t programs[5] = {REC,TRI,SAW,NOI,RINGMOD};

void HandleNoteOn (uint8_t channel, uint8_t note, uint8_t velocity){ 
  if (note < 108){
    play_tone(1, note, (channel-1)*0x7, programs[channel-1]);
  }
}

void HandleNoteOff (uint8_t channel, uint8_t note, uint8_t velocity){
  if (note < 108){
    play_tone(0, note, (channel-1)*0x7, programs[channel-1]);
  }
}

void setup(){
  delay(2000);
  Wire.begin();
  delay(500);
  reset_siduino();
  usbMIDI.setHandleNoteOff(HandleNoteOff);
  usbMIDI.setHandleNoteOn(HandleNoteOn);
  Serial.begin(9600);
}

void loop(){
  usbMIDI.read();
}

// send command to SIDuino chip
void sid(char msg[]){
  Wire.beginTransmission(1); // transmit to device #1 - siduino
  for (int i=0; i<sizeof(msg); i++){
    Wire.write(msg[i]);
  }
  Wire.endTransmission();    // stop transmitting
}

void set_adsr ( uint16_t adsr, uint8_t adsr_voice){
  char msg[4] = {adsr_voice+ATTACKDECAY, (adsr&~0x00ff)>>8, adsr_voice+SUSTAINRELEASE, adsr&~0xff00};
  sid(msg);
}

// pitch=16.77*frequency
void set_frequency(uint16_t pitch, uint8_t freq_voice)  {
  char msg[4] = {freq_voice, pitch&0xFF, freq_voice+1, pitch>>8};
  sid(msg);
}

void set_wave(uint8_t waveform, uint8_t gate, uint8_t wave_voice)  {
  char msg[2] = {wave_voice+CONTROLREG, waveform|gate};
  sid(msg);
}

void set_pulsewidth(uint16_t pulsewidth, uint8_t pulse_voice) {
  char msg[4] = {pulse_voice+PWLO, pulsewidth&0xFF, pulse_voice+PWHI, pulsewidth>>8};
  sid(msg);
}

uint16_t pitch_table[108] PROGMEM = {137, 145, 154, 163, 173, 183, 194, 205, 218, 231, 244, 259, 274, 291, 308, 326, 345, 366, 388, 411, 435, 461, 489, 518, 548, 581, 616, 652, 691, 732, 776, 822, 871, 922, 977, 1035, 1097, 1162, 1231, 1304, 1382, 1464, 1551, 1643, 1741, 1845, 1954, 2071, 2194, 2324, 2462, 2609, 2764, 2928, 3102, 3287, 3482, 3689, 3909, 4141, 4387, 4648, 4925, 5218, 5528, 5857, 6205, 6574, 6965, 7379, 7818, 8282, 8775, 9297, 9850, 10435, 11056, 11713, 12410, 13148, 13929, 14758, 15635, 16565, 17550, 18593, 19699, 20870, 22111, 23426, 24819, 26295, 27859, 29515, 31270, 33130, 35100, 37187, 39398, 41741, 44223, 46852, 49638, 52590, 55717, 59030, 62541 };
void play_tone(uint8_t mastergate, uint8_t note, uint8_t soundvoice,uint8_t waveform) {
  set_frequency(pgm_read_word(&(pitch_table[note])), soundvoice);
  set_wave(waveform, mastergate, soundvoice);
}


void reset_siduino() {
  uint8_t i;
  for (i=0; i<3; i++) {  // select all Voices
    set_wave(programs[0],0,i*7); // gate off (tone off)
    set_frequency(0,i*7); // frequency 0
    set_adsr(0x41F4,i*7); // adsr reset
    set_pulsewidth(0x0B00,i*7);
  }
}

Anyone gotten this to work? If so, does it seem like I am doing anything wrong?

Also, just to double-check that my teensy i2c output is correct, I put this code on an arduino, and hooked the teensy's i2c up to it:

#include <Wire.h>
#include <stdarg.h>

/*
TWI debugger.

hook clock up to A5 & data up to A4

*/

void setup() {
  Serial.begin(9600);
  Wire.begin(1);
  Wire.onReceive(receiveTWI);
}

void loop() {}

// http://linux.die.net/man/3/printf for usage
void p(char *fmt, ...){
  char tmp[128]; // resulting string limited to 128 chars
  va_list args;
  va_start (args, fmt );
  vsnprintf(tmp, 128, fmt, args);
  va_end (args);
  Serial.print(tmp);
}

char r,d;
void receiveTWI(int howMany) {
  while(0 < Wire.available()) {
    r = Wire.read();
    d = Wire.read();
    p("0x%02X, 0x%02X\n", r, d);
  }
}

I get this on init:

0x04, 0x40
0x00, 0x00
0x05, 0x41
0x02, 0x00
0x0B, 0x40
0x07, 0x00
0x0C, 0x41
0x09, 0x00
0x12, 0x40
0x0E, 0x00
0x13, 0x41
0x10, 0x00

This on noteon:

0x00, 0x24
0x04, 0x41

and this on noteoff

0x00, 0x24
0x04, 0x40