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

Adding Cartesian impedance/force control #106

Closed
wants to merge 9 commits into from

Conversation

rkojcev
Copy link
Contributor

@rkojcev rkojcev commented Sep 7, 2016

This changes add Impedance/Force control to grl via ros messages

See the following for details:
ros-industrial-consortium/majorana#3

@ahundt
Copy link
Owner

ahundt commented Oct 19, 2016

@rkojcev these changes are slightly out of date with @ashkan372 merged changes in #107, do you think you could merge and update?

Copy link
Owner

@ahundt ahundt left a comment

Choose a reason for hiding this comment

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

Looks like there is some cool stuff in here! Thanks! Could you update the pull request with changes for the comments below and so it merges with master? @ashkan372 could you also take a look?



// Printing dummy message:
getLogger().info("DUMMY: " + _currentKUKAiiwaState.dummy());
Copy link
Owner

Choose a reason for hiding this comment

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

this will probably overload the logger, won't it? should this be in an if(debug) or something like that?

/**
* Creates a FRI Session.
*/
public class GRL_Driver_ChangeModes extends RoboticsAPIApplication
Copy link
Owner

Choose a reason for hiding this comment

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

Can a more detailed explanation of this class be added here? What does it do? Why is it needed? How do I interact with it? Where is it used and why?

getLogger().info("GRL_Driver from github.com/ahundt/grl starting...\nZMQ Connecting to: " + _processDataManager.get_ZMQ_MASTER_URI());

// connect to the controlling application via ZeroMQ
ZMQManager zmq = new ZMQManager(_processDataManager.get_ZMQ_MASTER_URI(),getLogger());
Copy link
Owner

Choose a reason for hiding this comment

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

Will this conflict with @ashkan372's pull request #107? If not, will need to make the change to reconcile the differences! Perhaps it is designed to abstract the difference away across an interface?

}

}
// RISTO Start
Copy link
Owner

Choose a reason for hiding this comment

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

what does this mean? could you add an explanation of what's happening?



CartesianImpedanceControlMode cicm = new CartesianImpedanceControlMode();
// These parameters cant be changed online, only stiffnesses and damping, read KUKA documentation. However when we switch Control modes, this values can be set up.
Copy link
Owner

Choose a reason for hiding this comment

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

thanks! that's important info

@@ -17,9 +17,16 @@
#include <sensor_msgs/JointState.h>
#include <std_msgs/String.h>

#include <cartesian_impedance_msgs/SetCartesianImpedance.h>
Copy link
Owner

Choose a reason for hiding this comment

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

This might add an additional dependency, we will need a way to make this optional at the very least.

@rkojcev
Copy link
Contributor Author

rkojcev commented Nov 12, 2016

@ahundt thanks for the review!!! I will have a look at the most recent changes to grl make necessary changes and then make a new pull request with no conflicts and clean code.

I am currently with very few time to work on this. I will try to get back to it as soon as possible!!

@ahundt
Copy link
Owner

ahundt commented Nov 12, 2016

@rkojcev great thanks! Also when you update the changes be sure to avoid removing existing functionality if possible, I noticed all the FRI functionality gets taken out.

@ahundt
Copy link
Owner

ahundt commented Feb 4, 2017

Relevant issue:
ros-industrial-consortium/majorana#3

@ahundt ahundt added this to TODO Long Term in Plan Sep 22, 2017
@ahundt
Copy link
Owner

ahundt commented Dec 8, 2017

Closing due to inactivity. I'd love to merge this so feel free to let me know.

@ahundt ahundt closed this Dec 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Plan
TODO Long Term
Development

Successfully merging this pull request may close these issues.

None yet

2 participants