Writing a better code in better format

Hi guys I am a new learner in this field;
I am working on STR912x;

I am using the following code for sending a string which have saved in memory

          memset(sign_packet_data, 0, 0xFF);
              sign_packet_data = &sign_UART_txBuff[0];//char *sign_packet_data =0;
              strncpy(sign_packet_data, cmd,16);//cmd stores the string
              if(sign_packet_data != 0)
              {
               NbrOfDataToTransfer = strlen(sign_UART_txBuff);
               while(NbrOfDataToTransfer--)
               {
                UART_SendData(UART0, sign_UART_txBuff[TxCounter++]);
                while(UART_GetFlagStatus(UART0, UART_FLAG_TxFIFOFull) != RESET);
               }
               }

may i have a better way to do the same.

 RE: I am a new learner in this field

In what field, exactly?

- in any form of programming?

- in 'C' programming specifically?

- with microcontrollers generally?

- with the STR912x specifically?

For some basic tips on learning 'C', see: http://blog.antronics.co.uk/2011/08/08/

 RE: I am a new learner in this field

with the STR912x specifically:

I wish to know if I can use some function other than copying the data in the txbuffer everytime
i.e. strcpy

 RE: with the STR912x specifically

But there is nothing in the code you posted that relates specifically to the STR912x - it is all entirely generic 'C' code!!

"I wish to know if I can use some function other than copying the data in the txbuffer everytime e.g. strcpy"

Of course you can - why ever not?!

 RE: nothing in the code you posted relates specifically to the STR912x

OK, so this is rather specifc:

while( UART_GetFlagStatus(UART0, UART_FLAG_TxFIFOFull) != RESET );

But it could easily (and should?) be generalised to:

while( UART_IsBusy(UART0) );

or similar - thus keeping all the target-specific detail separate from the generic string handling.

 RE: Writing a better code in better format

You have to spend some own time thinking about what you want to do and how you may do it. Then ask more specific questions. By the way:

memset(sign_packet_data, 0, 0xFF);
sign_packet_data = &sign_UART_txBuff[0];//char *sign_packet_data =0;

You do a memset() before you assign a value to the sign_packet_data pointer. What did it point to before, i.e. what memory did you fill?

Second - you do a memset with size 255. Don't use hard-coded sizes. If you have an array, you can do sizeof(array). If you only have a pointer, you can't do sizeof(p) since that will just give the size of the pointer and not the size of the target of the pointer. But then you should have a constant somewhere, giving the size of the array.

strncpy(sign_packet_data, cmd,16);//cmd stores the string

One more hard-coded size in your code. How do you know that sign_packet_data - or cmd - are 16 bytes large? Why not use a named constant?

if (sign_packet_data != 0) { ... }

This test is meaningless. You have already assigned an address to sign_packet_data, so you already know that it isn't NULL. Probably, you intended to check if the string pointed to was non-zero, in which case you should have written:

if (*sign_packet_data != 0) { ... }

How do you know that there are any terminating zero when doing this call?

NbrOfDataToTransfer = strlen(sign_UART_txBuff);

sign_UART_txBuff was copied to the memory pointed at by sign_packet_data with a length of 16.

You have at one time used len 255 when doing a memset() of sign_packet_data - confusing change of length parameter.

And you did a strncpy() from cmd, in which case the copy can break before any terminating zero was copied - and your zero-fill using sign_packet_data was done before you assigned any address to sign_packet_data...

UART_SendData(UART0, sign_UART_txBuff[TxCounter++]);

Why do you jump between different representations?
Why assign a value to the pointer sign_packet_data if you are not using it while sending, but instead goes back to performing indexed access into sign_UART_txBuff?

Somehow, I think you have to sit down and figure out exactly what you want to do. Then spend time with every single source code line and make sure it is needed. And make sure you understand what it does. And make sure what is does is what you want it to do.