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

feat(fmgc): minimums exposed by fmgc #7887

Merged
merged 33 commits into from
Apr 16, 2023

Conversation

MikioDK
Copy link
Contributor

@MikioDK MikioDK commented Mar 19, 2023

Fixes #7610

Summary of Changes

The minimums get set through the BnrArinc when in CRZ or later and within 200NM of the destinition

Screenshots (if necessary)

References
https://discord.com/channels/738864299392630914/850710355877560350/1044558618982363176
and the issue itself #7610

Additional context

Discord username (if different from GitHub):
Mikio

Testing instructions

Test that displays and general aircraft operation is still okay.
Test that remote MCDU is working.

BARO minimums should not appear on the PFD during takeoff in the top corner or on the alt band.
BARO minimums should not make the altitude indicator, or meter altitude band Amber in color during takeoff.
RADIO minimums should not appear on the PFD during takeoff in the top corner.
RADIO minimums should not make the RA value on the bottom of Artifical Horizon on PFD green on takeoff.

BARO minimums should appear on the PFD when in a phase later than Cruise or if in Cruise and within 250NM of destination.
RADIO minimums should appear on the PFD when in a phase later than Cruise or if in Cruise and within 250NM of destination.
BARO minimums should make the altitude indicator amber when passing the minimums as normal, while having minimums displayed on FPD.
RADIO minimums should make the altitude indicator amber when passing the minimums as normal, while having minimums displayed on FPD

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

@MikioDK
Copy link
Contributor Author

MikioDK commented Mar 19, 2023

@2hwk I found the reason it broke remote MCDU. Was a small thing missing that must have created exceptions that blocked Simbridge connection.

@2hwk 2hwk added this to the v0.10.0 milestone Mar 22, 2023
@2hwk 2hwk added the QA Tier 1 label Mar 22, 2023
@beheh
Copy link
Member

beheh commented Mar 24, 2023

@MikioDK Could you please add testing instructions specifically around the minimums? E.g. BARO minimums should not appear on the PFD during takeoff in the top corner or on the alt band etc.

@MikioDK
Copy link
Contributor Author

MikioDK commented Mar 24, 2023

@beheh About that. Currently it is only exposing the Arinc429Word value, but no consumers. I can look into consuming the value in the PFD and possibly for the FWC in Rust as well. Depends on what @tracernz has to say about it.

@beheh
Copy link
Member

beheh commented Mar 24, 2023

I think the PFD is the important one. The FWC doesn't really matter because of the new Rust FWC, I'll start consuming it there once it's available. But I think updating the PFD would make sense.

@tracernz
Copy link
Member

I concur with beheh. 👍

@MikioDK MikioDK requested a review from beheh March 29, 2023 17:47
Copy link
Member

@beheh beheh left a comment

Choose a reason for hiding this comment

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

Thanks for the update! I've added another comment to my Failure Warning/NCD feedback, I think it's fairly quick to resolve. Once you've resolved a piece of feedback please mark it as resolved here in GitHub using the following button, and then you can re-tag me:

Resolve

@MikioDK MikioDK requested a review from beheh March 29, 2023 18:53
Copy link
Member

@beheh beheh left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@beheh beheh removed the Waiting For Response This issue or PR needs a response from the author label Mar 29, 2023
@beheh beheh changed the title feat(fmgc): A32nx minimums exposed by fmgc feat(fmgc): minimums exposed by fmgc Mar 29, 2023
@2hwk
Copy link
Member

2hwk commented Apr 15, 2023

QA Test Report:

  • Test that remote MCDU is working.
  • Test that displays and general aircraft operation is still okay.
    Test Aborted due to negatives

Negatives:
image

  • PFDs are not built correctly now, show up as black screen with react not loaded message.

Result:

  • Not Passed

Conclusions:
Remote MCDU now works, however the PFDs are not working.

@MikioDK
Copy link
Contributor Author

MikioDK commented Apr 15, 2023

Error was due to other ARINC429 changes that got merged in from master. PFD now loads

@2hwk
Copy link
Member

2hwk commented Apr 16, 2023

QA Report

Testing:

  • Test that displays and general aircraft operation is still okay.
  • Test that remote MCDU is working.
  • BARO minimums should not appear on the PFD during takeoff in the top corner or on the alt band.
  • BARO minimums should not make the altitude indicator, or meter altitude band Amber in color during takeoff.
  • RADIO minimums should not appear on the PFD during takeoff in the top corner.
  • RADIO minimums should not make the RA value on the bottom of Artifical Horizon on PFD green on takeoff.
  • BARO minimums should appear on the PFD when in a phase later than Cruise or if in Cruise and within 250NM of destination.
  • RADIO minimums should appear on the PFD when in a phase later than Cruise or if in Cruise and within 250NM of destination.
  • BARO minimums should make the altitude indicator amber when passing the minimums as normal, while having minimums displayed on FPD.
  • RADIO minimums should make the altitude indicator amber when passing the minimums as normal, while having minimums displayed on FPD

Negatives:

  • N/A

Results:

  • Passed

Conclusions:

  • LGTM

@2hwk 2hwk merged commit b46f3ff into flybywiresim:master Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimum values should only be transmitted from FMGC in CRZ and later
4 participants