Decision module #163
Decision module #163
Conversation
modules/decision/decision.py
Outdated
| 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 |
There was a problem hiding this comment.
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.
modules/decision/decision.py
Outdated
|
|
||
| def run( | ||
| self, | ||
| states: odometry_and_time.OdometryAndTime, |
There was a problem hiding this comment.
Change the naming to be more descriptive, it's the current state (not multiple states), so maybe current_state?
modules/decision/decision.py
Outdated
| if distance_to_best_bad <= self.__distance_tolerance: | ||
| # Issue a landing command if within tolerance |
There was a problem hiding this comment.
Put the comment before the if statement
modules/decision/decision.py
Outdated
| else: | ||
| # Move to best location if not within tolerance |
modules/decision/decision.py
Outdated
| ), | ||
| ) | ||
| else: | ||
| # Default to hover if no pad is found |
There was a problem hiding this comment.
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"
tests/test_decision.py
Outdated
| import pytest | ||
| from modules.decision import decision |
There was a problem hiding this comment.
2 lines between system module imports and local module imports. Also add a docstring to the file
tests/test_decision.py
Outdated
|
|
||
| # Test parameters |
There was a problem hiding this comment.
2 lines between imports and constant declarations (put the comment on the same line as the constant)
modules/decision/decision.py
Outdated
| dy = pad.position_y - current_position.odometry_data.position.east | ||
| return (dx**2 + dy**2) ** 0.5 | ||
|
|
||
| def weight_pads( |
There was a problem hiding this comment.
make this a private function
modules/decision/decision.py
Outdated
| def distance_to_pad( | ||
| pad: object_in_world.ObjectInWorld, | ||
| current_position: odometry_and_time.OdometryAndTime, | ||
| ): |
There was a problem hiding this comment.
format functions declarations like this
def func(arg1 :int,
arg2 :string) -> float:
pass
modules/decision/decision.py
Outdated
| def __weight_pads( | ||
| self, | ||
| pads: "list[object_in_world.ObjectInWorld]", | ||
| current_position: odometry_and_time.OdometryAndTime, | ||
| ): |
modules/decision/decision.py
Outdated
| def run( | ||
| self, | ||
| curr_state: odometry_and_time.OdometryAndTime, | ||
| pads: "list[object_in_world.ObjectInWorld]", | ||
| ): |
tests/test_decision.py
Outdated
| position_x, position_y, spherical_variance | ||
| ) | ||
| assert success | ||
| return pad |
There was a problem hiding this comment.
use yield in a pytest fixture
tests/test_decision.py
Outdated
| position_x, position_y, spherical_variance | ||
| ) | ||
| assert success | ||
| return pad |
tests/test_decision.py
Outdated
| def test_decision_within_tolerance( | ||
| self, decision_maker, best_pad_within_tolerance, pads, states | ||
| ): |
There was a problem hiding this comment.
same as function formatting comment above, but you don't need a type annotation for the return type
tests/test_decision.py
Outdated
| def test_decision_outside_tolerance( | ||
| self, decision_maker, best_pad_outside_tolerance, pads, states | ||
| ): |
tests/test_decision.py
Outdated
| res, command = decision_maker.run(states, []) | ||
|
|
||
| assert res == False | ||
| assert command is None # when no pads found |
There was a problem hiding this comment.
single empty line at end of file
modules/decision/decision.py
Outdated
| ) | ||
| # Default to do nothing if no pads are found | ||
| else: | ||
| return False, None |
There was a problem hiding this comment.
single empty line at end of file
tests/test_decision.py
Outdated
| 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 | ||
| ) |
There was a problem hiding this comment.
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.
tests/test_decision.py
Outdated
| 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)[ |
There was a problem hiding this comment.
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...
tests/test_decision.py
Outdated
| 1 | ||
| ] # Example altitude of -5 meters |
There was a problem hiding this comment.
if this fits on the line you don't need to make it multiline
tests/test_decision.py
Outdated
| orientation = drone_odometry_local.DroneOrientationLocal.create_new(0.0, 0.0, 0.0)[ | ||
| 1 | ||
| ] |
No description provided.