Skip to content

Allow building without serialization / json support#233

Open
jcelerier wants to merge 3 commits intomsteinbeck:masterfrom
jcelerier:patch-3
Open

Allow building without serialization / json support#233
jcelerier wants to merge 3 commits intomsteinbeck:masterfrom
jcelerier:patch-3

Conversation

@jcelerier
Copy link
Copy Markdown
Contributor

For some very small embedded builds :)

@msteinbeck
Copy link
Copy Markdown
Owner

Hi @jcelerier,

thanks for your PR. On which embedded system do you want to use TinySpline?

I'll have a look at your PR. So far, it looks well done, thanks.

Comment thread src/CMakeLists.txt Outdated
if(NOT TINYSPLINE_SERIALIZATION)
list(APPEND TINYSPLINE_C_DEFINITIONS "TINYSPLINE_NO_SERIALIZATION")
list(APPEND TINYSPLINE_CXX_DEFINITIONS "TINYSPLINE_NO_SERIALIZATION")
list(APPEND TINYSPLINE_BINDING_CXX_DEFINITIONS "TINYSPLINE_NO_SERIALIZATION")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We will probably never build one or more bindings without serialization support.

Comment thread src/tinyspline.c
#include "tinyspline.h"

#if !defined(TINYSPLINE_NO_SERIALIZATION)
#include "parson.h" /* serialization */
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

#include <stdio.h> can also be excluded as we only use FILE and fopen from it. I propose to move this line into the !defined(TINYSPLINE_NO_SERIALIZATION) block.

@jcelerier
Copy link
Copy Markdown
Contributor Author

hi, thanks for your quick answers! I'm targetting ESP8266, ESP32 and raspberry pi pico (RP2040) so far

@msteinbeck
Copy link
Copy Markdown
Owner

Very interesting. Isn't the dynamic allocation of memory a problem on these platforms?

Comment thread src/CMakeLists.txt
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.

2 participants