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(a380x): fix speed margin display above crossover altitude #9861

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BlueberryKing
Copy link
Member

Fixes #[issue_no]

Summary of Changes

The speed margins are upper-limited by VMO and MMO. Previously these values were hardcoded to the A320 values (350 kts and M.82). This PR makes them configurable per aircraft type.

Screenshots (if necessary)

Before:
image
After:
Screenshot 2025-02-14 165742

References

Additional context

Discord username (if different from GitHub):

Testing instructions

How to download the PR for QA

Every new commit to this PR will cause new A32NX and A380X artifacts 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, find and click on the PR Build tab
  4. Click on either flybywire-aircraft-a320-neo, flybywire-aircraft-a380-842 (4K) or flybywire-aircraft-a380-842 (8K) download link at the bottom of the page

@BlueberryKing BlueberryKing added the A380X Related to the A380X aircraft label Feb 14, 2025
Comment on lines +53 to +61
/**
* The maximum operating speed in knots
*/
VMO: number;

/**
* The maximum operating Mach number
*/
MMO: number;
Copy link
Member

@tracernz tracernz Feb 15, 2025

Choose a reason for hiding this comment

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

Suggested change
/**
* The maximum operating speed in knots
*/
VMO: number;
/**
* The maximum operating Mach number
*/
MMO: number;
/**
* The maximum operating speed in knots
*/
Vmo: number;
/**
* The maximum operating Mach number
*/
Mmo: number;

Our current style.
-e- hmm, well the rest of it is for some reason upper-snake-case? 😕 That wasn't ever FBW style.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we have this convention written down somewhere? I also prefer PascalCase for these parameters that do not change at runtime, but it does not appear that this has been enforced so far.

Copy link
Member

Choose a reason for hiding this comment

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

We should probably write them down, yeah.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have opened a separate PR to convert them #9865. It would be ideal to have a lint rule for this, but I have not found one that does this.

@BlueberryKing BlueberryKing force-pushed the fix-a380x-speed-margins branch from fc706d3 to 0c16431 Compare March 18, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A380X Related to the A380X aircraft QA Ready to Test
Projects
Status: 🟣 QA Review: Ready to Test
Development

Successfully merging this pull request may close these issues.

3 participants