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;