How come a line is not executed?

Go To Last Post
17 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have the following function:

int16_t DFPlayerSendCommand(uint8_t command, uint8_t parameter)
{
	uint8_t tempCommand[10] = {0x7E ,0xFF ,0x06 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0xEF};
	int16_t checksum;
	
	if (command == DFPLAYER_SET_EQ)
	{
		if (DFPlayerCurrentEQ == 5)
			DFPlayerCurrentEQ = 0;
		else
			DFPlayerCurrentEQ++;
				
		parameter = DFPlayerCurrentEQ;
	}
			
	tempCommand[3] = command;
	tempCommand[6] = parameter;
	
	checksum = _DFPlayerChecksum(tempCommand);
	
	tempCommand[7] = (uint8_t)(checksum >> 8);
	tempCommand[8] = (uint8_t)checksum;
		
	UARTSendArray(tempCommand, 10);
	
	// Wait for answer
	if (command == DFPLAYER_GET_EQ)
	{
		for (uint8_t i = 0; i < 10; i++)
			tempCommand[i] = UARTGetByte();
			
		checksum = (tempCommand[7] << 8) | (tempCommand[8]); // Glue --> int16_t
			
		// If MSG OK return answer
		if ((tempCommand[0] == DFPLAYER_RX_START_BYTE) && (tempCommand[9] == DFPLAYER_RX_END_BYTE) && (checksum == _DFPlayerChecksum(tempCommand)))
			return tempCommand[6];
	}
	
	return -1; // Nothing to return
}

It is being called once from main like so:

DFPlayerSendCommand(DFPLAYER_SET_PB_MODE, DFPlayerPlaybackMode);

The arguments hold the following values: 8, 2.

checksum gets the value 0xFEF1 which is correct, that checksum is split into 2 bytes like so:

tempCommand[7] = (uint8_t)(checksum >> 8);
tempCommand[8] = (uint8_t)checksum;

I've also checked the assembly code:

000000DA  STD Y+8,R25		Store indirect with displacement 
000000DB  STD Y+9,R24		Store indirect with displacement 

And I've checked that R24 = 0xF1 & R25 = 0xFE. However the send line is not being executed and when I check the array before being sent I get tempCommand[8] = 0 which is should not be.

 

Any suggestions what could cause this?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Have you stepped through the code in the debugger?
There could be a number of reasons and some not related to the code you’ve shown us. My first thought is running out of stack space.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What is a "send line"?

 

You have given us a few fragments of information -- why not give us everything?  All the values for defines.  All declarations.  All generated code.

 

You have said that the function is called with certain arguments.  How have you verified that?  ICE?  Simulator?  If so, you should be able to step through the machine code and see what is happening.

 

slow_rider wrote:
uint8_t tempCommand[10] = {0x7E ,0xFF ,0x06 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0xEF};

BTW, that is a very "interesting" way to have a lookup table.  You are forcing the C compiler to generate all the bytes onto the stack each time the function is invoked.  Takes cycles and takes stack space.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

Have you stepped through the code in the debugger?
There could be a number of reasons and some not related to the code you’ve shown us. My first thought is running out of stack space.

 

Yes I did. The MCU is ATmega128A. The function was reduced significantly from previous version which seemed to work fine. How can I check stack violations?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@theusch

Regarding uint8_t tempCommand[10], Assuming I need this "template" for the commands I'm sending and want to manipulate an array how else would I do that? If I would store this using PROGMEM attribute I would still need to fetch it during run-time for manipulations. The other alternative would be not to manipulate the array and make the code longer and harder to understand since I'll be loading bytes from the "PROGMEMed" array but deal with a number of variables as well. How would you suggest to do that?

 

main.c

#include <avr/io.h>
#include "DFPlayer.h"

int main(void)
{
	DFPlayerInit();

	DFPlayerSendCommand(DFPLAYER_PLAYBACK,0);

    while (1) 
    {
				
	}
}

dfplayers.h

#define DFPLAYER_RX_START_BYTE 0x7E
#define DFPLAYER_RX_END_BYTE   0xEF

#define DFPLAYER_PLAYBACK       0x0D
#define DFPLAYER_VOL_UP			0x04
#define DFPLAYER_VOL_DOWN		0x05
#define DFPLAYER_SET_EQ			0x07
#define DFPLAYER_GET_EQ			0x44
#define DFPLAYER_SET_PB_MODE	0x08

#define DFPLAYER_PB_MODE_REPEAT 0x00
#define DFPLAYER_PB_MODE_SINGLE 0x02
#define DFPLAYER_PB_MODE_RANDOM 0x03

#include "UART.h"

void DFPlayerInit(void);
int16_t DFPlayerSendCommand(uint8_t command, uint8_t parameter);
int16_t _DFPlayerChecksum(uint8_t arr[]);

dfplayers.c

#include <avr/io.h>
#include "DFPlayer.h"

uint8_t DFPlayerCurrentEQ;
uint8_t DFPlayerPlaybackMode = DFPLAYER_PB_MODE_SINGLE; // Default

void DFPlayerInit(void)
{
	UARTInit();
	DFPlayerCurrentEQ = (uint8_t)DFPlayerSendCommand(DFPLAYER_GET_EQ, 0); // Get EQ
	DFPlayerSendCommand(DFPLAYER_SET_PB_MODE, DFPlayerPlaybackMode); // Set playback mode
}

int16_t DFPlayerSendCommand(uint8_t command, uint8_t parameter)
{
	uint8_t tempCommand[10] = {0x7E ,0xFF ,0x06 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0xEF};
	int16_t checksum;
	
	if (command == DFPLAYER_SET_EQ)
	{
		if (DFPlayerCurrentEQ == 5)
			DFPlayerCurrentEQ = 0;
		else
			DFPlayerCurrentEQ++;
				
		parameter = DFPlayerCurrentEQ;
	}
			
	tempCommand[3] = command;
	tempCommand[6] = parameter;
	
	checksum = _DFPlayerChecksum(tempCommand);
	
	tempCommand[7] = (uint8_t)(checksum >> 8);
	tempCommand[8] = (uint8_t)checksum;
		
	UARTSendArray(tempCommand, 10);
	
	// Wait for answer
	if (command == DFPLAYER_GET_EQ)
	{
		for (uint8_t i = 0; i < 10; i++)
			tempCommand[i] = UARTGetByte();
			
		checksum = (tempCommand[7] << 8) | (tempCommand[8]); // Glue --> int16_t
			
		// If MSG OK return answer
		if ((tempCommand[0] == DFPLAYER_RX_START_BYTE) && (tempCommand[9] == DFPLAYER_RX_END_BYTE) && (checksum == _DFPlayerChecksum(tempCommand)))
			return tempCommand[6];
	}
	
	return -1; // Nothing to return
}

// Calculates checksum
int16_t _DFPlayerChecksum(uint8_t arr[])
{
	int16_t checksum = 0;
	
	for (uint8_t i = 1; i < 7; i++)
		checksum += arr[i];
	
	return -checksum;
}

 

Last Edited: Tue. Dec 12, 2017 - 10:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I use a "stack painter" derived from:

https://www.avrfreaks.net/forum/s...

 

David

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I've found that with the DFPlayer MP3 unit the checksum is optional in the message string.  The command will execute even if the checksum is not included.  Try sending 

{0x7E ,0xFF ,0x06 ,command ,0x00 ,0x00 ,parameter ,0xEF};

The biggest problem that I have with the DFPlayer is that there is no way to pause the playback and then restart it from the pause point.   The pause command is documented, but it doesn't work.  The stop command does actually halt the MP3 playback, but requires the unit to be completely reset after this command's execution to restore functionality. 

  I've got about 2000 MP3 songs on a 16Gigabyte SD card.  I can get a listing from doing a command-line dir command with the text redirected to a file when the SD card is inserted into a PC.  {E:\dir *.mp3  /s /b > c:\myMP3s\songlist.txt }      /s = enters all subdirectories  /b = brief listing

This listing matches the song number used by the DFPlayer.  I use a potentiometer connected to one of the AVR's 10-bit ADC channels.  If there is a new setting on the pot, indicating that I want to hear a new song, then the program multiplies the ADC result by 2 and sends this number to the DFPlayer to access the song.  The song number is the line number of the song listing file that the song title file name is displayed on.  I use the Software Serial library to send the commands to the DFPlayer module at 9600 baud.

This  module would be a lot easier to work with if all of its detailed documentation were not written only in Chinese.  But it's about the best of the $2 MP3 player-modules available on eBay.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Suit yourself on working with a local buffer.  I'm roundly lambasted for using globals anyway (regardless of how much more efficient it might be).

 

Tell which line is the "send line" that you are convinced is not carried out.  Show the generated code for that area.

 

[is one of the legs of the compound if test always false?]

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

slow_rider wrote:
However the send line is not being executed
By "send line" do you mean:

UARTSendArray(tempCommand, 10);

and like others asked, how are you observing that this is "not being executed". Are you talking about JTAG debugging, the simulator or some other observational technique or are you simply saying that the receiving end of the UARTSendArray() output does not appear to be getting the bytes. If so how do you know that's not simply an electrical thing?

 

I'd run this with JTAG or, failing that, in the simulator and right click select "Goto disassembly" so you can actually see the CALL/RCALL that invokes the UARTSendArray function and see if that opcode is executed. As it does not appear to be within a conditional block then I can see no reason how the compiler can avoid invoking it UNLESS it is inlined and optimised away or something? (the LSS should reveal that).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm running debug using JTAG and stepping through the code, looking at variables as I go.

I had a type-o in the post - the second line in not executing, and this refers to the second line from the dissembled code.

 

Looking at the following code from the function:

checksum = _DFPlayerChecksum(tempCommand);
	
tempCommand[7] = (uint8_t)(checksum >> 8);
tempCommand[8] = (uint8_t)checksum;

I can verify that checksum gets the correct value, this int16_t is split into two parts and loaded into the array in the next two lines of code, these are translated into assembly code by the compiler as so:

000000DA  STD Y+8,R25		Store indirect with displacement 
000000DB  STD Y+9,R24		Store indirect with displacement 

The value from R24 is not being loaded into the array, I can see the array just before stepping into 

UARTSendArray(tempCommand, 10);

and tempCommand[8] = 0 which is not the correct value. I also tried swapping the two lines of code, loading the lower byte first. That caused  tempCommand[8] to by loaded fine, but  tempCommand[7] = 0.

 

I'm not near my computer now, however I will try to remove the checksum variable and shorten the message, as Simonetta wrote. That might free up some ram. If that is the problem.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

slow_rider wrote:
I had a type-o in the post - the second line in not executing, and this refers to the second line from the dissembled code.
Do you think that readers here are pyschic? How can anyo0ne here apart from you know which line is the problematic one. You say the "second line" so do you mean:

1: int16_t DFPlayerSendCommand(uint8_t command, uint8_t parameter)
2: {
3:	uint8_t tempCommand[10] = {0x7E ,0xFF ,0x06 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0xEF};
4:	int16_t checksum;
5:	
6:	if (command == DFPLAYER_SET_EQ)
7:	{

the "second line" there is the opening brace of the function. Or by "second line" do you mean

1: int16_t DFPlayerSendCommand(uint8_t command, uint8_t parameter)
2: {
3:	uint8_t tempCommand[10] = {0x7E ,0xFF ,0x06 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0xEF};
4:	int16_t checksum;

but all that does is create a stack frame variable. (in fact you should be able to see that as SP will be adjusted by 12 bytes)

 

So how about telling us which line it is you think is not doing anything.

 

(BTW don't forget optimisation - the compiler will be spotting anything that is "pointless" and not generating code for it).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

slow_rider wrote:
I had a type-o in the post - the second line in not executing, and this refers to the second line from the dissembled code.
Do you think that readers here are pyschic? How can anyo0ne here apart from you know which line is the problematic one. You say the "second line" so do you mean:

 

000000DA  STD Y+8,R25		Store indirect with displacement 
000000DB  STD Y+9,R24		Store indirect with displacement 

I'll be looking at the code once I'm near my computer. I'll remove the local variables and check how that affects the stack pointer.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just posting random lines of disassembly (and the worst possible disassembly at that!) does not help. I want to know which line of the C it is that you claim not to be executing (the question you have posed in this thread)?

 

If you are saying that these two STD opcodes are not executing then that can only be because there's some kind of conditional branch before them that decides whether they execute or not. The very fact that they are using "Y+" suggests these are writes to a stack frame variable and as the code only has two stack frame variables:

3:	uint8_t tempCommand[10] = {0x7E ,0xFF ,0x06 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0x00 ,0xEF};
4:	int16_t checksum;

then one might assume that Y+1 to Y+10 are "tempCommand" and Y+11 and Y+12 are "checksum" so Y+8 and Y+9 are writes to tempCommand[7] and [8]. The code you show is:

tempCommand[7] = (uint8_t)(checksum >> 8);
tempCommand[8] = (uint8_t)checksum;

so clearly the two opcodes are those generated for those two lines of C.

 

So that's all working OK then? So what exactly are you claiming the problem here is?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

These are the same lines of code as in post #10, I've pasted the code and the disassembly of these two commands (which are):

tempCommand[7] = (uint8_t)(checksum >> 8);
tempCommand[8] = (uint8_t)checksum;

The second command seems to not execute, because when I pause the code just before executing

UARTSendArray(tempCommand, 10);

I can see that tempCommand[8] = 0 and that is not the value I'm expecting after the above lines of code are executed. R24 (as seen in the disassembled code) holds a value that needs to be inserted into the array and it does not. That is the problem.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

slow_rider wrote:
I can see
See how? You don't mean a watch window do you? What I'd do is follow execution into UARTSendArray. On entry to that R25:R24 will be a pointer to the buffer. Type that value into a Memory Window. What do the bytes there actually look like? Unlike debuggers the raw memory view almost never lies.

 

Also if you doubt the value in [7]/[8] then as you step over the two:

000000DA  STD Y+8,R25		Store indirect with displacement 
000000DB  STD Y+9,R24		Store indirect with displacement 

what is actually in R25 and R24 at that moment?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

M103C fuse, I guess.

Stefan Ernst

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sternst wrote:

M103C fuse, I guess.