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

AP_JSON: added JSON parsing library #26114

Merged
merged 14 commits into from
Feb 21, 2024
Merged

AP_JSON: added JSON parsing library #26114

merged 14 commits into from
Feb 21, 2024

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Feb 1, 2024

This is an alternative to #23673 that adds AP_JSON based on a re-working of picojson. It runs on both SITL and HAL_ChibiOS

I think this provides a nicer JSON API which is better for future usage of JSON

@tridge tridge requested a review from peterbarker February 1, 2024 04:30
@tridge tridge force-pushed the pr-ap-json branch 2 times, most recently from 3141e70 to 067377e Compare February 1, 2024 05:04
@rmackay9
Copy link
Contributor

rmackay9 commented Feb 1, 2024

This could be good for integration with various AI systems that all seem to use JSON..

@tridge
Copy link
Contributor Author

tridge commented Feb 1, 2024

This could be good for integration with various AI systems that all seem to use JSON..

I'd expect those to use a json implementation in lua (JSON is very easy in lua)

@tridge tridge force-pushed the pr-ap-json branch 2 times, most recently from 215a428 to 5795a7e Compare February 1, 2024 09:47
@tridge tridge force-pushed the pr-ap-json branch 3 times, most recently from c9a1b9b to e7604c1 Compare February 7, 2024 01:23
@tridge tridge added DevCallEU and removed DevCallEU labels Feb 7, 2024
# we use string find on these symbols, so this catches all types of throw calls
blacklist = ['std::__throw']

print("COMMAND: %s -C %s" % (task.env.get_flat('NM'), elfpath))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this print() should be removed

Copy link
Collaborator

@davidbuzz davidbuzz 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 to me once you explained the gmtime->gmtime_r changes

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Good so long as the Callisto model works on SITL-on-hardware.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Feb 19, 2024

Take a look at the benefit of std::array: https://godbolt.org/z/GraMsY3qP

At compile time, you can get an error when trying to access values out of range. Peter proposes doing a trivial example of changing a raw array to std::array and examining the binaries.

I would recommend enabling this in SITL when SITL is in debug mode. Coupled with using std::array for any arrays with size known at compile time, it allows for huge improvements in compile-time-safety with regards to array access.

@tridge
Copy link
Contributor Author

tridge commented Feb 20, 2024

tested on CubeOrange with Callisto model:
image

using gmtime_r makes gmtime thread safe
using gmtime_r makes gmtime thread safe
using gmtime_r makes gmtime thread safe
this cleans up our blacklist of library functions, and ensures there
can be no accidential use of std:: functions that cause exceptions in
flight code on HAL_ChibiOS
if scripting can't find an error handler it can call abort(). We don't
ever want to do that in ArduPilot
@tridge tridge merged commit f809737 into ArduPilot:master Feb 21, 2024
92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants