Skip to content

Fix updating mission item total #2667

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 6, 2025
Merged

Fix updating mission item total #2667

merged 1 commit into from
Oct 6, 2025
+12 −3

Conversation

irajkovic
Copy link
Contributor

Total number of items in a mission should only be updated when mission is downloaded. Downloading geofence or rally points should not affect this total.

@irajkovic
Copy link
Contributor Author

Just as a clarification, function convert_items is also called when downloading geofence, so number of points in geofence would be used to set the number of items in a mission.

@JonasVautherin
Copy link
Collaborator

Thanks! Would you mind fixing the style with ./tools/fix_style.sh?

@irajkovic irajkovic force-pushed the fix-mission-progress-total branch from 12b6bbf to 68ffa73 Compare September 30, 2025 13:23
@irajkovic
Copy link
Contributor Author

Pushed forced after reformatting the code. I only formatted my changes though, clang-format also reformatted unrelated file: ./tools/fix_style.sh that I didn't want to include in this commit as it's unrelated. Let me know if it's ok now.

JonasVautherin
JonasVautherin previously approved these changes Sep 30, 2025
View reviewed changes
Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

Sounds reasonable to me, but I'll let @julianoes review 😇

julianoes
julianoes reviewed Oct 4, 2025
View reviewed changes
src/mavsdk/plugins/mission_raw/mission_raw_impl.cpp Outdated
auto converted_result = convert_result(result);
auto converted_items = convert_items(int_items);

{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{
if (converted_result == MissionRaw::Result::Success) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for suggestion, added to the original commit.

src/mavsdk/plugins/mission_raw/mission_raw_impl.cpp Outdated
auto converted_result = convert_result(result);
auto converted_items = convert_items(items);

{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{
if (converted_result == MissionRaw::Result::Success) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

@irajkovic
Fix updating mission items total
40b02a7 
Total number of items in a mission should only be updated when
mission is downloaded. Downloading geofence or rally points
should not affect this total.
@irajkovic irajkovic force-pushed the fix-mission-progress-total branch from 68ffa73 to 40b02a7 Compare October 5, 2025 11:46
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Oct 5, 2025

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@JonasVautherin JonasVautherin mentioned this pull request Oct 5, 2025
Merged
@julianoes
Copy link
Collaborator

Thanks @irajkovic.

@julianoes julianoes merged commit fc0d3a5 into mavlink:main Oct 6, 2025
53 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julianoes julianoes julianoes approved these changes

@JonasVautherin JonasVautherin JonasVautherin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@irajkovic @JonasVautherin @julianoes