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

fix(fwc): Fixes the Master Warning on startup #8267

Merged

Conversation

Maximilian-Reuter
Copy link
Member

@Maximilian-Reuter Maximilian-Reuter commented Oct 24, 2023

Fixes #[issue_no]

Summary of Changes

On startup in C/D the aircraft plays a master warning until the startup EWD messages are empty. This was caused by the pseudo FWC checking whether the aircraft was on ground only after checking for dual engine failure which meant that the FWC didn't update the on ground status until after the check for dual engine failure and since the on ground status was initialized with false, the dual eng failure triggered on initial load.

This PR changes the order of those checks so that the setting of the on ground status happens first.

Screenshots (if necessary)

References

Additional context

Discord username (if different from GitHub): _chaoz_

Testing instructions

just a quick fix that shouldn't affect anything beyond the fwc initialization.

  1. start C/D
  2. power up the aircraft
  3. no master warning should occur

How to download the PR for QA

Every new commit to this PR will cause a new A32NX artifact to be created, built, and uploaded.

  1. Make sure you are signed in to GitHub
  2. Click on the Checks tab on the PR
  3. On the left side, click on the bottom PR tab
  4. Click on the A32NX download link at the bottom of the page

@Saschl Saschl added this to the v0.11.0 milestone Oct 25, 2023
@@ -1594,9 +1597,11 @@ export class PseudoFWC {
}

if (value.failure === 3) {
console.log(`Playing Master warning Source:${key}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls remove the logs and we're good to go :)

Copy link
Member Author

Choose a reason for hiding this comment

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

duh, knew I forgot smth

@Saschl
Copy link
Contributor

Saschl commented Oct 25, 2023

Also might be worth a changelog entry I think. Would go to 0.11 too imo EDIT: Tracers suggestion is better

@tracernz
Copy link
Member

Also might be worth a changelog entry I think. Would go to 0.11 too imo

I’d suggest keeping all the master changelogs under 0.12 and once the 0.11 release is done tidy the cherry-picked ones up all at once.

@Maximilian-Reuter
Copy link
Member Author

Also might be worth a changelog entry I think. Would go to 0.11 too imo EDIT: Tracers suggestion is better

is this even an issue on stable right now ?

@tracernz
Copy link
Member

Also might be worth a changelog entry I think. Would go to 0.11 too imo EDIT: Tracers suggestion is better

is this even an issue on stable right now ?

0.11 stable is master as of 2 days ago.

@Saschl
Copy link
Contributor

Saschl commented Oct 26, 2023

QA Report

Discord: saschl
Object of testing:
Tier of Testing : 1
Date : 26/10/2023

Testing Process:

Loaded the plane and powered up the plane. No master warning audible.
Slewed into the air with engines off. Master warning comes up as expected.

Testing Results:
Passed

@Saschl Saschl enabled auto-merge (squash) October 26, 2023 15:07
@Saschl Saschl merged commit b05a4c5 into flybywiresim:master Oct 26, 2023
6 checks passed
Saschl pushed a commit that referenced this pull request Oct 26, 2023
* Fixes the issue

* removed leftover logging
@2hwk 2hwk modified the milestones: v0.11.0, v0.11.2 Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✔️ Done
Development

Successfully merging this pull request may close these issues.

4 participants