-
Notifications
You must be signed in to change notification settings - Fork 37
Autonomous Landing Module #235
Conversation
maxlou05
left a comment
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
modules/auto_landing/auto_landing.py
Outdated
| self.__logger = local_logger | ||
|
|
||
| def run( | ||
| self, bounding_box: detections_and_time.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.
I think you'll be getting a MergedOdometryDetections object, which will have a list of Detection objects
modules/auto_landing/auto_landing.py
Outdated
| @@ -0,0 +1,100 @@ | |||
| """ | |||
| Auto-landing script. | |||
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.
Add calculates the parameters necessary for use with LANDING_TARGET MAVLink command
modules/auto_landing/auto_landing.py
Outdated
|
|
||
| class AutoLanding: | ||
| """ | ||
| Auto-landing script. |
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.
Copy new file docstring here
modules/auto_landing/auto_landing.py
Outdated
|
|
||
| import math | ||
|
|
||
| from pymavlink import mavutil |
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'm pretty sure you won't need this library
modules/auto_landing/auto_landing.py
Outdated
| vehicle = mavutil.mavlink_connection("tcp:localhost:14550") | ||
| try: | ||
| height_agl_mm = vehicle.messages[ | ||
| "GLOBAL_POSITION_INT" | ||
| ].relative_alt # copied from blue_only.py | ||
| height_agl = max(height_agl_mm / 1000, 0.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.
If we need altitude information, you cannot create another connection to the drone since FlightInterface already has one. You need to get the merged_odometry_detections.MergedOdometryDetections object from data merge worker. The altitude is agl which is the same as local altitude I think, so you can directly use that
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.
Also, this portion should also go into the run() function, as altitude will change as the drone lands, and is not constant
modules/auto_landing/auto_landing.py
Outdated
| ground_hyp = (x_dist**2 + y_dist**2) ** 0.5 | ||
| self.__logger.info(f"Required horizontal correction (m): {ground_hyp}", True) | ||
| target_to_vehicle_dist = (ground_hyp**2 + self.height_agl**2) ** 0.5 | ||
| self.__logger.info(f"Distance from vehicle to target (m): {target_to_vehicle_dist}", 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.
I think that instead of making 4 separate lines in the log, you can combine all of these into 1 line, so the timestamps look better (these calculations should be fast anyways)
modules/auto_landing/auto_landing.py
Outdated
|
|
||
| @classmethod | ||
| def create( | ||
| cls, angle_x: float, angle_y: float, target_dist: 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.
This method can be removed:
you dont really need the create method, just move the doc string inside the constructor.
just imagine this as a struct like in C++ to keeps things nice rather than an actual class
modules/auto_landing/auto_landing.py
Outdated
|
|
||
| def run( | ||
| self, odometry_detections: merged_odometry_detections.MergedOdometryDetections | ||
| ) -> "AutoLandingInformation": |
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.
Change this to accurately get the output:
tuple[bool, AutoLandingInformation]
modules/auto_landing/auto_landing.py
Outdated
|
|
||
| class AutoLandingInformation: | ||
| """ | ||
| Information necessary for the LANDING_TARGET MAVlink command. |
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 into the init docstring
modules/auto_landing/auto_landing.py
Outdated
| return True, AutoLandingInformation(angle_x, angle_y, target_to_vehicle_dist) | ||
|
|
||
|
|
||
| class AutoLandingInformation: |
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.
Reorder this so it is the first class in the file
modules/auto_landing/auto_landing.py
Outdated
|
|
||
| def __init__(self, angle_x: float, angle_y: float, target_dist: float) -> None: | ||
| """ | ||
| angle_x: Angle of the x coordinate of the bounding box (rads). |
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.
Add specifications to this, what angles are allowed?
maxlou05
left a comment
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
| angle_x = (x_center - self.im_w / 2) * (self.fov_x * (math.pi / 180)) / self.im_w | ||
| angle_y = (y_center - self.im_h / 2) * (self.fov_y * (math.pi / 180)) / self.im_h | ||
|
|
||
| height_agl = odometry_detections.odometry_local.position.down * -1 |
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.
Maybe a comment here that says height above ground level (AGL), and that down is how many meters down you are from home position, which is on the ground. Hence the * -1
| Returns an AutoLandingInformation object. | ||
| """ | ||
|
|
||
| x_center, y_center = odometry_detections.detections[0].get_centre() |
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.
Add comment with TODO: come up with a better algorithm to pick which detection to land at, if many are detected in one frame.
modules/auto_landing/auto_landing.py
Outdated
| from ..common.modules.logger import logger | ||
| from .. import merged_odometry_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.
Can you put this in alphabetical order? So .. goes before ..common
| from utilities.workers import queue_proxy_wrapper | ||
| from utilities.workers import worker_controller | ||
| from . import auto_landing | ||
| from ..common.modules.logger import logger |
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.
Can you put this in alphabetical order? So just put the last two on top
modules/auto_landing/auto_landing.py
Outdated
| True, | ||
| ) | ||
|
|
||
| time.sleep(self.period) |
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.
Can we move this to the worker and not the class' run() function? So remove the period from the class __init__ and create as well
| local_logger.info("Logger initialized", True) | ||
|
|
||
| result, auto_lander = auto_landing.AutoLanding.create( | ||
| fov_x, fov_y, im_h, im_w, period, local_logger |
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.
See comment about period
| if not result: | ||
| continue | ||
|
|
||
| output_queue.queue.put(value) |
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.
Place wait(period) down here, after this
maxlou05
left a comment
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.
Approved!
* intial commit * added files * removed old module * made class * made changes * added worker * made changes * issues commiting, now fixed * made changes * linter issues fixed * fixing linter issues * fixing code and linter issues * continuation of debugging * fixing linter issues * fixing logger initialization * fixing last linter issues * last few linter issues * last few changes * updated changes * fixing small linter issue * fixed run() * pulling new commit changes in common * fixing linter error * using first bbox * added AutoLandingInformation object * fixing small mistakes * redefined AutoLandingInformation class * added changes * fixed linter issues
Created the module for Autonomous Landing and making progress on creating the autonomous landing worker as well.