-
Notifications
You must be signed in to change notification settings - Fork 13
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
Inclomplete Configuration Examples and Missing Documentation #25
Comments
Hi @YBachmann, Thanks a lot for the suggestion! We would happily review a PR with these changes. One thing to keep in mind, is that there will be a "duplicate" of port info, in the yaml (pcic_port), and in the json (/ports/portX). So this should be clear in the documentation: if you accidentally provide a json file with a port number that differs from the one in the yaml, then the configuration will be done "for nothing", if that makes sense. |
Hey @lola-masson , Is there a reason why the Also, I tried removing components from the json config that are unnecessary for an example file ( |
Hi @lola-masson , |
Hi @YBachmann, |
Regarding the JSON structure, you do not need to use the complete JSON file, you can use only a subset. Considering the error message that you shared, my best guess is that there is an error in the JSON file you are trying to use. You need to preserve the hierarchy of the JSON, even if you remove some elements. So if you are changing a processing value, the file would look something like (replace with your actual config): {
"ports": {
"port0": {
"processing": {
"maxDistNoise": 0.1
}
}
}
} |
You are right, it seems I made some sort of mistake earlier. The only error I encountered was [ods_standalone-1] [ERROR] [1738765762.601717592] [ifm3d.vpu_a.ods]: Caught exception in callback for transition 10
[ods_standalone-1] [ERROR] [1738765762.601746188] [ifm3d.vpu_a.ods]: Original error: Unknown error 104011: /calibration: required property 'version' not found in object; instance: {} after I removed the "info": {
"calibration": {
"version": {
"major": 0,
"minor": 1,
"patch": 1
}
}, The only remaining question I have is regarding the duplicates in the ros parameter yaml file and the .json/.o3rcfg config_file: Those are the ones I noticed:
|
There are not exactly duplicate in the sense that they are not used the same way in the ROS node. |
Alright, thank you very much for the explanation 🙂 I will add a minimal default Just one more small question: The |
Good point. I guess it is possible, we do not check for file extension in the ROS node, only that it can be parsed as JSON. Thank you for the work in the PR! |
I noticed some discrepancies in the provided configuration files.
The file /config/ods_default_parameters.yaml includes a line
config_file: "config/examples/o3r_ods.json"
butconfig/examples/o3r_ods.json
does not exist in the repository.The other yaml files in the config folder (o3r_2d.yaml, o3r_3d.yaml, two_o3r_heads.yaml and camera_default_parameters.yaml) do not include the
config_file
parameter at all.This confused me at first because there is no direct hint or documentation that the yaml configuration files need a reference to another json configuration file.
Also by not providing a default configuration file users will get inconsistent behaviour when running the same launch command multiple times because not a fixed default configuration will be loaded but rather the last-saved settings from the VPU (which can get modified in different places like the vision assistant).
Proposed changes:
config_file
parameter in all yaml configuration files (e.g.config_file: "config/examples/o3r_port_settings.json"
)config/examples/o3r_port_settings.json
andconfig/examples/o3r_ods.json
)What do you think of this? I can happily create a PR to implement the proposed changes.
The text was updated successfully, but these errors were encountered: