Why doesn't this SD.open method work?

Projects Programming
1

I am breaking my current project into reusable classes. One of those is loading music from an SD card.

I have the following code with does not work:

	void SignalDEV_Audio::loadVGM(String dataFileName) {
		if(music) music.close();

		char __dataFileName[sizeof(dataFileName) + 1];
		dataFileName.toCharArray(__dataFileName, sizeof(__dataFileName) + 1);

		music = SD.open(__dataFileName);
                .....
	}

        .....

       audio.loadVGM("rtype.vgm");   //  compiles, but does not work...    :-(

However, when I put the filename in directly, it works fine.

	void SignalDEV_Audio::loadVGM(String dataFileName) {
		if(music) music.close();

//		char __dataFileName[sizeof(dataFileName) + 1];
//		dataFileName.toCharArray(__dataFileName, sizeof(__dataFileName) + 1);

		music = SD.open("rtype.vgm");
                ....
	}

       ....

      audio.loadVGM("rytpe.vgm");   //  this works...even though it doesn't matter what I put here because I hard-coded the name above

Any idea what I'm doing wrong?

Thanks!

2

This is wrong:

sizeof(dataFileName)

For the 'String' class (which you should be avoiding like the plague), you should use:

dataFileName.length();

This is also wrong:

dataFileName.toCharArray(__dataFileName, sizeof(__dataFileName) + 1);

It should be:

dataFileName.toCharArray(__dataFileName, sizeof(__dataFileName));
3

Sizeof(String) does not tell you the number of characters in the String - it only tells you the number of bytes needed to hold an instance of the String class. You can use the length() method to find the number of characters in the String.

The String class IMO causes more problems than it solves and I'd encourage you to use plain old c-strings (null-terminated char arrays) instead.

4

Thanks everyone. I couldn't reply fast enough. lol

The "+1" was a mistake. I shouldn't have put that in the question as it isn't what I actually have.

Here is the code I am actually using with the suggestion of using ".length()". However, still doesn't work.

void SignalDEV_Audio::loadVGM(String dataFileName) {
	if(music) music.close();

	char __dataFileName[dataFileName.length()];
	dataFileName.toCharArray(__dataFileName, sizeof(__dataFileName));

	music = SD.open(__dataFileName);

	if(music) {
		bufferCounter = 0;
		readVGMHeader();
		fillBuffer();
	}
}

Also, is there a better way to optimize this to not use String? Been in Java/C# so long that I've slipped since my C++ days. lol

5

It's time you did the obvious, println the filename variable you are using to open the file.

Java (I don't know C#) corrupts your C spirit. In MCU programming, wait maybe 5 years before using String class, that is, when all MCU's are doing 512KB memory.

Isn't it obvious you replaced the sizeof half-baked? What about this line?

dataFileName.toCharArray(__dataFileName, sizeof(__dataFileName));
6

Isn't it obvious you replaced the sizeof half-baked? What about this line?

What about that line? It's fine. The sizeof() the char array IS the number of characters it can hold.

7

cbmeeks:
The "+1" was a mistake. I shouldn't have put that in the question as it isn't what I actually have.

The "+1" is required in order for the array to be big enough to include the whole string plus the null terminator. If you omitted it, the string would be truncated. The String class is a train wreck waiting to happen anyway - I recommend you stop using it and use plain old c-strings (null-terminated char arrays) instead.

8

PeterH:

cbmeeks:
The "+1" was a mistake. I shouldn't have put that in the question as it isn't what I actually have.

The "+1" is required in order for the array to be big enough to include the whole string plus the null terminator. If you omitted it, the string would be truncated. The String class is a train wreck waiting to happen anyway - I recommend you stop using it and use plain old c-strings (null-terminated char arrays) instead.

I'm not currently at the bench so I can't test at the moment. But, I'm confused. How would I use the null-terminated char arrays in this example?

Thanks.

9

PaulS:

Isn't it obvious you replaced the sizeof half-baked? What about this line?

What about that line? It's fine. The sizeof() the char array IS the number of characters it can hold.

That's right.

Still the OP should really println the name.

10

See this line:

char __dataFileName[dataFileName.length()+1];

Does anybody know whether the above line calls the malloc()? the length of the char array is apparently undetermined at compile time.

OP, please stop using double underscore in front of your variable. Double underscore is customarily used by system variables, compile time defs etc. and should not be used in your case.

11

Thanks guys.

EDIT

I found this works even better. Not sure how efficient it is but seems to work pretty well.

void SignalDEV_Audio::loadVGM(char *filename) {

	if(music) music.close();

	music = SD.open(filename);

	if(music) {
		bufferCounter = 0;
		readVGMHeader();
		fillBuffer();
	}
}

I'm going to look into using maybe sprintf or something. I'm curious about the null-terminated char arrays now. And especially since String seems to be looked down on so much.

Thanks again!

12

Does anybody know whether the above line calls the malloc()?

No, it doesn't. malloc() allocates memory on the heap, so that it can persist when the allocating function ends.

That line of code causes allocation on the stack. That space goes away when the function returns and the stack pointer is rewound.

13

For anyone interested, this is the result of my work so far. :slight_smile: