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

Added a service to setup the active payload #50

Merged
merged 2 commits into from
Jul 20, 2020
Merged

Added a service to setup the active payload #50

merged 2 commits into from
Jul 20, 2020

Conversation

fmauch
Copy link
Collaborator

@fmauch fmauch commented Nov 27, 2019

This PR adds a service to setup the active payload through a ROS service.

This PR depends on ros-industrial/universal_robot#470 being accepted which is why this is a draft PR.

Comment on lines +328 to +395
<< " set_payload(" << req.payload << ", [" << req.center_of_gravity.x << ", " << req.center_of_gravity.y
<< ", " << req.center_of_gravity.z << "])" << std::endl
Copy link
Contributor

@gavanderhoorn gavanderhoorn Nov 27, 2019

Choose a reason for hiding this comment

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

Is relying on << to convert floats/doubles to string locale -safe?

Could end up with multiple commas in the string, which would probably upset UR's parser.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not think about that. I would never have though about the stream operator using locales in the background.

In a quick example I could not get a stream into locale-like formatting without explicitly setting the locale of the stream. However, to make sure, we could set the string's locale explicitly to classic

Copy link
Contributor

Choose a reason for hiding this comment

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

I just want to avoid another ros/urdfdom_headers#41.

If stream io doesn't suffer from this, all the better.

@ethanfowler
Copy link

Is there anything holding this up that I can help with? We (Shadow robot) want this functionality as soon as possible, though we can run on a branch in the meantime.

@lavarock1234
Copy link

Any update on this PR? Thanks very much in advance

@gavanderhoorn
Copy link
Contributor

The referenced PR has been merged into ros-industrial/ur_msgs.

@fmauch: I'd appreciate it if you could test the service a bit before I release it into Melodic.

@fmauch
Copy link
Collaborator Author

fmauch commented Jun 23, 2020

The referenced PR has been merged into ros-industrial/ur_msgs.

@fmauch: I'd appreciate it if you could test the service a bit before I release it into Melodic.

Sure thing!

@fmauch fmauch marked this pull request as ready for review June 23, 2020 19:13
@fmauch
Copy link
Collaborator Author

fmauch commented Jun 23, 2020

Just tested with a

rosservice call /ur_hardware_interface/set_payload "payload: 2.0
center_of_gravity:
  x: 0.2
  y: 0.3
  z: 0.1"

and verified with

rostopic pub /ur_hardware_interface/script_command std_msgs/String "data: 'def myProgram():

 textmsg(get_target_payload())

 textmsg(get_target_payload_cog())

end'"

Which printed
image

@fmauch
Copy link
Collaborator Author

fmauch commented Jun 24, 2020

It would be nice if we could add a check for success into the service. Currently, it succeeds, as soon as the script code got successfully sent to the socket.

Unfortunately, this isn't very easy to do as far as I understand. To my knowledge, there is no interface to get information about the payload data in

  • the primary interface
  • the RTDE interface
  • the dashboard interface

It could be implemented together with #171, where we would receive a PROGRAM_XXX_STARTED, PROGRAM_XXX_STOPPED message sequence on success and a PROGRAM_XXX_STARTED, RuntimeError, PROGRAM_XXX_STOPPED sequence in case of illegal input. Timing out on an answer should also be a failure.

Another option would be of course to send payload data back through a custom socket, but then we would be highly dependent on our actual program script code.

@gavanderhoorn
Copy link
Contributor

gavanderhoorn commented Jun 30, 2020

Update: ur_msgs was released into Melodic (v1.3.1) and includes the SetPayload service definition.


Edit: until Melodic sees a sync, the main repositories will not provide ur_msgs.

fmauch added 2 commits July 20, 2020 15:06
I am not sure whether this is necessary, but this will make sure that
decimal divider will be a dot.
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.

4 participants