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

Decision module #163

Merged
merged 24 commits into from
Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions modules/decision/decision.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
from .. import decision_command
TongguangZhang marked this conversation as resolved.
Show resolved Hide resolved
from .. import object_in_world
from .. import odometry_and_time


class Decision:
def __init__(self, tolerance: float):
self.__best_landing_pad = None
self.__weighted_pads = []
self.__distance_tolerance = tolerance

@staticmethod
def distance_to_pad(
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
pad: object_in_world.ObjectInWorld,
current_position: odometry_and_time.OdometryAndTime,
):
Copy link
Contributor

Choose a reason for hiding this comment

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

format functions declarations like this

def func(arg1 :int,
               arg2 :string) -> float:
    pass

"""
Calculate Euclidean distance to landing pad based on current position.
"""
dx = pad.position_x - current_position.odometry_data.position.north
dy = pad.position_y - current_position.odometry_data.position.east
return (dx**2 + dy**2) ** 0.5
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved

def weight_pads(
Copy link
Contributor

Choose a reason for hiding this comment

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

make this a private function

self,
pads: "list[object_in_world.ObjectInWorld]",
current_position: odometry_and_time.OdometryAndTime,
):
"""
Weights the pads based on normalized variance and distance.
"""
if not pads:
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
return None

distances = [self.distance_to_pad(pad, current_position) for pad in pads]
variances = [pad.spherical_variance for pad in pads]

max_distance = (
max(distances) or 1
) # Avoid division by zero if all distances are zero
max_variance = (
max(variances) or 1
) # Avoid division by zero if all variances are zero
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably don't want to set the max distance or variance to 1 if they're all 0. 1 is the maximum possible variance, but not the maximum possible distance (or the minimum possible distance), so this could cause a logic issue.
If all variances are 0, we should return no landing pads, if all distances are 0, that means we're on top of a landing pad and should land.


self.__weighted_pads = [
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
(pad, distance / max_distance + variance / max_variance)
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
for pad, distance, variance in zip(pads, distances, variances)
]

def __find_best_pad(self):
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
"""
Determine the best pad to land on based on the weighted scores.
"""
if not self.__weighted_pads:
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
return None
# Find the pad with the smallest weight as the best pad
self.__best_landing_pad = min(self.__weighted_pads, key=lambda x: x[1])[0]
return self.__best_landing_pad

def run(
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
self,
states: odometry_and_time.OdometryAndTime,
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the naming to be more descriptive, it's the current state (not multiple states), so maybe current_state?

pads: "list[object_in_world.ObjectInWorld]",
):
"""
Determine the best landing pad and issue a command to land there.
"""
self.weight_pads(pads, states)
best_pad = self.__find_best_pad()
if best_pad:
distance_to_best_bad = self.distance_to_pad(best_pad, states)
if distance_to_best_bad <= self.__distance_tolerance:
# Issue a landing command if within tolerance
Copy link
Contributor

Choose a reason for hiding this comment

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

Put the comment before the if statement

return (
True,
decision_command.DecisionCommand.create_land_at_absolute_position_command(
best_pad.position_x,
best_pad.position_y,
states.odometry_data.position.down,
),
)
else:
# Move to best location if not within tolerance
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

return (
True,
decision_command.DecisionCommand.create_move_to_absolute_position_command(
best_pad.position_x,
best_pad.position_y,
-states.odometry_data.position.down, # Assuming down is negative for landing
),
)
else:
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
# Default to hover if no pad is found
Copy link
Contributor

Choose a reason for hiding this comment

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

We're not returning a hover command, so change the comment to say something more like "Default to do nothing if no pad is found"

return False, None
Copy link
Contributor

Choose a reason for hiding this comment

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

single empty line at end of file

132 changes: 132 additions & 0 deletions tests/test_decision.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import pytest
from modules.decision import decision
Copy link
Contributor

Choose a reason for hiding this comment

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

2 lines between system module imports and local module imports. Also add a docstring to the file

from modules import decision_command
from modules import object_in_world
from modules import odometry_and_time
from modules import drone_odometry_local

# Test parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

2 lines between imports and constant declarations (put the comment on the same line as the constant)

TOLERANCE = 2


@pytest.fixture()
def decision_maker():
"""
Construct a Decision instance with predefined tolerance.
"""
decision_instance = decision.Decision(TOLERANCE)
yield decision_instance


# Fixture for a pad within tolerance
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
@pytest.fixture()
def best_pad_within_tolerance():
"""
Create a mock ObjectInWorld instance within tolerance.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove mention of mock.

What does "within tolerance" mean?

"""
position_x = 10.0
position_y = 20.0
spherical_variance = 1.0
success, pad = object_in_world.ObjectInWorld.create(
position_x, position_y, spherical_variance
)
assert success
return pad
Copy link
Contributor

Choose a reason for hiding this comment

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

use yield in a pytest fixture



# Fixture for a pad outside tolerance
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
@pytest.fixture()
def best_pad_outside_tolerance():
"""
Create a mock ObjectInWorld instance outside tolerance.
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
"""
position_x = 100.0
position_y = 200.0
spherical_variance = 5.0 # variance outside tolerance
success, pad = object_in_world.ObjectInWorld.create(
position_x, position_y, spherical_variance
)
assert success
return pad
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above



# Fixture for a list of pads
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
@pytest.fixture()
def pads():
"""
Create a list of mock ObjectInWorld instances.
"""
pad1 = object_in_world.ObjectInWorld.create(30.0, 40.0, 2.0)[1]
pad2 = object_in_world.ObjectInWorld.create(50.0, 60.0, 3.0)[1]
pad3 = object_in_world.ObjectInWorld.create(70.0, 80.0, 4.0)[1]
return [pad1, pad2, pad3]
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above



# Fixture for odometry and time states
Xierumeng marked this conversation as resolved.
Show resolved Hide resolved
@pytest.fixture()
def states():
"""
Create a mock OdometryAndTime instance with the drone positioned within tolerance of the landing pad.
"""
# Creating the position within tolerance of the specified landing pad.
position = drone_odometry_local.DronePositionLocal.create(9.0, 19.0, -5.0)[
Copy link
Collaborator

Choose a reason for hiding this comment

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

The construction pattern used for testing is as follows:

result, name_of_data = namespace.ClassName.create(...)
assert result
assert name_of_data is not None

# Use name_of_data somewhere...

1
] # Example altitude of -5 meters
Copy link
Contributor

Choose a reason for hiding this comment

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

if this fits on the line you don't need to make it multiline


orientation = drone_odometry_local.DroneOrientationLocal.create_new(0.0, 0.0, 0.0)[
1
]
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above


odometry_data = drone_odometry_local.DroneOdometryLocal.create(
position, orientation
)[1]

# Creating the OdometryAndTime instance with current time stamp
success, state = odometry_and_time.OdometryAndTime.create(odometry_data)
assert success
return state
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above



class TestDecision:
"""
Tests for the Decision.run() method.
"""

def test_decision_within_tolerance(
self, decision_maker, best_pad_within_tolerance, pads, states
):
Copy link
Contributor

Choose a reason for hiding this comment

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

same as function formatting comment above, but you don't need a type annotation for the return type

"""
Test decision making when the best pad is within tolerance.
"""
total_pads = [best_pad_within_tolerance] + pads
res, command = decision_maker.run(states, total_pads)

assert res
assert (
command.get_command_type()
== decision_command.DecisionCommand.CommandType.LAND_AT_ABSOLUTE_POSITION
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Place right side values into variables. Format of all tests should be:

Setup: Sets up objects to be tested and expected values (and assert the result)
Run: Run the method/function and get the actual value.
Test: Assert actual == expected.

Have spaces between each of the 3 sections, name the variables actual and expected.


def test_decision_outside_tolerance(
self, decision_maker, best_pad_outside_tolerance, pads, states
):
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

"""
Test decision making when the best pad is outside tolerance.
"""
total_pads = [best_pad_outside_tolerance] + pads
res, command = decision_maker.run(states, total_pads)

assert res
assert (
command.get_command_type()
== decision_command.DecisionCommand.CommandType.MOVE_TO_ABSOLUTE_POSITION
)

def test_decision_no_pads(self, decision_maker, states):
"""
Test decision making when no pads are available.
"""
res, command = decision_maker.run(states, [])

assert res == False
assert command is None # when no pads found
Copy link
Contributor

Choose a reason for hiding this comment

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

single empty line at end of file

Loading