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

Vehicle: ignore severity of PX4 preflight messages #12170

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaEtUgR
Copy link
Collaborator

@MaEtUgR MaEtUgR commented Dec 4, 2024

Problem

We had someone change this condition out of confusion downstream and I figured out there is no message with emergency severity containing the string "preflight" in the current PX4 version. I can still check if and where they might have existed on older versions but I'd suggest to remove this since for the events it doesn't make sense to sort the text by severity.

Solution

Ignore the severity. I think this was added originally such that emergency messages get output as yellow box and spoken even though they had the word "preflight" in it which usually resulted in a transparent box over the map.

Test Steps

I have not tested the change but went through the existing cases and it shouldn't make a difference with the current version of PX4.

Checklist:

  • Check what messages ever fulfilled this condition.

We had someone change this condition out of confusion downstream and I figured out there is no message with emergency severity containing the string "preflight" in the current PX4 version. I can still check if and where they might have existed on older versions but I'd suggest to remove this since for the events it doesn't make sense to sort the text by severity.
@MaEtUgR MaEtUgR self-assigned this Dec 4, 2024
@DonLakeFlyer
Copy link
Contributor

That severity check certainly seems odd. Could you also clean this up such that the two bools first check the firmware type on the vehicle as well. They are firmware specific tests so they should only run on vehicles with that firmware.

So for example:
const bool ardupilotPrearm = apmFirmware() && text.startsWith(QStringLiteral("PreArm"));

@DonLakeFlyer DonLakeFlyer self-requested a review December 14, 2024 21:27
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