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

Investigation file-based configuration of the buffer manager #39

Closed
traversaro opened this issue Jan 19, 2021 · 7 comments · Fixed by #65
Closed

Investigation file-based configuration of the buffer manager #39

traversaro opened this issue Jan 19, 2021 · 7 comments · Fixed by #65
Assignees

Comments

@traversaro
Copy link
Member

No description provided.

@Nicogene
Copy link
Member

Nicogene commented Feb 5, 2021

I found a very nice c++ library for parsing JSON files https://github.com/nlohmann/json

In particular, it can be imported also as a single header(see https://github.com/nlohmann/json/tree/develop/single_include/nlohmann) and in particular this feature is awesome.

We may implement the from_json and to_json in our BufferConfig struct!

cc @traversaro @AlexAntn @S-Dafarra

@traversaro
Copy link
Member Author

Good idea! I would however implement it in separate function/headers, so that we can easily add more conversions in the future, for example from/to yarp::os::Property as well, and permit to users to provide their own conversions, as adding more functions is easy, while adding methods to an existing class is impossible outside of the project.

cc @GiulioRomualdi

@traversaro
Copy link
Member Author

Regarding the use of single header, we can also support using like that (especially if we want to support 18.04 that has an old version: https://repology.org/project/nlohmann-json/versions), but I would have at least an option to find it externally via find_package(nlohmann_json).

@Nicogene
Copy link
Member

Nicogene commented Feb 5, 2021

In #65 we implemented the configuration from json, and that library is really easy to use!
For what we understood the default behaviour is that all the fields have to be specified in the .json file, no one is taken as optional

We have to investigate how to handle the case in which the user forgot a parameter

@S-Dafarra
Copy link
Collaborator

It would be nice to have keywords like all to store all the parameters, or not to put some specific parameter in the blacklist.

Nicogene added a commit that referenced this issue Feb 8, 2021
…truct

We have to move these in different header
It fixes #39
Nicogene added a commit that referenced this issue Feb 8, 2021
…truct

We have to move these in different header
It fixes #39
@barbalberto
Copy link

barbalberto commented Feb 9, 2021 via email

@Nicogene
Copy link
Member

Nicogene commented Feb 9, 2021

Also uses a lot of exceptions whenever anything is not perfect, which
personally I don't like so much.

Me neither, when the file misses some attribute is not clear what is missing or what it is wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants