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 Liao's Bootcamp #103

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

Lucas Liao's Bootcamp #103

wants to merge 12 commits into from

Conversation

lucasliao0403
Copy link

No description provided.

@lucasliao0403 lucasliao0403 changed the title Lucas Liao Lucas Liao's Bootcamp Sep 20, 2024
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

self.command_index = 0
self.commands = [
commands.Command.create_set_relative_destination_command(
waypoint.location_x, waypoint.location_y
Copy link
Member

Choose a reason for hiding this comment

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

This is a relative destination command, so your own position matters. You may end up going somewhere you didn't expect.


command = self.commands[self.command_index]
self.command_index += 1
elif report.status == drone_status.DroneStatus.HALTED and not self.has_sent_landing_command:
Copy link
Member

Choose a reason for hiding this comment

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

Although this can work for the bootcamp, what happens if the drone halts somewhere while it was on its way? Try to find a way to make sure it gets there.

self.command_index = 0
self.commands = [
commands.Command.create_set_relative_destination_command(
waypoint.location_x, waypoint.location_y
Copy link
Member

Choose a reason for hiding this comment

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

Same, relative command

command = self.commands[self.command_index]
self.command_index += 1

elif report.status == drone_status.DroneStatus.HALTED and not self.waypoint_reached:
Copy link
Member

Choose a reason for hiding this comment

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

Again, try to find a different way to guarantee the drone gets to the waypoint

Comment on lines 112 to 118
self.commands.append(
commands.Command.create_set_relative_destination_command(
landing_pad_locations[index].location_x - report.position.location_x,
landing_pad_locations[index].location_y - report.position.location_y,
)
)
command = self.commands[self.command_index]
Copy link
Member

Choose a reason for hiding this comment

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

You could just set command to your desired command rather than adding it to a list and then reading it from there.

)
command = self.commands[self.command_index]

elif (
Copy link
Member

Choose a reason for hiding this comment

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

Again, please ensure drone has reached the landing pad


# Get the Result object
prediction = ...
prediction = self.__model.predict(source=image)
Copy link
Member

Choose a reason for hiding this comment

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

This was supposed to be the predictions variable, and then take prediction to be the 0th index. Also, please read the comments above for which kwargs to include


# Plot the annotated image from the Result object
# Include the confidence value
image_annotated = ...
image_annotated = prediction[0].orig_img
Copy link
Member

Choose a reason for hiding this comment

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

Please use the plot function, and include the confidence value, not just the image


# Detach the xyxy boxes to make a copy,
# move the copy into CPU space,
# and convert to a numpy array
boxes_cpu = ...

boxes_cpu = boxes_xyxy.xyxy.cpu().numpy()
Copy link
Member

Choose a reason for hiding this comment

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

You can put the xyxy in the previous variable. Here, you should also detach to make a copy before moving it to CPU

return [], image_annotated
for i in range(np.shape(boxes_cpu)[0]):
# Create BoundingBox object and append to list
temp_box = bounding_box.BoundingBox.create(np.array(boxes_cpu[i]))[1]
Copy link
Member

Choose a reason for hiding this comment

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

Please use the result variable rather than discarding it. It contains important information if the creation succeeded or not. You also don't need to convert a numpy array to a numpy array, you already did so in the last step.

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

@staticmethod
def is_close(x: float, y: float, target_x: float, target_y: float, tolerance: float) -> bool:
"""Determines if coordinate x,y, is within tolerence of coordinate target_x, target_y."""
return abs(x - target_x) < tolerance and abs(y - target_y) < tolerance
Copy link
Member

Choose a reason for hiding this comment

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

Please use euclidean distance instead of manhattan distance

self.waypoint.location_y - report.position.location_y,
)

elif (
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this else statement can ever be reached, because the first part is checking whether the drone is halted. This can only be reached if the drone is not halted, but then you check if the drone is halted again, which can never happen.

@staticmethod
def is_close(x: float, y: float, target_x: float, target_y: float, tolerance: float) -> bool:
"""Determines if coordinate x,y, is within tolerence of coordinate target_x, target_y."""
return abs(x - target_x) < tolerance and abs(y - target_y) < tolerance
Copy link
Member

Choose a reason for hiding this comment

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

Please use euclidean distance

# self.waypoint_reached = True
dist = float("inf")
index = -1
for i, landing_pad in enumerate(landing_pad_locations):
Copy link
Member

Choose a reason for hiding this comment

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

You do not need to continuously calculate which landing pad is closest to you as you are moving towards the landing pad (this is unnecessary as you are never going to get closer to a different landing pad since you are moving straight towards the "closest" one)

landing_pad_locations[index].location_x - report.position.location_x,
landing_pad_locations[index].location_y - report.position.location_y,
)
# print("HALTED AND WAYPOINT REACHED\n\n\n\n\n\n\n\n")
Copy link
Member

Choose a reason for hiding this comment

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

You do not need to keep commented code, since git will save history. You can just delete it. You also seem to have a lot of print statements that print every iteration of run, which will be a lot of messages really fast. You can probably delete those too if you don't need it, as it won't clutter your screen. (also production doesn't need debug prints)

prediction = ...
prediction = predictions[0]

# print('\n\nHEYYYYYYYYYYY\n\n')
Copy link
Member

Choose a reason for hiding this comment

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

Same, can just delete these commented print statements

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