Skip to content

enable msgpack in zephyr port and add test for it. fix unpack issue#10927

Merged
tannewt merged 1 commit intoadafruit:mainfrom
FoamyGuy:zephyr_msgpack_and_fix
Apr 8, 2026
Merged

enable msgpack in zephyr port and add test for it. fix unpack issue#10927
tannewt merged 1 commit intoadafruit:mainfrom
FoamyGuy:zephyr_msgpack_and_fix

Conversation

@FoamyGuy
Copy link
Copy Markdown
Collaborator

@FoamyGuy FoamyGuy commented Apr 7, 2026

Enabled msgpack core module in the zephyr port and added test_msgpack.py to validate its functionality. I'm wondering if there is something else I need to do to get the rest of the autogen files to update? The only ones committed in this branch are the two devices that I built.

The test was initially failing when run on the native sim but passing when run on hardware. Claude assisted debug points to the issue being that the calls like mp_obj_dict_store(d, unpack(s, ext_hook, use_list), unpack(s, ext_hook, use_list)); do not guarantee the evaluation order of function arguments. So the compiler could evaluate them in the unintended order which was leading to dictionaries coming out as {value: key} instead of {key: value}. The fix is to split that statement up and call unpack in the intended order explicitly.

@FoamyGuy FoamyGuy requested a review from tannewt April 7, 2026 18:53
@tannewt
Copy link
Copy Markdown
Member

tannewt commented Apr 8, 2026

I'm wondering if there is something else I need to do to get the rest of the autogen files to update?

I usually run make all (there is make clean-all too.)

do not guarantee the evaluation order of function arguments.

Whoa! I did not know that. Python does left-to-right: https://docs.python.org/3/reference/expressions.html#evaluation-order

Copy link
Copy Markdown
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you!

@tannewt tannewt merged commit 83749c8 into adafruit:main Apr 8, 2026
568 checks passed
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