Skip to content

Derek Gou - Bootcamp PR#4

Closed
derekGou wants to merge 17 commits intoUWARG:mainfrom
derekGou:main
Closed

Derek Gou - Bootcamp PR#4
derekGou wants to merge 17 commits intoUWARG:mainfrom
derekGou:main

Conversation

@derekGou
Copy link

@derekGou derekGou commented Sep 9, 2025

🙏

Comment on lines 2 to 3
20:30:25: [INFO] [/Users/derekgou/autonomy-bootcamp-2025-p2/tests/integration/test_heartbeat_receiver.py | main | 111] Connected!
20:30:26: [INFO] [/Users/derekgou/autonomy-bootcamp-2025-p2/tests/integration/test_heartbeat_receiver.py | read_queue | 69] Connected
Copy link
Contributor

Choose a reason for hiding this comment

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

your main log is missing when the heartbeat receiver doesn't recieve any heartbeats

@@ -0,0 +1,2 @@
20:22:16: [INFO] [/Users/derekgou/autonomy-bootcamp-2025-p2/modules/common/modules/logger/logger_main_setup.py | setup_main_logger | 62] main logger initialized
Copy link
Contributor

Choose a reason for hiding this comment

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

you're missing the heartbeat_sender_drone_###### file

elif msg.get_type() == "ATTITUDE":
self.last_attitude = msg

# Only return if we have both types
Copy link
Contributor

Choose a reason for hiding this comment

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

you're missing the logic for what happens when it doesn't receive both types in 1 second - what should the logger show?

queue.queue.put(telemetry_data)
local_logger.info(f"Telemetry data queued: {telemetry_data}", False)
else:
break # no more messages right now
Copy link
Contributor

Choose a reason for hiding this comment

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

what would the logger show when it is missing telemetry data?

Comment on lines 62 to 63
data_queue: queue_proxy_wrapper.QueueProxyWrapper,
response_queue: queue_proxy_wrapper.QueueProxyWrapper,
Copy link
Contributor

Choose a reason for hiding this comment

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

while the logic you implement overall stays the same inputting the queues in the class implementation, the purpose of the worker is to use the logic in there

Copy link
Author

Choose a reason for hiding this comment

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

I'm not totally sure what to change here - I'm assuming you mean to move some logic into the worker, but I'm pretty sure that all of my logic is to fulfill the instructions left in the command.py file - am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm trying to say that inputting and taking output from queues should happen within your worker. they way you have it right now works, but the whole point of the worker is to deal with the queues in there

attitude = self.last_attitude
if position is not None and attitude is not None:
telemetry_data = TelemetryData(
time_since_boot=int(
Copy link
Contributor

Choose a reason for hiding this comment

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

also your time_since_boot is weirdly off

Copy link
Author

Choose a reason for hiding this comment

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

lol I can't figure out why - are you allowed to tell me what I'm supposed to expect?

yaw_change %= 360
if yaw_change > 180:
yaw_change -= 360
queued.append(f"CHANGING_YAW: {yaw_change}")
Copy link
Contributor

Choose a reason for hiding this comment

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

your yaw_change calculations are also wrong

yaw_change -= 360
queued.append(f"CHANGING_YAW: {yaw_change}")

if abs(telemetry_data.z - self.target.z) > 0.5 or abs(telemetry_data.yaw - angle) > (
Copy link
Contributor

Choose a reason for hiding this comment

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

also the altitude change and the yaw change should not be the same mavlink message

Copy link
Author

Choose a reason for hiding this comment

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

the drone seems to only work if the altitude and yaw change messages are sent simultaneously when they are both triggered - the drone expects 26 messages, but 28 are sent if the altitude and yaw changes are separated. Am I missing something?

Copy link
Contributor

@ellyokes253 ellyokes253 Sep 20, 2025

Choose a reason for hiding this comment

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

actually jk forget this i read it wrong

Copy link
Author

Choose a reason for hiding this comment

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

lol probably because i just fixed it (i think) - should be ready for review tho

bootcamp_main.py Outdated
target=heartbeat_sender_worker.heartbeat_sender_worker,
work_arguments=(connection, HEARTBEAT_PERIOD),
input_queues=[],
output_queues=[heartbeat_queue],
Copy link
Contributor

Choose a reason for hiding this comment

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

heartbeat sender shouldn't have anything in the input or output queues (reference the worker schematic)

bootcamp_main.py Outdated
connection,
HEARTBEAT_PERIOD,
),
input_queues=[heartbeat_queue],
Copy link
Contributor

@ellyokes253 ellyokes253 Sep 17, 2025

Choose a reason for hiding this comment

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

this is also incorrect, i reccommend referencing the worker schematic photo to know where your queues should go

@@ -0,0 +1,58 @@
22:17:46: [INFO] [/Users/derekgou/autonomy-bootcamp-2025-p2/modules/command/command_worker.py | command_worker | 54] Logger initialized
22:17:46: [INFO] [/Users/derekgou/autonomy-bootcamp-2025-p2/modules/command/command.py | run | 91] Average velocity: [0.0, 0.0, 4.0]
22:17:46: [INFO] [/Users/derekgou/autonomy-bootcamp-2025-p2/modules/command/command.py | run | 128] 63.43494882292201
Copy link
Contributor

Choose a reason for hiding this comment

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

you shouldn't be logging the yaw here

Copy link
Contributor

Choose a reason for hiding this comment

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

this still needs to be fixed

1,
0,
0,
yaw_diff,
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to go to another parameter spot within self.connection.mav.command_long_send()

self,
args, # Put your own arguments here
):
def run(self, data: telemetry.TelemetryData) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

the return type isn't none if you return something

bootcamp_main.py Outdated
HEARTBEAT_PERIOD,
),
input_queues=[],
output_queues=[],
Copy link
Contributor

Choose a reason for hiding this comment

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

there should be something here

local_logger.error("Failed to create CommandWorker", True)
return
# Main loop: do work.
while not controller.is_exit_requested():
Copy link
Contributor

Choose a reason for hiding this comment

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

it's good practice to include controller.check_pause() within the main look as well. this check if the workers are requested to pause (it blocks/sleeps the worker until unpause). the same applies to all other workers

Comment on lines 22 to 23
controller: worker_controller.WorkerController,
queue: queue_proxy_wrapper.QueueProxyWrapper,
Copy link
Contributor

Choose a reason for hiding this comment

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

it's typically good practice to put the input/output queues above the controller - within WorkerProperties that the bootcamp_main.py uses, it follows this format

@ellyokes253
Copy link
Contributor

lgtm!

IshaanMittal07 added a commit to IshaanMittal07/DroneGroundStation that referenced this pull request Oct 18, 2025
IshaanMittal07 added a commit to IshaanMittal07/DroneGroundStation that referenced this pull request Oct 18, 2025
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