Add option to skip initial calibration and use zero offsets #72

prashanthr05 · 2020-06-11T12:18:08Z

This PR addresses #67.

The idl files were autogenerated due to addition of parameter ~~skipResetFTSensorOffset~~ startWithZeroFTSensorOffsets to idl/wholeBodyDynamicsSettings/wholeBodyDynamicsSettings.thrift. The autogenerated files are also committed along with this PR.

Since by default the parameter ~~skipResetOffset~~ startWithZeroFTSensorOffsets is set to false (as per the regular workflow), only configuration related to simulation was modified by this PR. devices/wholeBodyDynamics/app/wholebodydynamics-icub-external-six-fts-sim.xml

traversaro · 2020-06-11T12:28:03Z

devices/wholeBodyDynamics/WholeBodyDynamicsDevice.cpp

@@ -942,7 +961,7 @@ bool WholeBodyDynamicsDevice::applyLPFSettingsFromConfig(const yarp::os::Propert
    {        
        return false;
    }
-
+       


Suggested change

Addressed in b495053

traversaro · 2020-06-11T12:28:33Z

devices/wholeBodyDynamics/WholeBodyDynamicsDevice.cpp

+    {
+        this->setFTSensorOffsetsToZero();
+        validOffsetAvailable = true;
+        ok = ok && true;


It is not really clear what is the point of this line.

I added that redundant code to debug and forgot to remove.

Addressed in b495053

traversaro · 2020-06-11T12:29:47Z

devices/wholeBodyDynamics/WholeBodyDynamicsDevice.cpp

+    if ( !(prop.check("skipResetOffset") && prop.find("skipResetOffset").isBool()) )
+    {
+        yWarning() << "wholeBodyDynamics: skipResetOffset bool parameter missing.";
+        yWarning() << "wholeBodyDynamics: setting skipResetOffset to the default value of false, please remember to resetOffset through the rpc port.";


This is misleading. On the real robot, you don't have to remember to call resetOffset .

Noted. I will remove that line.

Addressed in b495053

traversaro · 2020-06-11T12:44:50Z

Thanks! Minor comments: to be honest, the parameter name skipResetFTSensorOffset probably make sense to existing users that called resetOffets for thousands of times in their life, but I think it is misleading for new users, as it does suggest that the "FT Sensor offset reset" is skipped, while this option does exactly the opposite: is uses zero FT sensor offsets (so, "reset" offset) at the device startup, instead of performing calibration. For this reason, I think that calling something like startWithZeroFTSensorOffsets or similar (suggestions are welcome) would be more clear. Furthermore, also the description is not really clear: "This allows to skip manual calling of resetOffsets for removeing offsets on FT sensors". I think it would me more clear to first have a place were we state (also in the general description before this parameter) that at the beginning the device estimates the offset of the FT sensors, unless the option startWithZeroFTSensorOffsets is passed.

Furthermore note that a similar functionality (let user specify user-defined offsets, not just 0) is also provided in #45, so we should consider if the two options can coexist (probably yes, startWithZeroFTSensorOffsets can be an easy option to set all the FT sensor offset to zero (useful in simulation), while the option #45 can be used for more complex settings in which the offset need to be manually inserted.

@GiulioRomualdi @Giulero @gabrielenava @HosameldinMohamed

prashanthr05 · 2020-06-11T12:59:32Z

I think that calling something like startWithZeroFTSensorOffsets or similar (suggestions are welcome) would be more clear.

Agreed. I think startWithZeroFTSensorOffsets is clear. But I will wait a bit to understand if we get some inputs from other users.

Furthermore note that a similar functionality (let user specify user-defined offsets, not just 0) is also provided in #45, so we should consider if the two options can coexist (probably yes, startWithZeroFTSensorOffsets can be an easy option to set all the FT sensor offset to zero (useful in simulation), while the option #45 can be used for more complex settings in which the offset need to be manually inserted.

I revisited that PR and noticed that. Indeed, it allows the user to add an FT_OFFSET group to specify the custom offsets. I hope we will not be crowding the source code or the user experience too much by adding a redundant feature (startWithZeroFTSensorOffsets).

However, if its agreed to have both the options so that they can be used without any confusion, I will rename the parameter to what is agreed upon. Otherwise, we can close this PR.

traversaro · 2020-06-11T13:03:18Z

However, if its agreed to have both the options so that they can be used without any confusion, I will rename the parameter to what is agreed upon.

I agree that having both options make sense, especially for simulation users so that they do not need to add a complex FT_OFFSET group when they can just add a simple option.

traversaro · 2020-06-12T12:45:45Z

cc @lrapetti @diegoferigo

prashanthr05 · 2020-06-12T13:56:28Z

Agreed. I think startWithZeroFTSensorOffsets is clear. But I will wait a bit to understand if we get some inputs from other users.

I will proceed to rename the option with startWithZeroFTSensorOffsets, since there has not been any response so far.

traversaro · 2020-06-12T14:39:53Z

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

+## Unreleased
+### Added
+- Added 'skipResetOffset' to disable/enable bypassing of resetOffset rpc call for setting FT sensor offsets to zero. (See [!72](https://github.com/robotology/whole-body-estimators/pull/72)).


Suggested change

- Added 'skipResetOffset' to disable/enable bypassing of resetOffset rpc call for setting FT sensor offsets to zero. (See [!72](https://github.com/robotology/whole-body-estimators/pull/72)).

- Added 'startWithZeroFTSensorOffsets' to disable/enable bypassing of resetOffset rpc call for setting FT sensor offsets to zero. (See [!72](https://github.com/robotology/whole-body-estimators/pull/72)).

Addressed in 52813c1

traversaro · 2020-06-12T14:41:11Z

Approved, but check the CHANGELOG suggestion.

traversaro · 2020-06-12T15:13:41Z

Perfect!

prashanthr05 added PR-type:enhancement component:whole-body-dynamics labels Jun 11, 2020

prashanthr05 added this to the Iteration 45 - Back to the lab again milestone Jun 11, 2020

prashanthr05 requested review from traversaro and HosameldinMohamed June 11, 2020 12:18

prashanthr05 self-assigned this Jun 11, 2020

traversaro reviewed Jun 11, 2020

View reviewed changes

prashanthr05 force-pushed the feature/bypass-resetOffset branch from b495053 to ef3b7f6 Compare June 12, 2020 14:29

prashanthr05 added 2 commits June 12, 2020 16:32

[wholebodydynamics] add feature to start WBD with zero FT sensor offsets

4c83b40

[wholebodydynamics] update idl autogenerated

8e3d346

prashanthr05 force-pushed the feature/bypass-resetOffset branch from ef3b7f6 to 68fef48 Compare June 12, 2020 14:33

prashanthr05 requested a review from traversaro June 12, 2020 14:35

traversaro reviewed Jun 12, 2020

View reviewed changes

traversaro approved these changes Jun 12, 2020

View reviewed changes

Update CHANGELOG.md

52813c1

prashanthr05 force-pushed the feature/bypass-resetOffset branch from 68fef48 to 52813c1 Compare June 12, 2020 15:00

HosameldinMohamed approved these changes Jun 13, 2020

View reviewed changes

HosameldinMohamed merged commit 0c4cc01 into robotology:devel Jun 13, 2020

prashanthr05 deleted the feature/bypass-resetOffset branch June 15, 2020 11:09

prashanthr05 mentioned this pull request Jun 15, 2020

Add a .ini option to skip the initial calibration and just use zero offset #67

Closed

HosameldinMohamed mentioned this pull request Nov 9, 2020

Merge devel to master for a new release #89

Merged

traversaro mentioned this pull request Apr 5, 2024

Cleanup README.md removing reference to resetOffset and reduce references to iCub robotology/walking-controllers#172

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to skip initial calibration and use zero offsets #72

Add option to skip initial calibration and use zero offsets #72

prashanthr05 commented Jun 11, 2020 •

edited

Loading

traversaro Jun 11, 2020

prashanthr05 Jun 11, 2020

traversaro Jun 11, 2020

prashanthr05 Jun 11, 2020

prashanthr05 Jun 11, 2020

traversaro Jun 11, 2020

prashanthr05 Jun 11, 2020

prashanthr05 Jun 11, 2020

traversaro commented Jun 11, 2020

prashanthr05 commented Jun 11, 2020 •

edited

Loading

traversaro commented Jun 11, 2020

traversaro commented Jun 12, 2020

prashanthr05 commented Jun 12, 2020 •

edited

Loading

traversaro Jun 12, 2020

prashanthr05 Jun 12, 2020

traversaro commented Jun 12, 2020

traversaro commented Jun 12, 2020

	- Added 'skipResetOffset' to disable/enable bypassing of resetOffset rpc call for setting FT sensor offsets to zero. (See [!72](https://github.com/robotology/whole-body-estimators/pull/72)).
	- Added 'startWithZeroFTSensorOffsets' to disable/enable bypassing of resetOffset rpc call for setting FT sensor offsets to zero. (See [!72](https://github.com/robotology/whole-body-estimators/pull/72)).

Add option to skip initial calibration and use zero offsets #72

Add option to skip initial calibration and use zero offsets #72

Conversation

prashanthr05 commented Jun 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

traversaro commented Jun 11, 2020

prashanthr05 commented Jun 11, 2020 • edited Loading

traversaro commented Jun 11, 2020

traversaro commented Jun 12, 2020

prashanthr05 commented Jun 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

traversaro commented Jun 12, 2020

traversaro commented Jun 12, 2020

prashanthr05 commented Jun 11, 2020 •

edited

Loading

prashanthr05 commented Jun 11, 2020 •

edited

Loading

prashanthr05 commented Jun 12, 2020 •

edited

Loading