[Home]

Summary:ASTERISK-24332: [patch] Add ability to set "fromstring" per mailbox
Reporter:Leandro Dardini (ldardini)Labels:patch
Date Opened:2014-09-16 14:56:26Date Closed:
Priority:MinorRegression?
Status:Open/NewComponents:Applications/app_voicemail/NewFeature
Versions:12.5.0 13.18.4 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_voicemail.c.diff
Description:When using ODBC Voicemail, the "fromstring" variable is not read from the database and the default value is always used. Not only, but the code seems to check for the len of a not defined variable.
Comments:By: Leandro Dardini (ldardini) 2014-09-16 14:58:47.712-0500

Please review carefully because I am no more a C developer... however this patch seems to fix the issue.

By: Michael L. Young (elguero) 2014-09-16 20:15:12.660-0500

The "fromstring" is a global setting, not an individual mailbox setting.  Your patch changes the expected behavior.  Also, I think that the patch would break things for those who using a standard configuration file.

Where is this "fromstring" being placed? Is it in the ODBC table for the individual mailbox?  If so, it shouldn't be there.  The "fromstring" is supposed to be for the general configuration for voicemail for that system, if I am not mistaken.

By: Leandro Dardini (ldardini) 2014-09-17 05:42:00.081-0500

I don't agree with your point of view. What is valid for static configuration file is not valid for ODBC realtime interface. If I check the /etc/asterisk/voicemail.conf, I can find in the [general] section parameters like "serveremail" or "attachfmt" or "sayduration". All these parameters can be defined for each voicemail box using the ODBC realtime interface. In the code app_voicemail.c all these parameters are defined in the ast_vm_user struct for each voicemail box. I am stating the "fromstring" parameter is missing from the ast_vm_user struct and is impossible to set using the ODBC realtime interface.

By: Michael L. Young (elguero) 2014-09-17 10:53:32.269-0500

I think this email thread helps to clarify what I was trying to say:

http://lists.digium.com/pipermail/asterisk-users/2011-December/268615.html

That setting is not a per mailbox setting.  It is a global setting that is not meant to be overridden.  You should not assume that every setting in the general section can be overridden.

Now with that said, what it is you are trying to do is add a new feature, correct?  In that case this should not be marked as a bug, which is how I was approaching this issue.  I will mark this as an improvement request and we can then look at the patch from that standpoint.

Thank you for your contribution and for trying to clarify what you are trying to communicate.  It is appreciated as a whole.

By: Michael L. Young (elguero) 2014-09-17 11:14:58.049-0500

In regards to your current patch, you are removing the use of the global variable "fromstring".  That should be added back in.  By default, you would want to use the global variable and only if vmu->fromstring is set, then override it.

Since you are adding a setting to the mailbox, you should add the ability to see this setting in the manager output as well for the mailbox.

One other thing, it would be good to add this setting to the unit test that is in the code.

If you want to work on these items and update your patch, feel free to do so.  If you feel that you need someone to help you, I am willing to work up a patch for this new feature and then have you test it out for us.

By: Leandro Dardini (ldardini) 2014-09-17 16:11:47.960-0500

Really thank you for the time you are dedicating at this improvement. Yes, I need your help. I worked with C too many years ago and just understanding the asterisk code was a big effort. Writing something reasonable will be almost an impossible task for me. I will be really thankful if you can work up a patch for that and I'll be ready to test extensively.