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

rust: ability to convert to common serde map structure #1346

Merged
merged 10 commits into from
Jul 14, 2023

Conversation

silverjam
Copy link
Contributor

@silverjam silverjam commented Jul 12, 2023

Description

Adds new API function to return a serde_json::Value type that corresponds to what we get when running sbp2json -- this enables functionality that requires access to a JSON like object (with all the associated message data) without requiring that we parse JSON.

See here for an example usage: https://github.com/swift-nav/sbp-filter/pull/122

API compatibility

No API compatibility issues since we're just adding a new function.

JIRA Reference

https://swift-nav.atlassian.net/browse/DIP-2

@silverjam silverjam marked this pull request as ready for review July 13, 2023 17:34
Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

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

I'd like to see an easy change to prevent drifts in how json values would be crated inside of the lib out of msgs, and msgs are serialized into json.

rust/sbp/src/json/ser.rs Outdated Show resolved Hide resolved
@silverjam
Copy link
Contributor Author

@pcrumley This looking better?

Copy link
Contributor

@pcrumley pcrumley left a comment

Choose a reason for hiding this comment

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

Hey @silverjam I was hoping to see something like what I did in this stacked PR. #1349

See what you think, you can just merge that pr into this or change it however you'd like. I think it is worth making sure JsonOutput is constructed the same throughout the lib, but i'm ok either way

@silverjam silverjam changed the title Add sbp::json::to_value rust: ability to convert to common serde map structure Jul 14, 2023
@silverjam this is an example of the change I wanted to see. Whenever
JsonOutput is created it should be done by the same function. Therefore
if it ever is refactored there will be no drft.
@silverjam silverjam merged commit 9834893 into master Jul 14, 2023
12 checks passed
@silverjam silverjam deleted the silverjam/sbp-to-value branch July 14, 2023 05:50
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.

3 participants