[Home]

Summary:ASTERISK-29118: VoiceMail() should have an option to play greetings as Early Media
Reporter:Juan Carlos Castro y Castro (jccyc)Labels:patch
Date Opened:2020-10-09 11:31:51Date Closed:2020-12-01 11:24:44.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail/NewFeature
Versions:17.7.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) asterisk-voicemail-greetings-early-media-v2.patch
Description:It'd be nice if billsec in the CDR was zero if the person calling an unavailable endpoint, when executing VoiceMail(), hangs up before the “leave message” beep? That’s how cellphone operators here behave and I believe it's an useful feature.
Comments:By: Asterisk Team (asteriskteam) 2020-10-09 11:31:52.346-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. Please note that log messages and other files should not be sent to the Sangoma Asterisk Team unless explicitly asked for. All files should be placed on this issue in a sanitized fashion as needed.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur.

Please note that by submitting data, code, or documentation to Sangoma through JIRA, you accept the Terms of Use present at [https://www.asterisk.org/terms-of-use/|https://www.asterisk.org/terms-of-use/].
By: Juan Carlos Castro y Castro (jccyc) 2020-10-09 11:35:26.971-0500

The new option for early media is 'e', if it should be another letter please advise. Also, I send a progress message so you don't have to call Progress() in the dialplan beforehand.
By: Kevin Harwell (kharwell) 2020-10-12 14:57:39.336-0500

[~jccyc], would you be interested in going ahead and posting this patch to gerrit [1]?

[1] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage
By: Juan Carlos Castro y Castro (jccyc) 2020-10-14 18:29:20.220-0500

I'm setting it up as we speak. Turns out I CAN change my e-mail even if my old one is unaccessible.
By: Juan Carlos Castro y Castro (jccyc) 2020-10-14 18:54:26.914-0500

The Change-Id didn't appear when editing the commit message. I did the setup process correctly.
By: Kevin Harwell (kharwell) 2020-10-15 12:35:16.285-0500

hrm odd. I wonder if the gerrit commit script didn't get copied. In the top level source directory try the following:
{noformat}
$ git review -s
{noformat}
Any kind of error? If that doesn't work try copying it directly with the following (again from your local "asterisk" source directory):
{noformat}
scp -p -P 29418 <your user name>@gerrit.asterisk.org:hooks/commit-msg ".git/hooks/"
{noformat}
Once it's copied down just amend your commit, and save and it should add it:
{noformat}
$ git commit --amend
{noformat}
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 14:35:32.378-0500

Ok, starting from scratch in another machine, Ubuntu 20.04, running as root. Apparently it did go through but it got "merged" (if that makes sense) with a previous commit from another author. When trying "git commit" without --amend it'd say there's no changes.

{{
jcastro@pbx:~$ cat >.ssh/config
Host asterisk
 Hostname gerrit.asterisk.org
 Port 29418
 User jcastro
jcastro@pbx:~$ mkdir src
jcastro@pbx:~$ git clone -b 17 asterisk:asterisk asterisk-17-git
Cloning into 'asterisk-17-git'...
remote: Counting objects: 20918, done
remote: Finding sources: 100% (9464/9464)
remote: Total 345201 (delta 2122), reused 340585 (delta 2122)
Receiving objects: 100% (345201/345201), 206.87 MiB | 6.03 MiB/s, done.
Resolving deltas: 100% (260062/260062), done.
jcastro@pbx:~$ sudo apt-get -y install git-review
Reading package lists... Done
Building dependency tree      
Reading state information... Done
The following NEW packages will be installed:
 git-review
0 upgraded, 1 newly installed, 0 to remove and 0 not upgraded.
Need to get 35.6 kB of archives.
After this operation, 161 kB of additional disk space will be used.
Get:1 http://br.archive.ubuntu.com/ubuntu focal/universe amd64 git-review all 1.27.0-1 [35.6 kB]
Fetched 35.6 kB in 0s (336 kB/s)      
Selecting previously unselected package git-review.
(Reading database ... 142600 files and directories currently installed.)
Preparing to unpack .../git-review_1.27.0-1_all.deb ...
Unpacking git-review (1.27.0-1) ...
Setting up git-review (1.27.0-1) ...
Processing triggers for man-db (2.9.1-1) ...
jcastro@pbx:~$ git config --global --add user.email jccyc1965@gmail.com
jcastro@pbx:~$ git config --global --add user.name "Juan Carlos Castro y Castro"
jcastro@pbx:~$ cd asterisk-17-git
jcastro@pbx:~/asterisk-17-git$ git checkout -b ASTERISK-29118
Switched to a new branch 'ASTERISK-29118'
jcastro@pbx:~/asterisk-17-git$ patch -p1 <~/asterisk-voicemail-greetings-early-media-v2.patch
patching file apps/app_voicemail.c
jcastro@pbx:~/asterisk-17-git$ git add apps/app_voicemail.c
jcastro@pbx:~/asterisk-17-git$ git commit
Aborting commit due to empty commit message.
jcastro@pbx:~/asterisk-17-git$ ls .git/hooks/
applypatch-msg.sample  fsmonitor-watchman.sample  pre-applypatch.sample  pre-merge-commit.sample    pre-push.sample    pre-receive.sample
commit-msg.sample      post-update.sample         pre-commit.sample      prepare-commit-msg.sample  pre-rebase.sample  update.sample
jcastro@pbx:~/asterisk-17-git$ scp -p -P 29418 jcastro@gerrit.asterisk.org:hooks/commit-msg ".git/hooks/"                
commit-msg                                                                                                                                                    100% 1790    12.2KB/s   00:00    
jcastro@pbx:~/asterisk-17-git$ git commit --amend      
[ASTERISK-29118 811ae7d2d5] asterisk: Add verbose message stating support status.
Author: Joshua C. Colp <jcolp@sangoma.com>
Date: Mon Oct 12 07:30:52 2020 -0300
2 files changed, 29 insertions(+), 5 deletions(-)
jcastro@pbx:~/asterisk-17-git$ git commit
On branch ASTERISK-29118
nothing to commit, working tree clean
jcastro@pbx:~/asterisk-17-git$ git review 17
Creating a git remote called 'gerrit' that maps to:
       ssh://jcastro@gerrit.asterisk.org:29418/asterisk.git
remote:
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: (-)
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: refs: 1 (|)
remote: Processing changes: refs: 1, done            
remote: commit 31b9226: warning: subject >50 characters; use shorter first paragraph        
remote: commit 31b9226: Missing issue-id in commit message        
remote: Commit 31b9226347bfcc0dee29b0dd1a1631996acaa15b not associated to any issue        
remote:
remote: Hint: insert one or more issue-id anywhere in the commit message.        
remote:       Issue-ids are strings matching (ASTERISK-[0-9]+)        
remote:       and are pointing to existing tickets on its-jira Issue-Tracker        
To ssh://gerrit.asterisk.org:29418/asterisk.git
! [remote rejected]       HEAD -> refs/for/17%topic=ASTERISK-29118 (change https://gerrit.asterisk.org/c/asterisk/+/15052 closed)
error: failed to push some refs to 'ssh://jcastro@gerrit.asterisk.org:29418/asterisk.git'
}}

By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 14:38:06.543-0500

No, it seems it didn't. What's wrong?
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 14:41:44.050-0500

NOW it went through, I hadn't typed the issue ID in the commit message.

remote:
remote: Processing changes: (\)
remote: Processing changes: (|)
remote: Processing changes: (/)
remote: Processing changes: new: 1 (-)
remote: Processing changes: new: 1 (\)
remote: Processing changes: new: 1 (|)
remote: Processing changes: refs: 1, new: 1 (|)
remote: Processing changes: refs: 1, new: 1 (|)
remote: Processing changes: refs: 1, new: 1 (|)
remote: Processing changes: refs: 1, new: 1, done            
remote: commit e5087c2: warning: subject >50 characters; use shorter first paragraph        
remote:
remote: SUCCESS        
remote:
remote:   https://gerrit.asterisk.org/c/asterisk/+/15064 voicemail: add option 'e' top play greetings as early media [NEW]        
remote:
To ssh://gerrit.asterisk.org:29418/asterisk.git
* [new branch]            HEAD -> refs/for/17%topic=ASTERISK-29118

J.H.C., I can't even format text properly in these comments.
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 14:46:51.053-0500

I also had errors when trying to cherry pick to master:

jcastro@pbx:~/asterisk-17-git$ git review --cherrypick 15064 master
Downloading refs/changes/64/15064/1 from gerrit
There was a problem applying changeset contents to the current branch.
The following command failed with exit code 1
   "git cherry-pick FETCH_HEAD"
-----------------------
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

   git commit --allow-empty

Otherwise, please use 'git cherry-pick --skip'
On branch ASTERISK-29118
You are currently cherry-picking commit e5087c284c.
 (all conflicts fixed: run "git cherry-pick --continue")
 (use "git cherry-pick --skip" to skip this patch)
 (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean
-----------------------

By: Kevin Harwell (kharwell) 2020-10-15 14:53:20.826-0500

Glad you were able to finally upload it, and thanks for submitting the patch.
By: Kevin Harwell (kharwell) 2020-10-15 14:55:56.033-0500

It's possible you might be getting the cherry pick errors due to "merged" patches. In your 17 review I see code from a previous user submitted patch (the stuff in main/asterisk.c). Try removing that, re-uploading the 17 patch and then cherry-picking
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 15:01:16.658-0500

By "removing that" you mean what, reverting main/asterisk.c to its contents before that change?
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 15:12:32.479-0500

Now the review only has my file (and it's a different one, the commit message came with two change ids and I tried deleting the new one), but the cherry pick still gives me an error.

jcastro@pbx:~/asterisk-17-git$ git review --cherrypick 15065 18
Downloading refs/changes/65/15065/1 from gerrit
There was a problem applying changeset contents to the current branch.
The following command failed with exit code 1
   "git cherry-pick FETCH_HEAD"
-----------------------
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

   git commit --allow-empty

Otherwise, please use 'git cherry-pick --skip'
On branch review/juan_carlos_castro_y_castro/ASTERISK-29118
Your branch and 'gerrit/17' have diverged,
and have 1 and 1 different commits each, respectively.
 (use "git pull" to merge the remote branch into yours)

You are currently cherry-picking commit 1d68bdf4a6.
 (all conflicts fixed: run "git cherry-pick --continue")
 (use "git cherry-pick --skip" to skip this patch)
 (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean
-----------------------

By: Kevin Harwell (kharwell) 2020-10-15 15:13:16.012-0500

{quote}
By "removing that" you mean what, reverting main/asterisk.c to its contents before that change?
{quote}
The change in _main/asterisk.c_ has already been merged into the code base (see review [15052|https://gerrit.asterisk.org/c/asterisk/+/15052]. I am not entirely sure how those changes ended  as part of your patch. If you rebase (17 branch):
{noformat}
$ git rebase 17
{noformat}
what happens?
By: Kevin Harwell (kharwell) 2020-10-15 15:15:03.733-0500

Try a "git pull" first actually. It appears your branch might be "behind".
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 15:15:43.601-0500

I can't bash my head against this much more, I have to do actual work. I hope this process is in an acceptable state now.
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 15:16:46.926-0500

Aaaaaand we have a catch-22.

jcastro@pbx:~/asterisk-17-git$ git pull
fatal: You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).
Please, commit your changes before you merge.

By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 15:19:44.527-0500

Did the rebase, no differences. I'm out of here. Really.

jcastro@pbx:~/asterisk-17-git$ git rebase 17
First, rewinding head to replay your work on top of it...
Applying: voicemail: add option 'e' top play greetings as early media
jcastro@pbx:~/asterisk-17-git$ git diff
jcastro@pbx:~/asterisk-17-git$ git add apps/app_voicemail.c
jcastro@pbx:~/asterisk-17-git$ git commit --amend
fatal: You are in the middle of a cherry-pick -- cannot amend.
jcastro@pbx:~/asterisk-17-git$

By: Kevin Harwell (kharwell) 2020-10-15 15:20:32.517-0500

Ah yeah 17 branch looks as it should now. I went ahead and abandoned your first one as it is no longer needed.
By: Kevin Harwell (kharwell) 2020-10-15 15:22:31.499-0500

No worries. I got them cherry-picked. Once you got the second 17 review up they cherry-picked cleanly through the UI.

Thanks again!
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 15:23:19.960-0500

Should I delete my source tree and re-clone it?
By: Juan Carlos Castro y Castro (jccyc) 2020-10-15 15:24:39.759-0500

I really REALLY hope next time I submit a fix it takes me less than 20x the work it took to actually make the fix. Cheers!
By: Kevin Harwell (kharwell) 2020-10-15 15:37:56.135-0500

{quote}Should I delete my source tree and re-clone it?{quote}
Might be easiest to start things over. Once you have a "fresh" environment you could pull down your review/patch using something like the following:
{noformat}
$ git review -d <gerrit review id>
{noformat}
So in your case for the 17 branch it should be:
{noformat}
$ git review -d 15065
{noformat}
Sorry about the setup issues. On the plus side it should be easier next time :-) If you do decide to submit more patches, or run into other problems I'd recommend joining us in our IRC #asterisk-dev channel [1]. It's easier to discuss things in real time. Also others are around to help there too.

[1] https://wiki.asterisk.org/wiki/display/AST/IRC

Thanks again!
By: Juan Carlos Castro y Castro (jccyc) 2020-10-17 09:39:18.659-0500

I've made some changes suggested by Sean, re-submitted successfully, but the cherripick gave me an error AGAIN. I did a "git commit --allow-empty" as it suggested and I got this output. Is this OK?

jcastro@pbx:~/src/asterisk-17-git$ git review --cherrypick 15065 18
Downloading refs/changes/65/15065/3 from gerrit
There was a problem applying changeset contents to the current branch.
The following command failed with exit code 1
   "git cherry-pick FETCH_HEAD"
-----------------------
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

   git commit --allow-empty

Otherwise, please use 'git cherry-pick --skip'
On branch review/juan_carlos_castro_y_castro/ASTERISK-29118
Your branch and 'gerrit/17' have diverged,
and have 1 and 2 different commits each, respectively.
 (use "git pull" to merge the remote branch into yours)

You are currently cherry-picking commit 5b012ed16e.
 (all conflicts fixed: run "git cherry-pick --continue")
 (use "git cherry-pick --skip" to skip this patch)
 (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean
-----------------------
jcastro@pbx:~/src/asterisk-17-git$ git commit --allow-empty
[review/juan_carlos_castro_y_castro/ASTERISK-29118 69c522050d] voicemail: add option 'e' top play greetings as early media
Author: Joshua C. Colp <jcolp@sangoma.com>
Date: Mon Oct 12 07:30:52 2020 -0300
jcastro@pbx:~/src/asterisk-17-git$
By: Kevin Harwell (kharwell) 2020-10-19 09:40:21.552-0500

Not quite sure what's going on with the cherry picks (looks like maybe the branch is "behind" the remote?). No worries though as it looks like it cherry picked cleanly from the gerrit UI ([~seanbright] took care of it).

For future reference though you can cherry pick from the gerrit UI if you like by selecting the 3-vertical dot dropdown button in the top right, and the choosing "cherry pick". Although to be fair sometimes that does now work too.
By: Friendly Automation (friendly-automation) 2020-12-01 11:24:45.023-0600

Change 15059 merged by George Joseph:
voicemail: add option 'e' to play greetings as early media

[https://gerrit.asterisk.org/c/asterisk/+/15059|https://gerrit.asterisk.org/c/asterisk/+/15059]
By: Friendly Automation (friendly-automation) 2020-12-01 11:24:57.071-0600

Change 15058 merged by George Joseph:
voicemail: add option 'e' to play greetings as early media

[https://gerrit.asterisk.org/c/asterisk/+/15058|https://gerrit.asterisk.org/c/asterisk/+/15058]
By: Friendly Automation (friendly-automation) 2020-12-01 11:25:15.714-0600

Change 15112 merged by George Joseph:
voicemail: add option 'e' to play greetings as early media

[https://gerrit.asterisk.org/c/asterisk/+/15112|https://gerrit.asterisk.org/c/asterisk/+/15112]