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

refactor: remote failures provider #6971

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 40 additions & 0 deletions docs/a320-coherent-triggers.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,43 @@ See `FMMessage` in `src/shared/src/FmMessages.ts`.
- Payload: `[text: string]`
- `text`:
- The MCDU text of the message to remove

## Failures

### Triggers

- A32NX_NEW_FAILURES_STATE
- Dataflow: Failures orchestrator -> Remote failures providers
- Notifies any remote failures providers of available, active and changing failures
- Payload: `[active: number[], changing: number[]]`
- `all`: every failure currently active
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this should be active?

Suggested change
- `all`: every failure currently active
- `active`: every failure currently active

- `changing`: every failure currently changing

- A32NX_ACTIVATE_FAILURE
- Dataflow: Remote failures providers -> Failures orchestrator
- Requests a failure to be activated through the failure orchestrator
- Payload: `[identifier: number]`
- `identifier`: failure identifier

- A32NX_DEACTIVATE_FAILURE
- Dataflow: Remote failures providers -> Failures orchestrator
- Requests a failure to be deactivated through the failure orchestrator
- Payload: `[identifier: number]`
- `identifier`: failure identifier

- CONFIRM_ACTIVATE_FAILURE
- Dataflow: Failures orchestrator -> Remote failures providers
- Confirms to remote providers that a failure was activated
- Payload: `[identifier: number]`
- `identifier`: failure identifier

- CONFIRM_DEACTIVATE_FAILURE
Comment on lines +46 to +52
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- CONFIRM_ACTIVATE_FAILURE
- Dataflow: Failures orchestrator -> Remote failures providers
- Confirms to remote providers that a failure was activated
- Payload: `[identifier: number]`
- `identifier`: failure identifier
- CONFIRM_DEACTIVATE_FAILURE
- A32NX_CONFIRM_ACTIVATE_FAILURE
- Dataflow: Failures orchestrator -> Remote failures providers
- Confirms to remote providers that a failure was activated
- Payload: `[identifier: number]`
- `identifier`: failure identifier
- A32NX_CONFIRM_DEACTIVATE_FAILURE

- Dataflow: Failures orchestrator -> Remote failures providers
- Confirms to remote providers that a failure was deactivated
- Payload: `[identifier: number]`
- `identifier`: failure identifier

- A32NX_REQUEST_FAILURES_STATE
- Dataflow: Remote failures providers -> Failures orchestrator
- Requests the orchestrator to immediately send a `A32NX_NEW_FAILURES_STATE` message
- Payload: `[]`
16 changes: 16 additions & 0 deletions jest/ViewListener.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export let listeners = {};
Copy link
Member

Choose a reason for hiding this comment

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

/external/jest/ViewListener.ts
  1:1  error  Parsing error: 'import' and 'export' may appear only with 'sourceType: module'


/** @type {Partial<ViewListener.ViewListener>} */
export const MockViewListener = {
triggerToAllSubscribers: jest.fn((event, ...args) => {
const subs = listeners[event];

const jsonArgs = args.map((it) => JSON.stringify(it)).map((it) => JSON.parse(it));
Copy link
Member

Choose a reason for hiding this comment

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

thought: You copy the arguments. If your goal is to protect the caller of triggerToAllSubscribers from suddenly having its own objects changed under him, then this works. If your goal is for every individual subscriber to have their own copy of objects, then this will need to move within the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

The goal here is to reproduce what MSFS does (value -> json -> value), and lose the prototypes and object identities.


if (subs) {
for (const sub of subs) {
sub(event, ...jsonArgs);
}
}
}),
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
};
};

41 changes: 40 additions & 1 deletion jest/setupJestMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,48 @@ global.SimVar.GetSimVarValue = jest.fn((name, _, __) => {
return 0;
}
});
global.SimVar.SetSimVarValue = jest.fn((name, _, value, __) => {

global.SimVar.SetSimVarValue = jest.fn((name, _, value) => {
return new Promise((resolve, _) => {
values[name] = value;
resolve();
});
});

let listeners = {};

global.beforeEach(() => {
listeners = {};
});

/** @type {Partial<ViewListener.ViewListener>} */
const MockViewListener = {
Copy link
Member

Choose a reason for hiding this comment

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

question: Is the code duplicate intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. The TS file wasn't meant to be committed.

triggerToAllSubscribers: jest.fn((event, ...args) => {
const subs = listeners[event];

const jsonArgs = args.map((it) => JSON.stringify(it)).map((it) => JSON.parse(it));
Copy link
Member

Choose a reason for hiding this comment

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

thought: You copy the arguments. If your goal is to protect the caller of triggerToAllSubscribers from suddenly having its own objects changed under him, then this works. If your goal is for every individual subscriber to have their own copy of objects, then this will need to move within the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

The goal here is to reproduce what MSFS does (value -> json -> value), and lose the prototypes and object identities.


if (subs) {
for (const sub of subs) {
sub(...jsonArgs);
}
}
}),
};

global.RegisterViewListener = jest.fn((name, _, __) => {
return MockViewListener;
});

/** @type {Partial<import("typings/fs-base-ui").Coherent>} */
global.Coherent = {
on(event, callback) {
const subs = listeners[event];

if (subs && !subs.includes(callback)) {
subs.push(callback);
} else {
listeners[event] = [callback];
}
},
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { QueuedSimVarReader, SimVarReaderWriter, TransactionReader, QueuedSimVarWriter, TransactionWriter } from '..';
import { flushPromises } from '../../test-functions';
import { flushPromises } from '../../test-values';

test('read/write', async () => {
const failureIdentifier = 1;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { QueuedSimVarReader, TransactionReader } from '.';
import { CallbackReader, SimVarReaderWriter, QueuedSimVarWriter } from '..';
import { flushPromises } from '../../test-functions';
import { flushPromises } from '../../test-values';

describe('TransactionReader', () => {
test('confirms receival', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { flushPromises } from '../../test-functions';
import { flushPromises } from '../../test-values';
import { QueuedSimVarWriter } from '.';
import { SimVarReaderWriter } from '..';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { TransactionWriter } from './transaction-writer';
import { SimVarReaderWriter, Updatable, Writer, QueuedSimVarWriter } from '..';
import { flushPromises } from '../../test-functions';
import { flushPromises } from '../../test-values';

describe('TransationWriter', () => {
test('waits for receival confirmation', async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/failures/src/failures-orchestrator.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FailuresOrchestrator } from '.';
import { getActivateFailureSimVarName, getDeactivateFailureSimVarName } from './sim-vars';
import { flushPromises } from './test-functions';
import { flushPromises } from './test-values';

describe('FailuresOrchestrator', () => {
test('stores configured failures', () => {
Expand Down
59 changes: 50 additions & 9 deletions src/failures/src/failures-orchestrator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { QueuedSimVarWriter, SimVarReaderWriter } from './communication';
import { getActivateFailureSimVarName, getDeactivateFailureSimVarName } from './sim-vars';
import { FailuresProvider } from './failures-provider';
import { FailuresTriggers, PrefixedFailuresTriggers } from './triggers';

export interface Failure {
identifier: number,
Expand All @@ -9,9 +11,10 @@ export interface Failure {
/**
* Orchestrates the activation and deactivation of failures.
*
* Only a single instance of the orchestrator should exist within the whole application.
* Only a single instance of the orchestrator should exist within the whole application. Its data can be accessed using
* a {@link RemoteFailuresProvider}
*/
export class FailuresOrchestrator {
export class FailuresOrchestrator implements FailuresProvider {
Copy link
Member

Choose a reason for hiding this comment

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

thought: I have the feeling we're missing some abstractions in these changes. Now this type needs to know about Coherent's on, RegisterViewListener, and publishing. Maybe there's some additional work here?

private failures: Failure[] = [];

private activeFailures = new Set<number>();
Expand All @@ -22,53 +25,91 @@ export class FailuresOrchestrator {

private deactivateFailureQueue: QueuedSimVarWriter;

private readonly listener = RegisterViewListener('JS_LISTENER_SIMVARS', null, true);

private triggers: FailuresTriggers;

constructor(simVarPrefix: string, failures: [number, string][]) {
this.activateFailureQueue = new QueuedSimVarWriter(new SimVarReaderWriter(getActivateFailureSimVarName(simVarPrefix)));
this.deactivateFailureQueue = new QueuedSimVarWriter(new SimVarReaderWriter(getDeactivateFailureSimVarName(simVarPrefix)));

failures.forEach((failure) => {
this.failures.push({
identifier: failure[0],
name: failure[1],
});
});

this.triggers = PrefixedFailuresTriggers(simVarPrefix);

Coherent.on(this.triggers.ActivateFailure, (identifier: number) => {
this.activate(identifier);
});

Coherent.on(this.triggers.DeactivateFailure, (identifier: number) => {
this.deactivate(identifier);
});

Coherent.on(this.triggers.RequestFailuresState, () => {
this.publishStateUpdate();
});

this.publishStateUpdate();
}

update() {
this.activateFailureQueue.update();
this.deactivateFailureQueue.update();
}

private publishStateUpdate() {
this.listener.triggerToAllSubscribers(this.triggers.NewFailuresState, Array.from(this.activeFailures), Array.from(this.changingFailures));
}

private publishConfirmActivate(identifier: number) {
this.listener.triggerToAllSubscribers(this.triggers.ConfirmActivateFailure, identifier);
}

private publishConfirmDeactivate(identifier: number) {
this.listener.triggerToAllSubscribers(this.triggers.ConfirmDeactivateFailure, identifier);
}

/**
* Activates the failure with the given identifier.
*/
async activate(identifier: number): Promise<void> {
this.changingFailures.add(identifier);

this.publishStateUpdate();

await this.activateFailureQueue.write(identifier);
this.changingFailures.delete(identifier);
this.activeFailures.add(identifier);

this.publishConfirmActivate(identifier);
this.publishStateUpdate();
}

/**
* Deactivates the failure with the given identifier.
*/
async deactivate(identifier: number): Promise<void> {
this.changingFailures.add(identifier);

this.publishStateUpdate();

await this.deactivateFailureQueue.write(identifier);
this.changingFailures.delete(identifier);
this.activeFailures.delete(identifier);

this.publishConfirmDeactivate(identifier);
this.publishStateUpdate();
}

/**
* Determines whether or not the failure with the given identifier is active.
*/
isActive(identifier: number): boolean {
return this.activeFailures.has(identifier);
}

/**
* Determines whether or not the failure with the given identifier is currently
* changing its state between active and inactive.
*/
isChanging(identifier: number): boolean {
return this.changingFailures.has(identifier);
}
Expand Down
39 changes: 39 additions & 0 deletions src/failures/src/failures-provider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { Failure } from '@flybywiresim/failures';

export interface FailuresProvider {
/**
* Activates the failure with the given identifier.
*/
activate(identifier: number): Promise<void>

/**
* Deactivates the failure with the given identifier.
*/
deactivate(identifier: number): Promise<void>

/**
* Determines whether the failure with the given identifier is active.
*/
isActive(identifier: number): boolean

/**
* Determines whether the failure with the given identifier is currently
* changing its state between active and inactive.
*/
isChanging(identifier: number): boolean

/**
* Returns all available failures
*/
getAllFailures(): Readonly<Readonly<Failure>[]>

/**
* Returns all active failures
*/
getActiveFailures(): Set<number>

/**
* Returns all changing failures
*/
getChangingFailures(): Set<number>
}
1 change: 1 addition & 0 deletions src/failures/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { FailuresConsumer } from './failures-consumer';
export { FailuresOrchestrator } from './failures-orchestrator';
export { RemoteFailuresProvider } from './remote-failures-provider';
export type { Failure } from './failures-orchestrator';
export { A320Failure } from './a320';
Loading