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

Readme: re init readme #13

Closed
wants to merge 1 commit into from

Conversation

khancyr
Copy link
Collaborator

@khancyr khancyr commented Jan 21, 2022

remove old info
Add initial info

@srmainwaring
Copy link
Collaborator

srmainwaring commented Jan 22, 2022

Looks great thanks @khancyr, thanks for updating. I've added some feedback and suggested references.

So you can expect things to not be up-to-date.
This assume that your are using Ubuntu 16.04 or Ubuntu 18.04
Ubuntu able to run full 3D graphics: Native, VM, WSL, Docker.
Ignition Gazebo version 5.x (Ignition Edifice) or greater. See https://ignitionrobotics.org/docs/edifice/install_ubuntu

## Usage :
Copy link
Collaborator

Choose a reason for hiding this comment

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

This section could be 'Install' as it covers installing prerequisites and the plugin. The 'Usage' section could cover how to launch gazebo with one of the example models and configure SITL.

There is more information on the project wiki https://github.com/ArduPilot/ardupilot_gazebo/wiki that can be linked to from the Usage section.

</spherical_coordinates>
````
Rangefinder
See https://ignitionrobotics.org/docs/edifice/troubleshooting#ubuntu
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should add a license and credits section. The current license file is GPL3 which is consistent with the rest of ArduPilot. I'm in favour of that, but tridge and the other developers may have a view on what's best.

Regarding credits: the initial port to Ignition (non-JSON version) was carried out by the OSRF team and can be referenced here: https://ignitionrobotics.org/api/gazebo/6.4/ardupilot.html

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 have made the original plugin GPL, but I am pretty sure it cannot be GPL due to linkage issue with gazebo. Therefore maybe a LGPLv3 would fit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@khancyr I'm not sure about the fine details of when a library can / can't be GPL. Is the thinking that because the plugin links to Ignition that there is some transitive requirement that Ignition also be GPL (which it's not and won't be)?

It would surprise me if a downstream library (the plugin) could force an upstream licensing requirement (Ignition), but I really don't know the specifics - so will leave that one to the dev-team's lawyers - but I'd be interested to know the argument for / against what ever way it goes.

Update

I see what the issue is now - it's the issue of the using program (Ignition) and the plugin being regarded as a combined work. Another option might be to add the linking exception clause to the license:

https://en.wikipedia.org/wiki/GPL_linking_exception

Copy link
Collaborator

@srmainwaring srmainwaring left a comment

Choose a reason for hiding this comment

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

@khancyr we don't have an install section in the CMakeFile.txt yet, so we could exclude that from the instructions for the moment until the code's been updated. Otherwise LGTM.

In any case I expect this will be an ongoing work as we take on feedback from users.

@khancyr
Copy link
Collaborator Author

khancyr commented Feb 2, 2022

I have rework this again based on your wiki info.
I will update the prs tomorrow.
I prefer to use the readme for general user as the GitHub wiki is a bit hidden and won't be cloned.

@srmainwaring
Copy link
Collaborator

Superseded by #21

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.

None yet

2 participants