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

Command API does not sufficiently abstract MAVLink_mission_item_message #67

Open
hamishwillee opened this issue Apr 21, 2015 · 1 comment
Assignees

Comments

@hamishwillee
Copy link
Contributor

The Command object is, I believe, a thin wrapper around a mavutil.mavlink.MAVLink_mission_item_message. It allows you to create a command object like this:

cmd = Command(0,
      0,
      0,
      mavutil.mavlink.MAV_FRAME_GLOBAL_RELATIVE_ALT,
     mavutil.mavlink.MAV_CMD_NAV_WAYPOINT,
      0, 0, 0, 0, 0, 0,
       lat, lon, altitude)
cmds.add(cmd)

The values of the parameters are the same as the mavutil.mavlink.MAVLink_mission_item_message and I believe this to be a bug.

The problem is that I don't believe that most of these can or should be set by the user. It may be that they are already correctly set by the system, but IMO they should also be hidden. If we can't hide them we need to clearly state what values can be set, and this is not documented anywhere!

Specifically

  • target_system
    • - each part of a mavlink system gets a "target system" id for routing. This includes every UAV, GSCs etc. The number given has a default but is configurable
    • 0 is used for broadcast - goes to all systems in the network. For companion computers we would use this.
    • The thing is though, in practise a user can only get a command from the API for a specific Vehicle. This means that the target system is already known, and the user should not have to set it.
  • target_component:
    • This is a subsytem id within a target MAVLink system. Typically it is just the pixhawk, and you might as well use 0 (broadcast value)
    • It is possible for this to be an individual component like the camera.
    • Arguably this is useful, but again, the vehicle will know what subsystems it has (if any) and which commands can possibly be relevant to those subsystems.
  • seq:
    • The sequence number. When missions are uploaded to the autopilot, any command out of sequence is rejected. Given the fact that these are added etc in order by the system, there is no reason for the number not to be assigned automatically on upload.
  • frame - this is useful, but I will have to document.
  • current: current - Not intended for use in missions (represents the "curent command" but since we upload set of missions and ardupilot decides which we're up to is meaningless)
  • autocontinue: Intended to mark the end of a mission in the end of mission. Not implemented.

So in summary, lets hide target_system, target_component, seq, current, autocontinue from the API.

@hamishwillee
Copy link
Contributor Author

@mrpollo You indicated that target_system, target_component, sequence number are handled by the API. As understand it you disagreed with removing them from the API because even though they aren't particularly useful right now (and are not necessarily implemented in Autopilots) there is no reason they might not be useful in future. I see your point.

I'm keeping this open and assigning to you because AFAIK there is still no real purpose for exposing: seq (the mission handling code should always handle this), current and autocontinue are both not used/meaningless and unlikely to ever be useful. If you still think this is not needed, then feel free to close.

@hamishwillee hamishwillee assigned mrpollo and unassigned hamishwillee May 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants