Skip to content

Add unsigned ints and 64 bit types to msgpack.unpack #7802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

Neradoc
Copy link

@Neradoc Neradoc commented Mar 28, 2023

Add missing formats to msgpack unpack.

In mspack.unpack()

  • add support for unsigned ints (they were treated as signed)
  • add support for 64 bits ints (signed and unsigned)
  • fix support for 32 bit ints by not using small ints
  • add support for double precision floats (without the precision)

C python seems to always use doubles for floats, and seems to use unsigned ints for positive integers, so this should fix unpacking msgpack payloads from C python. I also discovered in tests that the previous implementation would drop high bits in 32-bits ints instead of erroring or using big ints (for numbers that don't fit in a small int).

Error message changed, since 0xc1 is now the only invalid byte, as it is in the specs.
(Is there an existing message that could be reused ?)

This PR doesn't change the int packing behavior, which is still limited to small ints.

I did not add conditional compiling for ports without big ints, because if you can't fit big ints, you can't fit msgpack.

[Non exhaustive] Test code below for the new features, unpacking bytes generated from C python or manually.

Type Before After (and Python 3)
int8 ✅ -6Fh ✅ -6Fh
int16 ✅ -F6Fh ✅ -F6Fh
int32 ✅ 3FFFFFFFh ✅ 3FFFFFFFh
int32 🚫 91h ✅ -7FFFFF6Fh
int64 ⚠️ Not Implemented ✅ 0x1122334455667788
int64 ⚠️ Not Implemented ✅ -0x100
uint8 🚫 -0x6F ✅ 0x91
uint16 🚫 -0xF6F ✅ 0xF091
uint32 ✅ 0x3FFFFFFF ✅ 0x3FFFFFFF
uint32 🚫 0x91 ✅ 0x80000091
uint64 ⚠️ Not Implemented ✅ 0x11223344
uint64 ⚠️ Not Implemented ✅ 0xFFFFFFFFFFFFFFFF
float ✅ 2.1427 ✅ 2.1427
double ⚠️ Not Implemented ✅ -1154.12
import msgpack
from io import BytesIO
def decode(data, val):
    try:
        out = msgpack.unpack(BytesIO(data))
        if out == val: icon = "✅"
        elif abs(1 - out/val) < 10**-6: icon = "✅"
        else: icon = "🚫"
        if isinstance(out, int): out = f"{out:X}h"
        print(f"{icon}   {out}")
    except NotImplementedError as ex:
        print("⚠️     Not Implemented")
# int
decode(b'\xd0\x91', -0x6F)
decode(b'\xd1\xF0\x91', -0xF6F)
decode(b'\xd2\x3f\xff\xff\xff', 0x3fffffff)
decode(b'\xd2\x80\x00\x00\x91', -0x7FFFFF6F)
# int64
decode(b'\xd3\x11"3DUfw\x88', 0x1122334455667788)
decode(b'\xd3\xff\xff\xff\xff\xff\xff\xff\x00', -256)
# uint
decode(b'\xcc\x91', 0x91)
decode(b'\xcd\xF0\x91', 0xF091)
decode(b'\xce\x3f\xff\xff\xff', 0x3fffffff)
decode(b'\xce\x80\x00\x00\x91', 0x80000091)
# uint64
decode(b'\xcf\x00\x00\x00\x00\x11\x22\x33\x44', 0x11223344)
decode(b'\xcf\xff\xff\xff\xff\xff\xff\xff\xff', 0xffffffffffffffff)
# float
decode(b'\xca@\t!\xfb', 2.1426990032196045)
# double
decode(b'\xcb\xc0\x92\x08v+j\xe7\xd5', -1154.1154)

change msgpack error message when format invalid
Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. I was worried that the addition of a double might drag in a bunch of double-floating-point routines, but the difference seems minimal: 8-12 bytes on SAMd51, and about 100 bytes on RP2040.

We had that problem before: #1855

@dhalbert dhalbert merged commit 9a0fa5c into adafruit:main Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants