[Home]

Summary:ASTERISK-13864: [patch] add possibility to read
Reporter:Clod Patry (junky)Labels:
Date Opened:2009-03-30 14:18:22Date Closed:2011-06-07 14:03:25
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Functions/func_volume
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) volume_read.diff
Description:Currently, with VOLUME() with can only set values with that function.

In some cases, it could be useful to read the existing value.


****** ADDITIONAL INFORMATION ******

Here's the tests i've made:
exten => 141,1,Set(VOLUME(rx)=4);
exten => 141,n,NooP(volume rx is ${VOLUME(rx)});
exten => 141,n,Set(VOLUME(tx)=-3);
exten => 141,n,NooP(volume rx is ${VOLUME(rx)});
exten => 141,n,NooP(volume tx is ${VOLUME(tx)});
exten => 141,n,Set(VOLUME(tx)=$[${VOLUME(tx)} + 4]);
exten => 141,n,NooP(volume tx is ${VOLUME(tx)});suppose to get 1 (-3+4)
exten => 141,n,NoOp(volume invalid is ${VOLUME(junky)});

and here's the result:
shooter*CLI> console dial 141@default
 == Console is full duplex
   -- Executing [141@default:1] Set("Console/dsp", "VOLUME(rx)=4") in new stack
   -- Executing [141@default:2] NoOp("Console/dsp", "volume rx is 4") in new stack
   -- Executing [141@default:3] Set("Console/dsp", "VOLUME(tx)=-3") in new stack
   -- Executing [141@default:4] NoOp("Console/dsp", "volume rx is 4") in new stack
   -- Executing [141@default:5] NoOp("Console/dsp", "volume tx is -3") in new stack
   -- Executing [141@default:6] Set("Console/dsp", "VOLUME(tx)=1") in new stack
   -- Executing [141@default:7] NoOp("Console/dsp", "volume tx is 1") in new stack
   -- Executing [141@default:8] NoOp("Console/dsp", "volume invalid is INVALID") in new stack
Comments:By: Clod Patry (junky) 2009-03-30 14:30:00

That new version doesnt init datastore, as pointed per eliel on IRC.
By: Mark Michelson (mmichelson) 2009-03-30 14:38:32

Good idea, JunK-Y. There are some issues with your patch though.

1. This will not compile in dev mode because you have mixed declarations with the rest of the code. If you move the *buf = '\0'; line to after the declarations, this warning will go away.

2. Always make sure to lock the channel when performing datastore operations.

3. The portion of code which handles if the datastore cannot be found is incomplete. You create the datastore and the volume_information struct; however, you never attach the datastore to the channel, nor do you set the datastore's data field to point to the newly created volume_information struct.

4. The last item is a minor coding guidelines violation. For one-line if statements, you should use curly braces.
By: Mark Michelson (mmichelson) 2009-03-30 14:52:23

Ah, I notice that a lot of the volume_read function was copied from volume_write. volume_write has several of the same problems that I mentioned in ~102421.


By: Eliel Sardanons (eliel) 2009-04-01 14:38:59

You have removed the code that initialize the audiohook, that is correct, but you also need to avoid creating a datastore on that channel if you are reading the value. Just reply with a ast_copy_string(buf, "0", len);

If you allocate a datastore, then the next call to valume_write won't create the audiohook.
By: Leif Madsen (lmadsen) 2009-09-18 09:18:42

Setting status to Feedback while we await changes by JunK-Y. I'm sure this has fallen off the radar of the last few months due to GSoC, but hopefully he can pick it back up soon.
By: Jason Parker (jparker) 2009-10-06 12:00:45

I think this is superseded by ASTERISK-13708.  It adds the ability to read, along with the rewrite.  Thoughts?
By: Leif Madsen (lmadsen) 2009-10-26 10:05:40

I'm going to agree with Qwell here, so I'm going to close this for now unless someone else disagrees.