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 section for usage hints #27

Merged
merged 19 commits into from
Dec 10, 2018
Merged

Conversation

jontje
Copy link
Contributor

@jontje jontje commented Nov 28, 2018

  • Added brief descriptions of the library's primary classes.
  • Added references to an optional RobotWare Add-In called StateMachine Add-In.

@jontje jontje requested a review from gavanderhoorn November 28, 2018 15:58
@jontje
Copy link
Contributor Author

jontje commented Nov 29, 2018

I forgot to mention that this PR is related to issue #17

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.

Looks ok. I've added some minor wording suggestions.


![EGM sketch](docs/images/egm_sketch.svg)
![EGM sketch](docs/images/egm_sketch.png)
Copy link
Member

Choose a reason for hiding this comment

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

Did you have a specific reason for converting this to PNG?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The .svg looked good in Chrome, Edge and Explorer but then I used Firefox and it was not showing the greyed out sections correctly.

@@ -41,11 +42,43 @@ This library is intended to be used with the UDP variant of EGM, and it supports

### Recommendations

* This library has been verified to work with RobotWare `6.06.01`. Other version are expected to work, but this cannot be guaranteed at the moment.
* This library has been verified to work with RobotWare `6.06.01`. Other versions are expected to work, but this cannot be guaranteed at the moment.
Copy link
Member

Choose a reason for hiding this comment

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

I know we already do in some places, but it might be good to repeat the "don't use 6.07 with this library"-warning here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add the warning


### [Optional] StateMachine Add-In

The purpose of the RobotWare Add-In is to *ease the setup* of ABB robot controllers. It is made for both *real physical controllers* and *virtual controllers* simulated in RobotStudio. If the Add-In is selected during a RobotWare system installation, then the Add-In will load several RAPID modules and system configurations based on the system specifications (e.g. number of robots and present options).
Copy link
Member

Choose a reason for hiding this comment

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

And then what? What does it actually make easier? From this description it sounds like it "just" installs some files.

I seem to remember it actually also configures things for you. Perhaps clarify this a bit by adding a sentence that better describes what sort of work the add-in does which the user doesn't need to do any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, I will clarify that it makes up a ready-to-run RAPID program, which is also customizable. The intention is to use the Add-In together with external system that want to interact with the robot in some way. For example, the external system could contain all the decision making and it wants to use the robot as a “worker resource”.

It takes about 10-20 minutes to install a new robot system, and if the Add-In is included, then it should just be to connect the external system to the robot.

@jontje
Copy link
Contributor Author

jontje commented Dec 7, 2018

@gavanderhoorn, thanks for the review, and I think I have addressed all your comments now.

@gavanderhoorn
Copy link
Member

Looks ok like this. Thanks for iterating 👍

Feel free to merge.

@jontje jontje merged commit 9ae271c into ros-industrial:master Dec 10, 2018
@jontje jontje deleted the readme_update branch December 10, 2018 09:27
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