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

Feature: SF Audio amplifier mute 2.10 port #3753

Merged
merged 10 commits into from
Sep 14, 2023

Conversation

mha1
Copy link
Contributor

@mha1 mha1 commented Jul 3, 2023

This is a port of #3741 to main

This PR adds "Audio amplifier mute" to the SF/GF list. Possible use cases:

  • total silence, no noise, not only volume down
  • home brew BT Audio module picking up the audio signal at the amplifier input to use wireless ear buds. Silencing the speakers is necessary. Using SF makes that easy

@3djc
Copy link
Collaborator

3djc commented Jul 14, 2023

Why are you doing it differently than your 2.9 version ? 2.9 had the function always defined, but not this one

@mha1
Copy link
Contributor Author

mha1 commented Jul 14, 2023

Sorry, what do you mean?

@3djc
Copy link
Collaborator

3djc commented Jul 14, 2023

Sorry, what do you mean?

You answered it there : #3753 (comment)

@mha1
Copy link
Contributor Author

mha1 commented Jul 21, 2023

@3djc you good with this?

@3djc
Copy link
Collaborator

3djc commented Jul 21, 2023

On my phone because of travel. Ok on the principle, haven’t tested.

@mha1
Copy link
Contributor Author

mha1 commented Jul 21, 2023

@pfeerick I know the SF AUDIO_AMP_OFF shouldn't have gone in 2.9. But now it is and I found I confused FUNCTION_* with FUNC_*. 2.9 tested ok but it's still wrong and I don't know about the sideeff ects. Can you please try and cherry pick 3ced094 and 336919e to 2.9. Or should I create a 2.9 (yuck) PR?

@gagarinlg
Copy link
Member

@pfeerick I know the SF AUDIO_AMP_OFF shouldn't have gone in 2.9. But now it is and I found I confused FUNCTION_* with FUNC_*. 2.9 tested ok but it's still wrong and I don't know about the sideeff ects. Can you please try and cherry pick 3ced094 and 336919e to 2.9. Or should I create a 2.9 (yuck) PR?

We expect you at the EdgeTX fest, on your knees and begging for forgiveness.

@mha1
Copy link
Contributor Author

mha1 commented Jul 21, 2023

bless your heart

@Eldenroot
Copy link
Contributor

Thats why I do not understand cherrypicking into 3 branches :)

@mha1
Copy link
Contributor Author

mha1 commented Jul 21, 2023

Just 2. 33% less effort :-)

@pfeerick
Copy link
Member

pfeerick commented Jul 25, 2023

lol... just 1 branch actually 😛 Has been cherry-picked and squished into one commit, but not pushed yet.

@mha1 mha1 force-pushed the PR_SF_Audio_Amp_Mute_2.10 branch from 5f52089 to fac6d1c Compare August 2, 2023 11:42
@pfeerick pfeerick added this to the 2.10 milestone Aug 3, 2023
@pfeerick pfeerick force-pushed the PR_SF_Audio_Amp_Mute_2.10 branch from fac6d1c to 643dd9d Compare August 4, 2023 04:00
@raphaelcoeffic
Copy link
Member

@pfeerick I assigned you this PR so you can clear your name (and @3djc as your chaperon to make double sure the code is ok).

@3djc
Copy link
Collaborator

3djc commented Aug 12, 2023

Tested ok on T20 (and tested function is not there on X7 where it should not)
Nothing obviously wrong in the code that I could spot

@mha1 mha1 force-pushed the PR_SF_Audio_Amp_Mute_2.10 branch from 643dd9d to fac6d1c Compare September 12, 2023 18:42
@mha1 mha1 force-pushed the PR_SF_Audio_Amp_Mute_2.10 branch from fac6d1c to 759aee1 Compare September 12, 2023 18:48
@pfeerick
Copy link
Member

pfeerick commented Sep 14, 2023

Ah ... fk... think I just forgot to merge this after the rebase passed build tests 🤷‍♂️ (thanks for the eyes and tests JC :) )

@pfeerick pfeerick merged commit d30499f into EdgeTX:main Sep 14, 2023
@mha1
Copy link
Contributor Author

mha1 commented Sep 14, 2023

thx - no problemo

pfeerick added a commit that referenced this pull request Sep 21, 2023
@pfeerick pfeerick mentioned this pull request Mar 11, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants