Improved rf230bb.c synchronization/locking. Removed unnecessary locks

during Tx (single-threaded environment). Added protection to Rx thread
from buffer access by ISR.
This commit is contained in:
Ivan Delamer 2011-12-06 14:08:05 -07:00
parent 27daa94030
commit 57e686179c

View file

@ -305,7 +305,8 @@ hal_rx_frame_t rxframe[RF230_CONF_RX_BUFFERS];
* \retval STATE_TRANSITION The radio transceiver's state machine is in * \retval STATE_TRANSITION The radio transceiver's state machine is in
* transition between two states. * transition between two states.
*/ */
static uint8_t //static uint8_t
uint8_t
radio_get_trx_state(void) radio_get_trx_state(void)
{ {
return hal_subregister_read(SR_TRX_STATUS); return hal_subregister_read(SR_TRX_STATUS);
@ -506,8 +507,6 @@ flushrx(void)
rxframe[rxframe_head].length=0; rxframe[rxframe_head].length=0;
} }
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/
static uint8_t locked, lock_on, lock_off;
static void static void
on(void) on(void)
{ {
@ -581,19 +580,6 @@ off(void)
ENERGEST_OFF(ENERGEST_TYPE_LISTEN); ENERGEST_OFF(ENERGEST_TYPE_LISTEN);
} }
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/
#define GET_LOCK() locked = 1
static void RELEASE_LOCK(void) {
if(lock_on) {
on();
lock_on = 0;
}
if(lock_off) {
off();
lock_off = 0;
}
locked = 0;
}
/*---------------------------------------------------------------------------*/
static void static void
set_txpower(uint8_t power) set_txpower(uint8_t power)
{ {
@ -858,17 +844,11 @@ rf230_transmit(unsigned short payload_len)
{ {
int txpower; int txpower;
uint8_t total_len; uint8_t total_len;
uint8_t radiowason;
uint8_t tx_result; uint8_t tx_result;
#if RF230_CONF_TIMESTAMPS #if RF230_CONF_TIMESTAMPS
struct timestamp timestamp; struct timestamp timestamp;
#endif /* RF230_CONF_TIMESTAMPS */ #endif /* RF230_CONF_TIMESTAMPS */
GET_LOCK();
/* Save receiver state */
radiowason=RF230_receive_on;
/* If radio is sleeping we have to turn it on first */ /* If radio is sleeping we have to turn it on first */
/* This automatically does the PLL calibrations */ /* This automatically does the PLL calibrations */
if (hal_get_slptr()) { if (hal_get_slptr()) {
@ -954,7 +934,7 @@ rf230_transmit(unsigned short payload_len)
#if defined(__AVR_ATmega128RFA1__) #if defined(__AVR_ATmega128RFA1__)
sei(); sei();
#endif #endif
PRINTF("rf230_transmit:\n"); PRINTF("rf230_transmit: %d\n", (int)total_len);
#if DEBUG>1 #if DEBUG>1
/* Note the dumped packet will have a zero checksum unless compiled with RF230_CONF_CHECKSUM /* Note the dumped packet will have a zero checksum unless compiled with RF230_CONF_CHECKSUM
* since we don't know what it will be if calculated by the hardware. * since we don't know what it will be if calculated by the hardware.
@ -979,7 +959,7 @@ rf230_transmit(unsigned short payload_len)
#if RF230_CONF_AUTORETRIES #if RF230_CONF_AUTORETRIES
tx_result = hal_subregister_read(SR_TRAC_STATUS); tx_result = hal_subregister_read(SR_TRAC_STATUS);
#else #else
tx_result=0; tx_result=RADIO_TX_OK;
#endif #endif
#ifdef ENERGEST_CONF_LEVELDEVICE_LEVELS #ifdef ENERGEST_CONF_LEVELDEVICE_LEVELS
@ -991,14 +971,6 @@ rf230_transmit(unsigned short payload_len)
set_txpower(txpower & 0xff); set_txpower(txpower & 0xff);
} }
/* Restore receive mode */
if(radiowason) {
DEBUGFLOW('l');
on();
} else {
off();
}
#if RF230_CONF_TIMESTAMPS #if RF230_CONF_TIMESTAMPS
setup_time_for_transmission = txtime - timestamp.time; setup_time_for_transmission = txtime - timestamp.time;
@ -1012,24 +984,24 @@ rf230_transmit(unsigned short payload_len)
ENERGEST_OFF(ENERGEST_TYPE_TRANSMIT); ENERGEST_OFF(ENERGEST_TYPE_TRANSMIT);
if(RF230_receive_on) { if(RF230_receive_on) {
DEBUGFLOW('l');
ENERGEST_ON(ENERGEST_TYPE_LISTEN); ENERGEST_ON(ENERGEST_TYPE_LISTEN);
on();
} else { } else {
#if RADIOALWAYSON #if RADIOALWAYSON
/* Enable reception */ /* Enable reception */
on(); on();
#else #else
/* Go to lower power TRX_OFF state (turn off PLL) */ off();
hal_subregister_write(SR_TRX_CMD, CMD_FORCE_TRX_OFF); PRINTF("rf230_transmit: turning radio off\n");
#endif #endif
} }
RELEASE_LOCK();
#if RF230_INSERTACK #if RF230_INSERTACK
ack_pending = 0; ack_pending = 0;
#endif #endif
if (tx_result==1) { //success, data pending from adressee if (tx_result==1) { //success, data pending from addressee
tx_result=RADIO_TX_OK; //handle as ordinary success tx_result=RADIO_TX_OK; //handle as ordinary success
} }
@ -1051,6 +1023,7 @@ rf230_transmit(unsigned short payload_len)
} else if (tx_result==5) { //Expected ACK, none received } else if (tx_result==5) { //Expected ACK, none received
DEBUGFLOW('n'); DEBUGFLOW('n');
tx_result = RADIO_TX_NOACK; tx_result = RADIO_TX_NOACK;
PRINTF("rf230_transmit: ACK not received\n");
RIMESTATS_ADD(badackrx); //ack was requested but not received RIMESTATS_ADD(badackrx); //ack was requested but not received
} else if (tx_result==7) { //Invalid (Can't happen since waited for idle above?) } else if (tx_result==7) { //Invalid (Can't happen since waited for idle above?)
DEBUGFLOW('o'); DEBUGFLOW('o');
@ -1076,7 +1049,6 @@ rf230_prepare(const void *payload, unsigned short payload_len)
ack_seqnum=*(((uint8_t *)payload)+2); ack_seqnum=*(((uint8_t *)payload)+2);
#endif #endif
GET_LOCK();
DEBUGFLOW('p'); DEBUGFLOW('p');
// PRINTF("rf230: sending %d bytes\n", payload_len); // PRINTF("rf230: sending %d bytes\n", payload_len);
@ -1094,9 +1066,7 @@ rf230_prepare(const void *payload, unsigned short payload_len)
#if RADIOSTATS #if RADIOSTATS
RF230_sendfail++; RF230_sendfail++;
#endif #endif
#if DEBUG PRINTF("rf230_prepare: packet too large (%d, max: %d)\n",total_len,RF230_MAX_TX_FRAME_LENGTH);
printf_P(PSTR("rf230_prepare: packet too large (%d, max: %d)\n"),total_len,RF230_MAX_TX_FRAME_LENGTH);
#endif
ret = -1; ret = -1;
goto bail; goto bail;
} }
@ -1130,7 +1100,6 @@ rf230_prepare(const void *payload, unsigned short payload_len)
bail: bail:
RELEASE_LOCK();
return ret; return ret;
} }
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/
@ -1146,9 +1115,7 @@ rf230_send(const void *payload, unsigned short payload_len)
#endif #endif
if((ret=rf230_prepare(payload, payload_len))) { if((ret=rf230_prepare(payload, payload_len))) {
#if DEBUG PRINTF("rf230_send: Unable to send, prep failed (%d)\n",ret);
printf_P(PSTR("rf230_send: Unable to send, prep failed (%d)\n"),ret);
#endif
goto bail; goto bail;
} }
@ -1169,20 +1136,13 @@ rf230_off(void)
return 0; return 0;
} }
/* If we are called when the driver is locked, we indicate that the /* If we are currently receiving a packet, we still call off(),
radio should be turned off when the lock is unlocked. */ as that routine waits until Rx is complete (packet uploaded in ISR
if(locked) { so no worries about losing it). If using RX_AACK_MODE, chances are
lock_off = 1; the packet is not for us and will be discarded. */
return 1;
}
/* If we are currently receiving a packet
we don't actually switch the radio off now, but signal that the
driver should switch off the radio once the packet has been
received and processed, by setting the 'lock_off' variable. */
if (!rf230_isidle()) { if (!rf230_isidle()) {
lock_off = 1; PRINTF("rf230_off: busy receiving\r\n");
return 1; //return 1;
} }
off(); off();
@ -1196,11 +1156,6 @@ rf230_on(void)
DEBUGFLOW('q'); DEBUGFLOW('q');
return 1; return 1;
} }
if(locked) {
lock_on = 1;
DEBUGFLOW('r');
return 1;
}
on(); on();
return 1; return 1;
@ -1349,7 +1304,16 @@ PROCESS_THREAD(rf230_process, ev, data)
rf230_pending = 0; rf230_pending = 0;
packetbuf_clear(); packetbuf_clear();
/* Turn off interrupts to avoid ISR writing to the same buffers we are reading. */
HAL_ENTER_CRITICAL_REGION();
len = rf230_read(packetbuf_dataptr(), PACKETBUF_SIZE); len = rf230_read(packetbuf_dataptr(), PACKETBUF_SIZE);
/* Restore interrupts. */
HAL_LEAVE_CRITICAL_REGION();
PRINTF("rf230_read: %u bytes lqi %u\n",len,rf230_last_correlation);
RF230PROCESSFLAG(1); RF230PROCESSFLAG(1);
if(len > 0) { if(len > 0) {
packetbuf_set_datalen(len); packetbuf_set_datalen(len);
@ -1370,10 +1334,13 @@ PROCESS_THREAD(rf230_process, ev, data)
PROCESS_END(); PROCESS_END();
} }
/* Get packet from Radio if any, else return zero. /* Read packet that was uploaded from Radio in ISR, else return zero.
* The two-byte checksum is appended but the returned length does not include it. * The two-byte checksum is appended but the returned length does not include it.
* Frames are buffered in the interrupt routine so this routine * Frames are buffered in the interrupt routine so this routine
* does not access the hardware or change its status. * does not access the hardware or change its status.
* However, this routine must be called with interrupts disabled to avoid ISR
* writing to the same buffer we are reading.
* As a result, PRINTF cannot be used in here.
*/ */
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/
static int static int
@ -1433,44 +1400,40 @@ if (!RF230_receive_on) {
rf230_time_of_departure = 0; rf230_time_of_departure = 0;
#endif /* RF230_CONF_TIMESTAMPS */ #endif /* RF230_CONF_TIMESTAMPS */
// can't use PRINTF as interrupts are disabled
// PRINTSHORT("r%d",rxframe[rxframe_head].length); // PRINTSHORT("r%d",rxframe[rxframe_head].length);
PRINTF("rf230_read: %u bytes lqi %u crc %u\n",rxframe[rxframe_head].length,rxframe[rxframe_head].lqi,rxframe[rxframe_head].crc); //PRINTF("rf230_read: %u bytes lqi %u crc %u\n",rxframe[rxframe_head].length,rxframe[rxframe_head].lqi,rxframe[rxframe_head].crc);
#if DEBUG>1 #if DEBUG>1
{ {
uint8_t i; //uint8_t i;
PRINTF("0000"); //PRINTF("0000");
for (i=0;i<rxframe[rxframe_head].length;i++) PRINTF(" %02x",rxframe[rxframe_head].data[i]); //for (i=0;i<rxframe[rxframe_head].length;i++) PRINTF(" %02x",rxframe[rxframe_head].data[i]);
PRINTF("\n"); //PRINTF("\n");
} }
#endif #endif
// GET_LOCK();
//if(len > RF230_MAX_PACKET_LEN) { //if(len > RF230_MAX_PACKET_LEN) {
if(len > RF230_MAX_TX_FRAME_LENGTH) { if(len > RF230_MAX_TX_FRAME_LENGTH) {
/* Oops, we must be out of sync. */ /* Oops, we must be out of sync. */
DEBUGFLOW('u'); DEBUGFLOW('u');
flushrx(); flushrx();
RIMESTATS_ADD(badsynch); RIMESTATS_ADD(badsynch);
// RELEASE_LOCK();
return 0; return 0;
} }
if(len <= AUX_LEN) { if(len <= AUX_LEN) {
DEBUGFLOW('s'); DEBUGFLOW('s');
PRINTF("len <= AUX_LEN\n"); //PRINTF("len <= AUX_LEN\n");
flushrx(); flushrx();
RIMESTATS_ADD(tooshort); RIMESTATS_ADD(tooshort);
// RELEASE_LOCK();
return 0; return 0;
} }
if(len - AUX_LEN > bufsize) { if(len - AUX_LEN > bufsize) {
DEBUGFLOW('v'); DEBUGFLOW('v');
PRINTF("len - AUX_LEN > bufsize\n"); //PRINTF("len - AUX_LEN > bufsize\n");
flushrx(); flushrx();
RIMESTATS_ADD(toolong); RIMESTATS_ADD(toolong);
// RELEASE_LOCK();
return 0; return 0;
} }
/* Transfer the frame, stripping the footer, but copying the checksum */ /* Transfer the frame, stripping the footer, but copying the checksum */
@ -1500,8 +1463,8 @@ if (!RF230_receive_on) {
#if RF230_CONF_CHECKSUM #if RF230_CONF_CHECKSUM
if(checksum != crc16_data(buf, len - AUX_LEN, 0)) { if(checksum != crc16_data(buf, len - AUX_LEN, 0)) {
DEBUGFLOW('w'); DEBUGFLOW('w');
PRINTF("checksum failed 0x%04x != 0x%04x\n", //PRINTF("checksum failed 0x%04x != 0x%04x\n",
checksum, crc16_data(buf, len - AUX_LEN, 0)); // checksum, crc16_data(buf, len - AUX_LEN, 0));
} }
#if FOOTER_LEN #if FOOTER_LEN
if(footer[1] & FOOTER1_CRC_OK && if(footer[1] & FOOTER1_CRC_OK &&
@ -1547,7 +1510,7 @@ if (!RF230_receive_on) {
#if FOOTER_LEN #if FOOTER_LEN
} else { } else {
DEBUGFLOW('x'); DEBUGFLOW('x');
PRINTF("bad crc"); //PRINTF("bad crc");
RIMESTATS_ADD(badcrc); RIMESTATS_ADD(badcrc);
len = AUX_LEN; len = AUX_LEN;
} }
@ -1572,9 +1535,7 @@ if (!RF230_receive_on) {
void void
rf230_set_txpower(uint8_t power) rf230_set_txpower(uint8_t power)
{ {
GET_LOCK();
set_txpower(power); set_txpower(power);
RELEASE_LOCK();
} }
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/
uint8_t uint8_t
@ -1632,17 +1593,6 @@ rf230_cca(void)
uint8_t cca=0; uint8_t cca=0;
uint8_t radio_was_off = 0; uint8_t radio_was_off = 0;
/* If the radio is locked by an underlying thread (because we are
being invoked through an interrupt), we preted that the coast is
clear (i.e., no packet is currently being transmitted by a
neighbor). */
if(locked) {
DEBUGFLOW('|');
// return 1; rf230 hangs on occasion?
return 0;
}
/* Turn radio on if necessary. If radio is currently busy return busy channel */ /* Turn radio on if necessary. If radio is currently busy return busy channel */
/* This may happen when testing radio duty cycling with RADIOALWAYSON */ /* This may happen when testing radio duty cycling with RADIOALWAYSON */
if(RF230_receive_on) { if(RF230_receive_on) {