For loop doesn't seem to want to increment the variable more than once

I'm wanting to build an MP3 player, or at least, try. I'm starting out with the VS1011E as I already had one of the chips lying around.

First of all I just want to verify that all the hardware actually functions before I start so if there's a problem I can be pretty sure it's in software, not hardware.
I came across and am trying to use the test sketches from someone else's project here: Arduino MP3 to do a quick test.

I doubt I should post the whole lot of code here, as it would take up a lot of space. I'll post the part I am having problems with in case there is some really stupid error I'm overlooking.
But the files in question can be downloaded here: Arduino MP3 or GitHub: GitHub - brokentoaster/ArduinoMP3: The Arduino MP3 shield is an add on board for developing Arduino based systems that incorporate MP3 decoding, SD cards and Joysticks. It is initially based on the work done as part of the ButterflyMP3 Project.
(I would have asked the person who did that project but I can't find a way to contact him on his website at all)

I've got the same Decoder chip - the VS1011E - that he uses in his project, and an ATMega168 running a 12MHz clock with an Optiboot bootloader compiled for 12MHz and of course custom target board defined in Arduino IDE.

Maybe Optiboot or the custom clock speed is the problem?
I also did have to update his files to use Arduino.h instead of WProgram.h - I don't know if that's a red flag for other potential problems down the line...

I have already tried the Joystick test sketch and it runs without issue, the output is all as expected.

The problem I'm having is with the MP3_TEST sketch that I tried afterwards:

// This is a Test for just the MP3 player chip and NOT MMC/SD card routines
// Should start up the board and then playsome sine wave tones.

#define BAUD 115200
#define FAT_NumberedSong_EN 1

#include <utils.h>
#include <mmc.h>
#include <vs1001.h>
#include <types.h>

MMC card;              // not used but doesn't work with out it 
vs1001 player;

char mp3_cs = 3;
char mp3_bsync = 2;
char mp3_dreq  = 15;
char mp3_reset = 14 ;
char debugpin = 8;

//////////////////////////////////// SETUP
void setup()
{
        char r,i;
        unsigned long lr;

        pinMode(debugpin,OUTPUT);           // setup debug trigger
        digitalWrite(debugpin, HIGH);

        digitalWrite(mp3_dreq, HIGH);       // turn on pullup resistors
        digitalWrite(mp3_bsync, LOW);       // turn off BSYNC
        digitalWrite(mp3_reset, LOW);       // turn off VS1003 
        digitalWrite(mp3_cs, HIGH);         // turn off VS10xx SPI

        pinMode(mp3_dreq, INPUT);           // set pin to input
        pinMode(mp3_bsync,OUTPUT);          // set pin to output
        pinMode(mp3_reset,OUTPUT);          // set pin to output
        pinMode(mp3_cs,OUTPUT);             // set pin to output

        // Init Serial comms and send a test message
        Serial.begin(BAUD);
        Serial.println("Arduino MP3 Shield VS1011 Test");       

        // Run some tests on the VS1011 chip
        Serial.print("Init IO ... ");
        player.init_io();
        Serial.println("OK");
        Serial.print("Init_chip ... ");
        player.init_chip();
        Serial.println("OK");     
        Serial.print("Sine_test ... "); 
        player.sine_test();
        Serial.println("OK");

        // Read out all the regesters
        for (i=0;i<16;i++){
                delay(10);
                Serial.print("Read Reg ");
                Serial.print(i,DEC);
                Serial.print(": ");
                Serial.println(readReg(i),HEX);  
        }
        
        // Done.
        Serial.println("DONE");
}

//////////////////////////////////// LOOP
void loop() 
{
        // Do nothing
}



/***************************************************************************
*       Name:	      debug_trigger
*	Description:  pulse the debug pin low then high again as fast as you can
*	Parameters:   <x> char number of times to pulse the line.
*	Returns:      none
***************************************************************************/
void debug_trigger(char x)
{
        char i;
        for (i=0;i<x;i++){
                digitalWrite(debugpin, LOW);
                digitalWrite(debugpin, HIGH);  
        }
}



/***************************************************************************
*       Name:	      readReg
*	Description:  read a register from the VS1011 and return 16 Bit response
*	Parameters:   <reg> byte address of register to read
*	Returns:      16 Bit data returned from the register
***************************************************************************/
uint16 readReg(uint8 reg)
{
        uint16 data;
        player.read(reg, 2,&data);
        return data;
}

From his website, the expected output on the serial monitor is as follows:

Arduino MP3 Shield VS1011 Test
Init IO ... OK
Init_chip ... OK
Sine_test ... OK
Read Reg 0: 820
Read Reg 1: 20
Read Reg 2: 0
Read Reg 3: 9800
Read Reg 4: 0
Read Reg 5: BB80
Read Reg 6: 0
Read Reg 7: 0
Read Reg 8: 0
Read Reg 9: 0
Read Reg 10: 0
Read Reg 11: 0
Read Reg 12: 0
Read Reg 13: 0
Read Reg 14: 0
Read Reg 15: 0
DONE

Whereas with mine, I just get:

Arduino MP3 Shield VS1011 Test
Init IO ... OK
Init_chip ... OK
Sine_test ... OK
Read Reg 0: 820
Read Reg 1: 20
Read Reg 1: 20
Read Reg 1: 20
Read Reg 1: 20

With the "Read Reg 1: 20" line repeated over and over forever...

I have tried changing the loop so it starts at another number, like 3, at which point it reads register 3, then 4, then keeps repeating the output for register 4 over and over.

Note that the registers all do print out the same numbers as his example (when the loop variable init value is changed), also that the Sine wave test passes, I can hear the tone fine.
I suspect that the VS1011E is working OK, as I doubt it could pass the IO\Init tests and also output a correct audio signal on both channels if it were faulty or wired incorrectly.

I have tried compiling both with Arduino 1.0 in Linux and Arduino 1.0.5 in Windows and both have the same issue.

Anyone got any ideas what might be going on here?

Perhaps you have run out of RAM? That can account for all sorts of odd behaviour.

Try shifting the constant prints

        Serial.println("Arduino MP3 Shield VS1011 Test");

into PROGMEM         Serial.println(F("Arduino MP3 Shield VS1011 Test"));   

Given that you only have 1K of RAM and that you have additional code in the full sketch, memory problems seem likely; does the reduced version you posted function correctly?

MarkT:
Perhaps you have run out of RAM? That can account for all sorts of odd behaviour.

Well, I found this: Arduino Playground - HomePage and gave it a try. Changed the code:

        // Read out all the regesters
        for (i=0;i<16;i++){
                delay(1000);
                Serial.print("Read Reg ");
                Serial.print(i,DEC);
                Serial.print(": ");
                Serial.println(readReg(i),HEX);  
                    //Serial.println(str);

                Serial.print("freeMemory()=");
                Serial.println(freeMemory());
                
        }

The result:

Arduino MP3 Shield VS1011 Test
Init IO ... OK
Init_chip ... OK
Sine_test ... OK
Read Reg 0: 820
freeMemory()=137
Read Reg 1: 20
freeMemory()=137
Read Reg 1: 20
freeMemory()=137
Read Reg 1: 20
freeMemory()=137

I don't know if 137 Bytes free is too low or not.

AWOL:
Try shifting the constant prints

        Serial.println("Arduino MP3 Shield VS1011 Test");

into PROGMEM         Serial.println(F("Arduino MP3 Shield VS1011 Test"));     

That does appear to have saved some RAM but not make any other difference:

Arduino MP3 Shield VS1011 Test
Init IO ... OK
Init_chip ... OK
Sine_test ... OK
Read Reg 0: 820
freeMemory()=245
Read Reg 1: 20
freeMemory()=245
Read Reg 1: 20
freeMemory()=245
Read Reg 1: 20
freeMemory()=245
Read Reg 1: 20

wildbill:
Given that you only have 1K of RAM and that you have additional code in the full sketch, memory problems seem likely; does the reduced version you posted function correctly?

Which version was this? I have only ever used the version I downloaded. There is no reduced version that I know of...

Agent24:
Which version was this? I have only ever used the version I downloaded. There is no reduced version that I know of...

...

Agent24:
I doubt I should post the whole lot of code here, as it would take up a lot of space. I'll post the part I am having problems with in case there is some really stupid error I'm overlooking.

Does the code you posted above ("the part I am having problems with") exhibit this problem?

Agent24:
I don't know if 137 Bytes free is too low or not.

IMO that's dangerously low, bearing in mind that the stack depth varies as function calls are made and automatic variables come in and out of scope - there's no reason to assume that the point where you called freeMemory() was the deepest point of the stack and the stack could easily have gone way past that point. It doesn't show there is definitely a memory problem, but it's low enough that it is a definite possibility.

Got it... The code I posted is just the code of the main sketch (.pde) and includes the loop that doesn't seem to work and which I assumed is the problem. It is not a reduced version. The one that doesn't work is the only one I have been using and the only one which exists.

There are nine other files (.c, .h and .cpp) that I did not post the code for, as I did not think anyone would appreciate a thread with ten different sections of quoted code. Maybe I was wrong. Would you like me to post the lot?

PeterH:

Agent24:
I don't know if 137 Bytes free is too low or not.

IMO that's dangerously low, bearing in mind that the stack depth varies as function calls are made and automatic variables come in and out of scope - there's no reason to assume that the point where you called freeMemory() was the deepest point of the stack and the stack could easily have gone way past that point. It doesn't show there is definitely a memory problem, but it's low enough that it is a definite possibility.

OK. Is there another way to get a more accurate reading of free memory?

Quick question about this:

uint16 readReg(uint8 reg)
{
        uint16 data;
        player.read(reg, 2,&data);
        return data;
}

You ask it to read '2' uint16's, and yet you pass it a reference to a variable which only has space for one. Doing so will likely cause memory corruption if the data is stored to a place which was not allocated for it.

Does changing the code to one of these two cure the problem?

uint16 readReg(uint8 reg)
{
        uint16 data;
        player.read(reg, 1,&data);
        return data;
}

OR

uint16 readReg(uint8 reg)
{
        uint16 data[2];
        player.read(reg, 2,data);
        return data[0];
}

Agent24:
There are nine other files (.c, .h and .cpp) that I did not post the code for, as I did not think anyone would appreciate a thread with ten different sections of quoted code. Maybe I was wrong. Would you like me to post the lot?

Under "Additional Options..." you can attach files.

Agent24:
OK. Is there another way to get a more accurate reading of free memory?

It's possible to use a guard word to see whether the stack has overflowed into the heap, but I haven't seen an example of Arduino code doing that.

As far as I can see the sketch is pretty simple but I have no idea what's going on inside those libraries you use and call. At a guess the problem happens inside the call to player.read() and you could confirm that by commenting out the call and seeing of the problem stops happening. You could also look inside the read() method to see what it does and add trace output or comment out parts of its functionality to try to track down which statement within that code causes the problem.

I don't know whether any of those libraries use interrupts but if they do then that's another possible place to be triggering the problem. Using the same approach, selectively disabling the interrupt handlers would enable you to see whether they are causing the problem.

Zip them up and attach them. The part that "seems not to work" can be a side effect of other things.

Why yes, it does. It works perfectly now.
Now if I actually knew what I was doing I probably would have spotted that myself...

Arrch:
Under "Additional Options..." you can attach files.

I have done so, for anyone interested.

PeterH:

Agent24:
OK. Is there another way to get a more accurate reading of free memory?

It's possible to use a guard word to see whether the stack has overflowed into the heap, but I haven't seen an example of Arduino code doing that.

As far as I can see the sketch is pretty simple but I have no idea what's going on inside those libraries you use and call. At a guess the problem happens inside the call to player.read() and you could confirm that by commenting out the call and seeing of the problem stops happening. You could also look inside the read() method to see what it does and add trace output or comment out parts of its functionality to try to track down which statement within that code causes the problem.

I don't know whether any of those libraries use interrupts but if they do then that's another possible place to be triggering the problem. Using the same approach, selectively disabling the interrupt handlers would enable you to see whether they are causing the problem.

That sounds like something I need to learn more about. In fact the whole Arduino system and C in general is something I need to learn more about...

Extra files.zip (26.8 KB)