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

Please review my PR and suggest the required changes #1

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

abyasingh
Copy link

No description provided.

Copy link
Member

@Nageshbansal Nageshbansal left a comment

Choose a reason for hiding this comment

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

images should be in images and videos/images directory.

Copy link
Member

@Nageshbansal Nageshbansal left a comment

Choose a reason for hiding this comment

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

directories names should be according to guidlines

README.md Outdated
A simulation of a basic drone model take-off up to a given altitude in a gazebo environment; with a PX4 autopilot supported by Mavros in Ubuntu.

![Final Product](https://github.com/abyasingh/Drone-Simulation/blob/main/Images%20and%20Videos/Simultion%20of%20drone.png)
Copy link
Member

Choose a reason for hiding this comment

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

images should be enclosed in <p></p> tags, so that they can be aligned in the center i.e align="center

README.md Outdated
<p align="justify">Finally, we needed a script for takeoff of our drone. A launch file was also needed. We create a python script in VS Code for this execution.</p>

![Takeoff_P1](https://github.com/abyasingh/Drone-Simulation/blob/main/Images%20and%20Videos/Take-off%20Scrpit%20P1.png)
Copy link
Member

Choose a reason for hiding this comment

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

instead of adding screenshots of code, you can use code-blocks

Copy link
Member

Choose a reason for hiding this comment

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

By code-blocks I meant, adding a snippet of the code in the markdown/Readme file. for e.g.

def test():
   return None

README.md Outdated

1. [Nagesh Bansal](https://github.com/Nageshbansal)
2. Harshini S.
Copy link
Member

Choose a reason for hiding this comment

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

add links to GitHub profiles for all of the mentors

README.md Show resolved Hide resolved
pass

def setTakeoff(self):
print('waiting for service')
Copy link
Member

Choose a reason for hiding this comment

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

add comments in code for each of the method/functions and classes

@abyasingh
Copy link
Author

Sir I've made the changes, please review.

Copy link
Member

@Nageshbansal Nageshbansal left a comment

Choose a reason for hiding this comment

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

change the filename of video

README.md Outdated
A simulation of a basic drone model take-off up to a given altitude in a gazebo environment; with a PX4 autopilot supported by Mavros in Ubuntu.

![Final Product](https://github.com/abyasingh/Drone-Simulation/blob/main/Images%20and%20Videos/Images/Simultion%20of%20drone.png)
Copy link
Member

Choose a reason for hiding this comment

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

images should be enclosed in the <p> tags, so that they can be aligned in the center

README.md Outdated
<p align="justify">Finally, we needed a script for takeoff of our drone. A launch file was also needed. We create a python script in VS Code for this execution.

![Takeoff_P1](https://github.com/abyasingh/Drone-Simulation/blob/main/Images%20and%20Videos/Images/Take-off%20Scrpit%20P1.png)
Copy link
Member

Choose a reason for hiding this comment

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

instead of adding screenshots of the code, you can add snippets or just add the URL to the script.

@abyasingh
Copy link
Author

Sir I have done all the changes recommended by you.

Copy link
Member

@Nageshbansal Nageshbansal left a comment

Choose a reason for hiding this comment

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

  1. remove the screenshots of the code from the images and videos directory
  2. add report in the Poster and Report directory
  3. Instead of adding link to the script, you can add a snippet of the code in your markdown, you can follow this tutorial for your reference https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-and-highlighting-code-blocks

@abyasingh
Copy link
Author

Yes Sir, I've done all the changes. Please review.

Copy link
Member

@Nageshbansal Nageshbansal left a comment

Choose a reason for hiding this comment

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

report should be in .pdf format

@abyasingh
Copy link
Author

Sir I've converted the report in pdf form.

@Nageshbansal
Copy link
Member

Are you done with all of the mentioned changes?

@abyasingh
Copy link
Author

Yes Sir

@SanjeevKrishnan
Copy link
Member

Yes Sir

@Nageshbansal Follow this up and tag me once the repo is ready for merge.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@abyasingh
Copy link
Author

SIr, I've done the changes

Copy link
Member

@Nageshbansal Nageshbansal left a comment

Choose a reason for hiding this comment

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

LGTM, Good work.
@SanjeevKrishnan, I think we're done with changes, let me know if we can merge this PR.

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.

3 participants