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

add jsmn (Jasmine) JSON parser #23673

Closed
wants to merge 14 commits into from

Conversation

peterbarker
Copy link
Contributor

No description provided.

@peterbarker peterbarker added the WIP label May 3, 2023
@khancyr
Copy link
Contributor

khancyr commented May 3, 2023

what is the advantage against picojson we are already using ?

@khancyr
Copy link
Contributor

khancyr commented May 3, 2023

https://github.com/zserge/jsmn

https://github.com/kazuho/picojson

jsmm have no dynamic allocation

@peterbarker
Copy link
Contributor Author

jsmm have no dynamic allocation

Yep. No standard library either. Really wanted json parsing on sitl-on-hardware, but had to cut it out.

@peterbarker peterbarker force-pushed the pr/jsmn-json-parser branch from 0580605 to 9b652ca Compare May 4, 2023 02:49
@peterbarker
Copy link
Contributor Author

I've done some more playing with this and this PR now replaced picojson with jasmine.

Cost of just having the jsmn in-play is 1424 bytes (i.e. create the parser and call it on a 1-byte string).

To actually walk through the tokens afterwards for Callisto.json to set parameters (in the same way that picojson did) costs an additional 1600 bytes.

Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

we need comments, and need xplane working

Copy link
Contributor Author

@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.

we need comments, and need xplane working

So you are happy with the way this is turning out and I should pursue it, then? That's what this most recent push was in aid of - to make sure people didn't immediately throw up when they saw it.

I believe jasmine might be able to be run as a progressive parser, too - I haven't tried that, but it does have this:

  /* The string is not a full JSON packet, more bytes expected */
  JSMN_ERROR_PART = -3

... that would be rather awesome if it worked.

@tridge tridge removed the DevCallEU label May 10, 2023
@peterbarker peterbarker force-pushed the pr/jsmn-json-parser branch 4 times, most recently from 44e9cf9 to b9bc101 Compare June 2, 2023 00:40
@peterbarker
Copy link
Contributor Author

Works for making a CubeOrange-SimOnHardware behave like a Callisto:
image

I've run this through Valgrind and fixed the one false-positive.

I've made sure that XPlane dref extraction works:
image

... and of course, Callisto still loads in SITL.

@peterbarker
Copy link
Contributor Author

peterbarker commented Jun 2, 2023

Flash cost for CubeOrange-SimOnHW is 3kB (+ whatever JSON you want in ROMFS e.g. 1kB for Callisto.json)

--- a/libraries/AP_HAL_ChibiOS/hwdef/CubeOrange-SimOnHardWare/hwdef.dat
+++ b/libraries/AP_HAL_ChibiOS/hwdef/CubeOrange-SimOnHardWare/hwdef.dat
@@ -6,4 +6,7 @@ include ../include/SimOnHW.inc
 # short board name override (23 chars)
 define CHIBIOS_SHORT_BOARD_NAME "CubeOrangeSimOnHardWare"
 
+ROMFS models/Callisto.json Tools/autotest/models/Callisto.json
+define AP_JSONPARSER_ENABLED 1
+
./Tools/scripts/sitl-on-hardware/sitl-on-hw.py --frame octa-quad:@ROMFS/models/Callisto.json --simclass MultiCopter --vehicle copter --board CubeOrange-SimOnHardware --upload

@tridge
Copy link
Contributor

tridge commented Jun 5, 2023

failed on libraries/SITL/examples/XPlane/xplane_alia.json

@peterbarker peterbarker force-pushed the pr/jsmn-json-parser branch from 03de2d2 to f16d3bc Compare June 6, 2023 00:41
@peterbarker
Copy link
Contributor Author

failed on libraries/SITL/examples/XPlane/xplane_alia.json

Fixed - it was just the maximum number of tokens we support in a file.

@peterbarker peterbarker force-pushed the pr/jsmn-json-parser branch 2 times, most recently from ebd842b to 5ebda0a Compare June 6, 2023 00:52
@peterbarker peterbarker force-pushed the pr/jsmn-json-parser branch from 5ebda0a to cd8e4e4 Compare June 6, 2023 01:51
@peterbarker
Copy link
Contributor Author

@Ryanf55 I've changed this to use strtol etc

@peterbarker peterbarker force-pushed the pr/jsmn-json-parser branch 2 times, most recently from d08d2e0 to 7d3686f Compare June 7, 2023 05:24
@peterbarker
Copy link
Contributor Author

Fixed XPlane.

@tridge tridge removed the DevCallEU label Jan 31, 2024
@peterbarker peterbarker force-pushed the pr/jsmn-json-parser branch 2 times, most recently from 0c22378 to a5b61f4 Compare February 1, 2024 06:29
@peterbarker
Copy link
Contributor Author

Going with tridge's picojson changes instead.

@peterbarker peterbarker closed this Feb 6, 2024
@peterbarker peterbarker deleted the pr/jsmn-json-parser branch February 6, 2024 01:43
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.

6 participants