wTrig.getVersion() array overrun Issue

[KieranReck]

ISSUE
Using the wTrig.getVersion() function as outlined in the WTrigAdvanced.ino example sketch causes an array overrun within the gWTrigVersion char array. This causes your arduino sketch to get stuck in a reset loop within the setup() function.

STEPS TO RECREATE
Run the WTrigAdvanced example file but with an empty loop() function. You will see that the sketch never prints the firmware version, number of tracks, or "WAV Trigger response not available" over the serial monitor and instead gets stuck resetting the setup() function. (the attached .txt sketch can be used to recreate the issue)
WTrigAdvanced(current).txt

PROPOSED FIX
In the wavTrigger.cpp file, modify line 236.

WAV-Trigger-Arduino-Serial-Library/wavTrigger.cpp

Line 236 in 60e89a3

pDst[++i] = 0;

Current:
pDst[++i] = 0;

Proposed:
pDst[i] = 0;

The overrun is occurring because when the FOR loop reaches its break condition, i = VERSION_STRING_LEN - 1. This is the maximum boundary of "char gWTrigVersion[VERSION_STRING_LEN]" so incrementing i again before adding the zero character takes it beyond bounds.

From my testing, this proposed fix appears to work well, but this is my first time venturing into .cpp and .h files so please don't take my word for it. Also, sorry if this issue report isn't formatted correctly, I have never submitted one before. Thanks for all of the hard work your done so far on this library!

[nomakewan]

However, the character string for the version is built in the FOR loop. Let's say just for giggles that the version string ends up being VERSION_141 and therefore VERSION_STRING_LEN is 11. When this string is done being returned in the FOR loop, it means that the following will be the case:

char[] pDst == {V, E, R, S, I, O, N, _, 1, 4, 1}
VERSION_STRING_LEN == 11
int i == 10

Okay. Now, say you take the prefix increment and remove it, so pDst[++i] = 0; becomes pDst[i] = 0;. What happens to our data? It will now look like this:

char[] pDst == {V, E, R, S, I, O, N, _, 1, 4, 0}

This is because i == 10, and pDst[10] contains the final character in the string, which was 1. Now our version number is incorrect, since the WAV Trigger is trying to tell us it's Version 1.41, but the function is returning Version 1.40. So there are actually two solutions.

1. Correct the .ino code. Instead of char gWTrigVersion[VERSION_STRING_LEN]; use char gWTrigVersion[VERSION_STRING_LEN+1];. If your character array doesn't go out of bounds, the problem is solved.
2. Remove the pDst[++i] = 0; line from getVersion() entirely. Honestly I am not sure why it is there, but I don't have enough experience with this library yet to interpret its intended purpose.

Hope that helps!

EDIT: After looking into some more code for another issue, I notice this in update() in the section that returns the version string from the RX buffer:

for (i = 0; i < (VERSION_STRING_LEN - 1); i++)
	version[i] = rxMessage[i + 1];
version[VERSION_STRING_LEN - 1] = 0;

This looks like a simple case of double-dipping to me. It seems like version[VERSION_STRING_LEN - 1] gets junk data from rxMessage[VERSION_STRING_LEN] in the last step of the FOR loop, and so gets zeroed once the loop exits. But if this is this case, then we can assume the version array is already clean, so there's absolutely no reason to clean it again in getVersion(). As such, solution 2 above is in fact correct.