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 external axes support #88

Merged
merged 7 commits into from
Mar 30, 2020

Conversation

jontje
Copy link
Contributor

@jontje jontje commented Mar 13, 2020

As per title.

This PR partially fixes issue #20.

Example:

It will now be possible to use abb_libegm to control a system that has one RAPID motion task for a robot, and one motion RAPID task for a track, by setting up an EGM communication channel for each of the them. And a system could look like this:

track_and_robot

@jontje jontje requested a review from gavanderhoorn March 13, 2020 07:54
@gavanderhoorn
Copy link
Member

So I find this PR hard to review, as I cannot (readily) test it.

Will the State Machine addin require any changes for this to work?

What is the limit on the nr of tasks that could be added this way?

@jontje
Copy link
Contributor Author

jontje commented Mar 24, 2020

So I find this PR hard to review, as I cannot (readily) test it.

I hope you believe me if I say I have tested it enough 😄

Will the State Machine addin require any changes for this to work?

The StateMachine Add-In has some issues with RAPID motion tasks that doesn't control TCP robots, but they are minor problems.

Regardless, abb_libegm and this PR doesn't know anything about that, and this PR will work with any arbitrary EGM setup that control external axes in a seperate RAPID motion task.

What is the limit on the nr of tasks that could be added this way?

If I remember correctly, then EGM can only handle 4 simultaneous channels. And I think the limit of RAPID tasks is 20 (with the Multitasking option) but only a few can be motion tasks (with the MultiMove option).

@gavanderhoorn
Copy link
Member

So I find this PR hard to review, as I cannot (readily) test it.

I hope you believe me if I say I have tested it enough smile

Whether I believe you is irrelevant. The whole point of doing these reviews is to catch potential problems caused by using the code with setups you haven't/didn't consider(ed).

@jontje
Copy link
Contributor Author

jontje commented Mar 24, 2020

Whether I believe you is irrelevant. The whole point of doing these reviews is to catch potential problems caused by using the code with setups you haven't/didn't consider(ed).

Naturally.

However, as far as I know, I am probably the only one with any deeper understanding of the implementation, so it is of course difficult for anyone else to make anything expect a cursory review.

Copy link
Member

@gavanderhoorn gavanderhoorn left a comment

Choose a reason for hiding this comment

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

It would be good to at some (minimal) documentation on how to setup such a system (ie: with an external axis), but perhaps this should be done on the side of the add-in.

@gavanderhoorn gavanderhoorn merged commit c91601d into ros-industrial:master Mar 30, 2020
@gavanderhoorn
Copy link
Member

Thanks @jontje.

@jontje
Copy link
Contributor Author

jontje commented Apr 6, 2020

Thanks for the review @gavanderhoorn, much appreciated!

It would be good to at some (minimal) documentation on how to setup such a system (ie: with an external axis), but perhaps this should be done on the side of the add-in.

Agreed, and I think it would make most sense to include it in the issue for sample codes,

@jontje jontje deleted the add_external_axes_support branch April 6, 2020 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants