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

TelemetryDeviceDumper: make the BM configurable from xml file #106

Merged
merged 2 commits into from
Mar 24, 2021

Conversation

Nicogene
Copy link
Member

It fixes #100

Before proceeding with #101, I would settle which parameters are configurable from the file and which ones are mandatory.

In this implementation, the configuration of the BM from JSON will overwrite the parameter-by-parameters configuration.
n_samples is mandatory, as it is mandatory also save_period if save_periodically is set to true.
At the end it checks, if both save_periodically and auto_save are set to false the configuration will fail and the device will close.

An issue that I see is that the configuration with the JSON and the configuration parameter-by-parameter are different.
experiment_name is file_name in the JSON configuration(this probably has to be fixed) and in the JSON configuration ALL the parameters are mandatory for #39 (comment) (also this has to be investigated)

THIS HAS TO BE TESTED ON THE ROBOT.

cc @S-Dafarra

@Nicogene Nicogene requested review from traversaro and AlexAntn March 24, 2021 09:07
@Nicogene Nicogene self-assigned this Mar 24, 2021
@Nicogene Nicogene force-pushed the feat/tddConfigurable branch from 5715258 to d4092ce Compare March 24, 2021 09:10
Copy link
Member

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

No need to do it know, but I think the sooner we add the README for the telemetryDeviceDumper, the better, as we can start incrementally documenting any new parameter that we add (even by just forwarding to the library docs), instead of accumulating docs deps.

@Nicogene
Copy link
Member Author

d6b79d5 adds telemetryDeviceDumper to the README

cc @traversaro @AlexAntn @S-Dafarra

@Nicogene
Copy link
Member Author

I would merge it and then test on the robot, if there is a bug in the configuration we can open another PR since we have not yet a stable version to maintain

@Nicogene Nicogene merged commit 327cc29 into master Mar 24, 2021
@Nicogene Nicogene deleted the feat/tddConfigurable branch March 24, 2021 11:12
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.

telemetryDeviceDumper: support configuration for the BM
2 participants