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.

Sign up for GitHub

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

Jump to bottom

Add joint and motor acceleration reading #492

Merged
merged 3 commits into from
Jan 10, 2022
Merged

Add joint and motor acceleration reading #492

merged 3 commits into from
Jan 10, 2022

Conversation

isorrentino
Copy link
Collaborator

No description provided.

@isorrentino isorrentino self-assigned this Jan 4, 2022
@isorrentino isorrentino mentioned this pull request Jan 5, 2022
Merged
GiulioRomualdi
GiulioRomualdi reviewed Jan 7, 2022
View reviewed changes
...obotInterface/YarpImplementation/include/BipedalLocomotion/RobotInterface/YarpSensorBridge.h Outdated
Comment on lines 252 to 254
double& jointAcceleration,
OptionalDoubleRef receiveTimeInSeconds = {}) final;

Copy link
Member

Choose a reason for hiding this comment

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

Could you keep the right indentation?

...obotInterface/YarpImplementation/include/BipedalLocomotion/RobotInterface/YarpSensorBridge.h Outdated
* @return true/false in case of success/failure
*/
bool getJointAccelerations(Eigen::Ref<Eigen::VectorXd> jointAccelerations,
OptionalDoubleRef receiveTimeInSeconds = {}) final;
Copy link
Member

Choose a reason for hiding this comment

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

As above

prashanthr05
prashanthr05 approved these changes Jan 7, 2022
View reviewed changes
Copy link
Collaborator

@prashanthr05 prashanthr05 left a comment

Choose a reason for hiding this comment

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

Thanks, @isorrentino ! super minor comments only related to the aesthetics.

...obotInterface/YarpImplementation/include/BipedalLocomotion/RobotInterface/YarpSensorBridge.h Outdated

/**
* Get all motors' accelerations in rad/s
* @param[out] parameter all motors' accelerations in radians per second squared
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param[out] parameter all motors' accelerations in radians per second squared
* @param[out] motorAccelerations all motors' accelerations in radians per second squared

...obotInterface/YarpImplementation/include/BipedalLocomotion/RobotInterface/YarpSensorBridge.h Outdated

/**
* Get all joints' accelerations in rad/s^2
* @param[out] parameter all joints' accelerations in radians per second squared
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param[out] parameter all joints' accelerations in radians per second squared
* @param[out] jointAccelerations all joints' accelerations in radians per second squared

src/RobotInterface/YarpImplementation/src/YarpSensorBridge.cpp Outdated
Comment on lines 366 to 368
bool YarpSensorBridge::getJointAcceleration(const std::string& jointName,
double& jointAcceleration,
OptionalDoubleRef receiveTimeInSeconds)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please correct the indentation.

src/RobotInterface/YarpImplementation/src/YarpSensorBridge.cpp Outdated
Comment on lines 389 to 390
bool YarpSensorBridge::getJointAccelerations(Eigen::Ref<Eigen::VectorXd> jointAccelerations,
OptionalDoubleRef receiveTimeInSeconds)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please correct the indentation.

src/RobotInterface/YarpImplementation/src/YarpSensorBridge.cpp Outdated
Comment on lines 898 to 900
bool YarpSensorBridge::getMotorAcceleration(const std::string& jointName,
double& motorAcceleration,
OptionalDoubleRef receiveTimeInSeconds)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please correct the indentation.

src/RobotInterface/YarpImplementation/src/YarpSensorBridge.cpp Outdated
Comment on lines 928 to 929
bool YarpSensorBridge::getMotorAccelerations(Eigen::Ref<Eigen::VectorXd> motorAccelerations,
OptionalDoubleRef receiveTimeInSeconds)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please correct the indentation.

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h Outdated

/**
* Get all motors' accelerations in rad/s^2
* @param[out] parameter all motors' accelerations in radians per second squared
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param[out] parameter all motors' accelerations in radians per second squared
* @param[out] motorAccelerations all motors' accelerations in radians per second squared

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h Outdated

/**
* Get all joints' accelerations in rad/s^2
* @param[out] parameter all joints' accelerations in radians per second squared
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* @param[out] parameter all joints' accelerations in radians per second squared
* @param[out] jointAccelerations all joints' accelerations in radians per second squared

@isorrentino
Copy link
Collaborator Author

isorrentino commented Jan 10, 2022 •
edited
Loading

Suggested changes pushed!

@GiulioRomualdi
Copy link
Member

GiulioRomualdi commented Jan 10, 2022 •
edited
Loading

The failure is not related to this PR 😄

spdlog uses fmt to format the output print. 12 hours ago the following commit has been merged Homebrew/homebrew-core@d032f2f and the latest version of fmt (8.1.1) is now installed with brew. As explained here, fmt requires a specializing formatter<T> to handle a custom type.

As you can notice, the action is failing while compiling the following line

bipedal-locomotion-framework/src/TextLogging/src/Logger.cpp

Line 35 in 0a758c5

log()->error("Failed to change verbosity to level {}", verbosity);

I think that in the nearest future we will have a similar issue also for the conda installation (cc @traversaro)

A possible solution is to implement what explained here (but I don't know if this approach is back-compatible with the old fmt version installed by ubuntu 20.04

image

Another possibility's to modify directly this line:

bipedal-locomotion-framework/src/TextLogging/src/Logger.cpp

Line 35 in 0a758c5

log()->error("Failed to change verbosity to level {}", verbosity);

@GiulioRomualdi
Copy link
Member

@isorrentino could you try to handle it in another PR? If you don't have time I can do it

@GiulioRomualdi GiulioRomualdi mentioned this pull request Jan 10, 2022
Merged
@GiulioRomualdi
Copy link
Member

@isorrentino could you try to handle it in another PR? If you don't have time I can do it

#495 should fix the problem

@GiulioRomualdi
Copy link
Member

Waiting for CI and then I will merge it

@traversaro
Copy link
Collaborator

The failure is not related to this PR 😄

Related issue: robotology/robotology-superbuild#991 .

@GiulioRomualdi GiulioRomualdi merged commit 892164b into ami-iit:master Jan 10, 2022
@isorrentino isorrentino deleted the addAccelerationReading branch January 10, 2022 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GiulioRomualdi GiulioRomualdi GiulioRomualdi left review comments

@prashanthr05 prashanthr05 prashanthr05 approved these changes

Assignees

@isorrentino isorrentino

Labels
🔨 component: RobotInterface
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@isorrentino @GiulioRomualdi @traversaro @prashanthr05