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

Add flight interface decision queue #189

Closed
wants to merge 13 commits into from
Closed

Conversation

Trotyl15
Copy link
Member

No description provided.

@Trotyl15 Trotyl15 changed the title add flight interface decision queue Add flight interface decision queue Jul 16, 2024
Copy link
Contributor

@TongguangZhang TongguangZhang left a comment

Choose a reason for hiding this comment

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

reviewed

main_2024.py Outdated
Comment on lines 219 to 221
flight_interface_to_data_merge_queue,
flight_interface_decision_queue,
controller,
Copy link
Contributor

Choose a reason for hiding this comment

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

flight_interface_to_data_merge_queue should go in the output_queues arg, and flight_interface_decision_queue should go in the input_queues arg

Comment on lines 9 to 12
from ..logger import logger
from ..common.mavlink.modules import drone_odometry
from ..common.mavlink.modules import flight_controller
from ..decision_command import DecisionCommand
Copy link
Contributor

Choose a reason for hiding this comment

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

to follow the design pattern, use from .. import decision_command instead (to match the importing of odometry_and_time

Comment on lines 22 to 26
MOVE_TO_RELATIVE_POSITION = 0
MOVE_TO_ABSOLUTE_POSITION = 1
LAND_AT_CURRENT_POSITION = 2
LAND_AT_RELATIVE_POSITION = 3
LAND_AT_ABSOLUTE_POSITION = 4
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 an enum class

Copy link
Contributor

@TongguangZhang TongguangZhang left a comment

Choose a reason for hiding this comment

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

we should add unit tests for the apply_decision function

Comment on lines 97 to 101
def apply_decision(self, decision: decision_command.DecisionCommand) -> None:
"""
Apply the given decision command to the flight controller.
"""
self.controller.upload_commands(decision)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think flight controller expects a list of dronekit.Command objects, not a DecisionCommand object, couple options here:

  1. we could either change the function in common to accept a list of DecisionCommand objects and use enums to determine what dronekit.Command object to send (I like this option)
  2. change this function to determine what dronekit.Command object to send based on enums
  3. change the interface between flight interface and decision to pass dronekit.Command objects instead of DecisionCommand objects

@Trotyl15 Trotyl15 closed this Sep 25, 2024
@Trotyl15 Trotyl15 deleted the add_decision_handling branch September 26, 2024 23:35
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