flybywiresim / aircraft

The A32NX & A380X Project are community driven open source projects to create free Airbus aircraft in Microsoft Flight Simulator that are as close to reality as possible.
https://flybywiresim.com
GNU General Public License v3.0
4.93k stars 1.01k forks source link

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

Open patmack14 opened 1 month ago

patmack14 commented 1 month ago

8639

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.

  1. Engine !Running -> Pull from entered block fuel
  2. 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.

  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 or flybywire-aircraft-a380-842 download link at the bottom of the page
patmack14 commented 1 month ago

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!

patmack14 commented 1 month ago

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:

  1. 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).
  2. 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 commented 1 month ago

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 commented 1 month ago

@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.