Skip to content
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

Make it possible to compile MPack on 8-bit embedded targets? #74

Closed
ethanjli opened this issue Jan 10, 2020 · 3 comments
Closed

Make it possible to compile MPack on 8-bit embedded targets? #74

ethanjli opened this issue Jan 10, 2020 · 3 comments

Comments

@ethanjli
Copy link
Contributor

ethanjli commented Jan 10, 2020

Currently, compiling mpack for some 8-bit embedded targets (such as Arduino AVR microcontrollers) fails because doubles are just 32-bit floats and not proper 64-bit floating-point numbers:

MPACK_STATIC_ASSERT(sizeof(double) == sizeof(uint64_t), "double is wrong size??");

If possible, it would be great to make MPack compilable for such targets, even if reading/writing doubles must be unsupported on such targets - for example by making a flag in mpack-defaults.h to omit the mpack_load_double, mpack_store_double, and mpack_encode_double functions, and to make mpack_tree_parse_node_contents and mpack_parse_tag return false instead of attempting to read doubles on when type 0xcb (double) is encountered. I've started to try out this idea in a fork I made, and I'll report back on whether I find obvious differences in a few basic use cases between a few 8-bit AVR microcontrollers and a few 32-bit ARM microcontrollers. Caveat is that I've never used mpack before, so maybe my understanding has some crucial gaps about why this wouldn't work or about what is the correct way to disable double reading/writing functionality.

Perhaps more graceful degradation of functionality on AVR-like targets would be to convert 32-bit doubles to and from the 64-bit representations stored in the unions in mpack_load_double, mpack_store_double. I'm not sure if it's possible to implement this in a portable way - the closest I've found is somebody's function to read a 32-bit float from a buffer containing a 64-bit floating point representation at https://www.microchip.com/forums/FindPost/258940, and the ArduinoJson library's msgpack deserializer implementation (readDouble and doubleToFloat).

@ethanjli
Copy link
Contributor Author

ethanjli commented Jan 27, 2020

Update: I've been using working on a project using the Writer, Reader, and Expect APIs with the changes I made under a new MPACK_DOUBLES flag at https://github.com/ethanjli/mpack/commits/develop, and I haven't encountered any issues so far! I get successful compilation on 8-bit AVR targets (tested with Arduino Uno and Arduino Micro) when I disable the MPACK_DOUBLES flag, and floats/doubles seem to be handled correctly when MPACK_DOUBLES is enabled and when it is disabled.

@ludocode
Copy link
Owner

ludocode commented May 7, 2020

Nice! I've been meaning to wrap up the floating point code in a preproc so it can be disabled entirely on microcontrollers. If you do actually use 32-bit float though, maybe it's better to have separate preprocs for float and double. I like the idea of supporting serialized doubles by rounding to floats if doubles aren't supported; this is what mpack_read_float() does anyway.

If you want to add your code to MPack you can create a PR for it. It looks good. We do have to make sure the code taken from ArduinoJson is attributed properly though.

ludocode added a commit that referenced this issue Jul 31, 2021
This hopefully completes AVR support for #74 and #79.
@ludocode
Copy link
Owner

Fixed, hopefully. See the discussion in #79.

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

No branches or pull requests

2 participants