Fix non trailing zero message

[d3473r]

This PR fixes a bug if the last field in the message is zero.
The field will get truncated and payload.readUInt8(start) will throw a range error.
This check start >= payload.length fixes the bug.

[nthorn552]

Hey there! I've been playing with this parser myself and I had a question about the truncation that happens here. From the MavLink V2 documentation:

'The protocol only truncates empty bytes at the end of the serialized message payload; any null bytes/empty fields within the body of the payload are not affected.'
https://mavlink.io/en/guide/serialization.html#payload_truncation

From my understanding of that, couldn't we say that once we work our way through the payload, any remaining fields are simply 0?

[d3473r]

Yep, i didn't realise that whole fields would be truncated. This PR fixes this issue. I could add a test with multiple null fields, but i think it should already work with the start >= payload.length check. We could improve the performance a little bit if we would return all remaining fields as zero immediately if start >= payload.length.
But truncation can also happen within a field, for example if we are reading an uint_32 which should have 4 bytes data. But if the value is small ie 2 there will be only one byte of data because of the truncation and we have to add the remaining 3 bytes to parse it as an uint_32.

[nthorn552]

Oh, good point, hadn't considered truncation in the middle of a larger-than-1-byte field! Thanks for explaining that.

I don't want to conflict with your PR, but would love to contribute, so I'm just going to send it to you directly and see what you think. I removed the payload.readUInt8(start) check because I think that can actually cause some false assumption of truncation, if I understand it correctly. Say the last field is a 32bit int, and the value is 0x0F00. If that is truncated, it will only be 0x0F. The current logic on line 82 (start >= payload.length || payload.readUInt8(start) === 0) will evaluate to true, which will put the current field's value to 0, when actually there was still non-zero bytes present.

If I'm wrong, feel free to throw it out haha, appreciate the guidance as I'm learning this protocol!

Code in my branch locally at this point:
if (payload.length >= start + field_length) {
... normal field read ...
} else if (start < payload.length) { // partially truncated field
... your partial truncation handling ...
} else { // fully truncated field
... original truncation handler ...
}

[d3473r]

Oh, good point, hadn't considered truncation in the middle of a larger-than-1-byte field! Thanks for explaining that.

I don't want to conflict with your PR, but would love to contribute, so I'm just going to send it to you directly and see what you think. I removed the payload.readUInt8(start) check because I think that can actually cause some false assumption of truncation, if I understand it correctly. Say the last field is a 32bit int, and the value is 0x0F00. If that is truncated, it will only be 0x0F. The current logic on line 82 (start >= payload.length || payload.readUInt8(start) === 0) will evaluate to true, which will put the current field's value to 0, when actually there was still non-zero bytes present.

If I'm wrong, feel free to throw it out haha, appreciate the guidance as I'm learning this protocol!

Code in my branch locally at this point:
if (payload.length >= start + field_length) {
... normal field read ...
} else if (start < payload.length) { // partially truncated field
... your partial truncation handling ...
} else { // fully truncated field
... original truncation handler ...
}

You are absolutely right, thanks 👍
Partially truncated fields with leading zeroes as in your example would not be parsed correctly.
I've added your example as a test.