patterncMinor
Writing an interface for networking in an embedded system
Viewed 0 times
networkingembeddedsystemwritinginterfacefor
Problem
I'm writing a driver in C for a WiFi module attached to a PIC18 microcontroller. I want to implement some functions that I'm familiar with in computer application level programming like Window's API recv function for WinSock.
The module itself already implements the WiFi communication protocol and the TCP/IP stack, and communicate with the PIC18 microcontroller through UART.
Is this the correct way to achieve this? Efficiency is very important here (It's an embedded system too).
The module itself already implements the WiFi communication protocol and the TCP/IP stack, and communicate with the PIC18 microcontroller through UART.
Is this the correct way to achieve this? Efficiency is very important here (It's an embedded system too).
#define RECEIVE_TIMEOUT 200
#define RECEIVE_RETRY_COUNT 4
UINT16 ReceiveData(void* _data, UINT16 length)
{
UINT16 received = 0;
UINT8 retry_count = 0;
while(received < length)
{
if(UART1_Data_Ready())
{
((BYTE*)_data)[received] = ReceiveByte();
received++;
continue;
}
if(retry_count == RECEIVE_RETRY_COUNT)
break;
retry_count++;
Delay_ms(RECEIVE_TIMEOUT/RECEIVE_RETRY_COUNT);
}
return received;
}Solution
Just a small note on sleeping when waiting for input. As others have said
your loop is inefficient in that it uses a simple busy loop. This has at
least two potential problems:
-
it prevent other activities from executing;
-
it uses power.
But these are not necessarily problems for your application. And if in
your application there really is nothing else to be done until bytes arrive
and if power consumption is not a problem, then a busy loop is by far the
simplest solution.
Using interrupts will inevitably complicate the rest of your code and should
be avoided unless really necessary (for example if there are other
computing tasks that need to run while waiting for input).
If 1 above is not important but 2 is (ie if you need to minimise power use)
you could put the core to sleep during waiting and have the byte-receive wake
it. This would retain the benefit of avoiding interrupt handling. You would
need to set a timer to wake the core too if you need a timeout.
On the details of your code, I dislike the use of Windows types in an embedded
system. What is wrong with
Your leading underscore on
pointless.
I don't object to passing in a
and
type and avoid casts. This makes absolutely no difference when the buffer is
used just once, as here and is just a personal preference.
And using
your loop is inefficient in that it uses a simple busy loop. This has at
least two potential problems:
-
it prevent other activities from executing;
-
it uses power.
But these are not necessarily problems for your application. And if in
your application there really is nothing else to be done until bytes arrive
and if power consumption is not a problem, then a busy loop is by far the
simplest solution.
Using interrupts will inevitably complicate the rest of your code and should
be avoided unless really necessary (for example if there are other
computing tasks that need to run while waiting for input).
If 1 above is not important but 2 is (ie if you need to minimise power use)
you could put the core to sleep during waiting and have the byte-receive wake
it. This would retain the benefit of avoiding interrupt handling. You would
need to set a timer to wake the core too if you need a timeout.
On the details of your code, I dislike the use of Windows types in an embedded
system. What is wrong with
int and char or their unsigned varieties?Your leading underscore on
_data is reserved by something or other and is anywaypointless.
I don't object to passing in a
void * (after all, C stdlib functions recvand
read do this), but I would assign it to a local variable of appropriatetype and avoid casts. This makes absolutely no difference when the buffer is
used just once, as here and is just a personal preference.
And using
continue is usually frowned on and often unnecessary.int ReceiveData(void *data, int length)
{
unsigned char *p = data;
int received = 0;
int retry = 0;
while (received < length)
{
if (UART1_Data_Ready())
{
p[received++] = ReceiveByte();
}
else if (++retry <= RECEIVE_RETRY_COUNT)
{
Delay_ms(RECEIVE_TIMEOUT/RECEIVE_RETRY_COUNT);
}
else break;
}
return received;
}Code Snippets
int ReceiveData(void *data, int length)
{
unsigned char *p = data;
int received = 0;
int retry = 0;
while (received < length)
{
if (UART1_Data_Ready())
{
p[received++] = ReceiveByte();
}
else if (++retry <= RECEIVE_RETRY_COUNT)
{
Delay_ms(RECEIVE_TIMEOUT/RECEIVE_RETRY_COUNT);
}
else break;
}
return received;
}Context
StackExchange Code Review Q#48325, answer score: 5
Revisions (0)
No revisions yet.