Change _int_decoder to allow uint32 into smaller types instead of OverflowException #32
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hrowing OverflowException - The Sparkplug B Protobuf spec defines the Metric value as (oneof) uint32 along with a datatype to convert to. - The _int_decoder method was too strict in throwing an Overflow Exception when the uint32 value was larger than its defined datatype because this ignores how twos-compliment numbers get sign-extended when cast to larger types. The value should be decoded using the datatype to mask off the desired number of bits and sign bit - This changes the behavior of decoding which will now cast any value to its desired type instead of throwing an exception
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposal for opening up the int_decoder logic to decode any value into its desired type instead of throwing an exception. We are hitting this error when decoding messages generated by a third-party Sparkplug device.
Summary
Handle decoding sign-extended integers instead of throwing an OverflowException
Why?
We are unable to parse certain messages sent from third-party devices because the _int_decoder method is too restrictive and throws when the value to parse is larger than its datatype’s max value. This makes intuitive sense but it ignores how two’s complement integers can get sign-extended when cast to larger types.
The SparkplugB protobuf file defines the Metric value as (oneof) UINT32 meaning any INT16 value needs to be encoded as UINT32.
The spec does not require the UINT32 value must be smaller than its associated datatype’s max value which the OverflowException checks for. I don’t think this exception is necessary because a two’s compliment number that gets sign-extended by a publisher to maintain its negative value and then interpreted as a UINT32 can be larger than its datatype’s max value. This is due to encoding and not because of an overflow.
For a concrete example we have a third-party device encoding a negative INT16 which needs to be encoded as UINT32 for the Protobuf layer. The publisher sign-extends smaller integer values up to INT32 before storing them as UINT32.
INT16 = -87 (0xFFA9)
INT32 = -87 (0xFFFFFFA9) - sign-extended to maintain negative value
UINT32 = 4294967209 (0xFFFFFFA9) - value parsed at the Protobuf layer
The _int_decoder then throws because 4294967209 > Int16 max value.
The _int_decoder method should instead be looking at the bits for the desired type and the sign bit to decode into the actual value.