-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: remote flyPad and displays #8598
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # fbw-a380x/src/systems/instruments/src/PFD/instrument.tsx # pnpm-lock.yaml
# Conflicts: # fbw-a380x/mach.config.js # fbw-a380x/src/systems/extras-host/index.ts # fbw-a380x/src/systems/extras-host/modules/common/AircraftPresetsList.ts # fbw-a380x/src/systems/extras-host/modules/key_interceptor/KeyInterceptor.ts # fbw-a380x/src/systems/extras-host/modules/version_check/VersionCheck.ts # fbw-common/src/systems/instruments/src/EFB/Efb.tsx # fbw-common/src/systems/instruments/src/EFB/ToolBar/ToolBar.tsx # fbw-common/src/systems/instruments/src/ND/pages/rose/RoseVorPage.tsx
5bf28a1
to
021b20c
Compare
airframeName: 'A320-251N', | ||
clientName: 'A32NX', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
airframeName: 'A320-251N', | |
clientName: 'A32NX', | |
airframeName: 'A320-251N', | |
clientName: 'A32NX', |
This maybe should use an existing configuration (or the new VFS-based solution currently in the planning stages in #8599)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok to put a TODO: here as well for this to be covered later.
@@ -23,7 +23,7 @@ import { VerticalSpeedIndicator } from './VerticalSpeedIndicator'; | |||
import { PFDSimvars } from './shared/PFDSimvarPublisher'; | |||
|
|||
export const getDisplayIndex = () => { | |||
const url = document.getElementsByTagName('a32nx-pfd')[0].getAttribute('url'); | |||
const url = document.querySelector('vcockpit-panel > *').getAttribute('url'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const url = document.querySelector('vcockpit-panel > *').getAttribute('url'); | |
const url = document.querySelector('vcockpit-panel > *').getAttribute('url'); |
May be a use case for xmlConfig via panel.xml in the future, for now this is ok.
export const getAirframeType = () => { | ||
if (window.FBW_REMOTE) { | ||
return 'A320_251N'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export const getAirframeType = () => { | |
if (window.FBW_REMOTE) { | |
return 'A320_251N'; | |
} | |
export const getAirframeType = () => { | |
if (window.FBW_REMOTE) { | |
return 'A320_251N'; | |
} |
Note for self: I'll need to take note of this in #8599 as well as potentially #8500
In the future, if we have remote instruments in multiple AC and variants, this might also need some thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are also deprecating the function getAirframeType() from usage in the EFB (PR 8500), may be better to switch to the new aircraft type L:Var for this and merge 8500 first? (Or again, long term solution, is the VFS based unified config)
@@ -85,9 +85,15 @@ interface BatteryStatus { | |||
export const usePower = () => React.useContext(PowerContext); | |||
|
|||
// this returns either `A380_842` or `A320_251N` depending on the aircraft | |||
export const getAirframeType = () => new URL( | |||
export const getAirframeType = () => { | |||
if (window.FBW_REMOTE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (window.FBW_REMOTE) { | |
if (window.FBW_REMOTE) { |
May want to abstract this into its own util or function so that its easier to manage, for a potential future use case if one day we also have to have bespoke remote EFB code in future in the EFB (Unlikely, but better to push this out into a dedicated function)
@@ -58,7 +59,7 @@ export const ToolBar = () => ( | |||
<ToolBarButton to="/atc" tooltipText={t('AirTrafficControl.Title')}> | |||
<BroadcastPin size={35} /> | |||
</ToolBarButton> | |||
<ToolBarButton to="/failures" tooltipText={t('Failures.Title')}> | |||
<ToolBarButton to="/failures" tooltipText={t('Failures.Title')} disabled={window.FBW_REMOTE}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above point about having an abstracted util function (that reads from window.FBW_REMOTE
). We don't need awareness of window
at this level.
|
||
constructor(options: RemoteClientOptions) { | ||
this.url = options.websocketUrl(); | ||
this.airframeName = options.airframeName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.airframeName = options.airframeName; | |
this.airframeName = options.airframeName; |
Maybe this should be aircraft instead of airframe (a bit pendatic but sure)
airframeName: 'A380-842', | ||
clientName: 'A380X', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
airframeName: 'A380-842', | |
clientName: 'A380X', | |
airframeName: 'A380-842', | |
clientName: 'A380X', |
See point about using either an existing config, or (plan) to implement the new VFS based solution with a TODO:
fbw-a32nx/mach.config.js
Outdated
msfsAvionicsInstrument, | ||
reactInstrument, | ||
} = getMachInstrumentBuilders({ | ||
templateIDPrefix: 'A32NX', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
templateIDPrefix: 'A32NX', | |
templateIDPrefix: 'A32NX', |
I'd actually like to read from .env, or some kind of markup here, but I'm not sure if its actually possible.
Looks like merge conflicts from lint changes (You probably already know, this is for the do not merge tag) |
# Conflicts: # fbw-a32nx/mach.config.js # fbw-a32nx/src/systems/extras-host/index.ts # fbw-a32nx/src/systems/instruments/src/Clock/instrument.tsx # fbw-a32nx/src/systems/instruments/src/EWD/instrument.tsx # fbw-a32nx/src/systems/instruments/src/ND/instrument.tsx # fbw-a32nx/src/systems/instruments/src/PFD/PFD.tsx # fbw-a32nx/src/systems/instruments/src/PFD/instrument.tsx # fbw-a32nx/src/systems/simbridge-client/src/common.ts # fbw-a380x/src/systems/extras-host/index.ts # fbw-common/src/systems/instruments/src/EFB/Efb.tsx # fbw-common/src/systems/instruments/src/EFB/ToolBar/ToolBar.tsx # fbw-common/src/systems/instruments/src/ND/pages/rose/RoseVorPage.tsx # fbw-common/src/systems/instruments/src/ND/shared/utils/MapParameters.ts # fbw-common/src/systems/shared/src/simbridge/components/ClientState.ts # package.json # pnpm-lock.yaml
Still my biggest wish!!! |
Why was this one removed from V0.12? The issue is opened more then two and halve years ago and had then already demo-pictures where it was working. Is it still possible? |
Fixes #6712
Fixes #6978
Depends on flybywiresim/simbridge#114
Summary of Changes
This PR adds the first implementation of the remote display bridge protocol, and enables it for certain instruments, including the flyPad.
Testing instructions
TBD
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.