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

common: extend MISSION_CURRENT #1869

Merged
merged 9 commits into from
Aug 18, 2022
Merged

common: extend MISSION_CURRENT #1869

merged 9 commits into from
Aug 18, 2022

Conversation

julianoes
Copy link
Collaborator

The message MISSION_CURRENT is a bit limited as it sends the current mission item sequence but no context.

My proposition would be to include the total as well as the status of an ongoing mission.

Related to #1868.

The message MISSION_CURRENT is a bit limited as it sends the current
mission item sequence but no context.

My proposition would be to include the total as well as the status of an
ongoing mission.
@hamishwillee
Copy link
Collaborator

@peterbarker Would appreciate your comment on this.

At the moment you can infer some of the states of the mission from the mode (particularly in ArduPilot where pause is a sub mode of auto). This is not so easy in PX4 where "pause = hold mode" (except after an "autocontinue" pause). I don't think it is easy in either case to know if a mission has actually completed.

So this adds state information to the MISSION_CURRENT, along with the total number of mission items, so a user doesn't need to separately query the mission for that.

@julianoes julianoes marked this pull request as ready for review August 10, 2022 10:16
@hamishwillee
Copy link
Collaborator

hamishwillee commented Aug 12, 2022

Mission states are a little complex and we may need to rethink this to make sure that we're sending the right information. What that information depends on who/how this is consumed. In summary though

Summary, IMO

  • States reflect mission engine when it is running: unknown, no-mission, not-started, active, paused, finished.
  • If you're in another mode the mission engine is "suspended" - this is a useful distinction or you need to have multiple states to also cover the suspended case. You might infer this from the current mode, but I think we should expose this as a field to avoid the system having to know anything about autopilot modes. It is effectively "not mission mode or submode".

This is a change from having an option "Paused not in auto". There is a difference between saying "Pausing a mission mission moves it into hold mode" and "PX4 implements pause as hold". I THINK we are the former - certainly the mode is advertised as Hold in QGC.

Make sense? There is some useful info about what the systems do below.


TLDR;

The "reality" is that we have a mission state machine that might have states "like[+]":

  • mission not uploaded
  • mission not yet started - mission uploaded and all jump counters[++] reset. But has not yet been started.
  • mission running - mission is executing mission items
  • mission paused - mission is pausing within the context of the mission mode - ie in a submode.
  • mission complete - stopped

[+]: I say "like" because some implementations use the same "state" mission stopped for mission not yet started and complete. Further, plane does not support paused, even in theory.
[++] Jump counters are separate from state.

Then on top of this we have the case where a mission may be suspended - i.e. you're in another mode like "Hold". In this case. The mission is not running at all but the state still exists and might be re-entered.

ArduCopter matches those states precisely (as sub modes).

  • so if you are paused you're still in mission mode. If moving you'll move to a graceful stop.
  • If you leave the mission to another mode it is suspended. Say you move to hold, you'd immediately hold rather than gracefully stop.
  • FYI you can't start mission from the initial state by calling the set mission current
  • ArduPilot does not implement autocontinue flag.

ArduPlane does not implement the mission paused state; you cannot pause mission, you can only suspend it by changing mode.

  • The pause/continue command is not implemented because it doesn't make sense with this paradigm. Though continue might, if we assumed that this also requires a switch-to-mode.
  • If you run out of items in the mission (complete) the internal state changes to mission-stop and the mode is forced to RTL. Sending message to set the current mission item sets the state back to active - you then have to change the mode back to resume.
  • The plane never wants to say it is paused.
  • The plane state is almost completely inferrable - you're either in mission mode and executing or stopped or you are in some other mode.
  • FYI if in auto you calling the set mission current will start the mission. But semantically being in auto means "you're running the mission" on plane, so this is expected.

PX4 does a mix of these two, implementing both pause sub modes and emulating pause-as-switch to mode:

  • It has mission sub mode for the pause state when autocontinue=false
  • It has mission sub mode for the complete state (right?).
  • Pausing a mission is implemented as moving to hold mode. But this is not done using a pause, but using a do_reposition.
  • Restarting a mission is implemented as moving back to mission mode.
  • Not sure/ do we know how PX4 sees an explicit mode change vs pause when running a mission? i.e. does it think of itself as in hold mode, or does it somehow know that actually it is in a paused mission mode?

So:

  • Arduplane might want to output states: unknown, no-mission, not-started, active, complete (internally, both "not started" and "complete" are "stopped").
  • ArduCopter and PX4 might want to output states: unknown, no-mission, not-started, active, paused, finished.

tridge
tridge previously requested changes Aug 15, 2022
message_definitions/v1.0/common.xml Outdated Show resolved Hide resolved
<extensions/>
<field type="uint16_t" name="total" invalid="UINT16_MAX">Total number of mission items. 0: Not supported, UINT16_MAX if no mission is present on the vehicle.</field>
<field type="uint8_t" name="status" enum="MISSION_FSM_STATE" invalid="0">Mission finite state machine state. MISSION_FSM_STATE_UNKNOWN if state reporting not supported.</field>
<field type="uint8_t" name="mission mode" minValue="0" maxValue="2" increment="1" invalid="0">Vehicle is in a mode that can execute mission items (not suspended). 0: Unknown, 1: Mission mode, 2: Not in mission mode (suspended).</field>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@julianoes Do you think this needs to be an enum?

@hamishwillee
Copy link
Collaborator

@julianoes @tridge I updated so all fields are 0 for the default "not supported" case.

In addition, have modified the reporting of state in line with the notes above. It now reports state using two fields. The first is the state of the mission state machine. The second is whether or not the mission is suspended (worded as whether it is in mission mode or not). That allows a recipient to absolutely know the what the mission state is.

Is it worth considering also reporting on whether jump counters are reset? If so, would making the last byte into an set of flags that includes suspended and jump counters be a better implementation?

@tridge
Copy link
Member

tridge commented Aug 17, 2022

@hamishwillee thanks for the changes, looks much better

I removed the finite state machine reference as I would consider that
the implementation rather than what should be in the spec. I also
renamed the field from status to mission_state while at it.

Also, I fixed the mission mode field name adding an underscore.
@hamishwillee hamishwillee dismissed tridge’s stale review August 18, 2022 02:22

This was addressed

@hamishwillee
Copy link
Collaborator

As discussed in dev call, this has been reviewed by ArduPilot and PX4 and can be merged.

Note that this:

  • adds extension fields to the previous mission in a way such that there is no change for systems that have not updated to emit the new information.
  • The previous definition said "emit on changed waypoint" while the new one says "stream". This reflects what both PX4 and ardupilot actually do. It is not inconceivable some other system might have implemented this exactly per the old spec.

Thanks for this @julianoes , and @tridge for helping me understand how ArduPlane works with missions.

@hamishwillee hamishwillee merged commit 76b794c into master Aug 18, 2022
@hamishwillee hamishwillee deleted the pr-current-extension branch August 18, 2022 02:27
@hamishwillee
Copy link
Collaborator

FYI @TSC21

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.

None yet

3 participants