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

Sub motor test fixes #9243

Merged
merged 5 commits into from
Dec 16, 2020
Merged

Conversation

Williangalvani
Copy link
Contributor

@jaxxzer jaxxzer self-requested a review December 8, 2020 17:32
Copy link
Collaborator

@jaxxzer jaxxzer left a comment

Choose a reason for hiding this comment

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

LGTM

sendMavCommand(_defaultComponentId, MAV_CMD_DO_MOTOR_TEST, true, motor, MOTOR_TEST_THROTTLE_PERCENT, percent, timeoutSecs, 0, MOTOR_TEST_ORDER_BOARD);
// Sub requires periodic motortest messages, and we send them at a high rate to improve responsiveness.
// On higher latency connection, `showError=true` caused lots of "Unable to send command: Waiting on previous response to same command." errors.
bool showError = !sub();
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't correct. This is an ArduSub problem not a "sub" problem isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. What do you think of just exposing showError on the signature void Vehicle::motorTest(int motor, int percent, int timeoutSecs, bool showError) and letting qml decide it?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, updated!

@jaxxzer jaxxzer added this to the Release V4.1 milestone Dec 16, 2020
@DonLakeFlyer DonLakeFlyer merged commit 8ad94d6 into mavlink:master Dec 16, 2020
@Williangalvani Williangalvani deleted the submotortest branch December 19, 2020 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants