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

Follow up to RTL based on remaining flight time #18711

Merged
merged 2 commits into from
Nov 30, 2021

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Nov 24, 2021

Describe problem solved by this pull request
Follow up to the review here: #18646 (review)

Describe your solution

  • Message for the user saying "Remaining flight time low" instead of "Flight time low"
  • Add \t at the end of the deprecated mavlink_log_emergency() message to make it not show in newer ground stations that support the events interface
  • Publish an appropriate event over the events interface
  • Use a round number of 100s as the default safety margin for flight time still have left after the full RTL
  • Increase the maximum value for the time margin parameter to 60 minutes to account for vehicles that are capable of flying multiple hours

Test data / coverage
I did not specifically test these changes yet.

@@ -3662,10 +3662,14 @@ void Commander::battery_status_check()
// Try to trigger RTL
if (main_state_transition(_status, commander_state_s::MAIN_STATE_AUTO_RTL, _status_flags,
_internal_state) == TRANSITION_CHANGED) {
mavlink_log_emergency(&_mavlink_log_pub, "Flight time low, returning to land");
mavlink_log_emergency(&_mavlink_log_pub, "Remaining flight time low, returning to land\t");
events::send(events::ID("commander_remaining_flight_time_rtl"), events::Log::Emergency,
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't Emergency a bit too extreme? For comparison, the legacy "RTL due to critical battery" triggers an event with:
events::Log::Critical, events::LogInternal::Info

Copy link
Member Author

Choose a reason for hiding this comment

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

You're completely right, thanks! I compared to

events::send(events::ID("commander_bat_crit_rtl"), {events::Log::Critical, events::LogInternal::Info},
which should have the same severity and it's like you describe externally Critical, internally Info.

I corrected the commit directly.

@MaEtUgR MaEtUgR force-pushed the rtl-remaining-flight-time-follow-up branch from a40de3b to 7fc3ba2 Compare November 30, 2021 08:49
@MaEtUgR
Copy link
Member Author

MaEtUgR commented Nov 30, 2021

@sfuhrer I rebased on master and corrected the severity level. Sorry for the delay.

@sfuhrer sfuhrer merged commit b59db7d into master Nov 30, 2021
@sfuhrer sfuhrer deleted the rtl-remaining-flight-time-follow-up branch November 30, 2021 18:46
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.

2 participants