fix(fms): use entered fob rather than fqi before start #8664

patmack14 · 2024-05-27T02:51:41Z

Fixes #[issue_no]

Summary of Changes

Added 2 lines:
1.A check to see if an engine is running->Pull from sensed fuel tank levels.
2. Engine !Running -> Pull from entered block fuel
3. Also added some class and method comments noting where top level parameters originate from...spent many hours on a wild goose chase on this PR! Recommend inserting into docs somewhere down the line.

Screenshots (if necessary)

References

@BlueberryKing and @BravoMike99.
As previously stated I no longer have access to the pilot Discord channel, unable to use previous verification method.

Additional context

Discord username (if different from GitHub):

Testing instructions

Prior to engine start, the Destination EFOB should be pulling from entered block fuel, after start the figure should switch over to using sensed fuel level in the tanks.

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.

Make sure you are signed in to GitHub
Click on the Checks tab on the PR
On the left side, find and click on the PR Build tab
Click on either flybywire-aircraft-a320-neo or flybywire-aircraft-a380-842 download link at the bottom of the page

patmack14 · 2024-05-27T02:53:00Z

Special thanks to @BlueberryKing for helping to knock some rust off(I've been on a dev hiatus) and throwing me a bone for showing me where the FOB is initially injected!

tracernz

Additional things to do:

Refactor getGrossWeight a little as it duplicates this FQI logic.
Check that all the consumers of getFoB can handle it being undefined, as it always returned a number before. Some definitely can not, so some changes will be needed. Perhaps a new getFqiFob function could be created, wrapping the simvar read, and the fuel pred page could call it instead.
Check that the AOC actually wants this value, or does it always want the FQI FoB? (Note that the AOC shouldn't actually directly access the FMS, as it lives in a different computer in the real aircraft, but fixing that is out of scope for this PR.)

Fix the PR title inline with https://www.conventionalcommits.org/en/v1.0.0/. Suggest fix(fms): use entered fob rather than fqi before start or something like that. Whatever you put here will become the commit title when this PR is squash-merged later.

In general you always need to be very mindful of type safety when working on the legacy JS code, as unlike typescript there is no automatic type checking.

.../html_ui/Pages/VCockpit/Instruments/Airliners/FlyByWire_A320_Neo/FMC/A32NX_FMCMainDisplay.js

.github/CHANGELOG.md

Co-authored-by: Michael Corcoran <[email protected]>

patmack14 · 2024-05-28T04:22:33Z

Additional things to do:

Refactor getGrossWeight a little as it duplicates this FQI logic.

Check that all the consumers of getFoB can handle it being undefined, as it always returned a number before. Some definitely can not, so some changes will be needed. Perhaps a new getFqiFob function could be created, wrapping the simvar read, and the fuel pred page could call it instead.

Check that the AOC actually wants this value, or does it always want the FQI FoB? (Note that the AOC shouldn't actually directly access the FMS, as it lives in a different computer in the real aircraft, but fixing that is out of scope for this PR.)

Fix the PR title inline with https://www.conventionalcommits.org/en/v1.0.0/. Suggest fix(fms): use entered fob rather than fqi before start or something like that. Whatever you put here will become the commit title when this PR is squash-merged later.

In general you always need to be very mindful of type safety when working on the legacy JS code, as unlike typescript there is no automatic type checking.

Have a few questions here:

There are getGrossWeight() and getGW() ..are these used for different reasons? seem to have somewhat repeated functionality.I can certainly condense both of these or combine them ( if that makes sense).
Regarding the type:
Rather than going down a type checking rabbit hole I was thinking of doing something like

//If do a null check rather than just else, it should basically retain the same functionality as original and still be undefined/NaN thus not messing with AOC and/or other unintended consequences?
else if(this.blockFuel){
              return this .blockFuel;
              }

Feel free to message on Discord to bounce ideas/thoughts if above does not work.

tracernz · 2024-05-28T06:56:26Z

Part of the job here is understanding the aircraft systems and how things should actually work before we touch the code, as then we can understand how the code should look. That part is unfortunately much more difficult than the code. You happen to have picked an area where the gross weight calculation code is relatively simple, but the systems changes required are definitely not trivial, and spread around quite a wide area of the FMS. All of the consumers of GW/FOB should handle those values not being undefined because they really are not available prior to engine start with no ZFW/FOB entered. Some of the things getGW does look very dubious, especially setting a local var, so there's opportunity for a good clean up there too.

patmack14 · 2024-05-31T02:39:24Z

@tracernz
I've been thinking about how to handle this-
Would finding the AOC customers of getFob and put the same if (!engineRunning) ->return type scheme work for everyone?
I agree handling it that way would cut back on unecesssary function calls/bouncing between classes.

removed comments and extra conversion

BlueberryKing · 2024-06-21T17:50:46Z

.../html_ui/Pages/VCockpit/Instruments/Airliners/FlyByWire_A320_Neo/FMC/A32NX_FMCMainDisplay.js

@@ -4996,10 +5001,9 @@ class FMCMainDisplay extends BaseAirliners {
    //TODO: Can this be util?
    getGW() {
        let fmGW = 0;
-        if (this.isAnEngineOn() && isFinite(this.zeroFuelWeight)) {
+        //Simplified to just checking fuelWeight as GetFOB handles what fuel level to use (block vs tank reading)
+        if (isFinite(this.zeroFuelWeight)) {


Careful, what happens if you've not entered a block fuel yet?

Hmm Yes Thanks for catching

Can the way the current aircraft release handles this scenario be used as a "control" or do the IRL pilots need to be consulted for what the behavior is in this situation?

Hmm Yes Thanks for catching

Can the way the current aircraft release handles this scenario be used as a "control" or do the IRL pilots need to be consulted for what the behavior is in this situation?

I'd have to check the places this function is called from, but replicating the current behavior would be a good starting point.

OK Sounds good.

Added an additional check to check for block fuel being entered to prevent "A32NX_FM_GROSS_WEIGHT" from being Null

2hwk · 2024-07-14T05:56:19Z

@BlueberryKing to approve as well

patmack14 · 2024-07-31T19:31:04Z

@BlueberryKing
Wanted to check in with you on this to see if you have had a chance to review this and see if it needs a few more tweaks or able to move to test?
Thanks!

BlueberryKing · 2024-08-06T18:58:11Z

...ui/Pages/VCockpit/Instruments/Airliners/FlyByWire_A320_Neo/CDU/ATSU/A320_Neo_CDU_AOC_Init.js

+        if (mcdu.isAnEngineOn()) {
+            const currentFob = formatWeight(NXUnits.kgToUser(mcdu.getFOB()));
+        }


I suggest we move this if block to the block further down where currentFob is used.

@BlueberryKing
Thinking you mean like this? (Removing line 106->108 and adding to Line 130)

currentFob = formatWeight(NXUnits.kgToUser(mcdu.getFOB())); if (currentFob) { fob = `{small}${currentFob}{end}[color]green`; } }`

You can remove these lines (106-108) now, right?

BlueberryKing · 2024-08-06T19:12:13Z

.../html_ui/Pages/VCockpit/Instruments/Airliners/FlyByWire_A320_Neo/FMC/A32NX_FMCMainDisplay.js

@@ -4996,10 +5001,9 @@ class FMCMainDisplay extends BaseAirliners {
    //TODO: Can this be util?
    getGW() {
        let fmGW = 0;
-        if (this.isAnEngineOn() && isFinite(this.zeroFuelWeight)) {
+        //Simplified to just checking fuelWeight as GetFOB handles what fuel level to use (block vs tank reading)
+        if (isFinite(this.zeroFuelWeight) && this.blockFuel) {


I don't think checking for this.blockFuel works here. With an engine running, the gross weight should be available (since fuel is available from FQI) even if no block fuel is entered.

What I suggest is fetching this.getFOB() and checking whether that's finite instead.

Also, I prefer Number.isFinite over isFinite since isFinite(null) is true which is rarely desired.

//Does this work? I removed unnecessary else block for simplicity sake as well since fmGw already is at 0.

getGW() { let fmGW = 0; const currFob = this.getFOB(); //Simplified to just checking fuelWeight as GetFOB handles what fuel level to use (block vs tank reading) if (Number.isFinite(this.zeroFuelWeight) && Number.isFinite(currFob)) { fmGW = (currFob + this.zeroFuelWeight); } SimVar.SetSimVarValue("L:A32NX_FM_GROSS_WEIGHT", "Number", fmGW); return fmGW; }

Add .isFinite type safety for and isAnEngineOn() logic instead of currentFoB()

pilotseyea350 · 2024-09-20T15:00:36Z

Quality Assurance Tester/Trainee Report

Discord Username : PilotEyesA350
Object of testing : #8664
Aircraft : A32NX
Tier of Testing : 1
Date : 20/09/2024

Testing Process:

Power Aircraft Up
Enter EDDF/EDDM in INIT A
Enter random DEP & ARR RWY & random SID/STAR
Delete any discontinuity
Refuel Aircraft through the FlyPad
Enter a higher FOB in INIT B than there actually is in the fuel tanks
Enter data in PERF
Verify EFOB at EDDM
Start any of the 2 engines
Check EFOB at EDDM has decreased compared to EFOB before engine start

Testing Results:
Passed

Negatives:
N/A

Conclusions:
Worked like a charm, thanks for your hard work !

Media:
N/A

@BlueberryKing

…8664) * FIx for flybywiresim#8639 * Update .github/CHANGELOG.md Co-authored-by: Michael Corcoran <[email protected]> * Update A32NX_FMCMainDisplay.js removed comments and extra conversion * adding to make sure an engine is running in AOC * additional gw safety Added an additional check to check for block fuel being entered to prevent "A32NX_FM_GROSS_WEIGHT" from being Null * updates per @BlueberryKing suggestions Add .isFinite type safety for and isAnEngineOn() logic instead of currentFoB() * forgot to remove 106->108..doh! * Fix formatting --------- Co-authored-by: Michael Corcoran <[email protected]> Co-authored-by: BBK <[email protected]>

FIx for flybywiresim#8639

7627364

tracernz reviewed May 27, 2024

View reviewed changes

.github/CHANGELOG.md Show resolved Hide resolved

patmack14 changed the title ~~FIx for #8639~~ fix(fms): use entered fob rather than fqi before start May 27, 2024

Update .github/CHANGELOG.md

736d81b

Co-authored-by: Michael Corcoran <[email protected]>

Merge branch 'master' into 8639fix

fa4857d

patmack14 added 2 commits June 8, 2024 22:30

Update A32NX_FMCMainDisplay.js

c1636ea

removed comments and extra conversion

adding to make sure an engine is running in AOC

1042974

BlueberryKing reviewed Jun 21, 2024

View reviewed changes

additional gw safety

e38c364

Added an additional check to check for block fuel being entered to prevent "A32NX_FM_GROSS_WEIGHT" from being Null

2hwk approved these changes Jul 14, 2024

View reviewed changes

alepouna requested a review from BlueberryKing August 4, 2024 01:42

BlueberryKing requested changes Aug 6, 2024

View reviewed changes

patmack14 and others added 5 commits September 7, 2024 21:14

updates per @BlueberryKing suggestions

6c4cbbf

Add .isFinite type safety for and isAnEngineOn() logic instead of currentFoB()

Merge branch 'master' into 8639fix

03396ca

forgot to remove 106->108..doh!

8ee780d

Merge branch 'master' into 8639fix

e858ac7

Fix formatting

ded22a4

BlueberryKing approved these changes Sep 20, 2024

View reviewed changes

BlueberryKing requested a review from tracernz September 20, 2024 09:33

tracernz approved these changes Sep 20, 2024

View reviewed changes

BlueberryKing added the QA Tier 1 label Sep 20, 2024

github-actions bot added the QA Passed label Sep 20, 2024

Merge remote-tracking branch 'upstream/master' into 8639fix

7ee80c6

BlueberryKing force-pushed the 8639fix branch from b63af05 to 7ee80c6 Compare September 21, 2024 18:46

BlueberryKing enabled auto-merge (squash) September 21, 2024 18:46

BlueberryKing merged commit b978453 into flybywiresim:master Sep 21, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(fms): use entered fob rather than fqi before start #8664

fix(fms): use entered fob rather than fqi before start #8664

patmack14 commented May 27, 2024

patmack14 commented May 27, 2024 •

edited

Loading

tracernz left a comment •

edited

Loading

patmack14 commented May 28, 2024 •

edited

Loading

tracernz commented May 28, 2024

patmack14 commented May 31, 2024

BlueberryKing Jun 21, 2024

patmack14 Jun 22, 2024

BlueberryKing Jun 23, 2024

patmack14 Jun 23, 2024

2hwk commented Jul 14, 2024

patmack14 commented Jul 31, 2024

BlueberryKing Aug 6, 2024

patmack14 Aug 9, 2024 •

edited

Loading

BlueberryKing Sep 14, 2024

BlueberryKing Aug 6, 2024

patmack14 Aug 9, 2024 •

edited

Loading

pilotseyea350 commented Sep 20, 2024

fix(fms): use entered fob rather than fqi before start #8664

fix(fms): use entered fob rather than fqi before start #8664

Conversation

patmack14 commented May 27, 2024

Summary of Changes

Screenshots (if necessary)

References

Additional context

Testing instructions

How to download the PR for QA

patmack14 commented May 27, 2024 • edited Loading

tracernz left a comment • edited Loading

Choose a reason for hiding this comment

patmack14 commented May 28, 2024 • edited Loading

tracernz commented May 28, 2024

patmack14 commented May 31, 2024

BlueberryKing Jun 21, 2024

Choose a reason for hiding this comment

patmack14 Jun 22, 2024

Choose a reason for hiding this comment

BlueberryKing Jun 23, 2024

Choose a reason for hiding this comment

patmack14 Jun 23, 2024

Choose a reason for hiding this comment

2hwk commented Jul 14, 2024

patmack14 commented Jul 31, 2024

BlueberryKing Aug 6, 2024

Choose a reason for hiding this comment

patmack14 Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

BlueberryKing Sep 14, 2024

Choose a reason for hiding this comment

BlueberryKing Aug 6, 2024

Choose a reason for hiding this comment

patmack14 Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

pilotseyea350 commented Sep 20, 2024

patmack14 commented May 27, 2024 •

edited

Loading

tracernz left a comment •

edited

Loading

patmack14 commented May 28, 2024 •

edited

Loading

patmack14 Aug 9, 2024 •

edited

Loading

patmack14 Aug 9, 2024 •

edited

Loading