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

Feature/guarded motion added distance until edge up #1199

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

P-ict0
Copy link
Contributor

@P-ict0 P-ict0 commented Jun 21, 2022

  • Modified move_down_until_force_sensor_edge_up in guarded_motion.py and arms.py so that the travel distance until an edge up is customizable
  • Added some comments to guarded_motion.py
  • Modified example for move_down_until_force_sensor_edge_up

@P-ict0 P-ict0 force-pushed the feature/guarded_motion_added_distance_until_edge_up branch from 0b3163d to 07468c5 Compare June 21, 2022 19:58
@P-ict0 P-ict0 force-pushed the feature/guarded_motion_added_distance_until_edge_up branch from 07468c5 to d399630 Compare June 21, 2022 20:01

# Sets the joint state to move down a certain distance (or 0 if distance is not set)
if distance_move_down is not None:
current_joint_state['arm_lift_joint'] = max(0, current_joint_state['arm_lift_joint'] - distance_move_down)
Copy link
Member

Choose a reason for hiding this comment

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

0 should be the lower limit of the joint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0 is the lowest the arm lift joint state value can be. With the distance_move_down it can be greater than 0

for part in arm.parts:
if isinstance(arm.parts[part], ForceSensor):
return True
return False


def move_down_until_force_sensor_edge_up(self, force_sensor=None, timeout=10, retract_distance=0.01):
def move_down_until_force_sensor_edge_up(self, force_sensor=None, timeout=10, retract_distance=0.01,
Copy link
Member

Choose a reason for hiding this comment

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

I think the final position is more interesting than the distance to move down.

Copy link
Contributor Author

@P-ict0 P-ict0 Jun 22, 2022

Choose a reason for hiding this comment

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

I agree, I thought of adding the distance to move down first before working on the final position since this requires to calculate at what height the force sensor is. I think the final point is an interesting feature for me to work on in the future

PetervDooren
PetervDooren previously approved these changes Jun 28, 2022
Copy link
Contributor

@PetervDooren PetervDooren left a comment

Choose a reason for hiding this comment

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

I think this is already an improvement over the previous version, although I agree with matthijs that the other functionality would be preffered in the long run.

force_sensor = self.parts["force_sensor"]

# Fill with required joint names (desired in hardware / gazebo impl)
goal = self._create_lower_force_sensing_goal(timeout)
goal = self._create_lower_force_sensing_goal(distance_move_down, timeout)
self._ac_joint_traj.send_goal(goal)

force_sensor.wait_for_edge_up(timeout)
Copy link
Contributor

Choose a reason for hiding this comment

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

The behaviour we currently create will wait for the timeout if its end goal is reached. So it might be waiting at the lower position without continuing.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is acceptable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this the same behaviour as before? but with a different distance to move down.

@P-ict0
Copy link
Contributor Author

P-ict0 commented Jun 29, 2022

I suggest that we decide before continuing:

  • We continue with this feature and fix the timeout issue (so that the arm doesn't wait for timeout at the lower position)
    or
  • Close this PR and work on implementing moving the force sensor to a certain point instead of a distance

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.

3 participants