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(rmp): RMP behaviour and lights #7105

Merged
merged 28 commits into from
Aug 16, 2022

Conversation

juliansebline
Copy link
Contributor

@juliansebline juliansebline commented Apr 24, 2022

Fixes #344
Fixes #164
Fixes #5033
Fixes #4591

Summary of Changes

This commit fixes the RMPs/ACPs's behaviour (VHFs only), their electrical power and add VHF 3 voice/data mode.

Screenshots (if necessary)

image
image

image
image

References

image
image

Additional context

Thanks shomas#2719 for the pîctures in the real aircraft!

My discord: Julian Sebline#8476

Testing instructions

Keep in mind both RMPs and ACPs are independent

  1. Open SimvarWatcher and load the config file found in the last comment.
  2. At startup:
    • CALL1 should be active on both sides
    • VHF1/2 volumes knobs should be on with values 0.8 and 0.4 (respectively)
    • Left RMP should be set with VHF1
    • Right RMP should be set with VHF2
    • Respective SEL light should not be lit
    • Simvars RECEIVE ALL and RECIEVE ALL should be set to 1 (for vPilot)
  3. On the left RMP, press VHF2 then VHF3
    • The left SEL led AND word should lit in orange on both sides
    • Back to VHF1, the lights should go off
  4. On the right RMP, press VHF1 then VHF3
    • The right SEL led AND word should lit in orange on both sides
    • Back to VHF2, the lights should go off
  5. On both sides, press VHF1/2/3 volume knobs
    • Each knob should turn on/off independently
    • All knobs can be on/off
    • Respective simvar should be set to 1 IF at least one knob of each side is on
    • Respective simvar should be set to 0 IF each knob of each side is off
    • When off, rotate a knob should be harder (to simulate real behaviour)
  6. On both sides, rotate VHF1/2/3 volume knobs
    • Each knob should rotate independently
    • Respective simvar should be set to the greatest value of both knobs
  7. Turn off all volumes knobs
    • Rotate them should not update their respective simvar
    • Turn them on, the simvar should be updated with the value of the knob OR the max value between the two knobs if both on
  8. On both sides, turn OFF all CALL (related to VHFs) push buttons
    • Their respective light is off
    • The simvars COM TRANSMIT are set to 0
    • The simvars PILOT/COPILOT TRANSMITTER TYPE are set to 4 (None)
  9. On both sides, turn ON all CALL (related to VHFs) push buttons
    • It's NOT possible to have two or more CALL push button active on one RMP
    • The simvars PILOT/COPILOT TRANSMITTER TYPE are set to 0 for COM1, 1 for COM2, 2 for COM3
    • The respective simvars COM TRANSMIT are set to 1 if at least one of the respective push buttons is on
  10. Push VHF3 on each RMP
    • The word DATA should appear centered in the active window
    • A frequency should appear in the standby window
  11. Push the transfer key
    • The text "VHF 3 VOICE" should appear on the ECAM (the underlying logic is not implemented yet)
    • Push the transfer key once gain, the message should disappear
  12. Rotate the knobs to change a frequency
    • Frequency decimal can now return to 0 after .975 even if the integer is 136 (tested irl)

For this part, refer to this link

It is very likely points 12 and 13 fail the test. I have just realized I did not implement the "unusable mechanism". I will implement it in the second PR even though I don't know how it is in real life. I will make the frequency windows blank and knobs off.

  1. Disconnect DC BUS 1 and AC BUS 1
    • VHF 3 (knob and push button) should be unusable
  2. Disconnect DC BUS 2 and AC BUS 2
    • VHF 2 (knob and push button) should be unusable
    • RMP 2 should be unusable

At this point, only VHF 1, RMP 1 and both ACPs should be usable. Engines, APU, GPU off.

  1. Disconnect DC ESS BUS and AC ESS BUS
    • Everything should be unusable

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

@juliansebline juliansebline changed the title fix: RMP behavior and lights [issue 344] fix(rmp): RMP behavior and lights Apr 25, 2022
@juliansebline
Copy link
Contributor Author

juliansebline commented Apr 26, 2022 via email

@tracernz
Copy link
Member

Thank you for your comment/ Would you have the precise list of phases affected ? I cannot find it on the internet. I suppose CLB CRZ DESCENT ?

BehEh provided the code above.

Requested changes

Co-authored-by: Benedict Etzel <[email protected]>
@@ -430,7 +431,7 @@
<FREQUENCY>3</FREQUENCY>
<UPDATE_CODE>
(A:COM TRANSMIT:#Radio_ID#, Bool) if{
#Button_ID# (&gt;L:XMLVAR_COM_Transmit_Channel)
<!-- #Button_ID# (&gt;L:XMLVAR_COM_PANEL#ID#_Transmit_Channel) -->
Copy link
Member

Choose a reason for hiding this comment

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

Why is this part commented out? Do we still need it, or is it unnecessary? We should either remove it, or make it real code again, or if not explain in the comment why it's commented out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was not able to spot the purpose for these lines of code. To me, it was part of all the problems.

Thefore, just in case for the future, I left the condition only.

Comment on lines 500 to 501
<!-- (L:XMLVAR_COM_#ID#_#FREQ_ID#_Switch_Down) -->
<!-- #Button_ID# (L:XMLVAR_COM_Transmit_Channel) == or 100 * -->
Copy link
Member

Choose a reason for hiding this comment

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

Same as above - let's either remove the comment, make it real code again, or add a notice why it's a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've commented these two lines because I couldn't understand their purpose. I couldn't see any changes with or without. But, as I am a newbie on this project and this language, I don't want to delete code I don't understand.

src/instruments/src/RMP/Components/StandbyFrequency.tsx Outdated Show resolved Hide resolved
src/instruments/src/RMP/Components/RadioPanelDisplay.tsx Outdated Show resolved Hide resolved
src/instruments/src/Common/EWDMessages.tsx Outdated Show resolved Hide resolved
src/instruments/src/EWD/elements/PseudoFWC.tsx Outdated Show resolved Hide resolved
src/instruments/src/EWD/elements/PseudoFWC.tsx Outdated Show resolved Hide resolved
Comment on lines 28 to 42
// // If the passed value prop is a number, we'll use formatFrequency to get string format.
let value = textDataModeVHF3;
let offsetx = '100%';

if (typeof props.value === 'number') {
if (props.value !== 0) {
value = formatFrequency(props.value);
}
} else {
value = props.value;
}

if (props.value === 0) {
offsetx = '85%';
}
Copy link
Member

Choose a reason for hiding this comment

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

This entire component feels a bit messy now. I expect one bug for example to be that if you're in data mode AND enable lights test, offsetx may be incorrectly at 85%.

How about something more like this:

interface Props {
    vale: string | number;
    dataMode?: boolean;
}
if (lightsTest) {
  content =  (
    <text x={offsetx} y="52%">
      8.8.8.8.8.8
    </text>
  );
}
else if (props.dataMode) {
  content =  (
    <text x="85%" y="52%">
     DATA
    </text>
  );
}
else {
  content =  (
    <text x="100%" y="52%">
     {value}
    </text>
  );
}
return  (
  <svg className="rmp-svg">
    {content}
  </svg>
);

Copy link
Contributor Author

@juliansebline juliansebline Apr 28, 2022

Choose a reason for hiding this comment

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

You are right. I'm gonna ask Shomas if he can check the behavior in the real airplane as well

juliansebline and others added 6 commits April 28, 2022 13:04
Requested changes: code and VHF3 VOICE format

Co-authored-by: Benedict Etzel <[email protected]>
Requested changes: code VHF3 voice

Co-authored-by: Benedict Etzel <[email protected]>
Requested changes: code VHF3 voice

Co-authored-by: Benedict Etzel <[email protected]>
Requested changes: textDataModeVHF3 to TEXT_DATA_MODE_VHF3
@2hwk 2hwk added this to the v0.9.0 milestone Apr 29, 2022
beheh
beheh previously requested changes Jun 22, 2022
.github/CHANGELOG.md Outdated Show resolved Hide resolved
From 0.4.0 to 0.8.0 for another one
@beheh beheh dismissed their stale review June 22, 2022 12:12

Implemented

@juliansebline
Copy link
Contributor Author

juliansebline commented Aug 3, 2022

Put these lines into a file with the extension .simvars and load it into the Watcher.

COM TRANSMIT:1,Bool,False
COM TRANSMIT:2,Bool,False
COM TRANSMIT:3,Bool,False
COM VOLUME:1,percent over 100,False
COM VOLUME:2,percent over 100,False
COM VOLUME:3,percent over 100,False
PILOT TRANSMITTER TYPE,Enum,False
COPILOT TRANSMITTER TYPE,Enum,False

@juliansebline juliansebline changed the title fix(rmp): RMP behavior and lights fix(rmp): RMP behaviour and lights Aug 3, 2022
@BlueberryKing BlueberryKing added the Exp Available on experimental branch (for testing) label Aug 7, 2022
@mico975
Copy link
Contributor

mico975 commented Aug 8, 2022

Quality Assurance Tester Report

Discord : mico#3145
Object of testing: (#7105
Tier of Testing : 1
Date : 08/08/2022

Testing Process:

At startup:
CALL1 should be active on both sides - OK
VHF1/2 volumes knobs should be on with values 0.8 and 0.4 (respectively) - OK
Left RMP should be set with VHF1 - OK
Right RMP should be set with VHF2 - OK
Respective SEL light should not be lit - OK
Simvars RECEIVE ALL and RECIEVE ALL should be set to 1 (for vPilot) - OK

On the left RMP, press VHF2 then VHF3
The left SEL led AND word should lit in orange on both sides - OK
Back to VHF1, the lights should go off - OK

On the right RMP, press VHF1 then VHF3
The right SEL led AND word should lit in orange on both sides - OK
Back to VHF2, the lights should go off - OK

On both sides, press VHF1/2/3 volume knobs
Each knob should turn on/off independently - OK
All knobs can be on/off - OK
Respective simvar should be set to 1 IF at least one knob of each side is on - OK
Respective simvar should be set to 0 IF each knob of each side is off - OK
When off, rotate a knob should be harder (to simulate real behaviour) - OK

On both sides, rotate VHF1/2/3 volume knobs
Each knob should rotate independently - OK
Respective simvar should be set to the greatest value of both knobs - OK

Turn off all volumes knobs
Rotate them should not update their respective simvar - OK
Turn them on, the simvar should be updated with the value of the knob OR the max value between the two knobs if both on - OK

On both sides, turn OFF all CALL (related to VHFs) push buttons
Their respective light is off - OK
The simvars COM TRANSMIT are set to 0 - OK
The simvars PILOT/COPILOT TRANSMITTER TYPE are set to 4 (None) - OK

On both sides, turn ON all CALL (related to VHFs) push buttons
It's NOT possible to have two or more CALL push button active on one RMP - OK
The simvars PILOT/COPILOT TRANSMITTER TYPE are set to 0 for COM1, 1 for COM2, 3 for COM3 - OK
The respective simvars COM TRANSMIT are set to 1 if at least one of the respective push buttons is on - OK

Push VHF3 on each RMP
The word DATA should appear centered in the active window - OK
A frequency should appear in the standby window - OK

Push the transfer key
The text "VHF 3 VOICE" should appear on the ECAM (the underlying logic is not implemented yet) - OK
Push the transfer key once gain, the message should disappear - OK

Rotate the knobs to change a frequency
Frequency decimal can now return to 0 after .975 even if the integer is 136 (tested irl) - OK

Disconnect DC BUS 1 and AC BUS 1 - not tested
VHF 3 (knob and push button) should be unusable

Disconnect DC BUS 2 and AC BUS 2 - not tested
VHF 2 (knob and push button) should be unusable
RMP 2 should be unusable
At this point, only VHF 1, RMP 1 and both ACPs should be usable. Engines, APU, GPU off.

Disconnect DC ESS BUS and AC ESS BUS - not tested
Everything should be unusable

Negatives:
See above in testing process

Testing Results:
Passed (electrical system config still needs to be tested by someon)

Conclusions:
LGTM

2hwk pushed a commit that referenced this pull request Aug 9, 2022
2hwk added a commit that referenced this pull request Aug 9, 2022
2hwk added a commit that referenced this pull request Aug 12, 2022
2hwk added a commit that referenced this pull request Aug 12, 2022
@2hwk
Copy link
Member

2hwk commented Aug 16, 2022

Disconnect DC BUS 1 and AC BUS 1

  • VHF 3 (knob and push button) should be unusable

Disconnect DC BUS 2 and AC BUS 2

  • VHF 2 (knob and push button) should be unusable
  • RMP 2 should be unusable
  • At this point, only VHF 1, RMP 1 and both ACPs should be usable. Engines, APU, GPU off.

Disconnect DC ESS BUS and AC ESS BUS

  • Everything should be unusable

Remaining items passed

@2hwk 2hwk enabled auto-merge (squash) August 16, 2022 13:24
@2hwk 2hwk disabled auto-merge August 16, 2022 13:25
@2hwk 2hwk enabled auto-merge (squash) August 16, 2022 13:29
@2hwk 2hwk merged commit f86d18f into flybywiresim:master Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Exp Available on experimental branch (for testing) QA Tier 1
Projects
None yet
7 participants