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

Proposal: Deprecate all non MISSION_*_INT usage with a breaking change #1285

Closed
DonLakeFlyer opened this issue Dec 24, 2019 · 9 comments · Fixed by #1389
Closed

Proposal: Deprecate all non MISSION_*_INT usage with a breaking change #1285

DonLakeFlyer opened this issue Dec 24, 2019 · 9 comments · Fixed by #1389
Labels

Comments

@DonLakeFlyer
Copy link
Contributor

As it currently stands there is a hole in the mavlink spec with respect to _INT usage. There is a capability bit on the vehicle side. But there is no clear way to know whether the GCS side will support uploads of MISSION_ITEM_INT items. The only sideways sort of way to do that would be for the vehicle to remember if the GCS did a MISSION_REQUEST_INT. To both deal with that and remove complexity from the mavlink spec I propose a breaking change to only allow support for the _INT variants of the protocol.

  • This would mark MISSION_ITEM, MISSION_REQUEST and MAV_PROTOCOL_CAPABILITY_MISSION_INT as deprecated.
  • A gcs is required to support the _INT based protocol sequence.
  • If a gcs receives old non _INT requests it always respond with _INT responses.This provides an interim transition path to the future while these messages still exist. I've tested this in QGC and it works fine against PX4 and ArduPilot.

This reflects the reality of PX4/ArduPilot/QGC/MP of today which supports the _INT based protocol. I don't see any reason to maintain the non _INT protocol moving forward. So gets reduce complexity and get rid of it.

Related to #8122

@LorenzMeier
Copy link
Member

That makes total sense. We need to tighten up the overall protocol.

@hamishwillee hamishwillee added Dev Call Issues to be discussed during the Dev Call and removed Dev Call Issues to be discussed during the Dev Call labels Jan 6, 2020
@hamishwillee
Copy link
Collaborator

@auturgy Any comment?

@hamishwillee
Copy link
Collaborator

@auturgy My understanding of your comments in dev call was that:

  • The problem was caused by ArduPilot not implementing the spec correctly. QGC works around this problem already for ardupilot. If people other implement the protocol correctly there will be no problem. So there isn't much value in doing this.
  • That @WickedShell uses the float versions.

Is that correct?

To me the value (to me) in this is simplifying the work for future implementations. Essentially a docs change that removes the old non-INT path. I believe I understand your objection as "not best use of time" rather than "bad idea". Can you please comment directly, because if it is the former, then I'd like to do the work.

@WickedShell
Copy link
Collaborator

I should clarify that, I don't use the float versions, but it can be argued that they actually better represent some situations such as local NED much better then the int does. (Not that most systems have sensors that can read that accurately)

@DonLakeFlyer
Copy link
Contributor Author

To me the value (to me) in this is simplifying the work for future implementations.

Yup, that is why I proposed it. Simpler is better.

@auturgy
Copy link
Collaborator

auturgy commented Feb 5, 2020

Lets move to a PR and have a look at the impact - I think it'll be ok, as the lost local NED precision isn't useable anyway

@amilcarlucas
Copy link
Collaborator

amilcarlucas commented Apr 2, 2020

Isn't the only message that uses both _INT and NED variant the old Gimbal messsage?

@stale
Copy link

stale bot commented Jun 1, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Jun 1, 2020
@hamishwillee hamishwillee added pinned Dev Call Issues to be discussed during the Dev Call labels Jun 2, 2020
hamishwillee added a commit that referenced this issue Jun 2, 2020
@hamishwillee hamishwillee removed the Dev Call Issues to be discussed during the Dev Call label Jun 2, 2020
@stale stale bot removed the stale label Jun 2, 2020
@hamishwillee
Copy link
Collaborator

To progress this, PR created. See #1389

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants