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

sig_analog: Allow immediate fake ring to be suppressed. #119

Merged

Conversation

InterLinked1
Copy link
Contributor

When immediate=yes on an FXS channel, sig_analog will start fake audible ringback that continues until the channel is answered. Even if it answers immediately, the ringback is still audible for a brief moment.
This can be disruptive and unwanted behavior.

This adds an option to disable this behavior, though the default behavior remains unchanged.

Resolves: #118
ASTERISK-30003
Imported from: https://gerrit.asterisk.org/c/asterisk/+/19715

UserNote: The immediatering option can now be set to no to suppress the fake audible ringback provided
when immediate=yes on FXS channels.

@InterLinked1
Copy link
Contributor Author

cherry-pick-to: 20
cherry-pick-to: 18

jcolp
jcolp previously approved these changes May 30, 2023
@gtjoseph gtjoseph added the cherry-pick-test Trigger dry run of cherry-picks label Jun 5, 2023
@github-actions github-actions bot added cherry-pick-testing-in-progress Cherry-Pick tests in progress cherry-pick-checks-passed Cherry-Pick checks passed cherry-pick-gates-failed Cherry-Pick gates failed and removed cherry-pick-test Trigger dry run of cherry-picks cherry-pick-testing-in-progress Cherry-Pick tests in progress labels Jun 5, 2023
Copy link
Member

@gtjoseph gtjoseph left a comment

Choose a reason for hiding this comment

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

No merger commits please.
Please squash.

@InterLinked1
Copy link
Contributor Author

No merger commits please. Please squash.

Whoops, I was attempting to fix a merge conflict but I guess that didn't work out.

Rebasing like with Gerrit doesn't seem to get anywhere. Is there a good way of squashing offline? I know the "Squash and merge" on GitHub somehow does it magically but attempts to squash via git rebase -i HEAD~2 haven't seemed to have made it better.

Alternately, rather than wrestling with this, is it fine if I just submit a new PR (same content, but base branch updated properly first) or will that mess things up?

When immediate=yes on an FXS channel, sig_analog will
start fake audible ringback that continues until the
channel is answered. Even if it answers immediately,
the ringback is still audible for a brief moment.
This can be disruptive and unwanted behavior.

This adds an option to disable this behavior, though
the default behavior remains unchanged.

ASTERISK-30003 #close
Resolves: asterisk#118

UserNote: The immediatering option can now be set to no to suppress
the fake audible ringback provided when immediate=yes on FXS channels.
@InterLinked1
Copy link
Contributor Author

@gtjoseph I'm not sure if you were trying to update something on this PR (I don't see that on other PRs) but I'm now seeing a notice that the branch is out of date, do I need to rebase or is it good as is? Don't want to just click the magic button, as I'm sure that might screw something up again.

@gtjoseph
Copy link
Member

@gtjoseph I'm not sure if you were trying to update something on this PR (I don't see that on other PRs) but I'm now seeing a notice that the branch is out of date, do I need to rebase or is it good as is? Don't want to just click the magic button, as I'm sure that might screw something up again.

"Out of date" is OK. It just means that something else was merged into "master" since you last updated the PR. You can rebase off of current master if you want, but it's not necessary. You should do it locally then force push the PR rather than using the "Update branch" button.

If it ever says "Merge conflict" then something went in that will make your PR fail to merge. You'll need to locally rebase your PR branch from master, resolve any conflicts and force push your PR back up again.

@InterLinked1
Copy link
Contributor Author

@gtjoseph I'm not sure if you were trying to update something on this PR (I don't see that on other PRs) but I'm now seeing a notice that the branch is out of date, do I need to rebase or is it good as is? Don't want to just click the magic button, as I'm sure that might screw something up again.

"Out of date" is OK. It just means that something else was merged into "master" since you last updated the PR. You can rebase off of current master if you want, but it's not necessary. You should do it locally then force push the PR rather than using the "Update branch" button.

If it ever says "Merge conflict" then something went in that will make your PR fail to merge. You'll need to locally rebase your PR branch from master, resolve any conflicts and force push your PR back up again.

Okay, thanks for clarifying. Still a little confused, since it says you've requested changes, but I don't see what the branch being out of date would have to do with that otherwise, is there something else I need to do? I addressed the rebase issue, but it sounds like there's another issue?

@gtjoseph gtjoseph added the cherry-pick-test Trigger dry run of cherry-picks label Jun 30, 2023
@github-actions github-actions bot added cherry-pick-testing-in-progress Cherry-Pick tests in progress and removed cherry-pick-test Trigger dry run of cherry-picks labels Jun 30, 2023
@gtjoseph
Copy link
Member

The only change I requested was the squash. Not sure why it didn't dismiss automatically when you re-submitted.

@github-actions github-actions bot added cherry-pick-checks-passed Cherry-Pick checks passed cherry-pick-gates-failed Cherry-Pick gates failed and removed cherry-pick-testing-in-progress Cherry-Pick tests in progress labels Jun 30, 2023
@github-actions
Copy link

Successfully merged to branch master and cherry-picked to ["20","18"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[new-feature]: chan_dahdi: Allow fake ringing to be inhibited when immediate=yes
3 participants