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(cond): Fix SD COND page duct ovht indicator #8234

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Eearslya
Copy link
Contributor

Summary of Changes

PR #8178 added a few failures for air conditioning, including a few changes to the status page. One of these was to indicate duct temperatures as amber if they exceed the overheat threshold. This is meant to happen at 80 degrees Celsius, but because of the way that the COND page converts all temperatures to Fahrenheit, it was triggering at 80 Fahrenheit instead.

This fixes the indicator to properly trigger above 80C (176F).

Screenshots (if necessary)

References

Additional context

Discord username (if different from GitHub): Eearslya

Testing instructions

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

@@ -130,7 +130,7 @@ type CondUnitProps = {

const CondUnit = ({ title, trimAirValve, cabinTemp, trimTemp, x, y, offset, hotAir } : CondUnitProps) => {
const rotateTemp = offset + (trimAirValve * 86 / 100);
const overheat = trimTemp > 80;
const overheat = trimTemp > 176; // Overheat indicator at 80C, temperature is given in F
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't make sense. The temp is only F here if you select alternate units. The problem is really that the unit conversions are done too early. Should be at the point they're used in the UI only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You...are absolutely correct, and I'm not sure how I managed to mix that logic up in my head. Somehow I had it flipped and thought it would convert to F if the units were set to C. I'll work on a more complete change which converts the units later.

@tracernz tracernz added this to the v0.12.0 milestone Oct 24, 2023
@2hwk 2hwk added the Waiting For Response This issue or PR needs a response from the author label Apr 12, 2024
@flogross89 flogross89 removed this from the v0.12.0 milestone Oct 26, 2024
@2hwk 2hwk added this to the Backlog milestone Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting For Response This issue or PR needs a response from the author
Projects
Status: 🔴 Code Review: In progress
Development

Successfully merging this pull request may close these issues.

4 participants