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

Gurvir - WARG Autonomy Bootcamp #131

Closed
wants to merge 11 commits into from
Closed

Gurvir - WARG Autonomy Bootcamp #131

wants to merge 11 commits into from

Conversation

Gurvirr
Copy link

@Gurvirr Gurvirr commented Dec 20, 2024

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

Comment on lines +130 to +131
if result:
bounding_boxes.append(box)
Copy link
Member

Choose a reason for hiding this comment

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

Normally in statistics, if one part (one of the bounding boxes) of the data (the entire image) is invalid, we discard the entire data point.
(You do not need to change anything here, just something to think about)

if not self.has_sent_landing_command and report.status == drone_status.DroneStatus.HALTED:

# If the drone is halted, start the move command
command = self.commands
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 self.commands is defined anywhere


# If the drone is halted, start the move command
command = self.commands
self.has_sent_landing_command = True
Copy link
Member

Choose a reason for hiding this comment

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

Please rename this variable to has_sent_move_command, because your comments indicate that you sent a move command and not a landing command. It's very confusing that you're setting has_sent_landing_command to True

Comment on lines 48 to 50
self.move_command = commands.Command.create_set_relative_destination_command(
self.destination_x, self.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.

Not sure what this is for? It is never used. Also, this move command is based on relative distance, so you need to take into account your current position

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.

Conditionally approved!

command = move_command
self.has_sent_move_command = True

elif distance_squared < acceptance_radius_squared and self.has_sent_move_command is True:
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 fine for this bootcamp, but what happens if the drone halts unexpectedly midway in the flight? (Just something to think about, you do not need to change anything)

@maxlou05 maxlou05 closed this Dec 22, 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