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

BSON support #1244

Closed
julian-becker opened this issue Sep 18, 2018 · 5 comments · Fixed by #1320
Closed

BSON support #1244

julian-becker opened this issue Sep 18, 2018 · 5 comments · Fixed by #1320
Assignees
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON release item: ✨ new feature solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@julian-becker
Copy link
Contributor

It would be great to have BSON included support in this great JSON library!

Motivation:
I recently encountered the need for it at work, where we are already using this library for JSON processing (for many diverse purposes). Recently, we have been developing applications leveraging the rosbridge (configured in BSON mode) for communication between devices. If we can cover the BSON support with the same JSON library we are already using this would be much preferred over introducing a new library into the mix.

First Prototypical Implementation:
Actually, I have already started a quick, prototypical implementation for BSON serialization and deserialization, and if you feel BSON-support should become part of the mainline, it could be used as a starting point, and I'd be willing to help with the implementation.

Open Questions:
There is a certain fundamental format mismatch between JSON and BSON such that not all JSON values can be mapped 1:1 into BSON and vice versa.
Before continuing with an implementation, it should be clarified how these conversions should be handled (e.g. BSON timestamps, BSON 128-bit wide floats, or "naked" JSON-values which are not objects)

@nlohmann
Copy link
Owner

I had a look at BSON before, but struggled with the absence of a 1:1 mapping between JSON and BSON. Right now, we already have some issues with MessagePack and CBOR which both support binary values (see #1129 for a recent issue), but I think we can be pragmatic there. What I would like to avoid is adding support for very specific types such as timestamp or UUIDs and rather focus on those values supported by JSON.

I did not find the time to look at your implementation. Once I have a rough idea, we should continue the discussion. And of course, PRs are welcome.

@nlohmann nlohmann added the aspect: binary formats BSON, CBOR, MessagePack, UBJSON label Sep 18, 2018
@nlohmann
Copy link
Owner

I head a look at the prototype - very nice! It's a pity that BSON uses little endian and the other formats big endian. So I guess we may need to add a parameter to the existing read/write number functions to avoid having two functions.

I think it would be best if you could open a PR. Then we can shift the discussion related concrete code examples to reviews there.

@julian-becker
Copy link
Contributor Author

Great, thanks for the welcoming feedback! I will do a little consolidation on the code and will open a PR within the next few days, and we can discuss the details there.

On a more general and long-term note regarding the support of binary formats, I was thinking maybe it would make sense to have a more 1-to-1 mapping between the serialized and deserialized form: So, instead of deserializing BSON-data directly into the nlohmann::json, it could make sense to have a proper nlohmann::bson which supports BSON fully and allows for a non-lossy roundtrip conversion of the data (similarly a nlohmann::cbor and nlohmann::message_pack). If the client then still desires to have the data in a nlohmann::json, we could provide corresponding projection functions (which could be parametrized/templatized such that the client has more explicit control over what happens with the cases that don't map 1-to-1 naturally between JSON and the other formats).

@nlohmann
Copy link
Owner

I can understand the idea of the 1-to-1 mapping, but I'm afraid this very specific usecase adds a lot of complexity to the code base. Let's first focus on basic BSON support.

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Oct 25, 2018
@nlohmann nlohmann added this to the Release 3.4.0 milestone Oct 25, 2018
@nlohmann nlohmann self-assigned this Oct 25, 2018
@ayounes-nviso
Copy link

Can this BSON support be the basis to support CBOR byte string #1129 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON release item: ✨ new feature solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants