Valid block headers

[linguini1]

Currently, block headers have an additional field that labels whether or not they are valid.

See modules/telemetry/v1/block.py.

This should not be the design of the block header class. When constructed, BlockHeader should raise an exception if any of the parameters it's given could be considered invalid. This forces the calling code to handle the exception there and then or bubble it up for the higher level code.

As always, tests may need to be updated to reflect this change.

[EliasJRH]

Started looking into this, the current implementation of having a valid field in BlockHeader allowed the class to be initialized regardless if all of its fields were valid or not. This is also allowed the parsing function to at least have a valid block length which it could use to skip over the block if it was invalid.

With that being said, I propose that in stead of raising the error inside the class, we leave it as is and add a validation function similarly to how its done with PacketHeader. I'm not a huge fan of this idea but it would be a simpler fix that's consistent with how we're already checking packet headers.

The alternative (for consistency among classes) is to modify both PacketHeader and BlockHeader to raise exceptions in their classes. This would also require that during parsing, if we encounter and invalid block header, we would still need to extract the block length from the hex string.

Any thoughts?

[linguini1]

I prefer the second approach, and we can add a function to get block length from an invalid block to be able to skip properly. However, this raises an additional question:

If we ever receive an invalid block, it is likely because the data has been corrupted in transmission (we can assume the telemetry system never sends invalid blocks). In this case, should we trust the length field in the invalid block? Or should we toss the entire packet since it's been corrupted? I'm inclined to toss the entire remaining packet because if we trust the invalid block's length and it ends up being corrupted, we might encounter further parsing issues. I believe we receive enough packets per second as-is that tossing the packet wouldn't have a significant effect, especially with how few erroneous packets we should be receiving using the RN2483's built-in error correction.

[EliasJRH]

I think tossing the remaining packet is fine. I understand that the current spec supports a single packet containing multiple data blocks, is that ever the case in practice?

[linguini1]

I think tossing the remaining packet is fine. I understand that the current spec supports a single packet containing multiple data blocks, is that ever the case in practice?

Yes it is, I think I have configured to send about 3 blocks per packet. I still think tossing the packet is viable though.