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

AP_ExternalAHRS: Provide warning for init failure #26601

Merged

Conversation

Ryanf55
Copy link
Collaborator

@Ryanf55 Ryanf55 commented Mar 23, 2024

Purpose

  • If the device hasn't initialized in 5 seconds, give a warning that it failed. Without this, you just fail pre-arm checks. Before, if the device was disconnected, this would say it initialized, when in reality only the driver was constructed.
  • A failure of the AHRS system to read any data was reported as a failure to get the origin, even if the backend knew that it was something more fundamental, such as no data received. After this PR, the backend gets to report pre-arm failures before origin failures.

No cable connected

image

Cable connected and data streaming configured

image

Other thoughts

perhaps we should keep track of the initialization state, and give a success when it changes to true (event-based)? I did this in a follow up commit. If the devs like it, I'll squash it together. Pete suggested reordering the pre-arm checks to check the backend before verifying the origin. I liked it, so that's incorporated.

tridge
tridge previously requested changes Mar 24, 2024
Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

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

better to add a pre arm failure

@Ryanf55
Copy link
Collaborator Author

Ryanf55 commented Mar 24, 2024

better to add a pre arm failure

Looks like if the device is not connected, you get the warning of ExternalAHRS: No origin.
MicroStrain knows why, but it can't share that info with the user because AP_ExternalAHRS::pre_arm_check already returned.

Do you have any way we could send multiple failures for the pre-arm check?

Here's the behavior of what's currently pushed when the MicroStrain device is disconnected.
image

@Ryanf55 Ryanf55 added the For-4.5 Planned for 4.5 release label Mar 24, 2024
@@ -238,7 +238,6 @@ bool AP_ExternalAHRS::pre_arm_check(char *failure_msg, uint8_t failure_msg_len)

if (!state.have_origin) {
hal.util->snprintf(failure_msg, failure_msg_len, "ExternalAHRS: No origin");
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this failure get lost now?

Maybe it would be better to re-order so that the backend pre-arms come before the origin check?

Copy link
Collaborator Author

@Ryanf55 Ryanf55 Mar 24, 2024

Choose a reason for hiding this comment

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

Doesn't this failure get lost now?

Maybe it would be better to re-order so that the backend pre-arms come before the origin check?

Yea, a re-order would solve it. The origin problem is likely to make the other ones get lost.

Copy link
Collaborator Author

@Ryanf55 Ryanf55 Mar 25, 2024

Choose a reason for hiding this comment

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

Yea, totally. I re-ordered it, and it works great now! Easy fix.

* If the device hasn't initialized in 5 seconds, give a warning

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
* Add generic health and time utils
* Fix bug only checking first GNSS system
* Use common logging struct
* Improve pre-arm log checks

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@Ryanf55 Ryanf55 force-pushed the microstrain7-initialize-failure-warning branch from 46cc7b9 to df6ee02 Compare March 25, 2024 00:01
* This allows the backend to report more detailed errors
* Before this, many pre-arm errors were hidden by origin failure
* If pre-arm could report multiple errors, that would be ideal

Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
@Ryanf55 Ryanf55 force-pushed the microstrain7-initialize-failure-warning branch from df6ee02 to 92a60d0 Compare March 25, 2024 00:05
@tridge tridge merged commit 35451c7 into ArduPilot:master Mar 26, 2024
91 checks passed
@rmackay9
Copy link
Contributor

This is included in 4.5.2-beta1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ExternalAHRS For-4.5 Planned for 4.5 release
Projects
Status: 4.5.2-beta1
Development

Successfully merging this pull request may close these issues.

None yet

5 participants