Generalize the blf-calibration-delta-updater script
#361
Conversation
…eterHandler YARP bindings
…-updater.py script
| robot_name icub | ||
|
|
||
| [left_leg] | ||
| expected_values (-45.9, 0.0, 0.0, 5.153, -46.9, -26.62) |
There was a problem hiding this comment.
You may add in_degrees in the name to have the unit of measurement clear
| parser.add_argument('-i', '--input', type=str, required=True, help='Input xml file') | ||
| parser.add_argument('-o', '--output', type=str, required=True, help='Output xml file') | ||
| parser.add_argument('-p', '--part', type=str, required=True, help='Name of the part.') | ||
| parser.add_argument('--config', type=str, required=False, help='Path or name of the configuration file', |
There was a problem hiding this comment.
This is slightly different from what is mentioned in the README. Not a big deal, but it could raise doubts
| new_offsets.append(compute_joint_offset(sensor_bridge, i, expected_values)) | ||
|
|
||
| updated_offset = np.array(offsets) + np.array(new_offsets) | ||
| print_info("Previous offsets = " + str(['%.4f' % offset for offset in offsets]) + " deg") | ||
| print_info("New offsets = " + str(['%.4f' % offset for offset in updated_offset]) + " deg") |
There was a problem hiding this comment.
I wonder if it could make sense to add some deadzones. Usually, we tend to avoid changing the deltas if the difference is very small.
There was a problem hiding this comment.
We may ask the user if he wants to change the offset for a given joint. 🤔
There was a problem hiding this comment.
That's a good option. Otherwise, also something that sets a specific entry of new_offsets to zero if it is lower than a specified threshold could do the trick.
Co-authored-by: Stefano Dafarra <stefano.dafarra@gmail.com>
GiulioRomualdi
changed the title
blf-calibration-delta-updater scriptblf-calibration-delta-updater script
|
Hi @S-Dafarra in 736b63b I tried to apply what you suggested |
| expected_values_in_degrees (-46.99, 0.0, 0.0, 6.68, -45.256, -26.72) | ||
| control_board "right_leg" | ||
| joints_list ( "r_hip_pitch", "r_hip_roll", "r_hip_yaw", "r_knee", "r_ankle_pitch", "r_ankle_roll") | ||
| tolerance_in_degrees (0.01, 0.01, 0.01, 0.01, 0.01, 0.01) |
| parser.add_argument('-i', '--input', type=str, required=True, help='Path to the input xml file containing the calibration deltas') | ||
| parser.add_argument('-o', '--output', type=str, | ||
| required=True, help='Path to the output xml file containing the calibration deltas') | ||
| parser.add_argument('-p', '--part', type=str, required=True, | ||
| help='Name of the group will be loaded in the configuration file. For instance left_leg or right_leg.') | ||
| parser.add_argument('--config', type=str, required=False, |
There was a problem hiding this comment.
The README documents only the "short" version (e.g. -p). It may be worth noting somewhere that also --part can be used, for example.
| = yarp::os::ResourceFinder::getResourceFinderSingleton().findFileByName( | ||
| filename); | ||
| return impl.setFromFile(filePath); | ||
| }) |
There was a problem hiding this comment.
Not a big problem, but inserting actual logic in the pybind11 code instead of leaving the logic on the C++ Side and just keep pybind11 code to contain glue code may be a dangerous path, especially if in the future we amt/hope to generate automatically the pybind11 glue code.
There was a problem hiding this comment.
I open an issue for this :)
This follows #358
TODO:
cc @S-Dafarra @isorrentino @traversaro