View previous topic :: View next topic |
Author |
Message |
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
problem copying ram buffer arrays |
Posted: Wed Mar 31, 2010 7:57 am |
|
|
I am trying to copy a muti-dimensional ( 2 dimension string buffer)
but neither memcpy or strcpy seem to work.
If I do it the hard way( see the for code below), it does work.
Any comments ?
Just for explanation, I have a serial isr, which flips between 2 buffers, this allows another command to come down the serial port, whilst the previous command is being parsed and processed.
In the isr, if the character is a CR, then I pop in 0x00 into the buffer, so string processing can work.
Code: |
void Process_RS232_Command ()
{
char RS232_receive_buffer[RS232_Buffer_length];
int8 Buffer_No;
if (Command_received_flag==FALSE) return; // nothing to do
debug_nops_5
Buffer_No=((~RS232_receive_string_buffer_no) & 0x01);
//memcpy(&RS232_receive_buffer,&(RS232_receive_string[0][Buffer_No]),RS232_Buffer_length);
//strcpy(&RS232_receive_buffer,&(RS232_receive_string[0][Buffer_No]));
for (i=0;i<RS232_Buffer_length;i++)
{
RS232_receive_buffer[i]=RS232_receive_string[i][Buffer_No];
}
|
|
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19505
|
|
Posted: Wed Mar 31, 2010 9:33 am |
|
|
Put your row number _first_.
In C, "Elements are stored by rows, so the rightmost subscript, or column, varies fastest as elements are accessed in storage order" (direct quote from K&R). So to do what you want, the 'buffer number', must be the first subscript.
Best Wishes |
|
|
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
|
Posted: Wed Mar 31, 2010 10:14 am |
|
|
@Ttelmah,
I will try it tomorrow.
BUT, are you saying that it should have worked, but slowly, or that it wouldnt work the way I had it ?
Mike |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Thu Apr 01, 2010 1:47 am |
|
|
What Ttelmah is saying is that memcpy and strcpy require your data to be in a continuous block of memory.
When creating a multidimensional array it assigns memory in the following way :-
array[3][10]
First block of 10 is at array[0] or &array[0][0] (they mean the same)
Next block of 10 is at array[1] or &array[1][0]
Next block of 10 is at array[2] or &array[2][0]
Each block is contiguous.
You are using your arrays as though the blocks are :-
array[0][0], array[1][0], array[2][0] which are not contiguous
So although your copy will work it will be copying the wrong chars.
You need to change your overall string handling routines.
Secondly
char RS232_receive_buffer[RS232_Buffer_length];
char *p;
p = RS232_receive_buffer; // p now = the address of the start of the array
p = &RS232_receive_buffer[0]; // p = the address of the first char (same as above)
p = &RS232_receive_buffer; // p now = the address of where the address of the array is stored! Not sure what this would return on a PIC using CCS)
The same goes for &(RS232_receive_string[0][Buffer_No])
What would look better would be, once you have corrected your strings
memcpy(RS232_receive_buffer, RS232_receive_string[Buffer_No], RS232_Buffer_length);
It may not like that so
memcpy(RS232_receive_buffer, &RS232_receive_string[Buffer_No][0], RS232_Buffer_length); |
|
|
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
|
Posted: Thu Apr 01, 2010 2:38 am |
|
|
@wayne,
many thanks for your explanation.
Also, I have always been confused about the use of pointers ( as are many people).
char *p;
p = RS232_receive_buffer; // p now = the address of the start of the array
if *p is a char, I assume its an int8, so will it work as an address pointer, since it would limit to 255 addresses.
Mike |
|
|
ckielstra
Joined: 18 Mar 2004 Posts: 3680 Location: The Netherlands
|
|
Posted: Thu Apr 01, 2010 2:58 am |
|
|
MikeW wrote: | if *p is a char, I assume its an int8, so will it work as an address pointer, since it would limit to 255 addresses. | *p is not a character but a 'pointer to a character'. The size of this pointer is something the compiler decides and will be large enough to cover the whole address space ( > 255 bytes). It is dangerous to make assumptions as the size could change in the next compiler release, best is to use the sizeof() operator. |
|
|
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
|
Posted: Thu Apr 01, 2010 4:47 am |
|
|
now I am totally confused !!!!!!!
It was working with my "kludge" when i originally posted.
here is a newer version of the interrupt receive routine.
---------------------------------------------------------------------------------
#define RS232_Buffer_length 50
CHAR RS232_receive_string[RS232_Buffer_length][2];
CHAR RS232_receive_string_swapped[2][RS232_Buffer_length];
int1 Command_received_flag=false;
int1 Command_Processed_flag=false;
int8 RS232_receive_string_buffer_no=0;
#define LF 0x0A // LF is ignored
#define CR 0x0D // Carriage return
#define End_Of_Message_Character CR // CR ends the string
#int_rda
void serial_isr() {
static int8 buffer_count=0;
int8 received_char;
received_char=fgetc(Serial_Port_PIC);
if (Command_Processed_flag==TRUE)
{
buffer_count=0;
Command_received_flag=FALSE;
Command_Processed_flag=FALSE;
}
if (received_char==End_Of_Message_Character) // currently defined as CR
{
Command_received_flag=TRUE;
received_char=0x00; // tell printf its the end of the string
RS232_receive_string_buffer_no++;
RS232_receive_string_buffer_no&=0x01; // only allow 0 or 1
}
if (received_char==LF)
{
RETURN; // ignore linefeeds
}
RS232_receive_string[buffer_count++][RS232_receive_string_buffer_no]=received_char;
RS232_receive_string_swapped[RS232_receive_string_buffer_no][buffer_count++]=received_char;
debug_nops_5
if(buffer_count >=RS232_Buffer_length) buffer_count=0; // Buffer full !!
} // end of function
---------------------------------
notice, I have now 2 buffer arrays, with the subscripts swapped.
the .sym file tells me
19A-1FD RS232_receive_string
1FE-261 RS232_receive_string_swapped
I send the serial port ascii "123456789"
@ 0x19A in ram there is 31 00 00 00 32 00 00 00 33 etc
@ 0x1FE in ram there is 00 31 00 32 00 33 etc
I was expecting contiguous 31 32 33 34 35 etc in at least one of the arrays
Mike
[/img] |
|
|
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
|
Posted: Thu Apr 01, 2010 5:02 am |
|
|
belay that last post.
i had buffer_count++ implemented twice.
however, if i swap the subscripts, it does put the 31 32 33 33 34 in contiguous bytes at 0x19A
so, i am still confused
i define the array number after the max of numbers of characters in the buffer, but when storing the character, it only seems to work, if i reverse this
here is the code
PIC18F4680 v4.104
----------------------------------------------
#define RS232_Buffer_length 50
CHAR RS232_receive_string[RS232_Buffer_length][2];
int1 Command_received_flag=false;
int1 Command_Processed_flag=false;
int8 RS232_receive_string_buffer_no=0;
#define LF 0x0A // LF is ignored
#define CR 0x0D // Carriage return
#define End_Of_Message_Character CR // CR ends the string
#int_rda
void serial_isr() {
static int8 buffer_count=0;
int8 received_char;
received_char=fgetc(Serial_Port_PIC);
if (Command_Processed_flag==TRUE)
{
buffer_count=0;
Command_received_flag=FALSE;
Command_Processed_flag=FALSE;
}
if (received_char==End_Of_Message_Character) // currently defined as CR
{
Command_received_flag=TRUE;
received_char=0x00; // tell printf its the end of the string
RS232_receive_string_buffer_no++;
RS232_receive_string_buffer_no&=0x01; // only allow 0 or 1
}
if (received_char==LF)
{
RETURN; // ignore linefeeds
}
RS232_receive_string[RS232_receive_string_buffer_no][buffer_count++]=received_char;
debug_nops_5
if(buffer_count >=RS232_Buffer_length) buffer_count=0; // Buffer full !!
} // end of function |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Thu Apr 01, 2010 6:36 am |
|
|
I noticed the buffer_count++ twice problem lol.
This
CHAR RS232_receive_string[RS232_Buffer_length][2];
first of all it reserves memory, as it happens it will reserve the same amount as :-
CHAR RS232_receive_string[2][RS232_Buffer_length];
Also the address of the first item will be [0][0] for both so you may see memcpy work for the fist string RS232_receive_string[0] which ever way you do it BUT the address of RS232_receive_string[1] will be at a different offset from RS232_receive_string[0] depending on which way you choose.
Think of it like this
array[x][y]
x0 has (y0, y1, y2, y3)
x1 has (y0, y1, y2, y3)
So a string stored at x1 will exist in addresses
y0 = (x1+0), y1 = (x1+1), y2 = (x1+2), y3 = (x1+3) etc.
Does that help ? |
|
|
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
|
Posted: Thu Apr 01, 2010 6:48 am |
|
|
@wayne,
i am even more confused now, lets keep it simple.
I allocate a 2 dimensional array with
CHAR RS232_receive_string[RS232_Buffer_length][2];.
I then accept characters and stuff them here.
with RS232_receive_string_buffer_no =0
RS232_receive_string[buffer_count++][RS232_receive_string_buffer_no] =received_char;
I would expect sending 123456789
to be stored in consecutive bytes as 31 32 33 34 35 36 67 38 39
starting at address 0x19A as specified by the .sym file.
it doesnt, it puts 31 00 32 00 33 00 34 00 35 etc.
it feels like a bug to me
Mike |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19505
|
|
Posted: Thu Apr 01, 2010 6:56 am |
|
|
Can I make a comment though.
Why move the data?.
Moving data around, takes time.
Why not just toggle the buffer number, and work on the other buffer?.
So, something like:
Code: |
int1 in_use=0;
#define ISR_BUFFER (in_use)
#define MAIN_BUFFER (in_use ^ 1)
#define CHANGE_BUFFER in_use^=1
char buffer[2][SIZEOF_BUFFER];
void isr(void) {
buffer[ISR_BUFFER][count]=getc();
//Do whatever you want here
if (condition_to_change_buffer) CHANGE_BUFFER;
}
//Then in main, talk to buffer[MAIN_BUFFER][parse_count]
|
Best Wishes |
|
|
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
|
Posted: Thu Apr 01, 2010 7:11 am |
|
|
@Ttelmah,
this starting to do my head in !!!.
I thought I was doing it reasonably efficiently.
1. define 2 buffers, and flick between them in the isr once a message has been received.
2. then outside the isr, process the message, so the isr can continue with a new message.
your response seems to go back to the array[bufferno][parse count] definition,
which was my original way, but i was told to change it to
array[parse count][bufferno]
either way, your reply doesnt seem to answer my last post, that the array ends up with 31 00 32 00 33 00 34 00 35 if i do it the "correct way".
Mike |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19505
|
|
Posted: Thu Apr 01, 2010 7:26 am |
|
|
The point about index order, only applies if you copy the buffer. The point I was making, was "why copy"?. Just parse the second buffer. Don't move data unless you have to....
Best Wishes |
|
|
MikeW
Joined: 15 Sep 2003 Posts: 184 Location: Warrington UK
|
|
Posted: Thu Apr 01, 2010 7:35 am |
|
|
@Ttelmah,
I dont disagree that it would be simpler to work on the buffers directly ( outside of the isr), rather than doing memcpy or strcpy etc.
However, having found a possible problem in saving to the arrays, i want to get to the bottom of the problem.
as I said earlier, in my original post, the code without memcpy gives the correct answer, so why bother to delve deeper, and waste more than a days effort.
The answer to that is, it will come back to bite me later if i dont understand whats going on.
Also, how can I manipulate the data in the ram array, if its in the wrong place ?
Mike |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Thu Apr 01, 2010 9:47 am |
|
|
There is no problem with multi-dimensional arrays it is just your understanding of them and how memory is allocated for them.
RS232_receive_string[buffer_count++][RS232_receive_string_buffer_no] =received_char;
this line will store the char in the array and the increment the index of the first dimension. because of the way they are stored the next time you store a character it will be at position (buffer_count * dimension2_size) e.g.
buf[5][2]; // Starts at memory address 0
buf[0][0] = 'a'; // this puts 'a' at address 0
buf[1][0] = 'b'; // this puts 'b' at address 2
buf[2][0] = 'c'; // this puts 'c' at address 4
but
buf[2][5]; // Starts at memory address 0
buf[0][0] = 'a'; // this puts 'a' at address 0
buf[0][1] = 'b'; // this puts 'b' at address 1
buf[0][2] = 'c'; // this puts 'c' at address 2
buf[1][0] = '1'; // will put '1' at address 5
buf[1][1] = '2'; // will put '2' at address 6
Each index of the second dimension is in a contiguous block.
Strings require data to be contiguous and end with a null. |
|
|
|