Skip to content

Conversation

kimichenn
Copy link
Contributor

@kimichenn kimichenn commented May 21, 2025

  • Replaces the old UDP code with Ardupilot RELAY2_FUNCTION for dropping airdrops.
  • Uses passthrough to access Mavlink directly to change the state of relay
  • Integration test drop_it_like_its_hot tests the airdrop mechanism by calling triggerAirdrop(), which is defined in airdrop_approach

@kimichenn kimichenn marked this pull request as ready for review May 30, 2025 00:58
@kimichenn kimichenn requested a review from miyatakazuya May 31, 2025 22:54
@miyatakazuya miyatakazuya requested a review from Copilot June 1, 2025 01:41
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the legacy UDP-based airdrop mechanism with ArduPilot’s RELAY2 relay control via Mavlink passthrough, updates tests, and wires the new helper into both the tick logic and GCS routes.

  • Introduces triggerRelay() in MavlinkClient and uses it in AirdropApproachTick
  • Adds a new integration test (drop_it_like_its_hot) and CMake entries for airdrop testing
  • Updates GCS POST /dodropnow handler to call triggerAirdrop instead of UDP

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/unit/cv/utilities_test.cpp Removed stray semicolon in test closing brace
tests/integration/mavlink_client.cpp Changed argument parsing to use OBCConfig and expect config
tests/integration/drop_it_like_its_hot.cpp Added new airdrop integration test with triggerAirdrop
tests/integration/CMakeLists.txt Registered drop_it_like_its_hot executable and deps
src/ticks/airdrop_approach.cpp Added triggerAirdrop helper; updated tick to call relay
src/network/mavlink.cpp Implemented MavlinkClient::triggerRelay() and reordered includes
src/network/gcs_routes.cpp Updated /dodropnow to use triggerAirdrop with hardcoded index
include/ticks/airdrop_approach.hpp Declared triggerAirdrop
include/network/mavlink.hpp Declared triggerRelay
deps/onnxruntime/onnxruntime.cmake Disabled DOWNLOAD_EXTRACT_TIMESTAMP
Comments suppressed due to low confidence (4)

tests/integration/mavlink_client.cpp:29

  • The error message still says "Expected use: bin/mavsdk [config]" but you now require 5 arguments; update the message to reflect the actual expected args or show usage syntax.
if (argc != 5) {

tests/integration/drop_it_like_its_hot.cpp:8

  • [nitpick] Typo in comment: "electro-pelectro-permanent" has a duplicated "pelectro"; consider "electro-permanent".
* For the 2025 

src/ticks/airdrop_approach.cpp:218

  • The ternary is inverted: when state is true it sets param2 to 0.0f (off) but comment says 1=on; flip to state ? 1.0f : 0.0f.
command.param2 = state ? 0.0f : 1.0f;               // 1=on, 0=off

src/network/gcs_routes.cpp:289

  • The handler uses a hardcoded airdrop index instead of parsing request.body; restore parsing logic or map the incoming JSON index.
std::optional<airdrop_t> next_airdrop_to_drop;

if (triggerAirdrop(state->getMav() , next_airdrop_to_drop.value())) {
LOG_RESPONSE(INFO, "Dropped Bottle Successfully", OK);
} else {
LOG_RESPONSE(INFO, "Failed to drop bottle", OK);
Copy link
Preview

Copilot AI Jun 1, 2025

Choose a reason for hiding this comment

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

Using OK status code for failure is misleading; use an error status (e.g., INTERNAL_ERROR) when the relay command fails.

Copilot uses AI. Check for mistakes.

miyatakazuya and others added 3 commits May 31, 2025 18:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@miyatakazuya
Copy link
Contributor

omg why did the goat @atar13 return 💀

Copy link
Contributor

@miyatakazuya miyatakazuya left a comment

Choose a reason for hiding this comment

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

I've tested this a couple times with the actual magnet and seems to actually work perfectly. One weird sorta related bug is that the relay is on by default, but once we send the HTTP request once, the default state is switched to off. Otherwise, looks good to merge.

@miyatakazuya miyatakazuya merged commit 2e7c1c9 into feat/cv-everything Jun 1, 2025
1 of 2 checks passed
@miyatakazuya miyatakazuya deleted the feat/cv-everything-airdrop branch June 1, 2025 01:53
@atar13
Copy link
Member

atar13 commented Jun 1, 2025

omg why did the goat @atar13 return 💀

I'm locked in. I'll keep the commits coming 🫡

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.

5 participants