Add Haptic Glove Module #72

kouroshD · 2021-10-07T22:31:58Z

I still have to add README file to the module
I have to fix the configuration files
I have to check the possible changes outside the scope of this PR on other modules and remove those changes if are unnecessary
After the latest changes, I have to run the code with the robot.

kouroshD · 2021-12-06T18:34:59Z

The PR is ready to be reviewed, except the points mentioned in the first comment.

kouroshD · 2021-12-15T19:41:22Z

I have tested many times during these days the code, and I applied bug fixes as well as updates on the configuration file. All the features are implemented. The only remaining point is the ReadMe documentation, that I may do it later. In any case, the PR is ready to be reviewed. I have also updated the CI, so that has dependency into wearables as well. Now, in fact, it compiles haptic glove module in all OS.

S-Dafarra

Some preliminary feedback

app/robots/iCubGazeboV2_5/leftFingersHapticRetargetingParams.ini

modules/HapticGlove_module/CMakeLists.txt

modules/Utils/src/Utils.cpp

traversaro · 2022-01-10T10:29:23Z

Given the size of the PR, if @S-Dafarra is already reviewing it I would remove myself from the reviewers. If there is any specific point on which you need my input @kouroshD feel free to point it out, thanks!

S-Dafarra

I mostly skimmed through the code. Most of the comments are simply suggestions

modules/HapticGlove_module/include/RobotController.hpp

modules/HapticGlove_module/include/RobotInterface.hpp

modules/HapticGlove_module/include/RobotMotorsEstimation.hpp

modules/HapticGlove_module/src/GloveControlHelper.cpp

modules/HapticGlove_module/src/LinearRegression.cpp

modules/HapticGlove_module/src/MotorEstimation.cpp

modules/HapticGlove_module/src/RobotInterface.cpp

modules/HapticGlove_module/src/RobotMotorsEstimation.cpp

kouroshD · 2022-01-14T11:08:42Z

With this fc89789, I have modified the writeStrict option of the remotecontrolboard from true to false.

kouroshD · 2022-01-14T11:13:39Z

another thing is that in the RemoteControlBoard the default carrier is set to udp as in https://github.com/robotology/yarp/blob/ce6f008c86ba58a652b04b6bffdc2b46e9b157e2/src/devices/RemoteControlBoard/RemoteControlBoard.cpp#L247-L250 and https://github.com/robotology/yarp/blob/ce6f008c86ba58a652b04b6bffdc2b46e9b157e2/src/devices/RemoteControlBoard/RemoteControlBoard.cpp#L307. And I remember, I could run the HapticGloveModule some months ago from the operator side as well, however with some difficulties. In any case, now we are running it the robot head.

modules/HapticGlove_module/CMakeLists.txt

Co-authored-by: Stefano Dafarra <[email protected]>

…ific member method

kouroshD · 2022-01-17T12:33:51Z

@S-Dafarra let me know if the responses are OK, if it needs further declarations/modifications.

S-Dafarra · 2022-01-17T13:22:53Z

@S-Dafarra let me know if the responses are OK, if it needs further declarations/modifications.

I think it is OK to me! I would just suggest opening an issue to remove hardcoded numbers when setting a haptic feedback

kouroshD · 2022-01-17T15:21:48Z

Before merging, I ran once the code in order to be sure it is running after changing the requested code changes, and I found a bug indeed related to those changes: remoteControlBoardsOpts.put("writeStrict", "off"); in e246257 .
Here is the video:

leftHandREtargetingForPR-2022-01-17_15.24.06.mp4

S-Dafarra · 2022-01-17T16:24:44Z

I guess you can also remove that line. By default it should be off

kouroshD · 2022-01-17T18:35:53Z

I guess you can also remove that line. By default it should be off

It is not. Look at here:

https://github.com/robotology/yarp/blob/165a46c6e034fc27b90cc0bd80b35419679b22c3/src/devices/RemoteControlBoard/RemoteControlBoard.h#L96-L97

S-Dafarra · 2022-01-17T18:40:43Z

I guess you can also remove that line. By default it should be off

It is not. Look at here:

https://github.com/robotology/yarp/blob/165a46c6e034fc27b90cc0bd80b35419679b22c3/src/devices/RemoteControlBoard/RemoteControlBoard.h#L96-L97

Ah nice catch. It seems that it enables it when writing to multiple joints. I guess this is to make sure that the commands sent to all the joints is coherent in time 🤔 .

traversaro · 2022-01-18T09:22:59Z

I guess you can also remove that line. By default it should be off

It is not. Look at here:
https://github.com/robotology/yarp/blob/165a46c6e034fc27b90cc0bd80b35419679b22c3/src/devices/RemoteControlBoard/RemoteControlBoard.h#L96-L97

Ah nice catch. It seems that it enables it when writing to multiple joints. I guess this is to make sure that the commands sent to all the joints is coherent in time 🤔 .

Yes, writeStrict is necessary for all the commands that require multiple messages for different joints, otherwise tipically the last one would be the only one considered.

S-Dafarra · 2022-01-18T09:25:44Z

So then, maybe it is worth leaving it with the default behavior 🤔

kouroshD · 2022-01-18T09:33:29Z

Yes, writeStrict is necessary for all the commands that require multiple messages for different joints, otherwise tipically the last one would be the only one considered.

@traversaro What do you mean by requiring multiple messages for different joints? Can you give an example?

traversaro · 2022-01-18T09:51:34Z

Yes, writeStrict is necessary for all the commands that require multiple messages for different joints, otherwise tipically the last one would be the only one considered.

@traversaro What do you mean by requiring multiple messages for different joints? Can you give an example?

Let's say that for some reason you are sending velocity reference joint-by-joint with: https://github.com/robotology/yarp/blob/165a46c6e034fc27b90cc0bd80b35419679b22c3/src/devices/RemoteControlBoard/RemoteControlBoard.cpp#L1914 . In that case, if you do not have writeStrict, only the last command will be considered.

kouroshD · 2022-01-18T10:01:46Z

Yes, writeStrict is necessary for all the commands that require multiple messages for different joints, otherwise tipically the last one would be the only one considered.

@traversaro What do you mean by requiring multiple messages for different joints? Can you give an example?

Let's say that for some reason you are sending velocity reference joint-by-joint with: https://github.com/robotology/yarp/blob/165a46c6e034fc27b90cc0bd80b35419679b22c3/src/devices/RemoteControlBoard/RemoteControlBoard.cpp#L1914 . In that case, if you do not have writeStrict, only the last command will be considered.

As I see in that snippet, it sends the joint i reference value with false write strict option. I am not seeing any part that is discarding other commands! booh! I can put it back in our code in default option, what is your suggestion? When I tried yesterday with off option, I did not see any strange behavior. In any case, in our code, we are always send vector of joint values not a single joint!

kouroshD · 2022-01-18T10:20:50Z

@S-Dafarra @traversaro If you do not have other comments or suggestion, I will merge the PR. Thanks. I will keep the history of PR as it is.

traversaro · 2022-01-18T10:24:51Z

@S-Dafarra @traversaro If you do not have other comments or suggestion, I will merge the PR. Thanks. I will keep the history of PR as it is.

Sure on my side, see #72 (comment) !

S-Dafarra · 2022-01-18T10:38:55Z

@S-Dafarra @traversaro If you do not have other comments or suggestion, I will merge the PR. Thanks. I will keep the history of PR as it is.

GO also for me!

kouroshD force-pushed the hapticGlove_PR branch 2 times, most recently from 10c2474 to 4bc993f Compare November 2, 2021 16:24

kouroshD force-pushed the hapticGlove_PR branch 2 times, most recently from 139ae79 to 37029ee Compare November 29, 2021 19:06

kouroshD force-pushed the hapticGlove_PR branch from e077eea to d7d5f1b Compare December 6, 2021 17:46

kouroshD marked this pull request as ready for review December 6, 2021 17:56

kouroshD requested review from GiulioRomualdi, traversaro and S-Dafarra December 6, 2021 17:57

kouroshD force-pushed the hapticGlove_PR branch 9 times, most recently from cbc64d2 to ba407e7 Compare December 15, 2021 16:12

kouroshD mentioned this pull request Dec 15, 2021

Update GitHub actions #79

Closed

S-Dafarra reviewed Jan 10, 2022

View reviewed changes

S-Dafarra approved these changes Jan 10, 2022

View reviewed changes

S-Dafarra reviewed Jan 14, 2022

View reviewed changes

modules/HapticGlove_module/CMakeLists.txt Outdated Show resolved Hide resolved

kouroshD added 3 commits January 17, 2022 13:31

Add haptic Glove Module, Add IAnalogSensor client device

83be854

add the IAnalogSensor device to the polydriver

878eedd

getting axis and joint values from simulation work

c96ed9b

kouroshD and others added 5 commits January 17, 2022 13:31

Apply suggestions from code review

a8ccda3

Co-authored-by: Stefano Dafarra <[email protected]>

update the writeStrict option of the remote control board

5885460

RobotInterface Configuration: the devices to open are added to a spec…

4448f13

…ific member method

add nolias() to the eigen matrix products, add default destructors

e71ce75

update the project cmake with the target_include_directories

fd5f5f9

kouroshD force-pushed the hapticGlove_PR branch from 9746ce3 to fd5f5f9 Compare January 17, 2022 12:33

fix the remote control board option write sctrict option

e246257

kouroshD force-pushed the hapticGlove_PR branch from e995d14 to ab05a12 Compare January 17, 2022 18:48

add the memory allocation check option for the kalman filter

e94eb10

kouroshD force-pushed the hapticGlove_PR branch from ab05a12 to e94eb10 Compare January 18, 2022 09:00

add enum value to stop the thumper haptic feedback for SenseGlove

8d336ef

kouroshD mentioned this pull request Jan 18, 2022

Think of an approach to avoid passing hard-coded values for discrete haptic commands #86

Open

kouroshD merged commit 0731fd7 into devel Jan 18, 2022

kouroshD mentioned this pull request Jan 21, 2022

Feature/communication values #87

Merged

kouroshD mentioned this pull request Feb 1, 2022

Change BufferedPort forceStrict to false robotology/wearables#137

Merged

S-Dafarra deleted the hapticGlove_PR branch December 14, 2022 09:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Haptic Glove Module #72

Add Haptic Glove Module #72

kouroshD commented Oct 7, 2021 •

edited

Loading

kouroshD commented Dec 6, 2021

kouroshD commented Dec 15, 2021 •

edited

Loading

S-Dafarra left a comment

traversaro commented Jan 10, 2022

S-Dafarra left a comment

kouroshD commented Jan 14, 2022

kouroshD commented Jan 14, 2022

kouroshD commented Jan 17, 2022

S-Dafarra commented Jan 17, 2022

kouroshD commented Jan 17, 2022

S-Dafarra commented Jan 17, 2022 •

edited

Loading

kouroshD commented Jan 17, 2022

S-Dafarra commented Jan 17, 2022

traversaro commented Jan 18, 2022

S-Dafarra commented Jan 18, 2022

kouroshD commented Jan 18, 2022

traversaro commented Jan 18, 2022

kouroshD commented Jan 18, 2022

kouroshD commented Jan 18, 2022 •

edited

Loading

traversaro commented Jan 18, 2022

S-Dafarra commented Jan 18, 2022

Add Haptic Glove Module #72

Add Haptic Glove Module #72

Conversation

kouroshD commented Oct 7, 2021 • edited Loading

kouroshD commented Dec 6, 2021

kouroshD commented Dec 15, 2021 • edited Loading

S-Dafarra left a comment

Choose a reason for hiding this comment

traversaro commented Jan 10, 2022

S-Dafarra left a comment

Choose a reason for hiding this comment

kouroshD commented Jan 14, 2022

kouroshD commented Jan 14, 2022

kouroshD commented Jan 17, 2022

S-Dafarra commented Jan 17, 2022

kouroshD commented Jan 17, 2022

S-Dafarra commented Jan 17, 2022 • edited Loading

kouroshD commented Jan 17, 2022

S-Dafarra commented Jan 17, 2022

traversaro commented Jan 18, 2022

S-Dafarra commented Jan 18, 2022

kouroshD commented Jan 18, 2022

traversaro commented Jan 18, 2022

kouroshD commented Jan 18, 2022

kouroshD commented Jan 18, 2022 • edited Loading

traversaro commented Jan 18, 2022

S-Dafarra commented Jan 18, 2022

kouroshD commented Oct 7, 2021 •

edited

Loading

kouroshD commented Dec 15, 2021 •

edited

Loading

S-Dafarra commented Jan 17, 2022 •

edited

Loading

kouroshD commented Jan 18, 2022 •

edited

Loading