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

Add message downsampler to avoid logger flooding when changing control mode #770

Merged
merged 12 commits into from
Nov 16, 2021

Conversation

mfussi66
Copy link
Member

@mfussi66 mfussi66 commented Nov 16, 2021

This PR aims to fix the issue: #768
To downsample the error messages rate, the logic of a simple Finite State Machine is implemented inside a yarp::os::Timer. The timer runs at 100Hz. If they exceed a threshold of 5 events, the FSM prints the number of error events occurred each second.
The proposed solution was tested on a desk setup with one joint, addressing the three types of error events mentioned in the issue.

The videos below show the results on the setup.

On the top left there's the Yarpmotorgui, where I change the control mode. On the top right there's the program tutorial_arm that sends the requests, and triggers the errors in the yarprobotinterface if the control mode is not the expected one. On the bottom there is the yarprobotinterface.

  • From Position to Velocity
downsampler_position_to_vel-2021-11-16_09.41.15.mp4
  • From Velocity to Position
downsampler_vel_to_pos-2021-11-16_09.58.11.mp4
  • From Position Direct to Velocity
downsampler_direct_to_vel-2021-11-16_10.07.27.mp4

@pattacini
Copy link
Member

cc @S-Dafarra

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Hi @mfussi66

Nicely done 👍🏻

I've only a minor comment and a request for improving the printouts.

@pattacini pattacini marked this pull request as ready for review November 16, 2021 10:06
@S-Dafarra
Copy link
Contributor

Thanks a lot! Just as a curiosity, I have seen in the video above, you only change the control mode of a single joint. What happens if you do the same with multiple joints?

@pattacini
Copy link
Member

pattacini commented Nov 16, 2021

Thanks a lot! Just as a curiosity, I have seen in the video above, you only change the control mode of a single joint. What happens if you do the same with multiple joints?

Good question!

In that file, there are only 3 "skipping"-like error messages and we address all of them, which are joint dependent.

@marcoaccame, how does it work with multi-joint commands? Are they unpacked and dealt with at the single joint level? If so we're ok, otherwise we're required to catch where else the error messages are yielded and manage them accordingly.

Perhaps, @mfussi66, you could quickly test the multi-joint approach as well (most likely straight on the robot).

@mfussi66
Copy link
Member Author

Perhaps, @mfussi66, you could quickly test the multi-joint approach as well (most likely straight on the robot).

Yep, I'll check the availability of the robot and book it asap. I assume the code could be tested also on iCub2? In case iCub3 is not available.

@pattacini
Copy link
Member

Perhaps, @mfussi66, you could quickly test the multi-joint approach as well (most likely straight on the robot).

Yep, I'll check the availability of the robot and book it asap. I assume the code could be tested also on iCub2? In case iCub3 is not available.

Yes, but wait for @marcoaccame's input first.

@S-Dafarra
Copy link
Contributor

@marcoaccame, how does it work with multi-joint commands? Are they unpacked and dealt with at the single joint level? If so we're ok, otherwise we're required to catch where else the error messages are yielded and manage them accordingly.

By experience, I think this is the case

@marcoaccame
Copy link
Contributor

@marcoaccame, how does it work with multi-joint commands? Are they unpacked and dealt with at the single joint level? If so we're ok, otherwise we're required to catch where else the error messages are yielded and manage them accordingly.

So far, every call to velocityMoveRaw(), positionMoveRaw() and setPositionRaw() for any joint contributes to reach the threshold.

So, if the threshold of events is 5 and the device is for instance left_arm-eb24-j4_7 which manages 4 joints, then one velocity move of the entire part increments 4 times and the next velocity mode exceeds 5 and triggers the threshold.

If desired, it is possible (and easy) to modify the implementation so that there is one counter per each joint.

@pattacini
Copy link
Member

pattacini commented Nov 16, 2021

Thanks @marcoaccame 👍🏻

So, if the threshold of events is 5 and the device is for instance left_arm-eb24-j4_7 which manages 4 joints, then one velocity move of the entire part increments 4 times and the next velocity mode exceeds 5 and triggers the threshold.

That's how it is supposed to work, so far so good 👍🏻

If desired, it is possible (and easy) to modify the implementation so that there is one counter per each joint.

We can certainly make things more general and configurable but I would like to go on with this current state to reach a first MVP.

Therefore, I'm waiting for the CI to complete and then merge the PR.

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Minor refactoring.

src/libraries/icubmod/embObjLib/mcEventDownsampler.h Outdated Show resolved Hide resolved
src/libraries/icubmod/embObjLib/mcEventDownsampler.h Outdated Show resolved Hide resolved
@S-Dafarra
Copy link
Contributor

I would just suggest a more descriptive message. I am afraid that Detected 1024 events on aggregate since the last message might be a little cryptic if you miss the first message about the wrong control mode.

@pattacini
Copy link
Member

pattacini commented Nov 16, 2021

I would just suggest a more descriptive message. I am afraid that Detected 1024 events on aggregate since the last message might be a little cryptic if you miss the first message about the wrong control mode.

We have just added the new logging component and we may rely on that.
See #770 (comment).

Perhaps, @mfussi66, you may call the component something like mc.msgdownsampler.skipped-cmd-wrong-mode instead of mc.downsampler.

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.

When a joint goes in HF, errors like positionMoveRaw: skipping command flood the logger
4 participants