Arm authorization #7377

zehortigoza · 2017-06-07T20:13:46Z

Allow PX4 to request flight authorization before arm.

Government entities like NASA are working in systems to managed the aero space for drones(https://utm.arc.nasa.gov/) but this also can have another uses by private companies.

Here we have a prof of concept application that authorizes mission based on the altitude of the waypoints of mission loaded. mavlink-router/mavlink-router#94

TSC21 · 2017-06-07T20:33:28Z

@zehortigoza this is very promising. But, if one is not using an external system to send the authorization, can one just send the authorization through a RC channel for example?

zehortigoza · 2017-06-07T22:21:07Z

@zehortigoza this is very promising. But, if one is not using an external system to send the authorization, can one just send the authorization through a RC channel for example?

Authorization through RC is something that we can do but I don't see any point in doing as it can not get mission data and process it.

TSC21 · 2017-06-07T23:29:18Z

Authorization through RC is something that we can do but I don't see any point in doing as it can not get mission data and process it.

I understand, but is this supposed to be a mandatory verification or just optional? 'cause not everyone plans to use companion computers or GCS links. If you consider it mandatory, you should add an option like the above.

zehortigoza · 2017-06-08T00:55:21Z

I understand, but is this supposed to be a mandatory verification or just optional? 'cause not everyone plans to use companion computers or GCS links. If you consider it mandatory, you should add an option like the above.

PX4 already had a parameter to only arm if there is a mission, I added(actually using the same byte, just using bitmask now) a new one to arm only if it got a mission authorization.

lfelipe · 2017-06-08T15:25:34Z

@sanderux pinged you for review as it seems from the last dev meeting that you are interested in something close to this (getting an external entity to validate flight based on weather conditions)

sanderux · 2017-06-08T23:10:23Z

src/modules/commander/mission_auth.cpp

+#include <uORB/topics/vehicle_command.h>
+#include <uORB/topics/vehicle_command_ack.h>
+
+#define MISSION_AUTH_SYSTEM_ID 10


Why is this hard?

Sure, I will make this a parameter.

sanderux · 2017-06-08T23:11:19Z

src/modules/commander/mission_auth.cpp

+#define MISSION_APPROVED_TIMEOUT_USEC (1000000 * 60)
+#define MISSION_AUTH_CHECK_DEFAULT_TIMEOUT_USEC (1000000 * 60)
+#define MISSION_AUTH_CMD_ACK_TIMEOUT_USEC (50000)
+


These should also be configurable

Sure, I will make this a parameter.

sanderux · 2017-06-08T23:13:15Z

src/modules/commander/mission_auth.cpp

+ *
+ ****************************************************************************/
+#include "mission_auth.h"
+


Are we only requesting auth for missions? i think it should be flight approval (auth for arm)

Ok, I will rename it to make more generic.

sanderux · 2017-06-08T23:14:03Z

src/modules/commander/mission_auth.cpp

+	state = MISSION_AUTH_WAITING_AUTH;
+
+	mavlink_log_critical(mavlink_log_pub, "Requesting mission authorization...");
+


Please avoid needless user feedback, at minimum this should be an info message

I don't think that this is needless, user should know why the first request did not armed the vehicle.

The message here only informs that the vehicle is requesting. If the response comes within a ms this message is a bit much

sanderux · 2017-06-08T23:17:22Z

src/modules/commander/mission_auth.cpp

+					MISSION_APPROVED_TIMEOUT_USEC / 1000000);
+			state = MISSION_AUTH_MISSION_APPROVED;
+			auth_timeout = now + MISSION_APPROVED_TIMEOUT_USEC;
+			break;


I don't think flight approval should require 2 arm commands, in a real life use case flight approval could be requested from the onboard computer (like checking weather conditions) that would only take a few ms. The vehicle should arm directly after receiving approval.

If there are use cases where flight approval is either manual or massively delayed you could make this configurable

It was our first approach but getting final approval for the mission through 3G/4G was taking 10~20 seconds when the signal was good.
Having 2 arms commands makes it more safe as you know that the second arm command will actually arm the drone immediately.

I have a suggestion here. From a user perspective, 2 arm commands are completely un-intuitive. I would suggest that you request authorization on pre-arm (safety switch pressed and system de-saftied).

Then wait for arming from the operator. If the auth has succeeded by then, directly arm - otherwise refuse arming citing no authorization yet. How does that sound?

Perhaps this should be configurable too

Safetyswitch is not always used, i would propose to make this configurable

sanderux · 2017-06-08T23:19:05Z

src/modules/commander/mission_auth.cpp

+		default:
+			mavlink_log_critical(mavlink_log_pub, "Mission denied");
+			state = MISSION_AUTH_IDLE;
+			break;


There should be a reason included in the message. Perhaps a set of pre-defined messages including an option to send nothing (in our case the onboard computer sends the reason directly to the GCS)

Ok, so should I move to a new MAVlink message instead of keep using COMMAND_LONG/COMMAND_ACK? Any other suggestion?

If the ACK includes a status code we could compile a list of reasons

sanderux · 2017-06-08T23:25:34Z

src/modules/mavlink/mavlink_receiver.cpp

+		orb_publish(ORB_ID(vehicle_command_ack), _command_ack_pub, &command_ack);
+	}
+}
+


For feedback and log purposes i think we should also register who approved the flight

Sounds good

LorenzMeier · 2017-06-17T05:01:54Z

@zehortigoza Commander is scheduled for a rewrite - do you feel this is something you're ready to embark on?

zehortigoza · 2017-06-27T00:56:44Z

New version with the fixes requested.

LorenzMeier · 2017-07-09T12:55:07Z

@zehortigoza Sorry for only getting back to this now - the 1.6 release consumed my attention. Could you please rebase this on master and join the dev call on Wednesday to discuss the concept?

Link: http://discuss.px4.io/t/weekly-dev-call-of-july-12-2017/3833

zehortigoza · 2017-07-10T23:00:37Z

@zehortigoza Sorry for only getting back to this now - the 1.6 release consumed my attention. Could you please rebase this on master and join the dev call on Wednesday to discuss the concept?

Link: http://discuss.px4.io/t/weekly-dev-call-of-july-12-2017/3833

sure

mrpollo · 2017-07-18T05:02:46Z

@zehortigoza can you attend the dev call this Wednesday 8 am PST? we would like to discuss this with you

zehortigoza · 2017-07-18T15:53:13Z

@mrpollo Sure I will be there.

zehortigoza · 2017-07-20T00:08:40Z

Updated it again, the first commits were send as separated PR #7613

new version

LorenzMeier · 2017-08-09T05:27:46Z

src/modules/commander/commander_params.c

@@ -614,3 +614,21 @@ PARAM_DEFINE_INT32(COM_ARM_MIS_REQ, 0);
 * @group Mission
 */
 PARAM_DEFINE_INT32(COM_POSCTL_NAVL, 0);
+
+/**
+ * Arm authorization parameters, this uint32_t will be spited between starting from the LSB:


typo: spited -> splitted.

LorenzMeier · 2017-08-09T05:32:53Z

src/modules/mavlink/mavlink_receiver.cpp

@@ -610,6 +620,34 @@ MavlinkReceiver::handle_message_command_ack(mavlink_message_t *msg)
 }

 void
+MavlinkReceiver::handle_message_command_ack2(mavlink_message_t *msg)


Could you please send a PR against the MAVLink devguide detailing the proposal?
https://github.com/mavlink/mavlink-devguide

Please also try to find a better name for ACK2 - it should have a notion of how ACK and the new ACK messages are different (like GPS and GPS_INT or ATTITUDE and ATTITUDE_QUAT).

Thanks!

LorenzMeier

Generally in good shape - if you add the documentation to upstream MAVLink so anyone else can also implement it and the spec is agreed-on we could move this forward.

zehortigoza · 2017-08-19T01:08:25Z

New version now that the mavlink side was merged.

The commits handling the new fields in command_ack were also sent in another PR: #7821

Could you please send a PR against the MAVLink devguide detailing the proposal?
https://github.com/mavlink/mavlink-devguide

I will write that until tuesday.

LorenzMeier · 2017-08-19T07:05:24Z

other PR is in

… new requirement Instead of having several bools to each requirement to arm, lets group then in a byte and use bitmask. This also add a new arm requirement "arm authorization" that will be implemented in another patch.

If 2 or more vehicle_command are queued a call to update() will return the oldest vehicle_command and set the _cmd_time to the timestamp of the last vehicle_command queued losing it. Using update_if_changed() fix this causing all item being consumed one at each call of send().

If the second bit of COM_ARM_MIS_EXT_REQ is set the vehicle will only arm after receive an authorization. The authorization flow: vehicle/external -> command: arm authorization request -> arm authorizer vehicle <- command ack with result in progress <- arm authorizer vehicle <- any data request <- arm authorizer vehicle -> data response -> arm authorizer vehicle <- command ack authorizing or denying <- arm authorizer Right now there is 2 ways to start the arm authorization request, that can be configured by COM_ARM_AUTH parameter. - One arm: When pilot request the vehicle to arm, it will request authorization blocking the arm process up to the timeout defined in COM_ARM_AUTH parameter. - Two arms request: The first arm request will request the authorization and will deny the first arm request, if authorizer approved the request, pilot can arm again within the authorized time and arm without any block. The arm authorizer can be running anywhere(compute board or PX4 itself) and it is responsible to request the mission list or any other information to vehicle before send a final response, it should send to vehicle a COMMAND_ACK with result = MAV_RESULT_IN_PROGRESS as soon as it receive the arm authorization request and the final result as after it got all the data that it needs authorize or deny the request.

zehortigoza · 2017-08-21T23:38:06Z

Rebased again and documentation PR sent mavlink/mavlink-devguide#16

LorenzMeier · 2017-08-27T20:16:29Z

Jenkins test this please.

zehortigoza force-pushed the mission-auth branch from 3a41f26 to 0c77b63 Compare June 7, 2017 20:29

lfelipe requested a review from sanderux June 8, 2017 15:24

magicrub mentioned this pull request Jun 8, 2017

Mission authorization ArduPilot/ardupilot#6405

Closed

sanderux previously requested changes Jun 8, 2017

View reviewed changes

zehortigoza force-pushed the mission-auth branch from 0c77b63 to 9031b71 Compare June 27, 2017 00:55

zehortigoza changed the title ~~RFC: Mission authorization~~ RFC: Arm authorization Jun 27, 2017

zehortigoza force-pushed the mission-auth branch from 9031b71 to 61d78d9 Compare June 27, 2017 17:36

zehortigoza mentioned this pull request Jul 13, 2017

Allow vehicle to receive a command_ack & Safely avoid send the same vehicle_command/command_ack back #7613

Closed

mrpollo added the devcall label Jul 18, 2017

zehortigoza force-pushed the mission-auth branch from 61d78d9 to 5501c38 Compare July 20, 2017 00:07

zehortigoza force-pushed the mission-auth branch 2 times, most recently from e51c551 to 76a08e1 Compare July 21, 2017 21:39

sanderux removed the devcall label Jul 26, 2017

zehortigoza force-pushed the mission-auth branch from 76a08e1 to d0f17c6 Compare August 8, 2017 22:14

zehortigoza requested review from LorenzMeier and dagar August 8, 2017 22:15

zehortigoza mentioned this pull request Aug 8, 2017

Arm authorization messages mavlink/mavlink#768

Merged

zehortigoza changed the title ~~RFC: Arm authorization~~ Arm authorization Aug 8, 2017

LorenzMeier reviewed Aug 9, 2017

View reviewed changes

LorenzMeier requested changes Aug 9, 2017

View reviewed changes

TSC21 mentioned this pull request Aug 18, 2017

Feature: Implement Mavlink arm authorization mavlink/mavros#775

Open

zehortigoza force-pushed the mission-auth branch from d0f17c6 to 420c99a Compare August 19, 2017 01:04

zehortigoza added 4 commits August 21, 2017 13:42

msg: Add arm authorization request command id

a51a6a4

zehortigoza force-pushed the mission-auth branch from 420c99a to 05aed72 Compare August 21, 2017 23:37

LorenzMeier approved these changes Aug 28, 2017

View reviewed changes

LorenzMeier merged commit 3fd7e3f into PX4:master Aug 28, 2017

		state = MISSION_AUTH_WAITING_AUTH;

		mavlink_log_critical(mavlink_log_pub, "Requesting mission authorization...");

Arm authorization #7377

Arm authorization #7377

Conversation

zehortigoza commented Jun 7, 2017 • edited Loading

TSC21 commented Jun 7, 2017 • edited Loading

zehortigoza commented Jun 7, 2017

TSC21 commented Jun 7, 2017 • edited Loading

zehortigoza commented Jun 8, 2017

lfelipe commented Jun 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LorenzMeier commented Jun 17, 2017

zehortigoza commented Jun 27, 2017

LorenzMeier commented Jul 9, 2017 • edited Loading

zehortigoza commented Jul 10, 2017

mrpollo commented Jul 18, 2017

zehortigoza commented Jul 18, 2017

zehortigoza commented Jul 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LorenzMeier left a comment

Choose a reason for hiding this comment

zehortigoza commented Aug 19, 2017

LorenzMeier commented Aug 19, 2017

zehortigoza commented Aug 21, 2017

LorenzMeier commented Aug 27, 2017

zehortigoza commented Jun 7, 2017 •

edited

Loading

TSC21 commented Jun 7, 2017 •

edited

Loading

TSC21 commented Jun 7, 2017 •

edited

Loading

LorenzMeier commented Jul 9, 2017 •

edited

Loading