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

Block arming if takeoff waypoint is inactive and MIS_TKOFF_REQ is set to 1 #13824

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all 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
19 changes: 19 additions & 0 deletions src/modules/commander/Arming/PreFlightCheck/checks/preArmCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,18 @@

#include <ArmAuthorization.h>
#include <systemlib/mavlink_log.h>
#include <uORB/Subscription.hpp>
#include <uORB/topics/vehicle_command_ack.h>
#include <uORB/topics/position_setpoint_triplet.h>

#include <lib/parameters/param.h>

bool PreFlightCheck::preArmCheck(orb_advert_t *mavlink_log_pub, const vehicle_status_flags_s &status_flags,
const safety_s &safety, const arm_requirements_t &arm_requirements, const vehicle_status_s &status)
{
bool prearm_ok = true;
int32_t tkoff_check = 0;
param_get(param_find("MIS_TAKEOFF_REQ"), &tkoff_check);

// USB not connected
if (!status_flags.circuit_breaker_engaged_usb_check && status_flags.usb_connected) {
Expand Down Expand Up @@ -143,6 +149,19 @@ bool PreFlightCheck::preArmCheck(orb_advert_t *mavlink_log_pub, const vehicle_st
}
}

// This requires a valid takeodd waypoint to be set
uORB::Subscription setpoint_sub(ORB_ID(position_setpoint_triplet));
position_setpoint_triplet_s setpoint = {};
setpoint_sub.update(&setpoint);
Copy link
Contributor

Choose a reason for hiding this comment

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

I have two concerns here:

  1. The triplet is quite big and hence this needs CPU to copy and RAM to store it.
  2. The flow is generally commander -> navigator but here the commander is subscribing to navigator again to check what it is doing when really navigator should just be executing as told by commander.

Alternatives I see:

  1. Add this check into the mission feasibility checker. So when it checks that the mission is valid it also checks if it is set to the beginning?
  2. Don't start a mission in the controller even if armed if the item is not a takeoff item.

@MaEtUgR do you have other ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

@julianoes How about using the topic mission_result for this? To me this seems to be a topic which navigator uses to report various information regarding the current mission.
Wouldn't it be as simple as checking if mission_result.seq_current == 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, good idea!


if ((setpoint.current.type != position_setpoint_s::SETPOINT_TYPE_TAKEOFF || !setpoint.current.valid)
&& tkoff_check == 1) {
mavlink_log_critical(mavlink_log_pub, "Arming denied! Takeoff not active");
prearm_ok = false;
}




return prearm_ok;
}