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.

Sign up for GitHub

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

Add ICAO flight rules option #100

Merged
merged 7 commits into from
Jan 25, 2025
Merged

Add ICAO flight rules option #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
be3c0e9
Add ICAO Weather Condition Indicators
neilenns Jan 18, 2025
063f762
Update template and other stuff
neilenns Jan 18, 2025
f28c67c
Address coderabbit feedback
neilenns Jan 18, 2025
059d1ca
Fix HTML
neilenns Jan 18, 2025
ff502a1
Check visibility in meters
neilenns Jan 19, 2025
7cd1477
Fix mistake in cloud level calculation
neilenns Jan 22, 2025
31fa0d6
Fix calculation issues
neilenns Jan 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,18 @@
"Atis",
"atisletter",
"callsign",
"CAVOK",
"elgato",
"getv",
"handlev",
"KHIO",
"KLMT",
"KPDX",
"KSEA",
"KUAO",
"KLMT",
"LIFR",
"MVFR",
"sdpi",
"streamdeck",
"Typeguard",
"Updatev",
Expand Down
12 changes: 10 additions & 2 deletions com.neil-enns.vatis.sdPlugin/images/actions/atisLetter/template.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 10 additions & 6 deletions com.neil-enns.vatis.sdPlugin/ui/atisLetter.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@
</sdpi-select>
</sdpi-item>

<sdpi-item>
<sdpi-checkbox
setting="showFaaFlightRules"
label="Show FAA flight rules indicator"
default="true"
></sdpi-checkbox>
<sdpi-item label="Flight rules">
<sdpi-select
setting="showFlightRules"
default="FAA"
aria-label="Select flight rules display type"
>
<option value="NONE">None</option>
<option value="FAA">FAA</option>
<option value="ICAO">ICAO</option>
</sdpi-select>
</sdpi-item>

<sdpi-item label="Current">
Expand Down
8 changes: 7 additions & 1 deletion src/actions/atisLetter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,19 @@ export class AtisLetter extends SingletonAction<AtisLetterSettings> {
}
}

export enum FlightRuleDisplay {
NONE = "NONE",
FAA = "FAA",
ICAO = "ICAO",
}

export interface AtisLetterSettings {
atisType?: AtisType;
showAltimeter?: boolean;
showWind?: boolean;
currentImagePath?: string;
observerImagePath?: string;
showFaaFlightRules?: boolean;
showFlightRules?: FlightRuleDisplay;
showLetter?: boolean;
showTitle?: boolean;
station?: string;
Expand Down
111 changes: 102 additions & 9 deletions src/controllers/atisLetter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AtisLetterSettings } from "@actions/atisLetter";
import { AtisLetterSettings, FlightRuleDisplay } from "@actions/atisLetter";
import { KeyAction } from "@elgato/streamdeck";
import { Controller } from "@interfaces/controller";
import {
Expand All @@ -10,18 +10,23 @@ import {
} from "@interfaces/messages";
import TitleBuilder from "@root/utils/titleBuilder";
import { stringOrUndefined } from "@root/utils/utils";
import { ATIS_LETTER_CONTROLLER_TYPE } from "@utils/controllerTypes";
import debounce from "debounce";
import { BaseController } from "./baseController";
import { ATIS_LETTER_CONTROLLER_TYPE } from "@utils/controllerTypes";

const defaultTemplatePath = "images/actions/atisLetter/template.svg";
const defaultUnavailableTemplatePath = "images/actions/atisLetter/template.svg";

/**
* Maximum cloud level in feet. Used as a fallback when no ceiling data is available.
* Default cloud level in feet. Used as a fallback when no ceiling data is available.
*/
const DEFAULT_CEILING_LEVEL_FEET = 9999;

/**
* Default visibility in meters. Used as a fallback when no visibility data is available.
*/
const DEFAULT_VISIBILITY_METERS = 9999;

/**
* Flight rules categories according to FAA standards
*/
Expand All @@ -33,6 +38,15 @@ enum FaaFlightRules {
UNKNOWN = "UNKNOWN", // Flight rules couldn't be calculated
}

/**
* Flight rules categories according to ICAO standards
*/
enum IcaoFlightRules {
IMC = "IMC", // Instrument flight conditions
VMC = "VMC", // Visual flight conditions
UNKNOWN = "UNKNOWN", // Flight rules couldn't be calculated
}

/**
* A StationStatus action, for use with ActionManager. Tracks the settings,
* state and Stream Deck action for an individual action in a profile.
Expand All @@ -47,6 +61,7 @@ export class AtisLetterController extends BaseController {
private _altimeter?: string;
private _pressure?: Value;
private _faaFlightRules: FaaFlightRules = FaaFlightRules.UNKNOWN;
private _icaoFlightRules: IcaoFlightRules = IcaoFlightRules.UNKNOWN;

private _suppressUpdates: boolean;
private _settings: AtisLetterSettings | null = null;
Expand Down Expand Up @@ -92,6 +107,7 @@ export class AtisLetterController extends BaseController {
this._pressure = undefined;
this._suppressUpdates = false;
this._faaFlightRules = FaaFlightRules.UNKNOWN;
this._icaoFlightRules = IcaoFlightRules.UNKNOWN;

this.refreshDisplay();
}
Expand Down Expand Up @@ -119,11 +135,11 @@ export class AtisLetterController extends BaseController {
}

/**
* Gets the show FAA flight rules setting.
* @returns {boolean} True if the flight rules indicator should show. Defaults to true if no setting is provided.
* Gets the show flight rules setting.
* @returns {FlightRuleDisplay} The type of flight rules indicator to show. Defaults to FAA if no setting is provided.
*/
get showFaaFlightRules(): boolean {
return this.settings.showFaaFlightRules ?? true;
get showFlightRules(): FlightRuleDisplay {
return this.settings.showFlightRules ?? FlightRuleDisplay.FAA;
Comment on lines +138 to +142
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add migration notes for the breaking change

The breaking change related to FlightRuleDisplay default value is not documented. While the code maintains backward compatibility by defaulting to FlightRuleDisplay.FAA, this change should be documented to help users understand:

  • What changed in the flight rules display settings
  • The new default behavior
  • Steps required if they need to modify their implementation

Consider adding these details to README.md or creating a CHANGELOG.md file.

🔗 Analysis chain

Verify the default value in production code.

The getter's default value of FlightRuleDisplay.FAA maintains backward compatibility, which is good. However, since this is a breaking change as mentioned in the PR objectives, we should ensure this default value is documented in the migration guide.

Let's check if there's a migration guide or documentation update:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Look for migration guide or documentation updates
rg -g '!node_modules' -g '*.{md,txt}' -i 'migration|upgrade|breaking.?change' .

Length of output: 81

Script:

#!/bin/bash
# Look for documentation files and any mentions of flight rules
echo "=== Searching for documentation files ==="
fd -e md -e txt

echo -e "\n=== Searching for FlightRuleDisplay mentions in docs ==="
rg -g '*.{md,txt}' "FlightRuleDisplay|flight.?rules?"

echo -e "\n=== Checking for CHANGELOG ==="
fd -i changelog

Length of output: 400

}

/**
Expand All @@ -147,6 +163,27 @@ export class AtisLetterController extends BaseController {
this.refreshDisplay();
}

/**
* Gets the ICAO flight rules for the current ATIS.
* @returns {IcaoFlightRules} The current ICAO flight rules.
*/
get icaoFlightRules(): IcaoFlightRules {
return this._icaoFlightRules;
}

/**
* Sets the ICAO flight rules for the current ATIS and updates the action display.
*/
set icaoFlightRules(newValue: IcaoFlightRules) {
if (this._icaoFlightRules === newValue) {
return;
}

this._icaoFlightRules = newValue;

this.refreshDisplay();
}

/**
* Gets the name for the ATIS action.
* @returns {string | undefined} The station name.
Expand Down Expand Up @@ -290,7 +327,7 @@ export class AtisLetterController extends BaseController {
}

/**
* Sets the isNewAtis state and refreshes the action dipslay.
* Sets the isNewAtis state and refreshes the action display.
*/
public set isNewAtis(newValue: boolean) {
this._isNewAtis = newValue;
Expand Down Expand Up @@ -390,11 +427,66 @@ export class AtisLetterController extends BaseController {
this.altimeter = value.altimeter;
this.pressure = value.pressure;
this.calculateFaaFlightRules(value.ceiling, value.prevailingVisibility);
this.calculateIcaoFlightRules(value.ceiling, value.prevailingVisibility);
this.enableUpdates();

this.refreshDisplay();
}

/**
* Calculates ICAO flight rules based on ceiling and visibility values.
* @param {Value | undefined} ceiling - The ceiling height in hundreds of feet.
* @param {Value | undefined} prevailingVisibility - The visibility in meters.
* @private
*/
private calculateIcaoFlightRules(
ceiling?: Value,
prevailingVisibility?: Value
) {
// No visibility data might mean CAVOK so assume unlimited visibility.
if (!prevailingVisibility) {
prevailingVisibility = {
actualValue: DEFAULT_VISIBILITY_METERS,
actualUnit: Unit.Meter,
};
}

// If the ceiling is null then set it to the default.
if (!ceiling) {
ceiling = {
actualValue: DEFAULT_CEILING_LEVEL_FEET,
actualUnit: Unit.Feet,
};
}

// Rules are only applicable to meters and feet.
if (
prevailingVisibility.actualUnit !== Unit.Meter ||
ceiling.actualUnit !== Unit.Feet
) {
this.icaoFlightRules = IcaoFlightRules.UNKNOWN;
return;
}

const cloudLevel = ceiling.actualValue;
const visibility = prevailingVisibility.actualValue;

if (visibility < 0 || cloudLevel < 0) {
this.icaoFlightRules = IcaoFlightRules.UNKNOWN;
return;
}

// The checks are in this order to ensure the most restrictive, rather than least restrictive,
// is applied.
// Visibility is in meters.
// Cloud levels are in 100s of feet.
if (visibility < 5000 || cloudLevel < 15) {
this.icaoFlightRules = IcaoFlightRules.IMC;
} else {
this.icaoFlightRules = IcaoFlightRules.VMC;
}
}
Comment on lines +436 to +488
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Incorrect ICAO flight rules implementation

The implementation has several issues:

  • The JSDoc comment incorrectly states "ceiling height in hundreds of feet" while the code uses actual feet
  • The VMC/IMC threshold of 15 feet is incorrect (standard ICAO minimums are typically 1000-3000 feet)
  • Assuming CAVOK for missing visibility is inconsistent with the FAA implementation and potentially unsafe
🔗 Analysis chain

Verify ICAO flight rules calculation logic.

Please verify the following aspects of the implementation:

  1. The assumption that missing visibility means CAVOK needs verification, as it could also indicate unavailable data.
  2. The comment states "Cloud levels are in 100s of feet" but the code treats them as actual feet.
  3. The VMC/IMC threshold of 15 feet ceiling seems unusually low for ICAO standards.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for ICAO VMC/IMC definitions in documentation
echo "=== Searching for ICAO VMC/IMC documentation ==="
rg -i "vmc|imc|ceiling.*feet|visibility.*met(er|re)s?" .

echo -e "\n=== Searching for CAVOK handling ==="
rg -i "cavok|ceiling.and.visibility.ok" .

Length of output: 2499

Script:

#!/bin/bash
# Search for test files and ceiling/visibility handling
echo "=== Searching for test files ==="
fd -e test.ts -e spec.ts

echo -e "\n=== Searching for ceiling value handling ==="
rg -i "ceiling.*value|ceiling.*conversion" .

echo -e "\n=== Searching for visibility value handling ==="
rg -i "visibility.*value|visibility.*conversion" .

Length of output: 1737

Script:

#!/bin/bash
# Search for FAA implementation and Value interface
echo "=== Searching for FAA flight rules implementation ==="
rg -A 20 "calculateFaaFlightRules" .

echo -e "\n=== Searching for Value interface definition ==="
ast-grep --pattern 'interface Value'

Length of output: 3009


/**
* Calculates FAA flight rules based on ceiling and visibility values.
* @param {Value | undefined} ceiling - The ceiling height in hundreds of feet.
Expand Down Expand Up @@ -495,7 +587,8 @@ export class AtisLetterController extends BaseController {
title: this.title,
wind: this.wind,
faaFlightRules: this.faaFlightRules,
showFaaFlightRules: this.showFaaFlightRules,
icaoFlightRules: this.icaoFlightRules,
showFlightRules: this.showFlightRules,
};

if (this.isNewAtis) {
Expand Down
3 changes: 2 additions & 1 deletion src/events/streamdeck/atisLetter/updateAtisLetterSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export const handleUpdateAtisLetterSettings = (

const requiresRefresh =
savedAction.settings.station !== settings.station ||
savedAction.settings.atisType !== settings.atisType;
savedAction.settings.atisType !== settings.atisType ||
savedAction.settings.showFlightRules !== settings.showFlightRules;

savedAction.settings = settings;

Expand Down