msgpack not choosing type correctly

[polygnomial]

CircuitPython version

Adafruit CircuitPython 7.3.0-dirty on 2022-05-27; PyCubedv05-MRAM with samd51j19

Code/REPL

import msgpack, io
c = io.BytesIO()
msgpack.pack(145, c)
c.seek(0)
c.getvalue()

Behavior

Output in CircuitPython is b'\xd1\x00\x91', which is an int16, but this is encoded on unix as b'\xcc\x91', which is a uint8. The value is < 255 and positive, so I think it should be a uint8. Additionally, CircuitPython decodes b'\xcc\x91' as -111, which I'm pretty sure is incorrect per the msgpack spec.

Description

This causes problems for passing msgpack'd data back and forth between the two platforms. I recompiled the firmware with different pin definitions, but I don't think that is the issue.

Additional information

No response

[dhalbert]

@bboser @iot49 of interest to you

[polygnomial]

I'm poking around in the msgpack implementation, and best I can tell it never packs anything as a uint. That doesn't seem to be a huge problem in my use case, but unpacking is a bit of an issue. I am thinking for now I will get around this by on both sides encoding as a negative number (which will cause the packer to encode as a signed int) and then I will flip the sign after unpacking.

[Neradoc]

I looked at it briefly when somebody brought it up on discord a few weeks ago, it's an issue that msgpack can't really decode what is encoded by C python. I think adding uints is straightforward, but there's 64 bit types to support too, including doubles which is the default that C-python uses for encoding floats it seems.

circuitpython/shared-module/msgpack/__init__.c

Lines 485 to 490 in 884371f

	case 0xc1: // never used
	case 0xcb: // float 64
	case 0xcf: // uint 64
	case 0xd3: // int 64
	default:
	mp_raise_NotImplementedError(translate("64 bit types"));

[iot49]

Python does not have native uint's - hence the encoder never generates them in the output. The decoder converts received uint's to int's.

You may consider adding special encoders (e.g. pack_uint) to serve your purpose.

[iot49]

Additionally, CircuitPython decodes b'\xcc\x91' as -111, which I'm pretty sure is incorrect per the msgpack spec.

Yes, that would be a bug. I have no time to fix right now.

[Neradoc]

I think adding uints is straightforward

To be more accurate when I say that, a simple (uint*_t) cast works for ints that fit in a CP int (below 0x3FFFFFFF) but higher numbers require to use long ints,. I'll see if I can look into that.

Python does not have native uint's - hence the encoder never generates them in the output.

Note that C-Python does use uint for positive ints in msgpack. A matter of implementation choices I guess. I don't think we need it in CP because it's more code that does not add functionality, it only makes the packing potentially smaller.

❯ python
Python 3.9.6 (default, Aug 11 2021, 01:29:00) 
>>> import msgpack
>>> msgpack.packb(151)
b'\xcc\x97'

Versus b'\xd1\x00\x97' in CP.