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

Lucas Kim's bootcamp #102

Closed
wants to merge 14 commits into from
Closed

Lucas Kim's bootcamp #102

wants to merge 14 commits into from

Conversation

Raptors65
Copy link

No description provided.

Copy link
Member

@maxlou05 maxlou05 left a comment

Choose a reason for hiding this comment

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

Reviewed

relative_y = self.waypoint.location_y - report.position.location_y

# Check if the waypoint has already been reached
if abs(relative_x) < allowed_error and abs(relative_y) < allowed_error:
Copy link
Member

Choose a reason for hiding this comment

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

Normally, we use Euclidean distance (L2 norm) rather than taxicab/manhattan distance (L1 norm). Try to use that instead

@@ -4,6 +4,8 @@
Travel to designated waypoint and then land at a nearby landing pad.
"""

import math
Copy link
Member

Choose a reason for hiding this comment

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

In this bootcamp, you are not allowed to import any libraries. The bootcamp can be completed without importing math or using square roots. You can think about what is your purpose for calculating distance

# Create BoundingBox object and append to list
_, box = bounding_box.BoundingBox.create(xyxy_box)

if box is None:
Copy link
Member

Choose a reason for hiding this comment

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

We have a "success" indicator returned from the create function, you can use that instead. It's more readable and can handle more cases than box does not exist. Maybe the box exists but is malformed.

# Do something based on the report and the state of this class...
if report.status == drone_status.DroneStatus.HALTED:
# Check if the waypoint has already been reached
if self.started_moving_to_landing_pad:
Copy link
Member

Choose a reason for hiding this comment

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

Although in this bootcamp, you are guarantee to make it to your destination, sometimes due to wind you end up somewhere else. Please use the acceptance_radius just like you used the "error" from the other task.

# Check if the waypoint has already been reached
if self.started_moving_to_landing_pad:
command = commands.Command.create_land_command()
elif self.started_moving_to_waypoint:
Copy link
Member

Choose a reason for hiding this comment

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

Same here, please confirm that you are indeed within the acceptance radius of the waypoint

Copy link
Member

@maxlou05 maxlou05 left a comment

Choose a reason for hiding this comment

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

Reviewed


# Check if the landing pad or waypoint has already been reached
if (
self.__squared_distance(report.position, closest_landing_pad) < allowed_error
Copy link
Member

Choose a reason for hiding this comment

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

For this, the function run() has a parameter acceptance_radius which you should use. The "error" should thus be squared for this case.

and self.has_reached_waypoint
):
command = commands.Command.create_land_command()
elif self.__squared_distance(report.position, self.waypoint) < allowed_error:
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Comment on lines 73 to 76
closest_landing_pad = min(
landing_pad_locations,
key=lambda pad: self.__squared_distance(report.position, pad),
)
Copy link
Member

Choose a reason for hiding this comment

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

I think you moved it outside of the conditions. It should probably be after line 84, when you have reached the waypoint.

# Create BoundingBox object and append to list
success, box = bounding_box.BoundingBox.create(xyxy_box)

if not success:
Copy link
Member

Choose a reason for hiding this comment

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

In statistics, we normally discard the entire data point (the whole image) if part of it has gone wrong (a bounding box).
(You do not need to change anything, just something to think about)

Copy link
Member

@maxlou05 maxlou05 left a comment

Choose a reason for hiding this comment

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

Approved

@maxlou05 maxlou05 closed this Sep 21, 2024
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