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 conceptual sketch, and filled out the readme file #14

Merged
merged 5 commits into from
Jun 12, 2018

Conversation

jontje
Copy link
Contributor

@jontje jontje commented Jun 11, 2018

As per title.

@jontje jontje requested a review from gavanderhoorn June 11, 2018 13:46
README.md Outdated
* Subscriptions (i.e. receiving notifications when resources are updated).
* Uploading/downloading/removing files.
* Checking controller state (e.g. motors on/off, auto/manual mode and RAPID execution running/stopped).
* And more.
Copy link
Member

Choose a reason for hiding this comment

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

This seems a bit vague: the first sentence in this section states "several of the RWS services, but not all of them". Then this item in the list says "and more".

Ideally we should be explicit about what is supported. Would it be possible to include an exhaustive list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that is true, I will clarify this.

README.md Outdated

### Recommendations

* The current version of this library is recommended to be used with **RobotWare 6.06.01**.
Copy link
Member

Choose a reason for hiding this comment

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

The RWS functionality doesn't suffer from the same incompatibilities as the EGM one, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it is just that some of the implemented methods will not work with earlier versions (e.g. since some of the RWS services don't exist there). Currently it is 6.06.01 that I have primarily verified against, so that is why I recommend it.

@gavanderhoorn
Copy link
Member

Could you also address #5 with this update to the readme?

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Jun 12, 2018

SVG renders strangely for me. Was there something wrong with the PNG?


Edit: looks like the bounding box of the svg is not correctly set (lots of unneeded empty space around the figure).

@jontje
Copy link
Contributor Author

jontje commented Jun 12, 2018

I think I have addressed all the current remarks. Regarding the sketch (you were too fast! :)), I felt that the png file was a bit pixelated so I tried with svg instead. Also, the drawing program I used auto removed the empty space for png but not for svg, which I noticed after I pushed the changes.

@gavanderhoorn gavanderhoorn merged commit 33804de into ros-industrial:master Jun 12, 2018
@gavanderhoorn
Copy link
Member

Nice update again @jontje 👍

@jontje jontje deleted the readme_and_sketch branch August 20, 2018 08:02
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