-
Notifications
You must be signed in to change notification settings - Fork 0
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
Integrate YOLO model #6
Conversation
sjay05
commented
Mar 24, 2024
- Added command line option to run script with YOLO or finding largest contour
- Uses 2023 CV model
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.
Reviewed. Let's change it to have 2 functions - contour and yolo and main calls the function depending on the argument
blue_only.py
Outdated
if largest_contour is not None: | ||
# Draw a rectangle around the largest circular contour | ||
x, y, w, h = cv2.boundingRect(largest_contour) |
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.
let's change this to early exiting like above
if largest_contour is None:
continue
blue_only.py
Outdated
if best_landing_pad is not None: | ||
x, y = best_landing_pad.x_1, best_landing_pad.y_1 | ||
w = best_landing_pad.x_2 - best_landing_pad.x_1 | ||
h = best_landing_pad.y_2 - best_landing_pad.y_1 | ||
found_coordindates = 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.
let's change this to early exiting like above
if largest_contour is None:
continue
blue_only.py
Outdated
h = best_landing_pad.y_2 - best_landing_pad.y_1 | ||
found_coordindates = True | ||
|
||
if found_coordindates: |
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.
remove this if condition (since we want to have early exiting above)
models/best-2n.pt
Outdated
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.
we try to not have models in the repositories (since their file size is really large) - let's add .pt files to the gitignore
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.
Removed.
yolo_decision.py
Outdated
label = int(boxes.cls[i]) | ||
confidence = float(boxes.conf[i]) | ||
result, detection = Detection.create(bounds, label, confidence) | ||
if result and detection: |
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.
remember to return nothing if the create method fails (similar to the bootcamp)
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.
So if we have one good detection and one failed one, is it better to return nothing and continue? My original thought was it would be okay for the drone to land at the best landing pad among the valid detections.
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.
For now, I have edited it to return nothing if it fails.
blue_only.py
Outdated
if args.method == "yolo": | ||
yolo_model = yolo_decision.DetectLandingPad("cpu", 0.5, "models/best-2n.pt") |
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.
move this to the section where you use the yolo model
blue_only.py
Outdated
if args.method == "yolo": | ||
yolo_model = yolo_decision.DetectLandingPad("cpu", 0.5, "models/best-2n.pt") |
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.
let's add these parameters to a config file (look at config.yaml in computer-vision-python for an example)
blue_only.py
Outdated
last_image_time = current_milli_time() | ||
# break | ||
found_coordindates = True | ||
else: |
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.
to be explicit let's make this
elif args.method == "yolo"
Updated.
|
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.
Mostly revert to the old architecture - we just want the step of determining the landing pad to be in a function, not the whole setup and loop and mavlink message sending.
blue_only.py
Outdated
if args.method == "contour": | ||
return run_with_contours() | ||
|
||
if args.method == "yolo": | ||
model_device = config["yolo_detect_target"]["device"] | ||
detect_confidence = config["yolo_detect_target"]["confidence"] | ||
model_path = config["yolo_detect_target"]["model_path"] | ||
|
||
return yolo_detect(model_device, detect_confidence, model_path) |
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.
sorry I didn't mean to copy the entire script into the functions - we want to have the while loop in main, send the mavlink in main, etc. but just get the x, y, w, h from the contour or yolo function depending on the argparse. I was thinking more like - the line numbers are from the main branch and this isn't exact but like architecturally this is what I was thinking
def detect_landing_pads_contour():
# basically copy paste lines 148-173
return x, y, w, h
def detect_landing_pads_yolo():
# implement the class and functions in yolo_detection.py to get a landing pad
x, y, w, h = ... # interpret the results
return x, y, w, h
def main():
# setup stuff and argparse
while True:
# lines 87 - 148
if args.method == "contour":
x, y, w, h = detect_landing_pads_contour()
if args.method == "yolo":
x, y, w, h = detect_landing_pads_yolo()
if x, y, w, h is None: # not exactly this but basically check that we have a bounding box
return
# lines 174 onward
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.
Reviewed
blue_only.py
Outdated
return message_id == pymavlink.mavutil.mavlink.MAVLINK_MSG_ID_RADIO_STATUS | ||
|
||
|
||
def detect_landing_pads_contour(image: np.ndarray) -> "None | tuple": |
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.
let's change the function to return False, None
and True, output
blue_only.py
Outdated
return tuple(cv2.boundingRect(largest_contour)) | ||
|
||
|
||
def detect_landing_pads_yolo(image: np.ndarray, config: dict) -> "None | tuple": |
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.
same as above, let's change the function to return False, None
and True, output
blue_only.py
Outdated
if args.method == "contour": | ||
bounding_box = detect_landing_pads_contour(image) | ||
elif args.method == "yolo": | ||
bounding_box = detect_landing_pads_yolo(image, config) | ||
|
||
if bounding_box is not None: |
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.
from above, let's do result, bounding_box = detect(image)
and check the result instead of the bounding box to see if it's valid
blue_only.py
Outdated
if args.method == "contour": | ||
bounding_box = detect_landing_pads_contour(image) | ||
elif args.method == "yolo": | ||
bounding_box = detect_landing_pads_yolo(image, config) | ||
|
||
if bounding_box is not None: |
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.
let's do early exiting here - instead of if result
, let's do
if not result:
continue
blue_only.py
Outdated
loop_counter = 0 | ||
last_time = current_milli_time() | ||
if current_milli_time() - last_image_time > 100: | ||
if is_pad_detected: |
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.
same as above, instead of checking this here, have it exit early so we don't have to use a variable here
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