adafruit / AdafruitClassLibrary

Windows IoT Core libraries for Raspberry Pi
MIT License
26 stars 11 forks source link

Consider adding unit tests (and in particular around the GPS/NMEA parsing) #3

Open lloydjatkinson opened 7 years ago

lloydjatkinson commented 7 years ago

Hi,

There are no unit tests for the library; it would be great if there were some so that future library changes can be tested before being deployed. Imagine someone updating their NuGet packages and finding their sensor no longer works as they expected!

For example, some unit tests around parsing of the date time would be ideal, as currently there seems to be a workaround:

https://github.com/adafruit/AdafruitClassLibrary/blob/91d444ccb6a6934ef352c1eed255f771c9e19ba6/AdafruitClassLibrary/GPS.cs#L904-L916

Another area that definitely needs unit testing is the parser functionality. There are several methods like the snippet below. What happens if, for example, the GPS module stops communicating because it crashed? If this happened during a serial transmission of the NMEA string you are very likely to have an incomplete string and you will encounter a IndexOutOfRangeException because the code is looking at specific elements. As far as I can tell, this exception will then bubble up for the user to deal with. In combination with unit testing, a robust parser can be built which can return a "sorry, this is a bad string, can't do anything with it" result instead of leaving it to chance and forcing the user to deal with out of bounds exceptions.

        public GPSRMC(string sentence)
        {
            string[] splitString = sentence.Split(new char[] { ',', '*' });

            TimeStamp = ParseTimeStamp(splitString[0]);            // Time Stamp
            Valid = splitString[1] == "A" ? true : false;          // validity -A - ok, V - invalid
            Latitude = ParseDouble(splitString[2]);                // current Latitude
            LatHemisphere = splitString[3];                        // North/ South
            Longitude = ParseDouble(splitString[4]);               // current Longitude
            LonHemisphere = splitString[5];                        // East/ West
            Speed = ParseDouble(splitString[6]);                   // Speed in knots
            Course = ParseDouble(splitString[7]);                  // True course
            DateStamp = ParseDateStamp(splitString[8]);            // Date Stamp
            MagVariation = ParseDouble(splitString[9]);            // Magnetic Variation
            VarDirection = splitString[10];                        // East/ West

            LatDegrees = ToDegrees(Latitude);
            LonDegrees = ToDegrees(Longitude);
        }
driverblock commented 7 years ago

Thanks for the tip. I'll put this in the queue to work on. Kind of tied up on other things right now...