Skip to content

Support mavlink opaque ID#21839

Merged
dagar merged 3 commits intomainfrom
support_mavlink_opaque_id
Nov 29, 2023
Merged

Support mavlink opaque ID#21839
dagar merged 3 commits intomainfrom
support_mavlink_opaque_id

Conversation

@KonradRudin
Copy link
Copy Markdown
Contributor

@KonradRudin KonradRudin commented Jul 12, 2023

Solved Problem

This PR adds support for the Mavlink Mission opaque ID as defined in mavlink/mavlink#2012. It is based on the earlier implementation for the mission checksum support in #18418, but the mission checksum in Mavlink will be deprecated in mavlink/mavlink#2010.

Solution

  • Add an opaque Id for the mission, geofence and rally points. The opaque id is based on a Crc32 checksum as in the implementation in Implement mission checksum #18418
  • Opaque id is send from the System in the MISSION_COUNT on download. On upload, the opaque id is sent on the final MISSION_ACK
  • The opaque Id is also streamed continuously in the MISSION_CURRENT message, such that a GCS can detect plan changes.
  • The opaque id internally also replaces the mission counter, as those serves the same purpose.

Changelog Entry

For release notes:

Feature: Add Support for Mavlink mission opaque Id.

Alternatives

Opqaue id could also just be a counter instead of the Crc32. The hash implementation has the advantage that on loading the same mission (e.g. also from a file) should result in the same Id and thus the potentially multiple connected GCS does not need to download it again. Also it would allow for the feature, that a GCS does not need to download the mission on first connection if the old stored mission and opaque id is the same.

Test coverage

  • Unit/integration test: SITL testing by observing the mavlink message sent from the system.

Context

  • Before this can be merged, the respective mavlink PR should be merged

@KonradRudin KonradRudin force-pushed the support_mavlink_opaque_id branch from ec6f0b5 to 490cd52 Compare July 12, 2023 07:10
@KonradRudin KonradRudin force-pushed the support_mavlink_opaque_id branch 4 times, most recently from 8e4a9fb to 943b8e9 Compare July 14, 2023 15:01
@KonradRudin KonradRudin force-pushed the support_mavlink_opaque_id branch 3 times, most recently from 4ae0101 to ac307bd Compare October 17, 2023 10:39
Comment thread .gitmodules Outdated
Comment thread msg/Mission.msg Outdated
Comment thread src/modules/mavlink/mavlink_mission.cpp
Comment thread src/modules/mavlink/mavlink_mission.cpp Outdated
@hamishwillee
Copy link
Copy Markdown
Contributor

hamishwillee commented Nov 14, 2023

@KonradRudin Looks like this is getting close. Let me know when it is ready to merge and you have test results/stuff we can use to validate this (so that I can push for merging of the MAVLink changes).

dagar
dagar previously approved these changes Nov 20, 2023
@dagar
Copy link
Copy Markdown
Member

dagar commented Nov 20, 2023

As soon as mavlink/mavlink#2012 is merged we can update the mavlink submodule and bring this in.

@KonradRudin KonradRudin dismissed stale reviews from dagar and ThomasDebrunner via 216dc7a November 20, 2023 19:53
@KonradRudin KonradRudin force-pushed the support_mavlink_opaque_id branch from 486aa7e to 216dc7a Compare November 20, 2023 19:53
@KonradRudin
Copy link
Copy Markdown
Contributor Author

rebased on main again

@KonradRudin
Copy link
Copy Markdown
Contributor Author

@hamishwillee This PR was discussed now in the maintainer call and in agreement wirh @dagar . This side is ready now

@KonradRudin KonradRudin force-pushed the support_mavlink_opaque_id branch from 216dc7a to 34e7d85 Compare November 29, 2023 10:16
@KonradRudin
Copy link
Copy Markdown
Contributor Author

Upstream mavlink is merged now. Update the mavlink submodule

@KonradRudin KonradRudin force-pushed the support_mavlink_opaque_id branch from 34e7d85 to 096098b Compare November 29, 2023 12:32
@KonradRudin KonradRudin marked this pull request as ready for review November 29, 2023 12:37
@KonradRudin KonradRudin requested a review from dagar November 29, 2023 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

5 participants