[Tinyos-devel] cc2420 radio stack and deputy

[David Gay]

I found two sets of problems in the CC2420 radio stack when using
Deputy on a micaz:

- First, in CC2420TransmitP.nc, m_msg was used in a bunch of places
where it could be null (just after boot). I added the obvious if's to
deal with that.

- Second, the lower-levels of the CC2420 radio stack were passing the
total packet length (well, the value of the packet's length byte
actually) as the payload length, which caused a runtime error for
packets where payload-size + header-size > max-payload size. As I
couldn't see any good reason why this was being done, I propose the
following patch which passes the correct payload length in
CC2420ReceiveP. There was also a symmetric misuse of the length in the
send path (starting in CC2420ActiveMessageP), which this patch also
fixes:

Index: CC2420.h
===================================================================
RCS file: /cvsroot/tinyos/tinyos-2.x/tos/chips/cc2420/CC2420.h,v
retrieving revision 1.11
diff -u -r1.11 CC2420.h
--- CC2420.h    17 Jun 2008 07:28:24 -0000      1.11
+++ CC2420.h    20 Jun 2008 22:54:56 -0000
@@ -138,6 +138,8 @@
   MAC_FOOTER_SIZE = sizeof( uint16_t ),
   // MDU
   MAC_PACKET_SIZE = MAC_HEADER_SIZE + TOSH_DATA_LENGTH + MAC_FOOTER_SIZE,
+
+  CC2420_SIZE = MAC_HEADER_SIZE + MAC_FOOTER_SIZE,
 };

 enum cc2420_enums {
Index: CC2420ActiveMessageP.nc
===================================================================
RCS file: /cvsroot/tinyos/tinyos-2.x/tos/chips/cc2420/CC2420ActiveMessageP.nc,v
retrieving revision 1.16
diff -u -r1.16 CC2420ActiveMessageP.nc
--- CC2420ActiveMessageP.nc     11 Jun 2008 00:46:23 -0000      1.16
+++ CC2420ActiveMessageP.nc     20 Jun 2008 22:54:56 -0000
@@ -57,10 +57,6 @@
 }
 implementation {

-  enum {
-    CC2420_SIZE = MAC_HEADER_SIZE + MAC_FOOTER_SIZE,
-  };
-
   /***************** AMSend Commands ****************/
   command error_t AMSend.send[am_id_t id](am_addr_t addr,
                                          message_t* msg,
@@ -73,7 +69,7 @@

     signal SendNotifier.aboutToSend[id](addr, msg);

-    return call SubSend.send( msg, len + CC2420_SIZE );
+    return call SubSend.send( msg, len );
   }

   command error_t AMSend.cancel[am_id_t id](message_t* msg) {
@@ -179,10 +175,10 @@
     }

     if (call AMPacket.isForMe(msg)) {
-      return signal Receive.receive[call AMPacket.type(msg)](msg,
payload, len - CC2420_SIZE);
+      return signal Receive.receive[call AMPacket.type(msg)](msg,
payload, len);
     }
     else {
-      return signal Snoop.receive[call AMPacket.type(msg)](msg,
payload, len - CC2420_SIZE);
+      return signal Snoop.receive[call AMPacket.type(msg)](msg, payload, len);
     }
   }

Index: csma/CC2420CsmaP.nc
===================================================================
RCS file: /cvsroot/tinyos/tinyos-2.x/tos/chips/cc2420/csma/CC2420CsmaP.nc,v
retrieving revision 1.5
diff -u -r1.5 CC2420CsmaP.nc
--- csma/CC2420CsmaP.nc 17 Jun 2008 07:28:24 -0000      1.5
+++ csma/CC2420CsmaP.nc 20 Jun 2008 22:54:56 -0000
@@ -134,7 +134,7 @@
       m_msg = p_msg;
     }

-    header->length = len;
+    header->length = len + CC2420_SIZE;
     header->fcf &= 1 << IEEE154_FCF_ACK_REQ;
     header->fcf |= ( ( IEEE154_TYPE_DATA << IEEE154_FCF_FRAME_TYPE ) |
                     ( 1 << IEEE154_FCF_INTRAPAN ) |
Index: receive/CC2420ReceiveP.nc
===================================================================
RCS file: /cvsroot/tinyos/tinyos-2.x/tos/chips/cc2420/receive/CC2420ReceiveP.nc,v
retrieving revision 1.12
diff -u -r1.12 CC2420ReceiveP.nc
--- receive/CC2420ReceiveP.nc   17 Jun 2008 07:28:24 -0000      1.12
+++ receive/CC2420ReceiveP.nc   20 Jun 2008 22:54:56 -0000
@@ -326,16 +326,17 @@
   task void receiveDone_task() {
     cc2420_metadata_t* metadata = call CC2420PacketBody.getMetadata(
m_p_rx_buf );
     cc2420_header_t* header = call CC2420PacketBody.getHeader( m_p_rx_buf);
+    uint8_t length = header->length;
     uint8_t tmpLen __DEPUTY_UNUSED__ = sizeof(message_t) -
(offsetof(message_t, data) - sizeof(cc2420_header_t));
     uint8_t* COUNT(tmpLen) buf = TCAST(uint8_t* COUNT(tmpLen), header);

-    metadata->crc = buf[ rxFrameLength ] >> 7;
-    metadata->lqi = buf[ rxFrameLength ] & 0x7f;
-    metadata->rssi = buf[ rxFrameLength - 1 ];
+    metadata->crc = buf[ length ] >> 7;
+    metadata->lqi = buf[ length ] & 0x7f;
+    metadata->rssi = buf[ length - 1 ];

     if(passesAddressCheck(m_p_rx_buf)) {
       m_p_rx_buf = signal Receive.receive( m_p_rx_buf, m_p_rx_buf->data,
-          rxFrameLength );
+                                          length - CC2420_SIZE);
     }

     atomic receivingPacket = FALSE;


If noone objects, I'll check this in on Monday (the change to get
length from the header rather than the global rxFrameLength is
probably not necessary, but feels safer).

Now I just need to figure out why a safe base station is unhappy...

David