-
Notifications
You must be signed in to change notification settings - Fork 249
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
Alex Liu - Autonomy Bootcamp 2023 #105
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good start! I left some comments on your code, feel free to ask question on you thread if you have any.
|
||
# Loop over the boxes list and create a list of bounding boxes | ||
bounding_boxes = [] | ||
# BC NOTE: Need for error checking from returned tuple from BoundingBox.create() | ||
bounding_boxes = [bounding_box.BoundingBox.create(rectangle)[1] for rectangle in boxes_cpu] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the creation of a BoundingBox fails, you would want to return an empty list. Doing that with list comprehension will like kinda clunky so I recommend you refactor this into a for loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to find the method definition of BoundingBox.create
, it returns a tuple of a bool
and a BoundingBox
. If the returned boolean is false that means the creation of the boxes failed. You don't have to check for the conditions for failure manually that's done for in the BoundingBox.create
method, you will have to implement returning an empty list of any of the calls to the create
method fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha I was wondering what that creation flag was used for
@@ -70,6 +74,12 @@ def run( | |||
|
|||
# Do something based on the report and the state of this class... | |||
|
|||
if report.status == drone_status.DroneStatus.HALTED: | |||
if not self.should_land: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function runs in an infinite loop, the drone will land at the second iteration of the loop. What you want to do is to navigate to the waypoint until you are within the acceptance radius, then create the land command. (Returning the land command will exit out of the loop)
if not landing_pad_locations: | ||
return commands.Command.create_null_command() | ||
sorted_locations = landing_pad_locations.copy() | ||
sorted_locations.sort( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a decent solution but it would be faster to use min instead (min in python also has a key attr). This will bring this part down from O(nlogn) to O(n).
) | ||
) | ||
closest_pad: location.Location = sorted_locations[0] | ||
rel_x = closest_pad.location_x - reference_location.location_x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try to avoid abbreviating variable names
command = commands.Command.create_land_command() | ||
elif self.finished_goal: | ||
command = self.create_close_pad_goal(report.position, landing_pad_locations) | ||
self.should_land = True # should land on the next instruction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set this once you reach the acceptance radius
self.should_land = True # should land on the next instruction | ||
else: | ||
command = self.goal | ||
self.finished_goal = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set this once you reach the acceptance radius
Improved Task 1, 3, and 4 based on feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was leaving comments but I think I confused you, mb 😅. You only want to send command to the drone when it is halted, when it is halted, you check if it is within the acceptance radius, if yes then land if no then set on course to the waypoint again.
command = self.goals.pop(0) | ||
else: | ||
command = commands.Command.create_land_command() | ||
case drone_status.DroneStatus.MOVING: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't have to do anything while the Drone is moving, we just let it finish it's last command. The loop is for making fine adjustments when on top of the landing pad.
) | ||
else: | ||
command = commands.Command.create_land_command() | ||
case drone_status.DroneStatus.MOVING: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't have to do anything while the Drone is moving, we just let it finish it's last command. The loop is for making fine adjustments when on top of the landing pad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just made another commit, I'm unsure what you meant by making fine adjustments (perhaps I am supposed to reroute to the destination if position != destination)? I've left it for now just to resolve the other issues first.
command = commands.Command.create_land_command() | ||
case drone_status.DroneStatus.MOVING: | ||
# if the current position is close enough to the destination. | ||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check this when the Drone is halted.
bounding_boxes = [] | ||
for rectangle in boxes_cpu: | ||
optional_bounding_box = bounding_box.BoundingBox.create(rectangle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deconstruct the tuple result, box = bounding_box.BoundingBox.create(rectangle)
optional_bounding_box = bounding_box.BoundingBox.create(rectangle) | ||
# why is it that this one returns False while other functions might return None for a failure? | ||
# still unsure why .shape() is required | ||
if optional_bounding_box[0]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for failure and return an empty array if that is the case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're almost there! You just need to sort out some stuff with the loop logic. I you don't understand what I mean you can shoot me a msg on Discord or find me at the weekly auto meeting.
@@ -70,6 +87,17 @@ def run( | |||
|
|||
# Do something based on the report and the state of this class... | |||
|
|||
if report.status == drone_status.DroneStatus.HALTED: | |||
if self.goals: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see why you have a list of "goals". You can simply check if the drone is within the acceptance radius, land if true, set relative destination if false.
@@ -70,6 +119,31 @@ def run( | |||
|
|||
# Do something based on the report and the state of this class... | |||
|
|||
# only execute when "drone" is ready for another instruction | |||
if report.status == drone_status.DroneStatus.HALTED: | |||
if self.goals: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see why you have a list of "goals". You can simply check if the drone is within the acceptance radius, land if true, set relative destination if false.
else: | ||
# return an empty list if any of them fail??? | ||
bounding_boxes.clear() # clear the boxes if any were made before the failed one | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This solution works, but consider that
if not result:
return [], image_annotated
achieves the same result with less code (and slightly more performance since you don't have to clear bounding_boxes
). (You do not need to change anything here, just something to think about).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mhm, this one was a bit of a 50/50 choice for me. I was thinking "if the flow goes to the return statement anyways, I'll just make sure the return tuple is correct then return". HOWEVER, I like the idea of better efficiency :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super close, you should be able to finish this time around!
command = commands.Command.create_set_relative_destination_command( | ||
self.waypoint.location_x, self.waypoint.location_y | ||
) | ||
self.landing = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're super close, you have to set this to true only when the drone is within the acceptance radius, you code will look something like
if drone is halted:
if drone is within acceptance radius:
land the drone
else
move the drone
@@ -70,6 +113,34 @@ def run( | |||
|
|||
# Do something based on the report and the state of this class... | |||
|
|||
# only execute when "drone" is ready for another instruction | |||
if report.status == drone_status.DroneStatus.HALTED: | |||
if not self.finding_landing_pad: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to task 3, super close just some minor changes in the conditionals:
if drone is halted:
if drone is looking for waypoint:
if drone is within acceptance radius:
get nearest landing pad and tell the drone that it should look for it next
else:
move the drone
else if drone is looking for landing pad:
if drone is within acceptance radius:
land the drone
else:
move the drone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just need to improve your variable names a bit and you should be done with the bootcamp!
|
||
@staticmethod | ||
def calculate_distance_squared( | ||
location_1: location.Location, to_x: float, to_y: float |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use more descriptive variable names
|
||
@staticmethod | ||
def get_relative_position( | ||
location_1: location.Location, location_2: location.Location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use more descriptive variable names
# ============ | ||
# ↑ BOOTCAMPERS MODIFY ABOVE THIS COMMENT ↑ | ||
# ============ | ||
|
||
@staticmethod | ||
def calculate_distance_squared(location_1: location.Location, to_x: float, to_y) -> float: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use more descriptive variable names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm.
Some tasks might've been misunderstood