bblanchon / ArduinoJson

📟 JSON library for Arduino and embedded C++. Simple and efficient.
https://arduinojson.org
MIT License
6.63k stars 1.1k forks source link

MsgPack bin8/bin16/bin32 support #2078

Closed Sanae6 closed 2 months ago

Sanae6 commented 2 months ago

I've added binary support to the msgpack deserializer and serializer, along with a fallback for the json serializer. I've also added some tests to the relevant files. Let me know if there's anything I can do to make this implementation better. Thank you for the amazing library :)

This would fix and close #922.

bblanchon commented 2 months ago

Hi @Sanae6,

Thank you for this excellent PR!

Before starting the review, I'd like you to remove LinkedBinary. Indeed, binary values should be treated like raw JSON strings and always be copied; this is part of a larger (incomplete) refactoring to reduce the size of VariantContent. Then, of course, simplify the names accordingly, since we no longer need the distinction between "owned" and "linked" binary values.

Best regards, Benoit

Sanae6 commented 2 months ago

I removed LinkedBinaryValue and renamed OwnedBinaryValue to BinaryValue. Not sure what to name Binary, but I left it as something sensible enough.

Sanae6 commented 2 months ago

Alright! Force pushed, seems nothing is broken :+1:

bblanchon commented 2 months ago

It seems that you lost your version of string_length_size_4.cpp while merging the conflict.

Sanae6 commented 2 months ago

Made the changes you wanted, testing MsgPackBinary comparison actually revealed my initial implementation didn't work, so I fixed that as well.

coveralls commented 2 months ago

Coverage Status

coverage: 99.465% (-0.1%) from 99.589% when pulling c47d19d80c3a0f07a9ac3e30c18f1f90ff9884a5 on Sanae6:7.x into cd4bf33132689eb6387fcc30148d712d2446e4fc on bblanchon:7.x.

bblanchon commented 2 months ago

I added the three missing tests, squashed, and merged.

Thank you again for this excellent contribution ❤️

bblanchon commented 1 week ago

This feature is available in ArduinoJson 7.1.0