Page 1 of 1

MSP_SET_RAW_GPS does not generate a response

PostPosted: Thu Jun 21, 2012 12:15 am
by Tommie
Due to an added break statement, MSP_SET_RAW_GPS does not generate a response while including unreachable code in its switch branch:

Code: Select all
   case MSP_SET_RAW_GPS:
     f.GPS_FIX = read8();
     GPS_numSat = read8();
     GPS_coord[LAT] = read32();
     GPS_coord[LON] = read32();
     GPS_altitude = read16();
     GPS_speed = read16();
     GPS_update |= 2; break;              // New data signalisation to GPS functions
     headSerialReply(0);
     break;


As a sidenote, is there any documentation what GPS_update should contain? Until recently, it was only used as a boolean flag, now it seems to be some kind of bitmask?

Re: MSP_SET_RAW_GPS does not generate a response

PostPosted: Thu Jun 21, 2012 12:33 am
by kos

Re: MSP_SET_RAW_GPS does not generate a response

PostPosted: Thu Jun 21, 2012 12:38 am
by Tommie
Yes, I am aware of that change. Did I mention that multiple instructions per line of code is considered harmful?

Re: MSP_SET_RAW_GPS does not generate a response

PostPosted: Thu Jun 21, 2012 1:15 pm
by Alexinparis
ok, noted ;)
GPS_update was a boolean, and MIS (one OSD developper) found usefull to use the second bit for GPS_FROM_OSD feature.
for other needs it's still compatible with a boolean value.

Re: MSP_SET_RAW_GPS does not generate a response

PostPosted: Thu Jun 21, 2012 1:17 pm
by Tommie
Please document such behaviour; code might rely on stuff like (GPS_Update == 1) and fail miserably.

Each variable should be accompanied by a comment describing its data format and purpose (e.g. it is not trivial to guess that headFreeHoldMode is not a boolean flag, but holds the reference heading in...what exactly? degrees? degrees/100?).

Re: MSP_SET_RAW_GPS does not generate a response

PostPosted: Thu Jun 21, 2012 9:38 pm
by Mis
The "break;" instruction after GPS_update |= 2; is obviously wrong. Please remove this.