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

Incremental JSON <-> struct encoder for implementing preset saving #45

Merged
merged 17 commits into from May 20, 2019

Conversation

@csboling
Copy link
Contributor

@csboling csboling commented May 17, 2019

This adds library support for JSON serialization, to be used for saving things like Ansible state to USB disk. For discussion see the lines threads Preset save to USB disk and Serialisation of presets. If this PR is accepted I'll open another to the Ansible repo that uses this encoder to implement the preset save feature.

This code implements an encoder which can convert a JSON document with a known structure to and from a C struct, or do any other kind of processing when recognized pieces of JSON data are found. Tokenizing an incoming JSON document uses a modified version of the jsmn library (forked prior to a recent reorganization of that project), which is included. It has the following features which make it suitable for embedded use and easy extensibility.

  • it performs no dynamic allocations.
  • it can operate incrementally on streaming input, processing tokens which span multiple reads. As such it has almost no minimum text buffer size for storing a portion of text.
  • it is able to skip large amounts of whitespace and entire sections of the JSON document that it does not understand, allowing it to load only recognized segments from a much larger document. Supplying all expected properties in an input JSON document is also not required. This allows a module to be fairly forward/backward compatible in the saved preset structures it will accept.
  • the processing of the incoming JSON document can be completely customized by supplying different callbacks for handling different nodes. An example data structure used to describe an expected document format can be found in the tests.
  • it is agnostic to how it might be used in a larger system, accepting simple callbacks for reading from an input stream, writing to an output stream, and copying decoded data to its destination. This should make it easy to use the encoder for other possible applications like reading/writing JSON to an I2C channel rather than to a USB disk.

Encoding functions for common data types are included, such as a decimal encoder which handles signed values, and a hex encoder for encoding large binary arrays into JSON as hex strings, allowing relatively easy human editability of the resulting document. Parsing/serializing floating point numbers is not currently implemented. Use of the decimal encoding functions does require static allocation of a 12 byte buffer, I believe this is the only static memory allocation required to use the library.

@tehn
Copy link
Member

@tehn tehn commented May 17, 2019

ambitious and impressive!

This should make it easy to use the encoder for other possible applications like reading/writing JSON to an I2C channel rather than to a USB disk.

gasp. :)


code looks good to me so far. i'll take a second look later today.

@tehn tehn requested a review from scanner-darkly May 17, 2019
src/arp.c Outdated
@@ -132,16 +132,19 @@ void arp_seq_init(arp_seq_t* s) {
}

bool arp_seq_set_state(arp_seq_t *s, arp_seq_state state) {
#ifndef RUNNING_TESTS

This comment has been minimized.

@scanner-darkly

scanner-darkly May 19, 2019
Member

curious, why is this needed? does it need to be done in other places that use irqs_pause/resume? if so, would it be better to move ifndef to the actual functions?

This comment has been minimized.

@csboling

csboling May 19, 2019
Author Contributor

I just found this conversation which leads me to believe that this #ifndef isn't needed on the latest master. This was how I made the tests pass when I first made this branch, I can clean this up.

This comment has been minimized.

@scanner-darkly

scanner-darkly May 19, 2019
Member

ah right, this rings a bell. i think either way is fine - irqs stuff is not needed for running tests anyway.

This comment has been minimized.

@csboling

csboling May 19, 2019
Author Contributor

Removed RUNNING_TESTS, realized there was actually an error testing json_read_object_cached, added a make target so that if you have valgrind installed you can run make leaks, and found some leaks in the test code - nothing actually affecting the JSON encoder itself I think.

@scanner-darkly
Copy link
Member

@scanner-darkly scanner-darkly commented May 19, 2019

this is great - super useful to have it be part of libavr32.

@tehn
tehn approved these changes May 20, 2019
@tehn tehn merged commit ec45a7f into monome:master May 20, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants