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

Standardising mavlink message strings #11796

Merged
merged 2 commits into from
Apr 10, 2019
Merged

Conversation

potaito
Copy link
Contributor

@potaito potaito commented Apr 5, 2019

I tried to standardise the mavlink message strings a tiny bit:

  • No ALL CAPS words (even when the situation is really bad. But let's face it: A lot of messages can indicate something really bad)
  • Begin message with a capital letter
  • No punctuation character at end of message
  • Minor English adjustments

I understand that this is a matter of taste to some extent, but let's see :)

@@ -1038,7 +1038,7 @@ Commander::handle_command(vehicle_status_s *status_local, const vehicle_command_
// if no high latency telemetry exists send a failed acknowledge
if (_high_latency_datalink_heartbeat > commander_boot_timestamp) {
cmd_result = vehicle_command_s::VEHICLE_CMD_RESULT_FAILED;
mavlink_log_critical(&mavlink_log_pub, "Control high latency failed, no hl telemetry available");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This guy exceeded 50 characters before the change.

Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

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

THAT'S SO MUCH NICER! 😄

@@ -2118,7 +2118,7 @@ Commander::run()

} else {
if (!status_flags.rc_input_blocked && !status.rc_signal_lost) {
mavlink_log_critical(&mavlink_log_pub, "MANUAL CONTROL LOST (at t=%" PRIu64 "ms)", hrt_absolute_time() / 1000);
mavlink_log_critical(&mavlink_log_pub, "Manual control lost (at t=%" PRIu64 "ms)", hrt_absolute_time() / 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion we should remove the timestamp here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine by me. I always wondered why this message should be exceptional 😅

}

if (_datalink_last_status_avoidance_system == telemetry_status_s::MAV_STATE_FLIGHT_TERMINATION) {
mavlink_log_critical(&mavlink_log_pub, "Avoidance system abort");
mavlink_log_critical(&mavlink_log_pub, "Avoidance system aborted");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mrivi I assume you know this: What's the meaning of aborted here? Is autonomous flight terminated and the flight controller stops listening to obstacle avoidance? Or is something wrong with the obstacle avoidance? I assume it's the former and not the latter. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

It means that there is something wrong with the avoidance and the vehicle doesn't accept anymore any setpoint from the OA system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we then say "Avoidance system rejected" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine for me

@dagar dagar requested a review from Antiheavy April 5, 2019 14:22
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Looks more user friendly 👍

@bkueng bkueng merged commit 4127adf into master Apr 10, 2019
@bkueng bkueng deleted the pr-user-friendly-commander-strings branch April 10, 2019 07:38
@hamishwillee
Copy link
Contributor

Late to the party. This is great!

@potaito
Copy link
Contributor Author

potaito commented May 3, 2019

Thanks @hamishwillee 😁

potaito added a commit that referenced this pull request Jun 17, 2019
potaito added a commit that referenced this pull request Jun 17, 2019
dagar pushed a commit that referenced this pull request Jun 17, 2019
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.

6 participants