Hi folks,

I just burned an hour trying to figure out why the radio module wasn't
delivering packets.  Turns out it was because the CRC was failing.  I
had changed the TOS_Msg structure by adding new fields on to the end
of it.  CRCPACKETOBJ's CRC-calculating code assumes that the CRC is
always going to be at the end of the structure, and that the CRC is
always exactly 2 bytes long.

This seems brittle to me (especially because there was no comment on
the structure warning me that 'crc' must go last).  Can I suggest an
alternative CRC-calculating algorithm that will work independent of
the crc's size or position in the structure?

When calculating a CRC:
 1- Set CRC field to 0
 2- Calculate CRC
 3- Put calculated CRC into structure

And when checking it:
 1- Copy calculated CRC out of structure
 2- Set CRC field of structure to 0
 3- Compute CRC of structure and compare

This should work regardless of the word size, crc's position in the
structure, and so forth.

Here is a patch to CRCPACKETOBJ.c that does it:

--- CRCPACKETOBJ.c      2001/05/22 05:50:26     1.1.1.1
+++ CRCPACKETOBJ.c      2001/06/27 07:53:32
@@ -44,16 +44,19 @@
 TOS_FRAME_END(PACKET_obj_frame);
 
 TOS_TASK(CRC_calc){
- ((TOS_MsgPtr)VAR(data))->crc = calcrc(VAR(data), sizeof(TOS_Msg) - 2);
+ ((TOS_MsgPtr)VAR(data))->crc = 0;
+ ((TOS_MsgPtr)VAR(data))->crc = calcrc(VAR(data), sizeof(TOS_Msg));
 #ifdef FULLPC_DEBUG
  printf("CRC: %x\n", ((TOS_MsgPtr)VAR(data))->crc);
 #endif
 }
 
 TOS_TASK(check_crc){
- int crc = calcrc(VAR(msg), sizeof(TOS_Msg) - 2);
+  int crc = ((TOS_MsgPtr)VAR(msg))->crc;
+  ((TOS_MsgPtr)VAR(msg))->crc = 0;
+
  VAR(state) = 0;
- if(crc == ((TOS_MsgPtr)VAR(msg))->crc){
+ if(crc == calcrc(VAR(msg), sizeof(TOS_Msg))) {
            TOS_MsgPtr tmp;
            tmp = TOS_SIGNAL_EVENT(PACKET_RX_PACKET_DONE)((TOS_MsgPtr)VAR(msg));
            if(tmp != 0) VAR(msg) = (char*)tmp;

Reply via email to