ardupilot icon indicating copy to clipboard operation
ardupilot copied to clipboard

Implement new mission opaque ID protocol

Open peterbarker opened this issue 3 years ago • 32 comments

This PR (now) fills in new extension fields in several messages, according to discussions here: https://github.com/mavlink/mavlink/pull/2012

The MISSION_ACK when you have successfully uploaded a mission now contains an opaque ID for the loaded mission. The MISSION_CURRENT message has three extra fields, one for each of the standard three MAV_MISSION_TYPEs (mission, rally, fence). We only support this field on mission at this point. The MISSION_COUNT message also gets a new opaque_id field. A GCS can associate the downloaded mission with the opaque_id in the MISSION_COUNT message they receive back from the autopilot. The GCS MUST check the opaque ID hasn't changed after they have all the items if it wants to be sure the downloaded mission is intact!

Replaces https://github.com/ArduPilot/ardupilot/pull/17370 - nothing changed, I just stuffed up when I went to reopen things.

peterbarker avatar May 24 '22 08:05 peterbarker

Relevant discussion: https://github.com/mavlink/mavlink/issues/1171

peterbarker avatar May 24 '22 08:05 peterbarker

@peterbarker When do you plan on running a test(s) with many waypoints to evaluate the cost of this operation?

A checksum generated on the vehicle is logically the safest way to check missions match, so if this is feasible, it would be a good thing to do. It also depends on how long it takes/whether it might be done in the background. While we'd like an instant update of mission changed, it it took 10 seconds, I'm not sure that this would be the end of the world if this was well understood by implementers.

If not, it would be good to know so we can look at other alternatives that meet the same use case. Various ideas have floated around:

  • The checksum is generated on GCS or companion and supplied to the vehicle in MISSION_ACK during upload (or a brand new mission item made for this purpose).
  • The vehicle emits a UUID rather supplied in the mission upload, probably as a new mission item. This allows a GCS to know if the mission is the one it just uploaded, and others to know it is not - and it can be cached. The mission item could be part of the plan, so a saved mission that was reloaded would be known to be the same.

We didn't like those options as much because we didn't want to mess with the existing mission items or mission messages, and because using a checksum means that you know two mission/plans are the same without having to trust some data stored in the mission. But if we need to look at these approaches it would be better to know sooner rather than later.

hamishwillee avatar Aug 17 '22 06:08 hamishwillee

@peterbarker When do you plan on running a test(s) with many waypoints to evaluate the cost of this operation?

AP: Took 2.178000ms to checksum 373 points (5.839142ms/1000 points)

That's obviously not something we can do in the main thread! That's going to complicate the implementation.

Should be feasible to do the calculation in a thread - I'll see how it looks.

peterbarker avatar Sep 20 '22 22:09 peterbarker

I've modified MAVProxy and pymavlink to use the checksum feature to avoid uploading a mission if it is identical to the one on the vehicle: https://github.com/ArduPilot/MAVProxy/pull/1104

MANUAL> wp load Tools/autotest/ArduPlane_Tests/MISSION_CHECKSUM/text.txt
MANUAL> Loaded 373 waypoints from Tools/autotest/ArduPlane_Tests/MISSION_CHECKSUM/text.txt
Got COMMAND_ACK: REQUEST_MESSAGE: ACCEPTED
< MISSION_CHECKSUM {mission_type : 0, checksum : 1752152414}
Mission checksum mismatch
Got MISSION_ACK: TYPE_MISSION: ACCEPTED

MANUAL> 
MANUAL> 
MANUAL> 
MANUAL> Loaded 373 waypoint in 7.46s
Sent all 373 waypoints
Got MISSION_ACK: TYPE_MISSION: ACCEPTED
AP: Flight plan received
< MISSION_CHECKSUM {mission_type : 0, checksum : 4264322242}
Flight battery 100 percent

MANUAL> 
MANUAL> 
MANUAL> wp load Tools/autotest/ArduPlane_Tests/MISSION_CHECKSUM/text.txt
MANUAL> Loaded 373 waypoints from Tools/autotest/ArduPlane_Tests/MISSION_CHECKSUM/text.txt
Got COMMAND_ACK: REQUEST_MESSAGE: ACCEPTED
< MISSION_CHECKSUM {mission_type : 0, checksum : 4264322242}
Mission checksum match

peterbarker avatar Sep 21 '22 06:09 peterbarker

The patches in here are now updating the checksum progressively to avoid the scheduling problems.

peterbarker avatar Sep 26 '22 23:09 peterbarker

@hamishwillee we discussed this in today's DevCall.

This is now end--to-end working.

We're not sure of the utility of this in the face of mission-transfer-via-ftp, however. Could you summarize the arguments for this feature, please?

peterbarker avatar Sep 27 '22 02:09 peterbarker

Thanks for taking the time to do this work.

MISSION_CHECKSUM is what tells you that the mission, geofence or rally point is different or has changed on the vehicle and needs to be re-synced. It is required in order to support plan synchronization in vehicles, companions, and GCS, in particular in MAVLink networks where there are multiple components that might be making changes.

Using MAVFTP to upload/download a mission is a lot more efficient than the standard protocol, but:

  • not much use if you don't know the mission has changed.
  • is an expensive operation to make if you don't need to. That is particularly true in multiple GCS systems.

MAVFTP is great if you only have a single GCS and Vehicle that might affect plans. It's fast enough that on connection you might just decide that you'll either swamp the vehicle mission, or upload the vehicle mission. Of course even if that takes 1 second, that's still longer than the time taken to get the checksum and decide you don't need to bother.

Upshot,

  • you need something like this if you want to efficiently make sure that multiple systems can remain synchronised with a mission. It doesn't have to be a checksum, but that is the "cleanest" way.
  • even if you can choose to always just upload/download a mission, this is more efficient.

hamishwillee avatar Sep 28 '22 02:09 hamishwillee

@hamishwillee there should not be a tie between the GCS checksum and autopilot checksum. The checksums just won't match for ArduPilot, as we don't store all the mission item fields in ArduPilot, so the checksum calculated on the GCS for a mission file will not match the one from the autopilot. If we're going to have a checksum it should be an opaque algorithm, do the GCS can tell it has changed, but cannot use it to tell if a file they have locally is the same as one on the autopilot this will also make it a whole lot cheaper to compute, as we can directly CRC the memory mapper storage of the mission in the autopilot instead of reformatting all the time into the mavlink format

tridge avatar Sep 28 '22 08:09 tridge

@tridge Thanks very much. That's disappointing.

There's an additional complication; if the GCS can't infer the same checksum then it can't know that a particular emitted checksum is the result of a mission it just uploaded. Therefore it would have no way of knowing the checksum matched it.

To meet the use case of knowing "has the mission changed" either:

  1. The autopilot either has to tell the GCS that "this" particular checksum was set by it, or
  2. The GCS has to tell the autopilot what checksum it should use.

I kind of like option 2 because

  1. The GCS has more grunt to calculate a checksum and I presume is more likely to do so consistently across missions than different flight stacks.
  2. If you embed the checksum in a MAVCMD in the mission - such as MAV_CMD_CHECKSUM - then this will live in the mission on the autopilot whether it is uploaded using MAVFTP or the normal MAVLink protocol. Upload or download, the value persists.
  3. We could reasonably consider not using a checksum but instead using a UUID or similar. I prefer checksums as they allow two "like" missions to be the same, but a UUID would serve the purpose.

The rest of the process could be the same.

Might that work?

Re option 1 I'm not sure how you'd inform the GCS that it was from "X" system/component ID, and I'm not sure if you're using an opaque checksum of the autopilot mission you might not be just better off using UUIDs.

hamishwillee avatar Sep 28 '22 11:09 hamishwillee

It does seem that this is "poor mans signing". For commercial/secure applications, the ideal solution is that the mission should be able to be cryptographically signed on the GCS and the signature validated on the autopilot. If there are data or algorithm issues with this, then those issues should be solved. This is the utopian solution, but the only way in the end to securely, reliably and completely solve the problem.

timtuxworth avatar Sep 28 '22 14:09 timtuxworth

@timtuxworth The proposal and use case has nothing to do with signing (at least for me). It is merely a way to allow any system on the network that is interested in the plan on the vehicle to know if it has a matching version or needs to update.

I'm not convinced of the value of a signed mission plan. If there is a need for the drone to only accept trusted plans wouldn't it make more sense to establish trust with the supplier and supplying channel than overlay this requirement on any (every) particular data you want to share?

If you @tridge @peterbarker strongly feel this is a requirement then let me know, otherwise answer to suggestion at https://github.com/ArduPilot/ardupilot/pull/20834#issuecomment-1260771823 appreciated.

hamishwillee avatar Sep 28 '22 23:09 hamishwillee

@tridge Thanks very much. That's disappointing.

just to clarify with this current PR.

A GCS can't create a mission, checksum it, ask the autopilot what checksum it has and expect it to ever match what it came up with. ArduPilot can not round-trip the entire mission_item_int sequence as we don't store the vast majority of the data that can be present in MISSION_ITEM_INT.

What a GCS can do is checksum a mission it has downloaded from the autopilot, ask the autopilot what it's got on it and expect that to match. This is because once the mission has been round-tripped to the autopilot the GCS has a copy of the mission which has been filtered through the autopilot's storage mechanisms.

The suggestion that came out of the DevCall yesterday is to just go fully opaque - so the GCS never checksums anything, just has some sort of autopilot-generated token which can be used to uniquely identify a mission configuration (almost certainly a checksum).

Note that a mission can be modifed on the autopilot through many means - e.g. partial upload, scripting, custom patches - so adding a UUID in the sense of a randomly-generated number to e.g. MISSION_COUNT doesn't really make sense as something for the vehicle to store alongside the stored mission.

If we were to go down the "autopilot unique token (probably checksum)" route, we could potentially solve the race condition (vs other GCSs changing the mission) issues by having the GCS include an opaque session identifier as an extension in the MISSION_COUNT packet used to initiate the transfer. This will not help if the user transfers the file via ftp - but, then, ftp already has a CRC function built-in (which we could already use to avoid the FTP transfer if we did that implementation in the GCSs!)

peterbarker avatar Sep 29 '22 00:09 peterbarker

THanks @peterbarker

I'm fine with an autopilot generated ID, that might just happen to be a checksum on the stored file data. For the use case it doesn't matter as long as it changes when the mission does to some value that is not going to match what is on all the GCS/companions on the network.

It is "slightly beneficial" if this is an opaque checksum of the file because then a change to the mission that reverts a change would still match on GCS that have not yet chosen to update. Do you maintain a separate file for the different parts of the plan - mission, geofence, rally points? It IS good if we can know you might only need to update part of the plan at a time.

If we were to go down the "autopilot unique token (probably checksum)" route, we could potentially solve the race condition (vs other GCSs changing the mission) issues by having the GCS include an opaque session identifier as an extension in the MISSION_COUNT packet used to initiate the transfer. This will not help if the user transfers the file via ftp - but, then, ftp already has a CRC function built-in (which we could already use to avoid the FTP transfer if we did that implementation in the GCSs!)

What I don't understand is how, after downloading a mission, the GCS knows that the checksum it see's is the one for the mission it just uploaded. I think what you are suggesting above is that either:

  1. It assumes the next MISSION_CHECKSUM emitted is the value for the update it just made - and we accept there is a race condition.
  2. I think above you're suggesting that a session be included in MISSION_COUNT and that be added to MISSION_CHECKSUM?
    • Alternatively maybe add the checksum to the final MISSION_ACK? Your way is "safer" IMO because the GCS really doesn't care much if it misses the final ACK. It is a little more efficient.
  3. For the MAVFTP case you get a CRC from FTP? I assume the CRC32 is the same as you are proposing we use as the checksum for "normal" upload? Perhaps I'm being naïve but I am not so concerned about race conditions in this case.

hamishwillee avatar Sep 29 '22 00:09 hamishwillee

It is "slightly beneficial" if this is an opaque checksum of the file because then a change to the mission that reverts a change would still match on GCS that have not yet chosen to update. Do you maintain a separate file for the different parts of the plan - mission, geofence, rally points? It IS good if we can know you might only need to update part of the plan at a time.

ArduPilot doesn't store any of those three things in files as such; they're all put into either RAMTRON or flash-page storage. The limitations of those storage media mean we need to "compress" the data into a very compact form.

If we were to go down the "autopilot unique token (probably checksum)" route, we could potentially solve the race condition (vs other GCSs changing the mission) issues by having the GCS include an opaque session identifier as an extension in the MISSION_COUNT packet used to initiate the transfer. This will not help if the user transfers the file via ftp - but, then, ftp already has a CRC function built-in (which we could already use to avoid the FTP transfer if we did that implementation in the GCSs!)

What I don't understand is how, after downloading a mission, the GCS knows that the checksum it see's is the one for the mission it just uploaded. I think what you are suggesting above is that either:

2. I think above you're suggesting that a session be included in MISSION_COUNT and that be added to MISSION_CHECKSUM?

Yes, my brain thought that, but apparently my fingers didn't keep up.

   * Alternatively maybe add the checksum to the final MISSION_ACK? Your way is "safer" IMO because the GCS really doesn't care much if it misses the final ACK. It is a little more efficient.

Yep - just in MISSION_CHECKSUM, I think. There is the problem that two GCSs may tryt to update at the same time and use the same token, however - and there's also the possibility that that final "MISSION_CHECKSUM" is lost along the way to the original GCS which is waiting for the unique identifier (probably checksum) for back from the autopilot for the mission it just uploaded. The first could be solved by including a systemid/component id in the MISSION_CHECKSUM message, the latter by always transmitting that systemid/componentid so long as the mission checksum remains unchanged.

3. For the MAVFTP case you get a CRC from FTP? I assume the CRC32 is the same as you are proposing we use as the checksum for "normal" upload? Perhaps I'm being naïve but I am not so concerned about race conditions in this case.

Not sure which checksum it is off the top of my head - but it's a separate operation (like get and put, you can "give me a crc". This still works in ArduPilot because we present the missions as files..

peterbarker avatar Sep 29 '22 02:09 peterbarker

The suggestion that came out of the DevCall yesterday is to just go fully opaque - so the GCS never checksums anything, just has some sort of autopilot-generated token which can be used to uniquely identify a mission configuration (almost certainly a checksum).

A unique mission token is a great idea, but it can't possibly be a checksum. A unique identifier has to be unique and immutable and a checksum is neither. A UID/GUID would work and its probably the best way to do this.

The implication of this great idea is that there needs to be a mission configuration "header" data entity stored separately from the mission/waypoint items, fence items and rally points. This gives us a place to store the checksum and if a "Mission Configuration Version" was added then it would be fairly straightforward to solve the potential race condition in calculating the checksum. It would work something like this:

mission_configuration.version gets incremented every time a mission/waypoint, fence or rally point is updated

Run the checksum calculation in a low priority loop Store the current mission_configuration.version in a temp "current version" variable Run the checksum update for the next mission item check that the current version temp variable still matches the mission_configuration.version - if it doesn't throw away the current WIP checksum and start again When you get to the end of the mission store the checksum in mission_configuration and also store a checksum_version in the mission _configuration.

Any consumer of the checksum needs to check that the current mission version matches the checksum_version (under a semaphore) - if it doesn't wait until it does.

There are a lot of other very useful implications of having a separate "Mission Configuration" data entity, which I've written up in the forum here https://discuss.ardupilot.org/t/mission-configuration-data-item/91382

timtuxworth avatar Sep 29 '22 13:09 timtuxworth

@timtuxworth I'm hesitant to talk about having message header information separate from the mission. These kinds of architectural changes are quite hard to get agreed across flight stacks - you certainly wouldn't get it without a compelling use case for the need for checksums and then discussion that shows that this is the least disruptive way to do it when implemented across flight stacks.

If the "value" is generated on the vehicle and cannot match a value generated on the GCS then for this use case having it as a checksum is at most a "nice to have". If we don't make it a checksum though, we should rename the message to UUID or whatever we use. If you strongly feel a checksum is something needed then I suggest you raise it in the ArduPilot dev call and get support there. Then it could be brought to the mavlink call.

@peterbarker I think I'm happy with having the session in the MISSION_COUNT and included in the MISSION_CHECKSUM. The MISSION_CHANGED message was recently removed but it used the system id and component id of the last changer (we though the checksum was better, but essentially this is coming around to an id that is essentially system id + component id + "checksum or UUID or at least something somewhat unique".

Anyway, I can see you're iterating. Please loop me in. The most important thing is the meet the use case and document it so we can all match match it in the same way.

hamishwillee avatar Oct 02 '22 22:10 hamishwillee

@timtuxworth I'm hesitant to talk about having message header information separate from the mission. These kinds of architectural changes are quite hard to get agreed across flight stacks - you certainly wouldn't get it without a compelling use case for the need for checksums and then discussion that shows that this is the least disruptive way to do it when implemented across flight stacks.

This is the crux of it @hamishwillee - what is/are the used cases being addressed?

As a quite new user of ArduPilot, I have found the following challenges with working with missions that IMO need an architectural solution to address, hence my suggestion:-

  1. As a pilot, I want to be able to load and save missions to and from the FC and be able to easily manage which mission is loaded and keep track of where (on the FC or on my PC) is the latest/correct version of the mission.
  2. As a pilot, I want to be able to validate that the correct mission is on the aircraft, so that I know I am flying the mission I intended.
  3. As a pilot, I want to be able to identify the mission, by some kind of id AND description, so that I can keep track of the specific mission and it's purpose.

Those are the core ones, but there are other things I think I should be able to do with missions that I find frustrating that I can't do. I don't know if others want these, but I'd be really surprised if not:- 5. As a pilot, I want to be able to load multiple missions on the FC prior to a trip to the field and select the mission to run before each flight, which might be a different mission for each flight, so that I can carefully preplan the mission but select what I want to fly based on the circumstances. For example I have a field where the wind sometimes comes from the south and sometimes from the north. I'd like to pre-program a north takeoff/land and south takeoff/land version of the mission and select the right one at the field without having to reload the mission onto the FC while at the field, or even worse - manually edit it on the field on a laptop screen in the glare of the sun making it hard to read and running the risk that I mess it up. 6. As a drone service provider, I want to be able to ensure that the correct mission is on the vehicle, and has not been tampered with or modified, so that I can be sure the vehicle is executing the contracted flight. Ideally this should be cryptographically verified. 7. As a drone service provider, I want to be able to create a known reliable version of a mission and load it on 10's or even 100's of vehicles knowing that I can be guaranteed that exactly the same mission is loaded on each vehicle. 8. As a drone service provider, I want to be able to link logs, footage and sensor data for a flight to a specific programmed mission so that I can provide auditable results to a customer guaranteeing that the data they receive for multiple flown missions is the result of a specific contracted mission plan. 9. As a mission planner, I'd like to be able to construct sub-missions that can be reused and composed into larger missions without having to code the entire mission from beginning to end. I'd like to have reusable geofences for specific fields and takeoff and landings for different wind conditions and be able to combine those with sub-missions for specific flights and destinations. This would help me to be able to reliably and correctly create missions with the minimum of effort and reduce the potential for errors. 10. 10. As a pilot, I’d like specific parameters and options, such as the “Home” location, or particular parameters to be associated with a particular mission, to prevent errors and potential crashes due to options or parameters not being set correctly for the mission being flown. 11. As a mission planner or pilot, I’d like to be able to link a mission to a particular vehicle or vehicle type to prevent potential crashes or other problems due to a mission being loaded onto the incorrect vehicle.

Just off the top of my head ...

timtuxworth avatar Oct 03 '22 00:10 timtuxworth

there should not be a tie between the GCS checksum and autopilot checksum. The checksums just won't match for ArduPilot, as we don't store all the mission item fields in ArduPilot, so the checksum calculated on the GCS for a mission file will not match the one from the autopilot. If we're going to have a checksum it should be an opaque algorithm, do the GCS can tell it has changed, but cannot use it to tell if a file they have locally is the same as one on the autopilot this will also make it a whole lot cheaper to compute, as we can directly CRC the memory mapper storage of the mission in the autopilot instead of reformatting all the time into the mavlink format

It might take a bit of work to get the checksums to match (for example by being very explicit about which fields are included in the checksum and how the checksum is calculated), but it could be made to match. @tridge -are you saying it should not be tied because it's hard to make it match, or is there another reason?

If I understand the use case correctly, if the checksum isn't going to match on the vehicle and the GCS then a checksum isn't a solution for the requirement, so either it has to be made to match, or a different solution must be found. This seems to be what @hamishwillee is saying too:

If the "value" is generated on the vehicle and cannot match a value generated on the GCS then for this use case having it as a checksum is at most a "nice to have".

timtuxworth avatar Oct 03 '22 02:10 timtuxworth

@timtuxworth Wr.t. your last message above ...

It is likely that ArduPilot knows that it doesn't support certain parameters in certain commands and chooses not to store those. SO if the params have values in the GCS mission those will be thrown away. It is also possible that some mission items are accepted (don't fail an upload) but discarded. The obvious solution is to include the data even if you don't use it but that is costly.

The algorithm is documented in the message:

The checksum for a mission, geofence or rally point definition is run over each item in the plan in seq order (excluding the home location if present in the plan), and covers the following fields (in order): frame, command, autocontinue, param1, param2, param3, param4, param5, param6, param7.

If you run that algorithm and the GCS has values for the fields you will get a different result.

FWIW I am not ignoring your set of use cases. These are reasonable things to want to do, but nothing I can do to progress most of these. Most of them would be out of scope for MAVLink - e.g. composing of missions. Not sure how to help with those.

hamishwillee avatar Oct 05 '22 03:10 hamishwillee

It is likely that ArduPilot knows that it doesn't support certain parameters in certain commands and chooses not to store those. SO if the params have values in the GCS mission those will be thrown away.

Yes - the vast majority of mission commands don't contain information anything close to the amount capable of being stored in a full MISSION_ITEM_INT - we get by with storing 14 bytes vs the ~35 bytes of a MISSION_ITEM_INT

It is also possible that some mission items are accepted (don't fail an upload) but discarded. The obvious solution is to include the data even if you don't use it but that is costly.

No, we don't drop mission items. That would do nasty things to your JUMPs, and the downloaded mission would look nothing like the uploaded mission (most people don't realise we lose a lot of fields when storing missions, but they would definitely notice losing items :-) )

peterbarker avatar Oct 05 '22 04:10 peterbarker

Current 2022-10-05 17:05

New theory is that we will not have a mission specific to the checksum, rather inform the GCS of the ID of the mission currently being executed in an existing message. The ID would also be present in the final MISSION_ACK for a mission tansfer (presumably only upon success).

diff --git a/message_definitions/v1.0/common.xml b/message_definitions/v1.0/co
mmon.xml
index 6be74440..a0ac6686 100644
--- a/message_definitions/v1.0/common.xml
+++ b/message_definitions/v1.0/common.xml
@@ -4151,6 +4151,8 @@
     <message id="42" name="MISSION_CURRENT">
       <description>Message that announces the sequence number of the current 
active mission item. The MAV will fly towards this mission item.</description>
       <field type="uint16_t" name="seq">Sequence</field>
+      <extensions/>
+      <field type="uint32_t" name="unique_id">If non-zero, uniquely identifie
s current onboard mission.  This must be the same number emitted in MISSION_CU
RRENT.unique_id</field>
     </message>
     <message id="43" name="MISSION_REQUEST_LIST">
       <description>Request the overall list of mission items from the system/
component.</description>
@@ -4185,6 +4187,7 @@
       <field type="uint8_t" name="type" enum="MAV_MISSION_RESULT">Mission res
ult.</field>
       <extensions/>
       <field type="uint8_t" name="mission_type" enum="MAV_MISSION_TYPE">Mission type.</field>
+      <field type="uint32_t" name="unique_id">If non-zero, uniquely identifies current onboard mission.  This must be the same number emitted in MISSION_CURRENT.unique_id</field>
     </message>
     <message id="48" name="SET_GPS_GLOBAL_ORIGIN">
       <description>Sets the GPS co-ordinates of the vehicle local origin (0,0,0) position. Vehicle should emit GPS_GLOBAL_ORIGIN irrespective of whether the origin is changed. This enables transform between the local coordinate frame and the global (GPS) coordinate frame, which may be necessary when (for example) indoor and outdoor settings are connected and the MAV should move from in- to outdoor.</description>

Revised 2022-10-05 16:17

diff --git a/message_definitions/v1.0/development.xml b/message_definitions/v1.0/development.xml
index fa25cadc..77b98965 100644
--- a/message_definitions/v1.0/development.xml
+++ b/message_definitions/v1.0/development.xml
@@ -22,5 +22,20 @@
       <field type="uint8_t" name="mission_type" enum="MAV_MISSION_TYPE">Mission type.</field>
       <field type="uint32_t" name="checksum">CRC32 checksum of current plan for specified type.</field>
     </message>
+
+    <message id="5353" name="MISSION_UNIQUE_ID">
+      <description>Unique identifier for the currently loaded mission on the autopilot.
+
+      This number should change any time the mission is changed, and should uniquely identify the current mission.  It should be considered opaque to any mavlink node other than the node holding the mission.
+
+      </description>
+      <field type="uint8_t" name="mission_type" enum="MAV_MISSION_TYPE">Mission type.</field>
+      <field type="uint32_t" name="unique_id">Unique ID.</field>
+      <field type="uint8_t" name="source_system">system which last modified this mission, if known.</field>
+      <field type="uint8_t" name="source_component">component of source_system  which last modified this mission, if known.</field>
+   </message>
   </messages>
+
 </mavlink>

Old 2022-10-05 15:27

diff --git a/message_definitions/v1.0/common.xml b/message_definitions/v1.0/common.xml
index 6be74440..af5b6181 100644
--- a/message_definitions/v1.0/common.xml
+++ b/message_definitions/v1.0/common.xml
@@ -4111,6 +4111,7 @@
       <field type="int16_t" name="end_index">End index, equal or greater than start index.</field>
       <extensions/>
       <field type="uint8_t" name="mission_type" enum="MAV_MISSION_TYPE">Mission type.</field>
+      <field type="uint32_t" name="token">Opaque token echoed to source system in some responses.</field>
     </message>
     <message id="39" name="MISSION_ITEM">
       <deprecated since="2020-06" replaced_by="MISSION_ITEM_INT"/>
@@ -4166,6 +4167,7 @@
       <field type="uint16_t" name="count">Number of mission items in the sequence</field>
       <extensions/>
       <field type="uint8_t" name="mission_type" enum="MAV_MISSION_TYPE">Mission type.</field>
+      <field type="uint32_t" name="token">Opaque token echoed to source system in some responses.</field>
     </message>
     <message id="45" name="MISSION_CLEAR_ALL">
       <description>Delete all mission items at once.</description>
diff --git a/message_definitions/v1.0/development.xml b/message_definitions/v1.0/development.xml
index fa25cadc..77b98965 100644
--- a/message_definitions/v1.0/development.xml
+++ b/message_definitions/v1.0/development.xml
@@ -22,5 +22,20 @@
       <field type="uint8_t" name="mission_type" enum="MAV_MISSION_TYPE">Mission type.</field>
       <field type="uint32_t" name="checksum">CRC32 checksum of current plan for specified type.</field>
     </message>
+
+    <message id="5353" name="MISSION_UNIQUE_ID">
+      <description>Unique identifier for the currently loaded mission on the autopilot.
+
+      This number should change any time the mission is changed, and should uniquely identify the current mission.  It should be considered opaque to any mavlink node other than the node holding the mission.
+
+      If the mission has been transfered to the node from some other node, the fields
+      </description>
+      <field type="uint8_t" name="mission_type" enum="MAV_MISSION_TYPE">Mission type.</field>
+      <field type="uint32_t" name="unique_id">Unique ID.</field>
+      <field type="uint8_t" name="source_system">system which last modified this mission, if known.</field>
+      <field type="uint8_t" name="source_component">component of source_system  which last modified this mission, if known.</field>
+      <field type="uint32_t" name="source_token">token supplied by source in one of MISSION_COUNT or MISSION_WRITE_PARTIAL_LIST as part of transfer (if known)</field>
+   </message>
   </messages>
+
 </mavlink>

peterbarker avatar Oct 05 '22 04:10 peterbarker

@timtuxworth a lot of what you desire there is really outside the scope of what the autopilot can be expected to do - particularly in regards to workflows, which are more the purview of a GCS and/or fleet management software.

That being said, @Hwurzburg did a neat little LUA script which allows you to select different missions based on a transmitter switch position: https://github.com/ardupilot/ardupilot/blob/master/libraries/AP_Scripting/applets/MissionSelector.md / https://github.com/ardupilot/ardupilot/blob/master/libraries/AP_Scripting/applets/MissionSelector.lua

peterbarker avatar Oct 05 '22 04:10 peterbarker

@peterbarker Thanks for all this.

No, we don't drop mission items.

Good. If you can't handle a mission you should reject it outright or store it all and reject it when evaluating mission feasibility.

Thanks for the patch report.

I'm not sure you need both source_token and system id/component id. Either of these is sufficient to identify the source. I think you can remove it safely from this and the count and the partial list and just rely on the system/component ID.

Consider. After updating the mission, the flight stack starts emitting MISSION_UNIQUE_ID with the systemid and component id and unique_id.

  • If the GCS has a matching unique_id to the one it has stored it can ignore the system id and component id - because it knows it has a matching mission.
  • If the GCS has a mismatch and has just updated a mission it might check the systemid and component ID to see if they match the target it just updated. If so, then it has sufficient information to update its record of the unique_id.
  • If it hasn't updated the mission and there is a mismatch in the unique id it knows there is a real mismatch in the mission.

The point is that I don't think the source token gives you anything extra here. The system/component ID might change eventually, but should be the only matching thing on the network after you have just updated a mission. Make sense?

This is simpler, because you only have to update the MISSION_UNIQUE_ID

hamishwillee avatar Oct 05 '22 05:10 hamishwillee

@timtuxworth a lot of what you desire there is really outside the scope of what the autopilot can be expected to do - particularly in regards to workflows, which are more the purview of a GCS and/or fleet management software.

The use cases don't specify the solution @peterbarker - only the requirements. But if you think about the solution for these, it's hard to see how many of these requirements could be satisfied by the GCS only, without some cooperation (and changes) to how the autopilot does things. I'm hoping that these considerations can be taken into account. In the end the goal is to make "ArduPilot" (end to end solution, not just the FC) better.

Just a reminder for anyone joining the thread, I've created a forum post here https://discuss.ardupilot.org/t/mission-configuration-data-item/91382 that discusses these requirements. Perhaps I've tagged it wrong, it doesn't seem to be getting much attention :-(

timtuxworth avatar Oct 05 '22 14:10 timtuxworth

Current 2022-10-05 17:05

New theory is that we will not have a mission specific to the checksum, rather inform the GCS of the ID of the mission currently being executed in an existing message. The ID would also be present in the final MISSION_ACK for a mission tansfer (presumably only upon success).

So does this mean there will also be a "MISSION_TYPE" mission item stored in the vehicle? (I'm assuming that, otherwise I don't see where you store the unique_id value and the other values you are suggesting).

If so, you really have just created a cut down version of the "Mission Configuration" I was suggesting, but having done that, why not create a true immutable "unique_id" for the mission, in addition to one that changes when the mission is updated? It would sure help in the long run.

timtuxworth avatar Oct 05 '22 14:10 timtuxworth

@timtuxworth Wr.t. your last message above ...

It is likely that ArduPilot knows that it doesn't support certain parameters in certain commands and chooses not to store those. SO if the params have values in the GCS mission those will be thrown away. It is also possible that some mission items are accepted (don't fail an upload) but discarded. The obvious solution is to include the data even if you don't use it but that is costly.

The algorithm is documented in the message:

The checksum for a mission, geofence or rally point definition is run over each item in the plan in seq order (excluding the home location if present in the plan), and covers the following fields (in order): frame, command, autocontinue, param1, param2, param3, param4, param5, param6, param7.

If you run that algorithm and the GCS has values for the fields you will get a different result.

I don't know the history of this, but just reading this, it seem that if the definition algorithm is changed to only include fields that will be common on the FC and GCS, it should be possible to guarantee the same result for the same mission. What am I missing here?

timtuxworth avatar Oct 05 '22 14:10 timtuxworth

If you run that algorithm and the GCS has values for the fields you will get a different result.

I don't know the history of this, but just reading this, it seem that if the definition algorithm is changed to only include fields that will be common on the FC and GCS, it should be possible to guarantee the same result for the same mission. What am I missing here?

The GCS does not know/should not need to know what parameters the autopilot implements for each and every command. If even one command in the mission has one parameter that is not stored the checksum breaks.

There are ways this might be fixed, but autopilots aren't willing to pay those either. For example, a flight stack might reject any command that has anything other than the "invalid value" for an unused parameter. That way the mission is either stored with a parameter or is invalid and could be ignored by the algorithm. But flight stacks/GCSs generally don't use the defaults properly and much might break while trying to enforce this.

hamishwillee avatar Oct 05 '22 22:10 hamishwillee

@peterbarker Last addition in https://github.com/ArduPilot/ardupilot/pull/20834#issuecomment-1267918001 sounds good.

  • UID returned in MISSION_ACK as extension field. Current GCS knows the ID at that point and can latch it.
  • UID streamed in MISSION_CURRENT as extension field. If it doesn't match GCS value then you know your mission is out of date.
  1. I assume this generalise to MAVFTP based mission upload - i.e. the GCS would get the checksum from MAVFTP for the UID and this is what it would store. This is also what MAVLINK would put in MISSION_CURRENT?
  2. My only concern here w.r.t. the original MISSION_CHECKSUM is that we had separate ids for the parts of the plan - mission, geofence, rally point. The MISSION_ACK is per part of the plan, so you will get a separate ID for each part of the plan, but you only added one ID to MISSION_CURRENT. Thoughts?

hamishwillee avatar Oct 05 '22 22:10 hamishwillee

Probably worth adding in some defines for this functionality. If we add support for Fence/Rally it will grow several hundred more bytes.

Cost:

Board               AP_Periph  blimp  bootloader  copter  heli  iofirmware  plane  rover  sub
Durandal                       1568   *           1568    1568              1568   1568   1568
HerePro             128                                                                   
Hitec-Airspeed      *                                                                     
KakuteH7-bdshot                888    *           888     888               880    888    912
MatekF405                      880    *           880     888               880    888    904
Pixhawk1-1M-bdshot             888                888     888               888    880    904
f103-QiotekPeriph   *                                                                     
f303-Universal      0                                                                     
iomcu                                                           *                         
revo-mini                      888    *           888     880               888    888    912
skyviper-v2450                                    888                                     

peterbarker avatar Jul 01 '23 05:07 peterbarker

@peterbarker How's this progressing? Note that https://github.com/mavlink/mavlink/pull/2029/ is also in play which also affects MISSON_CURRENT.

hamishwillee avatar Oct 03 '23 22:10 hamishwillee