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

Bootcamp Complete?- Muhammad Awan #19

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

Conversation

muhammad-awan0
Copy link

No description provided.

Copy link
Contributor

@TongguangZhang TongguangZhang left a comment

Choose a reason for hiding this comment

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

Great work Muhammad! Your logic is sound, mostly just some style issues to be fixed.

@@ -8,7 +8,6 @@
import numpy as np
import torch
import ultralytics

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: One empty line between importing installed modules and importing local modules (remember to not modify anything outside of the area indicated by the comments).

modules/bootcamp/detect_landing_pad.py Outdated Show resolved Hide resolved
modules/bootcamp/detect_landing_pad.py Outdated Show resolved Hide resolved

# Plot the annotated image from the Result object
# Include the confidence value
image_annotated = ...
image_annotated = prediction.plot(conf=0.7)
Copy link
Contributor

Choose a reason for hiding this comment

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

You've already set the confidence above - how do you tell it to use the confidence here?

modules/bootcamp/detect_landing_pad.py Outdated Show resolved Hide resolved
Comment on lines -76 to -78
# ============
# ↑ BOOTCAMPERS MODIFY ABOVE THIS COMMENT ↑
# ============
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure you're not deleting the comment guards, place your code inside of them. You're allowed to write helper functions outside of the comments.

modules/bootcamp/decision_simple_waypoint.py Outdated Show resolved Hide resolved
if report.status == drone_status.DroneStatus.HALTED and not self.waypoint_found:
command = commands.Command.create_set_relative_destination_command(
self.waypoint.location_x,
self.waypoint.location_y
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Add a comma after each argument, including the last argument.

modules/bootcamp/decision_simple_waypoint.py Outdated Show resolved Hide resolved
Comment on lines +97 to +98
elif report.status == drone_status.DroneStatus.HALTED and self.landing_pad_found:
command = commands.Command.create_land_command()
Copy link
Contributor

Choose a reason for hiding this comment

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

While the bootcamp specifies that the drone will always complete its movement, what if this wasn't guaranteed and it could halt on its own at any random time? (You do not need to change anything here, just something to think about).

Comment on lines +90 to +91
elif not within_range and status == drone_status.DroneStatus.HALTED:
command = commands.Command.create_set_relative_destination_command(way_x, way_y)
Copy link
Contributor

Choose a reason for hiding this comment

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

Change this one to also be relative distance, think about the cases where the drone doesn't start simulation at (0, 0).

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.

2 participants