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

Use common GCS functions from superclass #6553

Closed
wants to merge 20 commits into from

Conversation

peterbarker
Copy link
Contributor

No description provided.

Copy link
Member

@OXINARF OXINARF left a comment

Choose a reason for hiding this comment

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

Besides inline comments, looks good.

Any reason for MAVLINK_MSG_ID_MISSION_ACK not to be moved to common too?

/* fall through */
case MAVLINK_MSG_ID_MISSION_SET_CURRENT:
/* fall through */
case MAV_CMD_MISSION_START:
Copy link
Member

Choose a reason for hiding this comment

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

This seems wrong. Message ID will never be a command ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's... odd. Fixed.

break;
}

case MAVLINK_MSG_ID_MISSION_ITEM_INT:
Copy link
Member

Choose a reason for hiding this comment

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

Any reason the code below is duplicated from above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the way it came in from the donor library. I did see it, but decided I was moving code, not fixing it :-)

I'll add a patch on top to remove the duplication.

Copy link
Member

Choose a reason for hiding this comment

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

That's funny because I noticed this because I saw one of the vehicles not having the duplicated code 🙂

@peterbarker
Copy link
Contributor Author

@OXINARF I've moved the MISSION_ACK stuff up now.

@OXINARF
Copy link
Member

OXINARF commented Jul 10, 2017

@peterbarker Do you mind if I squash the last 4 commits?

@magicrub Again, why dev call topic? This is basically a non-functional change...

@peterbarker
Copy link
Contributor Author

@OXINARF Not at all - squash away!

@OXINARF
Copy link
Member

OXINARF commented Jul 12, 2017

As agreed, squashed and merged, thanks!

@OXINARF OXINARF closed this Jul 12, 2017
@peterbarker peterbarker deleted the gcs-remove-funcs branch July 20, 2017 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants