module msgpack #3659

ghost · 2020-11-09T02:46:00Z

Support for msgpack (https://msgpack.org/) encoding/decoding. Like json, but binary encoding.

Example:

import msgpack
from io import StringIO

objs = [
    ( True, False, None, 0, 1, -1, 'abc' ),
    { 'a': 1, 'b': (2, 3) },
    123,
    'abc',
    1.2345,
    ( 1, 'abc', 5.3 ),
    {
        "a tuple": (1, 42, 3.141, 1337, "help"),
        "a string": "bla",
        "another dict": {"foo": "bar", "key": "value", "the answer": 42}
    },
    tuple(range(50)),
]

for obj in objs:
    s = StringIO()
    msgpack.pack(obj, s)
    s.seek(0)
    out = msgpack.unpack(s)
    if obj == out:
        print("ok:", obj)
    else:
        print("!!! MISMACH:\n    {}\n    {}".format(obj, out))

Output:

ok: (True, False, None, 0, 1, -1, 'abc')
ok: {'a': 1, 'b': (2, 3)}
ok: 123
ok: abc
ok: 1.2345
ok: (1, 'abc', 5.3)
ok: {'another dict': {'the answer': 42, 'foo': 'bar', 'key': 'value'}, 'a tuple': (1, 42, 3.141, 1337, 'help'), 'a string': 'bla'}
ok: (0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49)

tannewt

Thank you for the PR! I've left a few suggestions. Let me know if you have any questions.

py/circuitpy_mpconfig.h

py/circuitpy_mpconfig.mk

ports/nrf/mpconfigport.mk

shared-bindings/msgpack/__init__.c

tannewt · 2020-11-09T23:59:44Z

(The CI error is due to missing documentation in shared-bindings' __init__.c. Take a look at time or struct for module-only examples.)

tannewt

Looks great! To fix the translation issue run make translate at the top level and commit the result. It'll add entries to locale/circuitpython.pot. Then, the CI should build every board.

ghost · 2020-11-12T02:17:19Z

Still fails for some architectures.

More significantly, the current version does not convert output to big endian. I have a fix in a few days.

ghost · 2020-11-12T20:33:17Z

Updated to correctly output binary in big endian format.
Verified with round-trips c-python -> circuitpython -> c-python: https://github.com/iot49/iot-notebooks/blob/main/tests/msgpack.ipynb.

Tests are fairly comprehensive, except big data structures (due to memory limitations).

tannewt

Looks like it needs another make translate.

tannewt · 2020-11-16T22:48:16Z

Looks like a number of SAMD21 boards are out of flash now. In the short term, we can disable them on an individual basis but in the longer term we'll want to follow up with a way to freeze the modules on SAMD21. The spresence build is also failing due to a type issue.

ghost · 2020-11-17T19:57:11Z

I've disabled msgpack for the builds that are tight on memory.

Regarding the spresence :

First error is:

In file included from ../../shared-module/msgpack/__init__.c:29:
spresense-exported-sdk/nuttx/include/inttypes.h:185:31: error: unknown type name 'wchar_t'
  185 | intmax_t  wcstoimax(FAR const wchar_t *nptr, FAR wchar_t **endptr, int base);
      |                               ^~~~~~~

I added #include <stddef.h> as the first import, now I get the error below. Looks to be a problem with whatever stddef.h the port imports. Anyway - no clue how to fix it; I simply disabled msgpack for this port as well.

spresense-exported-sdk/nuttx/include/inttypes.h:186:50: note: 'wchar_t' is defined in header '<stddef.h>'; did you forget to '#include <stddef.h>'?
../../shared-module/msgpack/__init__.c:60:13: error: conflicting types for 'read'
   60 | STATIC void read(msgpack_stream_t *s, void *buf, mp_uint_t size) {
      |             ^~~~
In file included from spresense-exported-sdk/nuttx/include/pthread.h:52,
                 from spresense-exported-sdk/nuttx/include/nuttx/sched.h:50,
                 from spresense-exported-sdk/nuttx/include/sched.h:49,
                 from spresense-exported-sdk/nuttx/include/stdio.h:48,
                 from ../../shared-module/msgpack/__init__.c:28:

ghost · 2020-11-17T20:29:07Z

For some reason, the checks are not re-run. I'd expect them to pass, but how can I know?

tannewt · 2020-11-17T21:19:09Z

@iot49 disabling is totally fine. @kamtom480 may know what the issue is.

I'm not sure why the CI didn't run. I'll look into it.

tannewt · 2020-11-17T21:24:35Z

Ah, I think you need to fix the merge conflict before the CI can run because it runs on the merge.

kamtom480 · 2020-11-17T21:47:44Z

@iot49 Regarding the spresence, could you rename the read and write function? I think the problem is in the header of the read and write functions in Spresense which are not compatible with your read and write functions.

kamtom480 · 2020-11-18T06:35:34Z

@iot49 Thank you for renaming the read and write functions. The second problem is in the Spresense SDK configuration. I will fix this. You can disable msgpack for the Spresense for now.

ghost · 2020-11-18T20:23:14Z

@tannewt Sorry to bother again - do you have an idea how to fix the remaining (new) 3 errors, all of the same format:

Run actions/upload-artifact@v2
  with:
    name: espressif_saola_1_wrover
    path: bin/espressif_saola_1_wrover
    if-no-files-found: warn
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.8.6/x64
With the provided path, there will be 36 file(s) uploaded
Error: read ECONNRESET

tannewt · 2020-11-19T19:01:52Z

@iot49 Those are network failures. You don't need to worry about them.

tannewt · 2020-12-01T00:08:04Z

Would you mind rebasing this? I'm a bit worried that the diff includes a lot of extra changes.

If you are uncomfortable with git, then I can do it for you.

ghost · 2020-12-02T00:03:51Z

| If you are uncomfortable with git, then I can do it for you.

I'd appreciate this. I've tried to fix this mess, but seem to only add to it. Hate to admit it.

ghost · 2020-12-10T02:41:22Z

Essentially complete, except for a newline somewhere in the documentation but I can't figure out where. I'd appreciate a hint if you have time to take a look. Thanks for fixing the branch.

Should tests be added to the tests directory?

The class is a strict sub-set of the c-version. I use it for communication between Python interpreters.

tannewt · 2020-12-10T18:51:15Z

Essentially complete, except for a newline somewhere in the documentation but I can't figure out where. I'd appreciate a hint if you have time to take a look. Thanks for fixing the branch.

I think you need more

//|

these lines lead to empty lines in the resulting rst. I think you need one after every section. See here for an example: https://github.com/adafruit/circuitpython/blob/main/shared-bindings/struct/__init__.c#L51

Should tests be added to the tests directory?

If you like. Not required.

The class is a strict sub-set of the c-version. I use it for communication between Python interpreters.

Nice!

ghost · 2020-12-10T20:12:20Z

I think you need more
//|

Thanks, did that!

Now CI is not running: "Workflow runs completed with no jobs"

tannewt · 2020-12-11T19:11:56Z

The build wasn't running due to the merge conflict. I've fixed it so make sure and update your local copy.

I think you'll need ... anywhere implementation would have been, such as in a function call.

tannewt · 2021-01-06T00:14:17Z

Looks like there is a doc build error:

/home/runner/work/circuitpython/circuitpython/shared-bindings/msgpack/index.rst:34:Definition list ends without a blank line; unexpected unindent.

ghost · 2021-01-06T01:07:27Z

Thanks for looking into this again. It's time to quit. You have better things to do and I have no clue which blank line is missing. I've used the library extensively and it works correctly as best I can tell. So if perhaps in the future someone has a need this might be a starting point. Sorry to have wasted your time.

…

On Tue, Jan 5, 2021 at 4:14 PM Scott Shawcroft ***@***.***> wrote: Looks like there is a doc build error: /home/runner/work/circuitpython/circuitpython/shared-bindings/msgpack/index.rst:34:Definition list ends without a blank line; unexpected unindent. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3659 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AL426C5DJMLGQGG4SEVKKA3SYOTONANCNFSM4TOYDHOA> .

tannewt · 2021-01-06T16:44:22Z

Thanks for looking into this again. It's time to quit. You have better things to do and I have no clue which blank line is missing. I've used the library extensively and it works correctly as best I can tell. So if perhaps in the future someone has a need this might be a starting point. Sorry to have wasted your time.

You are so close! I'll look now and see if I can find it. The line number is for the intermediate generated file.

ghost · 2021-01-06T16:57:40Z

But how do I get the intermediate file? Presumably I could run the build (would have to figure out how to do this) locally, and then look at that file. BTW: are you using a tool to help you format the documentation? It's very tedious with just a text editor (V-code). I need an AI tool that inserts the missing newline, rather than just telling it's missing -:)

…

On Wed, Jan 6, 2021 at 8:44 AM Scott Shawcroft ***@***.***> wrote: Thanks for looking into this again. It's time to quit. You have better things to do and I have no clue which blank line is missing. I've used the library extensively and it works correctly as best I can tell. So if perhaps in the future someone has a need this might be a starting point. Sorry to have wasted your time. You are so close! I'll look now and see if I can find it. The line number is for the intermediate generated file. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3659 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AL426CYFCWWTY37PM5BJ2Y3SYSHPNANCNFSM4TOYDHOA> .

tannewt · 2021-01-06T17:07:40Z

I just pushed the fixes to: cdad59f

You can build the docs locally with make html at the top level. There are some install instructions for sphinx here: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/sharing-our-docs-on-readthedocs I think you may also need to pip install sphinxcontrib-svg2pdfconverter and recommonmark too.

FoamyGuy · 2021-01-06T20:03:45Z

Looks like it ran out of space on a few more of the smaller devices.

I can try to disable this new module on those devices this week to see if that will get it passing CI.

Thanks for all of your work on this @iot49! This looks very cool, I am interested using this for a few different things. Am willing to help out any way that I can to get this to the finish line.

FoamyGuy · 2021-01-09T16:36:13Z

latest commit disables msgpack on the 3 boards that failed. 🤞

❯ make BOARD=qtpy_m0_haxpress

249216 bytes used, 4480 bytes free in flash firmware space out of 253696 bytes (247.75kB).
10908 bytes used, 21860 bytes free in ram for stack and heap out of 32768 bytes (32.0kB).

❯ make BOARD=stackrduino_m0_pro

251304 bytes used, 2392 bytes free in flash firmware space out of 253696 bytes (247.75kB).
10892 bytes used, 21876 bytes free in ram for stack and heap out of 32768 bytes (32.0kB).

❯ make BOARD=pca10100

233648 bytes used, 3920 bytes free in flash firmware space out of 237568 bytes (232.0kB).
41500 bytes used, 89572 bytes free in ram for stack and heap out of 131072 bytes (128.0kB).

FoamyGuy · 2021-01-09T18:18:27Z

🎉 its passing now. I also tested this successfully (albeit very basically) with this code:

import msgpack
from io import StringIO

s = StringIO()
msgpack.pack({"test":"hello","num":4}, s)
print(s)
s.seek(0)
out = msgpack.unpack(s)
print(out)

tannewt

Thanks @iot49 and @FoamyGuy. Merging now

ghost · 2021-01-13T17:54:47Z

Many thanks to @tannewt and @FoamyGuy to make it happen!

tannewt self-requested a review November 9, 2020 23:46

tannewt added cpython api modules from cpython enhancement labels Nov 9, 2020

tannewt added this to the 6.x.0 - Features milestone Nov 9, 2020

tannewt requested changes Nov 9, 2020

View reviewed changes

py/circuitpy_mpconfig.h Outdated Show resolved Hide resolved

py/circuitpy_mpconfig.mk Outdated Show resolved Hide resolved

ports/nrf/mpconfigport.mk Outdated Show resolved Hide resolved

shared-bindings/msgpack/__init__.c Outdated Show resolved Hide resolved

tannewt requested changes Nov 10, 2020

View reviewed changes

tannewt requested changes Nov 13, 2020

View reviewed changes

ghost requested a review from tannewt November 17, 2020 20:27

tannewt previously approved these changes Nov 17, 2020

View reviewed changes

ghost dismissed tannewt’s stale review via 5dfdcc0 November 18, 2020 00:26

Bernhard Boser added 6 commits December 1, 2020 18:38

add module msgpack

90c203a

removed empty line at end of py/circuitpy_mpconfig.mk

748472d

moved logic to shared-module and added documentation

513253b

removed blanks to make pre-commit check happy

e197826

removed blanks line at end of file

912691e

added signatures to documenation

f38bd59

ha-rvc added 5 commits December 7, 2020 17:45

fix typspec in function declarations

413885a

remove two extra closing angular brackets

1f14d39

uglify typing hints

9e7a7b9

typing ...

a5c3dc2

cosmetics

3c72432

add blank lines after all comment sections

6d2329f

ha-rvc and others added 2 commits December 10, 2020 12:15

try without ... in docs

d29184b

Merge branch 'main' into msgpack

344d3c5

ha-rvc and others added 4 commits December 19, 2020 19:06

speedup pack_bin, ext, str; catch short reads

c875d7c

Merge branch 'msgpack' of github.com:iot49/iotpython into msgpack

79e3c3d

added //| to (hopefully) get CI to pass

90a299b

Merge branch 'main' into msgpack

1a82555

Fix doc build with extra blank lines

cdad59f

disable msgpack on boards without room

13efbf2

tannewt approved these changes Jan 11, 2021

View reviewed changes

tannewt merged commit 9124529 into adafruit:main Jan 11, 2021

kevinjwalters mentioned this pull request Jan 19, 2021

Add feature to encode and decode MakeCode's infrared pulses adafruit/Adafruit_CircuitPython_IRRemote#26

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module msgpack #3659

module msgpack #3659

ghost commented Nov 9, 2020

tannewt left a comment

tannewt commented Nov 9, 2020

tannewt left a comment

ghost commented Nov 12, 2020

ghost commented Nov 12, 2020

tannewt left a comment

tannewt commented Nov 16, 2020

ghost commented Nov 17, 2020

ghost commented Nov 17, 2020

tannewt commented Nov 17, 2020

tannewt commented Nov 17, 2020

kamtom480 commented Nov 17, 2020

kamtom480 commented Nov 18, 2020

ghost commented Nov 18, 2020

tannewt commented Nov 19, 2020

tannewt commented Dec 1, 2020

ghost commented Dec 2, 2020

ghost commented Dec 10, 2020

tannewt commented Dec 10, 2020

ghost commented Dec 10, 2020

tannewt commented Dec 11, 2020

tannewt commented Jan 6, 2021

ghost commented Jan 6, 2021 via email

tannewt commented Jan 6, 2021

ghost commented Jan 6, 2021 via email

tannewt commented Jan 6, 2021

FoamyGuy commented Jan 6, 2021

FoamyGuy commented Jan 9, 2021

FoamyGuy commented Jan 9, 2021

tannewt left a comment

ghost commented Jan 13, 2021

module msgpack #3659

module msgpack #3659

Conversation

ghost commented Nov 9, 2020

tannewt left a comment

Choose a reason for hiding this comment

tannewt commented Nov 9, 2020

tannewt left a comment

Choose a reason for hiding this comment

ghost commented Nov 12, 2020

ghost commented Nov 12, 2020

tannewt left a comment

Choose a reason for hiding this comment

tannewt commented Nov 16, 2020

ghost commented Nov 17, 2020

ghost commented Nov 17, 2020

tannewt commented Nov 17, 2020

tannewt commented Nov 17, 2020

kamtom480 commented Nov 17, 2020

kamtom480 commented Nov 18, 2020

ghost commented Nov 18, 2020

tannewt commented Nov 19, 2020

tannewt commented Dec 1, 2020

ghost commented Dec 2, 2020

ghost commented Dec 10, 2020

tannewt commented Dec 10, 2020

ghost commented Dec 10, 2020

tannewt commented Dec 11, 2020

tannewt commented Jan 6, 2021

ghost commented Jan 6, 2021 via email

tannewt commented Jan 6, 2021

ghost commented Jan 6, 2021 via email

tannewt commented Jan 6, 2021

FoamyGuy commented Jan 6, 2021

FoamyGuy commented Jan 9, 2021

FoamyGuy commented Jan 9, 2021

tannewt left a comment

Choose a reason for hiding this comment

ghost commented Jan 13, 2021