zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.62k stars 6.5k forks source link

Add json-c as external module #59303

Closed parthitce closed 4 weeks ago

parthitce commented 1 year ago

Origin

JSON-C implements a reference counting object model that allows you to easily construct JSON objects in C, output them as JSON formatted strings and parse JSON formatted strings back into the C representation of JSON objects. It aims to conform to RFC 7159.

URL: https://github.com/json-c/json-c

Purpose

json-c provides friendly API to frame and parse json messages. It's well known in the Linux world and bringing it helps,

Comparing with available native json data handling, json-c helps,

Mode of integration

Mode of integration: External module Repository name: json-c Reason: To keep in sync with the open source version of json-c, which helps rebasing

Maintainership

Pull Request

Yet to prepare a final one based on https://github.com/linumiz/json-c/tree/zephyr

Description

Please refer to https://json-c.github.io/json-c/

What is its primary functionality?

What problem are you trying to solve?

Why is this the right component to solve it?

Dependencies

What other components does this package depend on?

Will the Zephyr project have a direct dependency on the component, or will it be included via an abstraction layer with this component as a replaceable implementation?

Revision

Version or SHA you would like to integrate initially

License

Please use an SPDX identifier (https://spdx.org/licenses/), such as BSD-3-Clause

nashif commented 1 year ago

@mrfuchs can you please take a look?

parthitce commented 1 year ago

@mrfuchs Ping! Thanks.

parthitce commented 1 year ago

Added the initial draft PR for review https://github.com/zephyrproject-rtos/zephyr/pull/60039

parthitce commented 1 year ago

@nashif @mrfuchs Ping for json-c!

parthitce commented 1 year ago

@nashif @mrfuchs Gentle ping! Please share your feedback and comments.

github-actions[bot] commented 11 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

parthitce commented 9 months ago

@nashif @mrfuchs Please have a look. Thanks.

github-actions[bot] commented 7 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

keith-zephyr commented 5 months ago

This issue looks like it fell through the cracks and wasn't looked at by the TSC.

parthitce commented 5 months ago

This issue looks like it fell through the cracks and wasn't looked at by the TSC.

Thanks, let me know if further details are required to discuss with TSC. Active PR: https://github.com/zephyrproject-rtos/zephyr/pull/66763

parthitce commented 5 months ago

Based on the in-person discussion by @ssekar15 with you Carles @carlescufi at EOSS, could you please help us proceed with this issue? We would like to get this json-c implementation get into Zephyr. Also please let us know the process and to join the TSC to discuss about this issue. Many thanks.

carlescufi commented 5 months ago

Based on the in-person discussion by @ssekar15 with you Carles @carlescufi at EOSS, could you please help us proceed with this issue? We would like to get this json-c implementation get into Zephyr. Also please let us know the process and to join the TSC to discuss about this issue. Many thanks.

The process: https://docs.zephyrproject.org/latest/contribute/external.html

Joining the TSC: https://github.com/zephyrproject-rtos/zephyr/wiki/Technical-Steering-Committee-%28TSC%29

But I have to repeat what I said in person: we cannot have two implementations of the same functionality (JSON) in Zephyr. This means that, for this to be included in Zephyr, we have to deprecate and remove the current JSON library in the main Zephyr tree. To do that we need to make sure that we select the right replacement for it. So we would need a bit more info regarding json-c and how it compares to both the existing Zephyr library and other external projects that also implement the same, like for example https://github.com/DaveGamble/cJSON.

github-actions[bot] commented 3 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

parthitce commented 2 months ago

Based on the in-person discussion by @ssekar15 with you Carles @carlescufi at EOSS, could you please help us proceed with this issue? We would like to get this json-c implementation get into Zephyr. Also please let us know the process and to join the TSC to discuss about this issue. Many thanks.

The process: https://docs.zephyrproject.org/latest/contribute/external.html

Joining the TSC: https://github.com/zephyrproject-rtos/zephyr/wiki/Technical-Steering-Committee-%28TSC%29

But I have to repeat what I said in person: we cannot have two implementations of the same functionality (JSON) in Zephyr. This means that, for this to be included in Zephyr, we have to deprecate and remove the current JSON library in the main Zephyr tree. To do that we need to make sure that we select the right replacement for it. So we would need a bit more info regarding json-c and how it compares to both the existing Zephyr library and other external projects that also implement the same, like for example https://github.com/DaveGamble/cJSON.

@carlescufi thanks for reviewing this. With external optional modules support, does moving it as optional module under submanifest makes sense? This way it's not pulled in by default and used only when when the user/developer enables the optional?

I understand the primary question about the comparison with best replacement. As far as json-c is concerned, it uses heap. I didn't profile yet the usage of heap, but do we have a way to profile heap in zephyr? One other question is, modules of this kind can use heap?

parthitce commented 4 weeks ago

After careful evaluation of memory footprint with json-c, am closing this issue. The main reasons are,

To keep the existing work for json-c, for future reference and also potential use for devices with more RAM (SRAM/PSRAM), implementation is moved to https://github.com/linumiz/rixx