Re: [asterisk-dev] Configuring multistream in chan_pjsip

[Mark Michelson]

On 06/05/2017 03:17 PM, Matt Fredrickson wrote:

On Mon, Jun 5, 2017 at 2:31 PM, Joshua Colp <jc...@digium.com> wrote:

On Mon, Jun 5, 2017, at 04:21 PM, Mark Michelson wrote:

Hi folks,

For those of you following along at home, I recently published review
https://gerrit.asterisk.org/#/c/5760/ , which is step one towards making
chan_pjsip multistream, i.e. supporting more than one stream of a given
media type. This initial review does not actually introduce multistream
support so much as it just makes use of multistream structures under the
hood to ease the transition.

Continuing on, one of the next steps we need to determine is how a user
of chan_pjsip should configure a channel that supports multiple streams
of a particular type.

To refresh everyone on how things currently work in pjsip.conf, you set
an "allow" option in order to determine what codecs a particular
endpoint supports.

[Alice]
type = endpoint
allow = ulaw,opus,h264





I propose the following configuration options to move forward.

offered_audio_streams = 1
offered_video_streams = 2



My only question is why, in a scenario where we don't have a hint, would
we want to make the number of offered streams configurable by the user?
Ultimately it's up to the application that is handling the channel to
decide what it wants and that is decided in the moment, not ahead of
time based on configuration.

I think maximum and minimum are useful for enforcing some constraints
though.

That echoes my thoughts as well.  +1 to not having the
"offered_audio/video_streams" options, but I'm ok with the limiting of
maximum stream count for now.

Cool. I'm all for having fewer options. Sounds like if a user wants a 
certain number of streams to be offered during origination, that should 
be a parameter for the origination, then.



--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Configuring multistream in chan_pjsip

[Mark Michelson]

 downside I see is that if you wanted to be able to configure 
separate capabilities for different streams, then that is not possible 
with this configuration scheme.


What are people's thoughts on this? Got better configuration ideas? Can 
you think of something I'm not thinking of? For the time being, I'd like 
to focus on the configuration file/realtime options moreso than dialplan 
functions that would pertain to a single call. We can get to that 
discussion after this part gets settled.


Thanks,
Mark Michelson

--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Asterisk 13 Queues - eventwhencalled=vars

[Mark Michelson]

On 03/06/2017 02:44 PM, Gabriel Ortiz Lour wrote:

Hi all,
  Just saw that there is no more the "vars" option, well, there is no 
more the "eventwhencalled" option.


  How can I get the "vars" feature now? I need the channel variables 
along whith the Agent** events. Any way without having to poke on the 
source?

Att.
Gabriel



Hi Gabriel,

First, this question is more suited for the asterisk-users list than the 
asterisk-developers list since this does not pertain directly to the 
Asterisk source code. I'll answer here anyway this time, though.


From the CHANGES file in Asterisk 12+:

 * The configuration options eventwhencalled and eventmemberstatus have 
been

   removed.  As a result, the AMI events QueueMemberStatus, AgentCalled,
   AgentConnect, AgentComplete, AgentDump, and AgentRingNoAnswer will 
always be
   sent.  The "Variable" fields will also no longer exist on the Agent* 
events.

   These events can be filtered out from a connected AMI client using the
   eventfilter setting in manager.conf.

In other words, the events will always be sent. You don't need to set 
the eventwhencalled option. Since channel snapshots are sent in these 
events, channel variables can already be included. In your manager.conf 
file, in the general section, set


channelvars = foo,bar,baz

Every time a channel is sent in a manager event, the channel variables 
foo, bar, and baz will be sent.


I hope this helps,
Mark Michelson

--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Symmetric RTP

[Mark Michelson]

On 02/23/2017 08:28 AM, Andreas Krüger wrote:

Hi,

I was wondering with the following setup, if you would use Symmetric NAT.

+--+
+--+| 89.192.76.11 | 
+--+
|Client 1  | LAN|   Router | Internet| 
72.87.123.32 |
| 172.32.11.17 + -> | 172.32.11.1  + --> | 
 Asterisk|

+--+  +--+ +--+


I'm using PJSIP, and have set 
the external_media_address, external_signaling_address and local_net 
for the transport.


The client is connecting with WSS and also using ICE.

force_rport is set to yes in ps_endpoints section for the client. Also 
direct_media is no.


Regards,

Andreas Krüger


I think that all depends on ICE is configured. If the client is not 
connecting to a STUN server, then ICE will only advertise the client's 
host candidate(s). Since Asterisk cannot reach the client directly, you 
would want to enable symmetric RTP so that Asterisk directs its media to 
the router instead.


If the client is using a STUN server, it should advertise the router's 
public IP address as a server reflexive candidate. The ICE connectivity 
checks should result in Asterisk sending to that address. Therefore you 
would not need to enable symmetric RTP in that case.
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Deadlock GIT 13

[Mark Michelson]

On 02/08/2017 11:26 AM, Ross Beer wrote:


Hi,


I'm getting a deadlock every so often with asterisk 13 GIT. There are 
a lot of low level locks:



#0  0x7f64ca6f052d in nanosleep () from /lib64/libc.so.6
[Current thread is 1 (Thread 0x7f64cdcc9700 (LWP 20745))]
#0  0x7f64ca6f052d in nanosleep () at /lib64/libc.so.6
#1  0x7f64ca6f03c4 in sleep () at /lib64/libc.so.6
#2  0x004fc844 in db_sync_thread (data=0x0) at db.c:980
__PRETTY_FUNCTION__ = "db_sync_thread"
#3  0x0060401e in dummy_start (data=0x1e04440) at utils.c:1235
__cancel_buf = {__cancel_jmp_buf = {{__cancel_jmp_buf =
{0, -361566044794415684, 140733401961727, 140070926194432, 507904,
0, -361566044802804292, 302566635355346364}, __mask_was_saved =
0}}, __pad = {0x7f64cdcc8df0, 0x0, 0x0, 0x0}}
__cancel_routine = 0x452814 
__cancel_arg = 0x7f64cdcc9700
__not_first_call = 0
ret = 0x7f64cdcc8df0
a = {start_routine = 0x4fc765 , data =
0x0, name = 0x1e055b0 "db_sync_thread   started at [ 1022]
db.c astdb_init()"}
#4  0x7f64cb3ee61a in start_thread (arg=0x7f64cdcc9700) at
pthread_create.c:334
__res = 
pd = 0x7f64cdcc9700
now = 
unwind_buf = {cancel_jmp_buf = {{jmp_buf =
{140070926194432, 302565808931837372, 140733401961727,
140070926194432, 507904, 0, -361566044792318532,
-361562863565433412}, mask_was_saved = 0}}, priv = {pad = {0x0,
0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
not_first_call = 
pagesize_m1 = 
sp = 
freesize = 
#5  0x7f64ca72a5fd in clone () at /lib64/libc.so.6


Should an issue be created on Jira?


Regards,


Ross



Hi Ross,

This may be the same issue that George discovered earlier today while 
doing some testing. If you apply the code change on this review 
https://gerrit.asterisk.org/#/c/4902/ , do you still see the issue?


Mark Michelson
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] SDP API Wiki Page

[Mark Michelson]

To piggyback on Josh's e-mail to the list, I have also placed a new page
[1] on the wiki detailing the plans to create an SDP API for Asterisk 15.

The core Asterisk team has a goal of creating a better video experience
(yes, that's purposely vague) in the upcoming major releases of Asterisk.
When "video" arises these days, the most common targets for video are
WebRTC endpoints. In order to get better WebRTC support, we need to add
support for a bunch of SDP parameters that we currently do not support. If
you look at Asterisk today, each channel driver that uses SDP to establish
media sessions contains their own SDP parsing/negotiating logic. If we are
going to be adding SDP features, it makes sense to have a single layer that
we can add options to without having to touch SDP-using channel drivers.
Furthermore, if new WebRTC-driven channel drivers get written, it is silly
to have to copy SDP logic around between all of those.

Thus the SDP API comes into being. The linked page describes the current
SDP negotiation process that occurs in the PJSIP channel driver, and then
it proposes an actual API. The API internals are deliberately not present
on the page since the API design is more important at this stage.

If people could take a look at the SDP API page and offer feedback, that
would be great. There are some specific questions at the bottom of the wiki
page that would be good to get answered.

Thanks,
Mark Michelson

[1] https://wiki.asterisk.org/wiki/display/AST/SDP+Work
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Problems with custom C-file: undefined reference

[Mark Michelson]

On 12/05/2016 04:53 PM, Julian Fleischhauer wrote:

Hey all,

I have a problem using a custom C-file. The error ouput received when 
compiling is given below.


error output
.../system.c:1330: undefined reference to `XML_ParserCreate'
...
.../system.c:1465: undefined reference to `sip_get_header'
...
.../system.c:1608: undefined reference to `pcap_lookupnet'
...


Before, I had all code in chan_sip.c. I want to transfer the code into 
a separate file (system.c) now. The dependencies given below 
MODULEINFO are customly set and worked fine when used in chan_sip.c. 
Can anyone figure out if something else has to be added somewhere? The 
code is quoted below:


sip.h
const char *sip_get_header(const struct sip_request *req, const char 
*name);


system.h
#include "asterisk/netsock2.h"

void ast_some_function(struct sip_pvt *p, struct ast_sockaddr *addr);

system.c
/*** MODULEINFO
 sqlite3
 expat
 pcap
extended
***/

#include "asterisk/system.h"
#include "asterisk/sip.h"

void ast_some_function(struct sip_pvt *p, struct ast_sockaddr *addr)
{
xmlParser = XML_ParserCreate(NULL);
   ...
   ast_copy_string(touser, sip_get_header(req, "To"), 
strlen(sip_get_header(req,   "To"))+1);

   ...
   pcap_lookupnet(pcapDev, , , errbuf);
}

chan_sip.c
#include "asterisk/system.h"
#include "asterisk/sip.h"

ast_some_function(p, addr);


Thank you for any help!

Regards,
Juliannn

I can explain why sip_get_header() is causing a problem. In order for 
functions to be callable from other modules, they need to be explicitly 
exported. This is why you'll see files like res/res_agi.exports.in, 
which define functions that can be called globally from that module. You 
have a couple of options here:


* Define a .exports.in file for your system.c file that exports the 
sip_get_header() function.
* Change "sip_get_header()" to "ast_sip_get_header()". Assuming that 
system.c is in the main/ directory, there is a project-wide exports file 
that makes it so any function starting with "ast" can be called externally.


Regarding the XML and pcap issues, the issues are either that
* You're not #including the necessary files.
* The libraries are not being linked during compilation.

The first problem is easy to fix: just #include the right files :). The 
second problem is a bit more tricky because you'll need to dig into 
Makefiles and try to figure out how libraries get linked when Asterisk 
is built.


--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] ARI versioning in 13 and 14

[Mark Michelson]

Hi!

We were just about to start making the next round of release candidates 
for Asterisk. In doing so, we knew that we needed to bump the minor 
versions of AMI and ARI since new stuff had been added since the 
previous release. For AMI, this was fairly trivial. For ARI...well...


Currently, Asterisk 13 reports the ARI version as being 1.9.0. Asterisk 
14 reports the version as being 1.10.0. Asterisk 14 contains ARI 
commands not present in 13. If we bump the ARI version in 13, then it 
becomes 1.10.0. It therefore would have the same version as the initial 
release of 14, which leads to confusion. An ARI developer might think 
this means that Asterisk 13 has gained the new ARI commands introduced 
in Asterisk 14. But it hasn't.


What should we do here? We've currently thought of a few of ways of 
going with this:


1) Just bump the minor versions and document somewhere that the ARI 
version is "local" to a particular release of Asterisk. Therefore 
Asterisk 14's 1.10.0 is not the same as Asterisk 13's 1.10.0


2) Bump the major version of ARI for each major release of Asterisk. We 
won't retroactively apply this to the upgrade from Asterisk 12 to 
Asterisk 13. So Asterisk 13 will have ARI versions 1.X.Y, Asterisk 14 
will have ARI versions 2.X.Y, and Asterisk 15 will end up with Asterisk 
3.X.Y


3) Set the major version of ARI identical to the major version of 
Asterisk. Going forward, Asterisk 13 will have ARI versions 13.X.Y, 
Asterisk 14 will have ARI versions 14.X.Y, and Asterisk 15 will have ARI 
versions 15.X.Y


My personal choice would be to go with option 2. Option 1 is just 
generally confusing and makes keeping track of ARI versions difficult. 
Option 3 is tempting, but it could also cause confusion since the minor 
versions of Asterisk and ARI will be out of sync (e.g. a user might 
assume that Asterisk 14.3.0 would require ARI version 14.3.0, when 
that's not actually the case).


Feel free to voice your support for one of these options or to suggest 
something of your own.


Thanks,
Mark Michelson


--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Sending control frame outside a call

[Mark Michelson]

On 11/09/2016 01:11 PM, wferre...@nc.rr.com wrote:

Hi,

I have a customer who is running 12.8.2 and since their software needs to be 
certified at considerable expense and time will not be upgrading Asterisk 
anytime soon. In addition due to security requirements the only connection 
between their Asterisks is a single IAX2 peer. They would like to support BFCP 
(Binary Floor Control Protocol – RFC 4582). The issue is BFCP sends information 
in the SIP SDP such as TCP connection and port info and then assumes the two 
endpoints have set up a socket between them to exchange BFCP protocol messages. 
I am not allowed to open a direct socket between the BFCP endpoints due to 
security concerns, all traffic, signaling, messaging must go over the single 
IAX2 connection. So I can open a socket from asterisk to the BFCP client/server 
using the port information received the SIP SDP. Then the BFCP endpoint sends 
the protocol message to the asterisk and I need to send it across the IAX2 
connection to the other asterisk who will then send it on to its intended 
destination. What I am trying to do is create a new  ast_control_frame_type and 
send the BFCP protocol message as data in a control frame. The problem I am 
running into is determining how to send a message to a peer outside a call. All 
the methods I have found such as ast_queue_control_data assume an active call 
that has been bridged. I also tried to use the socket send method using the 
sockfd defined in the iax2_peer structure, but it returned a -1. So what I am 
looking for is the base methods for sending/reading data to/from an IAX2 peer. 
Is this possible? If so, can anyone point me to some code that does what I want?


The hard part for answering this determining what you mean when you say 
you want to convey information to another party "outside a call" . I can 
read this a couple of ways:


1) A SIP endpoint will send Asterisk arbitrary data in some sort of 
request (like maybe an OPTIONS request), and you then want to send this 
data over IAX2 to the other side. The endpoints involved are not calling 
each other at all. It's just that information received in Asterisk by 
one endpoint has to then be conveyed out another interface.


2) A SIP endpoint will initiate a call that will end up going out the 
IAX2 side. Prior to call establishment, you need to send information out 
the IAX2 side


Let's go over each of these.

For situation (1), there are no ast_channel structures involved at all. 
So there's no way to be queuing frames or sending frames between 
channels. Instead, you'll need to use other methods within Asterisk. 
Since you're using Asterisk 12, your best bet is to make use o the 
Stasis message bus. The idea would be that the SIP channel driver that 
receives the incoming messages would raise a Stasis event. The IAX2 
channel driver would be subscribed to those events and potentially react 
by sending messages out where necessary. If you want to learn more about 
the Stasis message bus, there is a real good explanation of the API in 
include/asterisk/stasis.h in the comments at the top of the file. If you 
want to see examples of it in action, I think device state is a good 
place to look. You can look in main/devicestate.c to see where the 
topics and messages get created, as well as transmission of events. You 
can grep for "ast_device_state_message_type()" to find places that 
subscribe to device state messages.


For situation (2), You'll have a channel making an incoming call, 
presumably with some BFCP information in the incoming SDP. However at 
the point where you want to make an outbound call, the channels involved 
are not communicating via frames. That doesn't start happening until the 
outbound call gets established. In such a case, you'll have several 
options for getting the necessary information from the inbound channel 
to the outbound channel. One way I'll recommend is to make dialplan 
functions to retrieve BFCP information, set channel variables (preceded 
by "__" so they get inherited on the outbound channels), and then have 
chan_iax2 read those channel variables and communicate what is necessary 
prior to making the outbound call.


As far as the mechanics for sending information on the IAX2 socket, I'm 
afraid I can't help you there. I don't have much direct experience with 
the inner workings of chan_iax2, and the best I could offer would be 
speculation of what might work based on my reading of the code.



--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] ARI StasisEnd event vs. channel variables

[Mark Michelson]

I have opened https://issues.asterisk.org/jira/browse/ASTERISK-26492 and
have attached the patch there. Feel free to try it out and let me know on
the issue how it works for you.

On Fri, Oct 21, 2016 at 8:37 AM, Sébastien Duthil <sdut...@proformatique.com
> wrote:

> On 19/10/16 11:59 AM, Mark Michelson wrote:
> > I actually have a patch I've written locally in my spare time that
> > adds an ari.conf option called "channelvars" that works exactly the
> > same as for manager.conf. In other words, it modifies channel
> > snapshots to have the channel variables specified in ari.conf. Then,
> > every channel snapshot that is part of an event sent on the ARI
> > websocket will have those variables.
> >
> > I came up with this idea because in addition to Sebastien's comment
> > about wanting variables on StasisEnd, other people told me they want
> > channel variables on other events. Their reasoning being that they
> > don't want the delay of having to look up channel variables every time
> > channel state changes. By emitting a subset of channel variables
> > (those that you care about in ARI), the message size can be limited to
> > whatever you want.
>
> That's great news! Could you share this patch? Since other people have
> expressed such a need, and it would also solve the StasisEnd variables
> problem, I think this solutions is the best bet.
>
> > I had thought about making the list of channel variables per-ARI app
> > instead of global, but I don't know if that's actually desired.
>
> The underlining question would be: do people use ARI as one big
> application that does all the logic, or do they use ARI as many small
> applications that handle different parts of their logic? In the "one big
> application" case, global configuration is enough. In the "many small
> applications" case, the different applications will probably need
> different variables, and global configuration will clutter the events
> with variables that application mostly don't care about. I guess
> per-application configuration fits better with the philosophy of ARI.
>
> --
> Sébastien Duthil
>
>
>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] app_queue retry limit

[Mark Michelson]

On 10/20/2016 09:51 AM, marek cervenka wrote:

hi,

we have questions from busy call centers if the retry time parameter 
can be 0 (wait time before next agent call)


in app_queue its prohibited

  } else if (!strcasecmp(param, "retry")) {
q->retry = atoi(val);
if (q->retry <= 0) {
q->retry = DEFAULT_RETRY;
}

do you remember someone from the past if there were reasons for 
minimal 1 second wait time?


thanks

Marek



I checked the history a bit.


commit e068c5fcf98c7a7f625718ff8b7a2f8cb52cc794: "Allow 0 retry time"

https://issues.asterisk.org/jira/browse/ASTERISK-3550 .

The reporter seemed to want just what you wanted.


commit b6ccca75667b99ddfb8ed77b70b5dec27847bff3 was "Setting a retry of 
0 is generally not a good idea and shouldn't be allowed."


https://issues.asterisk.org/jira/browse/ASTERISK-7379 .

If you read that report, it sounds like a zero retry was making queues 
behave bizarrely. It was decided then to go back to not allowing a zero 
retry. The issue does not give a lot of detail about why the bad 
behavior was happening, but BJ Weschke pointed out that we were passing 
a zero timeout to ast_waitfor_digits(), which was odd.



--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] ARI StasisEnd event vs. channel variables

[Mark Michelson]

On 10/19/2016 06:45 AM, Matthew Jordan wrote:

On Tue, Oct 18, 2016 at 4:02 PM, Joshua Colp  wrote:

Matthew Jordan wrote:





There are a few wrinkles with emitting channel variables with
arbitrary events (of which StasisEnd would qualify).

When an event is emitted out of the APIs, it's typically being done so
as a result of a message having been published on the Stasis message
bus. The messages on the bus don't contain a reference to a channel,
but rather a snapshot of the channel. Those snapshots, by default,
don't contain any channel variables, primarily for performance
reasons. Additionally, since dialplan functions can be freely
interchanged with channel variables, it's impossible (or very costly)
to evaluate dialplan function expressions on each snapshot creation.

Unfortunately, you really do need to rely on the messages coming
across the bus. By the time code is processing the sending of a
StasisEnd event, the ast_channel itself may already be gone. As such,
the variables have to be stored on the snapshot in some fashion.

AMI accomplished this by creating a core concept known as 'manager
variables'. These are set directly on the ast_channel structure, and -
when a snapshot is created - are evaluated and copied onto the
snapshot. Again, this is a rather costly operation, but it does limit
the impact of including variables on messages, as you have to
explicitly tell Asterisk which state you want transmitted along with
the channel.

If we wanted a similar concept in ARI, the manager variables could be
reused. I would note that you absolutely will take a performance hit
when using them - building and conveying that much state in every
event is not cheap. Because StasisEnd is rather 'special' - often the
channel core isn't aware of it - I'm not sure how you would convey
variables only on that event and not on all the rest.


The actual raising of StasisEnd is explicitly done inside of Stasis and
lives there. When raised the channel is there and valid. It currently uses a
channel blob as the payload but this could be changed to something else that
would allow channel variables (only channel variables explicitly set on the
channel) to also be added. This would scope the channel variables addition
to only StasisEnd. Now... if we wanted channel variables on every message...
then yeah, it would be difficult.


Two thoughts:
(1) I wonder if anyone needs the state conveyed on events other than
StasisEnd? If Asterisk is maintaining all of your state for you, and
you need to know some part of the state in your external application,
how do you ask for it?

(2) Even if we emit variable state on StasisEnd, how would we know
which state to emit? Would we want to require users to provide it up
front in ari.conf, or through some more dynamic mechanism?


I actually have a patch I've written locally in my spare time that adds 
an ari.conf option called "channelvars" that works exactly the same as 
for manager.conf. In other words, it modifies channel snapshots to have 
the channel variables specified in ari.conf. Then, every channel 
snapshot that is part of an event sent on the ARI websocket will have 
those variables.


I came up with this idea because in addition to Sebastien's comment 
about wanting variables on StasisEnd, other people told me they want 
channel variables on other events. Their reasoning being that they don't 
want the delay of having to look up channel variables every time channel 
state changes. By emitting a subset of channel variables (those that you 
care about in ARI), the message size can be limited to whatever you want.


I had thought about making the list of channel variables per-ARI app 
instead of global, but I don't know if that's actually desired.


--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] I want to make my first patch but have run into a problem and don't know how to progress.

[Mark Michelson]

g ast_monitor_setjoinfiles(chan, 1) after ast_monitor_start() in 
start_automonitor() of bridge_builtin_features.c. This approach would be 
better than copying/pasting because it keeps the code in one spot and 
makes use of already tested code.


Hope this was helpful,
Mark Michelson
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] BRIDGEPEER on multi-party conferences: Thoughts?

[Mark Michelson]

An alternative could be to set BRIDGEPEER to something like "multi-party",
"conference", or the bridge ID if there are more than two participants.
This way, the only channel you have to set a variable on is the one
joining/leaving the bridge, which means you are not having to lock the
bridge at all. Yes, there would likely be a special case for when the
number of channels first goes from 2 to 3, but the majority of the time,
that's not what would be happening. This way, you still get the VarSet when
the channel enters and leaves, and you have a non-empty value for
BRIDGEPEER when the channel is with multiple parties in a bridge. You won't
get VarSets on the other channels in a bridge when a channel enters or
leaves, though. I'm not sure if that's something that people rely on or not.

On Wed, Aug 10, 2016 at 10:11 AM, Corey Farrell <g...@cfware.com> wrote:

> It might be a problem if BRIDGEPEER is being compared to an empty string
> to see if a channel is alone.
>
> What if you made BRIDGEPEER into a built-in channel variable (like
> ${EPOCH}).  This would mean that you wouldn't be setting any channels,
> you'd only do a lookup when requested.  One side-effect is that VarSet
> events would never be produced for this variable, not sure how much this
> would matter given better events to monitor ConfBridge joins/parts?
>
> On Tue, Aug 9, 2016 at 7:01 PM, Mark Michelson <mmichel...@digium.com>
> wrote:
>
>> Hi folks,
>>
>> I've been looking into a Digium customer issue where ConfBridge audio has
>> been dropping out. The main issue there had to do with DNS, and there is
>> currently a review [1] up to fix that.
>>
>> A secondary issue, though, is that there would be brief audio cutouts
>> when participants joined and left the conference. Looking into it, I
>> believe the problem is that when a participant enters or leaves the bridge,
>> the BRIDGEPEER channel variable is updated for every remaining participant
>> in the bridge. The basic algorithm is like this:
>>
>> * Lock the bridge
>> * Iterate through the channels in the bridge (a maximum of 11 of them)
>> * Lock the channel
>> * Append the channel to an array of channel names
>> * Unlock the channel
>> * Iterate through the channels in the bridge again (no upper limit this
>> time)
>> * Lock the channel
>> * Set the BRIDGEPEER channel variable using the names in the
>> generated array from before (comma-separated)
>> * Unlock the channel
>> * Unlock the bridge
>>
>> In addition, this same process occurs every time an announcement is
>> played into a bridge, such as join/leave beeps. Playing an announcement
>> involves pushing an announcer channel into the bridge and then removing the
>> announcer channel when the playback completes.
>>
>> My question to the list is this: do you find value in having the
>> participants in a multi-party bridge packaged into the BRIDGEPEER channel
>> variable? I know that for two-party bridges, there are probably lots of
>> scripts and dialplans out there that rely on that variable to be set; my
>> question specifically is for bridges with more than two parties.
>>
>> My thoughts on the matter are that since bridges are query-able now
>> directly, getting the list of participants from the bridge makes more sense
>> than trying to get the participants based on a single channel in that
>> bridge. In addition, the code places a hard limit on the number of channel
>> names it will actually put in the BRIDGEPEER variable. The code currently
>> has a #define that makes it so that only the first 11 channels in the
>> bridge will have their names in BRIDGEPEER. Because of the hard-coded
>> maximum, if you have more than 11 channels in the bridge, you can't get the
>> full list of participants using BRIDGEPEER.
>>
>> By not setting BRIDGEPEER on channels in multi-party bridges, we can
>> avoid holding the bridge ransom while calculating the value and setting the
>> variable.
>>
>> Let me know what your thoughts are on the matter.
>>
>> Thanks,
>> Mark Michelson
>>
>> [1] https://gerrit.asterisk.org/#/c/3445
>>
>>
>> --
>> _
>> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>>
>> asterisk-dev mailing list
>> To UNSUBSCRIBE or update options visit:
>>   http://lists.digium.com/mailman/listinfo/asterisk-dev
>>
>
>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] BRIDGEPEER on multi-party conferences: Thoughts?

[Mark Michelson]

Hi folks,

I've been looking into a Digium customer issue where ConfBridge audio 
has been dropping out. The main issue there had to do with DNS, and 
there is currently a review [1] up to fix that.


A secondary issue, though, is that there would be brief audio cutouts 
when participants joined and left the conference. Looking into it, I 
believe the problem is that when a participant enters or leaves the 
bridge, the BRIDGEPEER channel variable is updated for every remaining 
participant in the bridge. The basic algorithm is like this:


* Lock the bridge
* Iterate through the channels in the bridge (a maximum of 11 of them)
* Lock the channel
* Append the channel to an array of channel names
* Unlock the channel
* Iterate through the channels in the bridge again (no upper limit this 
time)

* Lock the channel
* Set the BRIDGEPEER channel variable using the names in the 
generated array from before (comma-separated)

* Unlock the channel
* Unlock the bridge

In addition, this same process occurs every time an announcement is 
played into a bridge, such as join/leave beeps. Playing an announcement 
involves pushing an announcer channel into the bridge and then removing 
the announcer channel when the playback completes.


My question to the list is this: do you find value in having the 
participants in a multi-party bridge packaged into the BRIDGEPEER 
channel variable? I know that for two-party bridges, there are probably 
lots of scripts and dialplans out there that rely on that variable to be 
set; my question specifically is for bridges with more than two parties.


My thoughts on the matter are that since bridges are query-able now 
directly, getting the list of participants from the bridge makes more 
sense than trying to get the participants based on a single channel in 
that bridge. In addition, the code places a hard limit on the number of 
channel names it will actually put in the BRIDGEPEER variable. The code 
currently has a #define that makes it so that only the first 11 channels 
in the bridge will have their names in BRIDGEPEER. Because of the 
hard-coded maximum, if you have more than 11 channels in the bridge, you 
can't get the full list of participants using BRIDGEPEER.


By not setting BRIDGEPEER on channels in multi-party bridges, we can 
avoid holding the bridge ransom while calculating the value and setting 
the variable.


Let me know what your thoughts are on the matter.

Thanks,
Mark Michelson

[1] https://gerrit.asterisk.org/#/c/3445

--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Icelandic numbers, dates and voicemail

[Mark Michelson]

I think you should feel free to submit this patch if you want it included
in mainline Asterisk. If you are interested in submitting you patch, see
https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage . That page details
how to submit a patch to the Asterisk project.

Just as a note, I don't think many of us are going to be able to properly
analyze the usage of Icelandic. However, we will gladly check code
correctness (i.e. it's not going to crash or anything).

On Thu, May 26, 2016 at 12:50 PM, Örn Arnarson  wrote:

> Hello,
>
> I've patched say.c and app_voicemail.c of Asterisk version 13.9.0 to
> support the Icelandic language w.r.t. dates and numbers. I was wondering
> whether or not you guys would want this submitted into the codebase?
> Icelandic is obviously a fringe language, so nobody else might use this,
> ever. Still, I figured I'd ask in case people would be interested.
>
> The code is somewhat tested (has been running for years -- since
> 1.6.2.17.2. I just ported it to 13.9.0 because we were upgrading), though
> of course we're probably not doing everything that's possible, so some
> portions might be untested. However, everything we've tried seems to work
> just fine (including very large numbers).
>
> I haven't submitted anything to the project before, so I don't know how
> much of a bother it is. Presumably I'd have to patch the most recent
> version of Asterisk in Git?
>
> Regards,
> Örn
>
>
>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Issues with ARI python example for recording

[Mark Michelson]

On 03/18/2016 06:18 AM, Nitesh Bansal wrote:

Hello,

I'm using the latest version of ari-py library.
I'm trying the following demo 
https://wiki.asterisk.org/wiki/display/AST/ARI+and+Media%3A+Part+1+-+Recording.


Everything is setup correctly, I can call Asterisk, but when I press 
the DTMF '#' to

stop the recording, my python library throws an exception.


Entering recording state
Recording voicemail at voicemail/ 6/1458299017.29
stopping recording LiveRecording(voicemail/ 6/1458299017.29)
ERROR:ari.client:Event listener threw exception
Traceback (most recent call last):
  File "build/bdist.linux-x86_64/egg/ari/client.py", line 100, in __run
callback(msg_json, *args, **kwargs)
  File "build/bdist.linux-x86_64/egg/ari/client.py", line 198, in 
extract_objects

event_cb(obj, event, *args, **kwargs)
  File "build/bdist.linux-x86_64/egg/ari/model.py", line 181, in fn_filter
fn(objects, event, *args, **kwargs)
  File 
"/root/asterisk_ari/ari-py/examples/ari_bridges/recording_demo/recording_state.py", 
line 39, in on_dtmf

self.recording.stop()
  File "build/bdist.linux-x86_64/egg/ari/model.py", line 155, in 
enrich_operation

return promote(self.client, oper(**kwargs), oper.json)
  File "build/bdist.linux-x86_64/egg/ari/model.py", line 354, in promote
resp.raise_for_status()
  File "/usr/lib/python2.7/dist-packages/requests/models.py", line 
773, in raise_for_status

raise HTTPError(http_error_msg, response=self)
HTTPError: 404 Client Error: Not Found

It is throwing error on the line
'self.recording.stop'

Personally, I can't see anything wrong with this code, any ideas what 
I'm doing wrong or

is there any bug in the ARI lib?

Thanks,
Nitesh


Hi.

It's hard to tell exactly what's going wrong here, but Asterisk is 
responding to the HTTP request with a 404. This likely means that 
Asterisk can't find the recording in question. Although it is also 
possible that the URI being passed by python to Asterisk does not 
"resolve" as expected. If you look at the HTTP traffic, you may be able 
to tell more clearly why the 404 is being sent.


It's certainly possible there's a bug in Asterisk, but I would be more 
willing to bet that there's some sort of error in the python example. 
Although, like you, I don't immediately see the problem in the python code.


Mark Michelson

--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] LibPRI and LibSS7 Migrated to git

[Mark Michelson]

Hi folks,

We have now migrated the LibPRI and LibSS7 projects to git [1] [2]. The
projects are also mirrored at github [3] [4] and git.asterisk.org [5] [6] .
Development in these projects should be done in git now.

The main development branches and tags have been imported, but team
branches have not. The subversion repositories are still around, so if you
want to migrate a team branch from subversion to gerrit, feel free to do
so. Instructions for creating team branches can be found on the Asterisk
wiki [7].

LibPRI had an interesting structure in subversion. The 1.4 branch served as
the development trunk, and there was no svn trunk. In gerrit, a master
branch has been created that is exactly the same as the 1.4 branch. I won't
try to dictate the development model to be used in git, but I have left the
option available to use a traditional git master or to use the same method
that had been used in subversion.

Mark Michelson

[1] https://gerrit.asterisk.org/#/admin/projects/libpri
[2] https://gerrit.asterisk.org/#/admin/projects/libss7
[3] https://github.com/asterisk/libpri
[4] https://github.com/asterisk/libss7
[5] http://git.asterisk.org/gitweb/?p=libpri.git;a=summary
[6] http://git.asterisk.org/gitweb/?p=libss7.git;a=summary
[7]
https://wiki.asterisk.org/wiki/display/~mjordan/Working+with+Team+Branches
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] reloading XML docs for 'core show application'

[Mark Michelson]

On 03/04/2016 09:14 AM, Jeremy Kister wrote:
after installing a module in asterisk, I want to also install some 
docs for it for "core show application"


I can easily insert the appropriate xml in 
documentation/core-en_US.xml but the only way I've found Asterisk will 
use the new xml is to restart.


Is there another way/method to get my docs in there without restarting ?



The best way to do this is to insert the XML directly into the source 
for your module. If you bound your XML inside a


/*** DOCUMENTATION
***/

section, then Asterisk will, during compilation/installation, add the 
XML to core-en_US.xml so that it will be present when Asterisk starts up.


There currently is no way to adjust XML documentation at runtime and 
have it reflected in "core show application" though. A restart is necessary.


--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Mixmonitor omits some frames

[Mark Michelson]

In most cases, flushing an audiohook is not a bad thing, nor is it
something to be worried about. That's why it's a debug message. If you want
it in your logs, then set the core debug level to 1 so it shows up. You
will get more messages than you are used to seeing, but by only setting the
core debug level to 1, you will not get as badly flooded as you might if
you were to set the level higher.

On Fri, Jan 29, 2016 at 4:15 AM, Tomec Martin  wrote:

> Hi all,
>
>
>
> we are solving issue with mixmonitor – in production asterisk, there are
> sometimes gaps in recordings (missing syllables), but we cannot reproduce
> it in test enviroment.
>
> I suppose it is caused by frames flushing in audiohook.c:
>
>
>
>*if* (*ast_test_flag(audiohook, **AST_AUDIOHOOK_TRIGGER_SYNC**)*
> && other_factory_samples && (our_factory_ms - other_factory_ms >
> AST_AUDIOHOOK_SYNC_TOLERANCE)) {
>
>  *ast_debug(1, **"Flushing audiohook %p so it remains in
> sync\n"**, audiohook)*;
>
>  ast_slinfactory_flush(factory);
>
>  ast_slinfactory_flush(other_factory);
>
>}
>
>
>
> My question is, why it is not logged as a warning. I think that warning
> could help to diagnose the problem in production enviroment. Am I missing
> some „rules“ for log levels, or am I right?
>
>
>
> Thanks for any hint
>
> Martin Tomec
>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Asterisk 13.7.0 app PAGE plays participant count

[Mark Michelson]

Either with Page or Confbridge itself. Either

1) The Page application is not telling Confbridge not to announce the
number of users in the conference properly.
2) The Confbridge application is announcing the number of users despite the
Page application's request for quiet.

As a workaround, you can set announce_user_count=no in confbridge.conf for
your confbridge user. However, this will also make it so you don't hear a
user count when you call Confbridge normally.

On Wed, Jan 27, 2016 at 6:35 PM, Ross Beer  wrote:

>
>  Hi,
>
> I'm having an issue with the PAGE feature in Asterisk 13.7.0, when
> connecting a PAGE it says 'there is only one other participant in the
> conference'.
>
> This shouldn't be happening, I know that PAGE used ConfBridge however it
> should only use this to bridge the call.
>
> The options I am using are as follows:
>
>
> Page(,ni)
>
>
> The options should suppress any messages however this is not the case. Is
> this currently a bug with the PAGE feature?
>
> Thanks,
>
> Ross
>
> --
> _
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Proposing change to Queue and missed calls behavior

[Mark Michelson]

On 12/04/2015 01:00 PM, Stian Hvatum wrote:

Hi,
I have a problem with an accompanying solution that I wish to share, but I am 
not sure if it is valuable enough or correct enough to be suggested as a patch 
to Asterisk.

When SIP-phones are members of a queue, they tend to accumulate "missed calls" unless the C-flag is 
applied. When the C-flag is applied there is no missed calls at all, since all hangups are marked as 
"answered elsewhere". I have created and attached a patch which makes the C-flag "answer 
elsewhere" all hangups that are caused by app_queue canceling the dial or the call is really answered 
elsewhere, but sets normal cause when caller actually hangs up before the call is answered.

I know the patch alters the behavior of the C-flag, and altering behavior of existing flags is 
probably a bad thing. I can try to create a new flag for this "missed call on caller 
hangup"-behavior if that is of any value. Also, I don't have much experience with the Asterisk 
source code, so if I break something by setting normal cause here I would be very happy if anyone 
would give me a hint about it. The only problem I have seen so far is that if the caller hangs up 
during an announcement or between app_queue's dial outs, the call is not marked as missed (as the 
previous call was "answered elsewhere" and no new calls went out to the phones).

The code is running a few places without causing any trouble as far as I can 
tell. I wrote this after a customer had a few thousand missed calls on his 
queue-connected phone...

Best regards and thanks for a great project!
Stian Hvatum


In my opinion, if the c-flag is set and the caller hangs up, you are 
correct that the "answered elsewhere" status should not be applied. I 
also think the "answered elsewhere" status should not be applied if a 
call to a single queue member times out before the member answers the 
call. The c-flag behavior should only be applied when multiple queue 
members' phones are ringing at the same time, and Asterisk has to cancel 
the outgoing call to certain members due to the call being answered by 
someone else.


I think the change you are suggesting would be welcome.

--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Bridges, T.38, and other good times

[Mark Michelson]

On 12/06/2015 07:57 PM, Matthew Jordan wrote:

Hello all -

One of the efforts that a number of developers in the community here 
at Digium have been at work at are cleaning up test failures exposed 
by Jenkins [1]. One of these, in particular, has been rather difficult 
to resolve - namely, fax/pjsip/directmedia_reinvite_t38 [2]. This 
e-mail goes over what has been accomplished, and asks some questions 
on how we might try and fix Asterisk under this scenario.


The directmedia_reinvite_t38 test attempts to do the following:
 (1) UAC1 calls UAC2 through Asterisk, with audio as the media. The 
dial is performed using the 'g' flag, such that UAC2 will continue on 
if UAC1 hangs up.
 (2) UAC1 and UAC2 are configured for direct media. Asterisk sends a 
re-INVITE to UAC1 and UAC2 to initiate direct media.
 (3) After responding with a 200 OK to the direct media requests, UAC1 
sends a re-INVITE offering T.38.

 (4) Asterisk sends an INVITE with T.38 to UAC2
 (5) UAC2 sends back a 200 OK for T.38; Asterisk sends that to UAC1. 
Asterisk switches out of a direct media bridge to a core bridge.
 (6) UAC1 hangs up. Asterisk sends a re-INVITE to UAC2 for audio back 
to Asterisk. UAC2 responds with a 200 OK for the audio.

 (7) Asterisk ejects UAC2 back to the dialplan.

It's important to note that this test never should have passed - an 
update to the test suite "fixed" the test erroneously passing, which 
led to us investigating why the scenario was failing. This test was 
copied over from an identical chan_sip test, which passes.


The PJSIP stack has two issues which make life difficult for it in 
this scenario:
(1) The T.38 logic is implemented in res_pjsip_t38. While that is 
_mostly_ a very good thing - as it keeps all the fax state logic 
outside of the channel driver - we are also a layer removed from 
interactions that occur in the channel driver. That makes it 
challenging to influence direct media checks and other 
Asterisk/channel interactions.
(2) Being very asynchronous, requests may be serviced that influence 
T.38 state while other interactions are occurring in the core. 
Informing the core of what has occurred can have more race conditions 
than what occurs in chan_sip, which is single threaded.


The first bug discovered when the test was investigated was an issue 
in step (2). We never actually initiated a direct media re-INVITE. 
This was due to res_pjsip_t38 using a frame hook, and not implementing 
the .consume_cb callback. That callback allows a framehook to inform 
the core (and also the bridging framework) of the types of frames that 
a framehook wants to consume. If a framehook needs audio, a direct 
media bridge will be explicitly denied, and - by default - the 
bridging framework assumes that framehooks will want all frames. 
Another bug that was discovered occurred in step (6). When UAC1 sends 
a BYE request, nothing informed UAC2 that the fax had ended - instead, 
it was merely ejected from the bridge. This meant that it kept its 
T.38 session going, and Asterisk never sent a re-INVITE to UAC2. Both 
of these bugs were fixed by 726ee873a6.


Except, unfortunately, the second bug wasn't really fixed.

726ee873a6 did the "right" thing by intercepting the BYE request sent 
by UAC1, and queueing up a control frame of type 
AST_CONTROL_T38_PARAMETERS with a new state of AST_T38_TERMINATED. 
This is supposed to be passed on to UAC2, informing it that the T.38 
fax has ended, and that it should have its media re-negotiated back to 
the last known state (audio) but also back to Asterisk (since we 
aren't going to be in a bridge any longer). Unfortunately, this code 
was insufficient.


A race condition exists in this case. On the one hand, we've just 
queued up a frame on UAC1's channel to be passed into the bridge, 
which should get tossed onto UAC2's channel. On the other hand, we've 
just told the bridging framework to kill UAC1's channel with extreme 
prejudice, thereby also terminating the bridge and ejecting UAC2 off 
into the dialplan. In the first case, this is an asynchronous, message 
passing mechanism; in the second case, the bridging framework inspects 
the channel to see if it should be hung up on *every frame* and 
*immediately* starts the hangup/shutdown procedure if it knows the 
channel should die. This is not asynchronous in any way. As a result, 
UAC1 may be hung up and the bridge dissolved before UAC2 ever gets its 
control frame from UAC1.


There were a couple of solutions to this problem that were tried:
(1) First, I tried to make sure that enqueued control frames were 
flushed out of a channel and passed over the bridge when a hangup was 
detected. In practice, this was incredibly cumbersome - some control 
frames should get tossed, others need to be preserved. What was worse 
was the sheer number of places the bridge dissolution can be 
triggered. While it wasn't hard to make sure we flushed frames off an 
ejected channel into a bridge, it was nigh impossible to ensure 

[asterisk-dev] ODBC improvements: Community opinions requested

[Mark Michelson]

Hi folks,

I apologize if this ends up being a wall of text

In Asterisk 13, we introduced two new features, the PJSIP channel driver 
and the sorcery configuration management tool, that have both made the 
stability and reliability of Asterisk better. However, when it comes to 
realtime configuration with ODBC, we've noticed that there is a bad 
trend of contention within res_odbc in Asterisk. This is because the 
PJSIP channel driver, unlike chan_sip, is multithreaded, meaning that 
multiple threads may be attempting to read or alter the database at the 
same time. Also, the use of sorcery, along with the use of different 
types of objects within the PJSIP channel driver, results in more 
database queries than were being performed with chan_sip.


Part of this issue has been addressed by adding a memory cache to 
sorcery, and there will be work down the pipe to decrease the number of 
unneeded database queries. In the meantime, though, addressing the 
contention issue is something I want to get fixed.


The big reason for the contention is that a default configuration of 
Asterisk results in a single connection to the database being created 
and shared between all threads. This connection is lock-protected, 
resulting in many threads potentially waiting on the lock in order to be 
able to use the connection. The res_odbc.conf file allows for an 
administrator to configure multiple connections for Asterisk to use, but 
this limitation is incredibly strict. If, for instance, you configure 
Asterisk to use five database connections, all five are in use, and a 
query is initiated, it will fail, rather than doing something graceful, 
like waiting for a connection to become available. Predicting the number 
of connections you'll need is incredibly difficult, and so you're likely 
to end up causing yourself big problems unless you set the limit to 
something absurdly high.


The obvious solution to this problem is to use multiple database 
connections by default in Asterisk. One provision provided by unixODBC 
is a connection pooling option that can be enabled in odbcinst.ini. With 
this, any time that a connection is requested, it is pulled from a pool 
of connections. When disconnected, rather than being freed, the 
connection is added back to the pool. unixODBC has configuration options 
that allow for connections in the pool to eventually time out if they 
haven't been used in a while, and it has an option to allow for the 
connection to only be used a certain number of times before it is freed.


The nice thing about this is that the connection pooling logic is 
provided completely under the hood, meaning all Asterisk ever has to do 
is request a connection and then disconnect it. A huge swath of code in 
res_odbc could be outright removed. There are two potential problems, 
though:


* There is no way for us to enforce the use of connection pooling from 
our code. If connection pooling is not configured in odbcinst.ini, then 
Asterisk would end up creating and freeing database connections for each 
query, which could be costly in terms of performance (though admittedly 
it may still be better than the current situation).
* The amount of data out there about connection pools is pretty limited. 
I had to figure out how it worked and what options were available from 
reading the unixODBC source code. More importantly, I don't have much 
data on the reliability/reputation of unixODBC connection pooling.


I'm curious what opinions people have about what would be best for 
Asterisk with regards to using multiple ODBC connections. Do you have 
any experience using unixODBC connection pooling? Was the experience 
positive? Was the performance difference between using connection 
pooling vs. not using connection pooling noticeable?


The alternative to using the connection pooling built into unixODBC 
would be to modify what currently exists in Asterisk to be less strict. 
The goal would be to work similar to how unixODBC works, in that 
multiple connections can be in use, connections can be re-used, and the 
limit on the number of open connections is "softer" than it currently is 
in Asterisk. The big disadvantage to this is having to maintain a 
connection pool within Asterisk instead of letting a lower level handle 
it for us. However, if connection pooling has a noticeable performance 
improvement over constantly opening/closing connections, then this may 
be a tempting alternative since we otherwise would not be able to 
enforce the use of connection pooling in unixODBC.


I want to know what community opinion is with regards to how to proceed 
here. Do we write ODBC code in such a way that it has no knowledge of 
connection pooling and *strongly* urge users to enable it in 
odbcinst.ini, or do we modify the existing connection pooling code in 
res_odbc to be more user-friendly?


Thanks in advance,
Mark Michelson

--
_
--

Re: [asterisk-dev] Transfer cmd (PJSIP not sending Referred-By but chan_sip does)

[Mark Michelson]

The answer to this is actually pretty simple: adding Referred-By in 
outgoing SIP REFERs is simply not implemented in chan_pjsip's 
chan_pjsip_transfer() function.


As far as the syntax required for the Transfer() application, that's 
probably a case where that needs to be clarified in documentation. There 
are lots of places in PJSIP configuration where we require full SIP URIs 
rather than just IP addresses or bare URIs (user@domain).


On 08/25/2015 10:00 AM, Dan Cropp wrote:


I asked the question on asterisk–users but did not receive a response, 
so I am sending the question here.


I am running Asterisk 13.5.0.

A call comes in, Asterisk answers it. After some actions, the call 
needs to be Transferred (SIP REFER) to another number.  The other 
switch is responsible for accepting the Transfer and tromboning the 
lines internally. It will also send a BYE so Asterisk no longer has 
the call.


The behavior works when I have the endpoint configured at chan_sip.  
It does not work when the endpoint is configured as PJSIP.  I worked 
with the other switch vendor and he determined chan_sip includes the 
Referred-By header. PJSIP does not include the Referred-By header.  
The other switch requires the Referred-By header to be present.


I tried setting the channel’s SIPREFERREDBYHDR variable before the 
Transfer command and that still did not force the Referred-By header 
to be part of the REFER packet.


I tried the PJSIP_HEADER add and it still did not add the Referred-By 
header to the REFER packet.


Is there a PJSIP setting to force the Referred-By to be part of the 
REFER packet?


chan_sip (succeeds)

19:27:32.512123 IP (tos 0x0, ttl 64, id 11492, offset 0, flags [none], 
proto UDP (17), length 630)


192.168.xxx.xxx.sip  192.168.yyy.yyy.sip: SIP, length: 602

REFER sip:3...@192.168.yyy.yyy:5060 SIP/2.0

Via: SIP/2.0/UDP 192.168.xxx.xxx:5060;branch=z9hG4bK58f4bd1d

Max-Forwards: 70

From: sip:3...@192.168.xxx.xxx;tag=as44000cf4

To: sip:3...@192.168.yyy.yyy;tag=7Iy0JkwDC

Contact: sip:3...@192.168.xxx.xxx:5060

Call-ID: jdeuqpak-00...@192.168.yyy.yyy 
mailto:jdeuqpak-00...@192.168.yyy.yyy


CSeq: 102 REFER

User-Agent: Asterisk PBX 13.5.0

Date: Thu, 20 Aug 2015 19:27:32 GMT

Refer-To: sip:3...@192.168.yyy.yyy

Referred-By: sip:3...@192.168.xxx.xxx:5060

Allow: INVITE, ACK, CANCEL, OPTIONS, BYE, REFER, SUBSCRIBE, 
NOTIFY, INFO, PUBLISH, MESSAGE


Supported: replaces, timer

Content-Length: 0

Pjsip

18:46:58.386372 IP (tos 0x0, ttl 64, id 38690, offset 0, flags [DF], 
proto UDP (17), length 654)


192.168.xxx.xxx.sip  192.168.yyy.yyy.sip: SIP, length: 626

REFER sip:3...@192.168.yyy.yyy:5060 SIP/2.0

Via: SIP/2.0/UDP 
192.168.xxx.xxx:5060;rport;branch=z9hG4bKPjec41c3b9-d734-482d-82c1-2a6f8d9452a3


From: 
sip:3...@192.168.xxx.xxx;tag=3c10f423-e468-42ea-87a1-658ae106581c


To: sip:3...@192.168.yyy.yyy;tag=WITKDakt

Contact: sip:192.168.xxx.xxx:5060

Call-ID: s6wk6l6q-00...@192.168.yyy.yyy 
mailto:s6wk6l6q-00...@192.168.yyy.yyy


CSeq: 981 REFER

Event: refer

Expires: 600

Supported: 100rel, timer, replaces, norefersub

Accept: message/sipfrag;version=2.0

Allow-Events: message-summary, presence, dialog, refer

Refer-To: sip:3...@192.168.yyy.yyy

Max-Forwards: 70

User-Agent: Asterisk PBX 13.5.0

Content-Length:  0

One other slight oddity.

To get chan_sip to Transfer

3...@192.168.yyy.yyy mailto:3...@192.168.yyy.yyy

To get PJSIP to Transfer with the correct Refer-To header, I had to 
include the  and sip:


_sip:3...@192.168.yyy.__yyy_

Have a great day!

Dan





-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Transfer cmd (PJSIP not sending Referred-By but chan_sip does)

[Mark Michelson]

That code gets you about 95% of the way there.

The biggest difference would be that the pjsip_msg_add_hdr() call may 
need to be broken up based on whether the SIPREFERREDBYHDR channel 
variable is set or not. To determine that, you can use the 
pbx_builtin_getvar_helper() function call declared in 
include/asterisk/pbx.h to retrieve the variable value, and the 
ast_strlen_zero() function to determine if the channel variable value is 
zero-length or not.


Other than that, you should be able to omit the call to 
pjsua_process_msg_data() since Asterisk doesn't use pjsua.


On 08/25/2015 12:35 PM, Dan Cropp wrote:


In doing a little research, it seems the Referred-By header could be 
added after the pjsip_xfer_initiate.


This is the approach PJSIP did for some code as far back as PJSIP 1.6.

/*

 * Create REFER request.

 */

status = pjsip_xfer_initiate(sub, dest, tdata);

if (status != PJ_SUCCESS) {

pjsua_perror(THIS_FILE, Unable to create REFER 
request, status);


pjsip_dlg_dec_lock(dlg);

return status;

}

/* Add Referred-By header */

gs_hdr = pjsip_generic_string_hdr_create(tdata-pool, str_ref_by,

 dlg-local.info_str);

pjsip_msg_add_hdr(tdata-msg, (pjsip_hdr*)gs_hdr);

/* Add additional headers etc */

pjsua_process_msg_data( tdata, msg_data);

/* Send. */

status = pjsip_xfer_send_request(sub, tdata);

if (status != PJ_SUCCESS) {

pjsua_perror(THIS_FILE, Unable to send REFER 
request, status);


pjsip_dlg_dec_lock(dlg);

return status;

}

Could anyone provider some insight into how difficult this might be 
for me to add and submit for approval?  Depending on the answer, my 
manager may be willing to let me work on this.


I've developed in C/C++ for over 25 years so I'm plenty familiar with 
the language.


I'm less familiar with the syntax and coding standards of Asterisk.  I 
know the group is very good at letting people know about their 
mistakes and how to fix them.


Have a great day!

Dan

*From:*asterisk-dev-boun...@lists.digium.com 
[mailto:asterisk-dev-boun...@lists.digium.com] *On Behalf Of *Dan Cropp

*Sent:* Tuesday, August 25, 2015 10:50 AM
*To:* Asterisk Developers Mailing List
*Subject:* Re: [asterisk-dev] Transfer cmd (PJSIP not sending 
Referred-By but chan_sip does)


Thank you Mark

*From:*asterisk-dev-boun...@lists.digium.com 
mailto:asterisk-dev-boun...@lists.digium.com 
[mailto:asterisk-dev-boun...@lists.digium.com] *On Behalf Of *Mark 
Michelson

*Sent:* Tuesday, August 25, 2015 10:30 AM
*To:* Asterisk Developers Mailing List
*Subject:* Re: [asterisk-dev] Transfer cmd (PJSIP not sending 
Referred-By but chan_sip does)


The answer to this is actually pretty simple: adding Referred-By in 
outgoing SIP REFERs is simply not implemented in chan_pjsip's 
chan_pjsip_transfer() function.


As far as the syntax required for the Transfer() application, that's 
probably a case where that needs to be clarified in documentation. 
There are lots of places in PJSIP configuration where we require full 
SIP URIs rather than just IP addresses or bare URIs (user@domain).


On 08/25/2015 10:00 AM, Dan Cropp wrote:

I asked the question on asterisk–users but did not receive a
response, so I am sending the question here.

I am running Asterisk 13.5.0.

A call comes in, Asterisk answers it. After some actions, the call
needs to be Transferred (SIP REFER) to another number.  The other
switch is responsible for accepting the Transfer and tromboning
the lines internally.  It will also send a BYE so Asterisk no
longer has the call.

The behavior works when I have the endpoint configured at
chan_sip.  It does not work when the endpoint is configured as
PJSIP.  I worked with the other switch vendor and he determined
chan_sip includes the Referred-By header.  PJSIP does not include
the Referred-By header.  The other switch requires the Referred-By
header to be present.

I tried setting the channel’s SIPREFERREDBYHDR variable before the
Transfer command and that still did not force the Referred-By
header to be part of the REFER packet.

I tried the PJSIP_HEADER add and it still did not add the
Referred-By header to the REFER packet.

Is there a PJSIP setting to force the Referred-By to be part of
the REFER packet?

chan_sip (succeeds)

19:27:32.512123 IP (tos 0x0, ttl 64, id 11492, offset 0, flags
[none], proto UDP (17), length 630)

192.168.xxx.xxx.sip  192.168.yyy.yyy.sip: SIP, length: 602

REFER sip:3...@192.168.yyy.yyy:5060 SIP/2.0

Via: SIP/2.0/UDP 192.168.xxx.xxx:5060;branch=z9hG4bK58f4bd1d

Max-Forwards: 70

From: sip:3...@192.168.xxx.xxx;tag=as44000cf4

To: sip:3...@192.168.yyy.yyy;tag=7Iy0JkwDC

Contact: sip:3...@192.168.xxx.xxx:5060

Call-ID

Re: [asterisk-dev] Transfer cmd (PJSIP not sending Referred-By but chan_sip does)

[Mark Michelson]

Yep, that looks like what I would expect it to look like. The only thing 
that immediately jumps out as unnecessary is the


if (ref_by_val  !ast_strlen_zero(ref_by_val))

line. You can get rid of the initial NULL check because 
ast_strlen_zero() does that for you.


If you wanted to submit this for inclusion in Asterisk, feel free to 
upload a review to https://gerrit.asterisk.org. Instructions can be 
found on the wiki [1]. Before submitting, I'd also be sure to read the 
Asterisk coding guidelines [2] since the current code would have coding 
guidelines findings on it.


[1] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage
[2] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines

On 08/25/2015 04:35 PM, Dan Cropp wrote:


Thank you Mark for the tips.

Is the code below close to what you were thinking?

I ran some initial tests and it seems to be working.  I can override 
the default Referred-By value by setting the SIPREFERREDBYHDR variable.


static void transfer_refer(struct ast_sip_session *session, const char 
*target)


{

pjsip_evsub *sub;

enum ast_control_transfer message = AST_TRANSFER_SUCCESS;

pj_str_t tmp;

pjsip_tx_data *packet;

if (pjsip_xfer_create_uac(session-inv_session-dlg, NULL, 
sub) != PJ_SUCCESS) {


 message = AST_TRANSFER_FAILED;

ast_queue_control_data(session-channel, AST_CONTROL_TRANSFER, 
message, sizeof(message));


return;

}

if (pjsip_xfer_initiate(sub, pj_cstr(tmp, target), packet) 
!= PJ_SUCCESS) {


message = AST_TRANSFER_FAILED;

ast_queue_control_data(session-channel, AST_CONTROL_TRANSFER, 
message, sizeof(message));


pjsip_evsub_terminate(sub, PJ_FALSE);

return;

}

/ Start of changes /

pjsip_hdr *hdr;

const pj_str_t str_ref_by = { Referred-By, 11 };

const char *ref_by_val = 
pbx_builtin_getvar_helper(session-channel, SIPREFERREDBYHDR);


pj_str_t tmp2;

if (ref_by_val  !ast_strlen_zero(ref_by_val))

  {

hdr = 
(pjsip_hdr*)pjsip_generic_string_hdr_create(packet-pool, str_ref_by, 
pj_cstr(tmp2, ref_by_val));


  }

else

  {

/* Add Referred-By header */

hdr = 
(pjsip_hdr*)pjsip_generic_string_hdr_create(packet-pool, str_ref_by, 
session-inv_session-dlg-local.info_str);


  }

pjsip_msg_add_hdr(packet-msg, hdr);

/ End of changes /

pjsip_xfer_send_request(sub, packet);

  ast_queue_control_data(session-channel, AST_CONTROL_TRANSFER, 
message, sizeof(message));


}

*From:*asterisk-dev-boun...@lists.digium.com 
[mailto:asterisk-dev-boun...@lists.digium.com] *On Behalf Of *Mark 
Michelson

*Sent:* Tuesday, August 25, 2015 1:17 PM
*To:* Asterisk Developers Mailing List
*Subject:* Re: [asterisk-dev] Transfer cmd (PJSIP not sending 
Referred-By but chan_sip does)


That code gets you about 95% of the way there.

The biggest difference would be that the pjsip_msg_add_hdr() call may 
need to be broken up based on whether the SIPREFERREDBYHDR channel 
variable is set or not. To determine that, you can use the 
pbx_builtin_getvar_helper() function call declared in 
include/asterisk/pbx.h to retrieve the variable value, and the 
ast_strlen_zero() function to determine if the channel variable value 
is zero-length or not.


Other than that, you should be able to omit the call to 
pjsua_process_msg_data() since Asterisk doesn't use pjsua.


On 08/25/2015 12:35 PM, Dan Cropp wrote:

In doing a little research, it seems the Referred-By header could
be added after the pjsip_xfer_initiate.

This is the approach PJSIP did for some code as far back as PJSIP 1.6.

/*

 * Create REFER request.

 */

status = pjsip_xfer_initiate(sub, dest, tdata);

if (status != PJ_SUCCESS) {

pjsua_perror(THIS_FILE, Unable to create REFER request, status);

pjsip_dlg_dec_lock(dlg);

return status;

}

/* Add Referred-By header */

gs_hdr = pjsip_generic_string_hdr_create(tdata-pool, str_ref_by,

 dlg-local.info_str);

pjsip_msg_add_hdr(tdata-msg, (pjsip_hdr*)gs_hdr);

/* Add additional headers etc */

pjsua_process_msg_data( tdata, msg_data);

/* Send. */

status = pjsip_xfer_send_request(sub, tdata);

if (status != PJ_SUCCESS) {

pjsua_perror(THIS_FILE, Unable to send REFER request, status);

pjsip_dlg_dec_lock(dlg);

return status;

}

Could anyone provider some insight into how difficult this might
be for me to add and submit for approval?  Depending on the
answer, my manager may be willing to let me work on this.

I've developed in C/C++ for over 25 years so I'm plenty familiar
with the language.

I'm less familiar with the syntax and coding standards of
Asterisk.  I know the group is very good at letting people know
about their mistakes and how to fix them

Re: [asterisk-dev] new module

[Mark Michelson]

On 07/21/2015 11:23 PM, Kelvin Chua wrote:

i am building a proof of concept for a new module,

my first step was to create a socket to listen to, accept connections,
show remote IP, accept data and print it in asterisk console.

so far i am able to load, unload, setup the socket with a few lines of 
code.

then i got stuck. any pointers?

here is what i have so far:

static struct ast_tcptls_session_args desc = {
.accept_fd = -1,
.master = AST_PTHREADT_NULL,
.tls_cfg = NULL,
.poll_timeout = 5000,   /* wake up every 5 seconds */
.periodic_fn = cleanup,
.name = Control Module,
.accept_fn = ast_tcptls_server_root,/* thread doing the 
accept() */

.worker_fn = session_do,/* thread handling the session */
};

static int load_module(void)
{
struct ast_sockaddr bindaddr;
ast_sockaddr_parse(bindaddr, 0.0.0.0:2223 
http://0.0.0.0:2223, 0);

ast_sockaddr_copy(desc.local_address, bindaddr);
ast_tcptls_server_start(desc);

return AST_MODULE_LOAD_SUCCESS;
}

question:
what would be the general flow of session_do() in this scenario?



Kelvin Chua


When your session_do() function is called, it is because a client has 
made a connection to you. The session_do() function takes an argument of 
type ast_tcptls_session_instance, whose definition you can find in 
include/asterisk/tcptls.h. This sructure contains the file descriptor 
from which data can be read from the socket, and it also contains an 
ast_sockaddr structure that contains IP/port information about the 
remote client that connected to you.


If you are interested in printing the remote IP address/port, I suggest 
looking at the various flavors of ast_sockaddr_stringify in 
include/asterisk/netsock2.h. As for reading data from the TCP 
connection, you can use typical *nix/BSD read() or recv().


As for the general flow, that's going to depend on what your module's 
overall goal is. Typically, your session_do() will need to run a loop 
that will last as long as the TCP connection is active. This loop can 
poll the TCP file descriptor in order to determine when new data has 
arrived from the client, then read that data and act on it. The big 
thing to keep in mind is that the TCP/TLS framework in Asterisk 
basically just takes care of the boilerplate of setting up a listening 
socket, accepting connections, plus some helper functions. Once the 
connection is accepted and your session_do() function is called, you now 
have sole responsibility over the connection and can essentially do what 
you please with it. When you are finished with your connection, you can 
call ast_tcptls_close_session_file() in order to tear down the TCP 
connection.


Hopefully that points you in the correct direction.
Mark Michelson
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Deadlock in pthread_exit due to lazy binding with libgcc

[Mark Michelson]

On 07/15/2015 08:41 AM, Yousf Ateya wrote:

Dear,

I started to see a  strange deadlock in some asterisk nodes. For every 
call, when calling pthread_exit from pbx_thread, the caller thread is 
stuck inside pthread_exit.


After a while, there will be tens-of-thousands of threads having the 
same backtrace. After some googling, I found this happens because of 
the default lazy linking of gcc linker.


Related issue of stackoverflow: 
http://stackoverflow.com/questions/11954527/dlopen-malloc-deadlock


Tried to recompile asterisk using:
export LDFLAGS=-Wl,-z,now
./configure  make  make install

and this deadlock problem didn't happen again; the problem cause is 
lazy binding with libgcc.


Shall we add this option by default or add it in menuselect?


snip



--
Yousf Ateya,
StarkBits
www.starkbits.com http://www.starkbits.com



Thanks for this report. Based solely on the man page for ld(1), it 
sounds like load-time binding would, at most, cause module loading to 
take longer. Are there any other potential issues to making this change?


Mark Michelson
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Sorcery Caching Wiki Page

[Mark Michelson]

Hey fellow developers,

I have created a wiki page that details the finer points of sorcery caching
[1]. The intended audience for the page is Asterisk administrators. If you
have any critiques of the page, please feel free to mention them.

Mark Michelson

[1] https://wiki.asterisk.org/wiki/display/AST/Sorcery+Caching
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Review Request: Sorcery Caching

[Mark Michelson]

I've had a look at the page now, and here are my thoughts:

1) One thing that isn't really made clear is the interaction between 
multiple sorcery wizards. In a real-world example, you'd want to hit the 
memory_cache first and then hit the database afterwards if you couldn't 
retrieve the object from the cache. Does the order in which items are 
configured in sorcery.conf dictate the order in which wizards are 
consulted? Right now, I don't think that's the case. Or is it just that 
sorcery will automatically see cached stores as being higher priority 
than others?


2) I agree with Scott's assessment that the global expiration isn't the 
best idea. The object lifetime option makes much more sense.


3) Object expiration interval is something I feel most people wouldn't 
really want to set. They'd just hope that something sane was done by 
default. I think that defaulting to 60 seconds isn't that great a plan 
either, especially if objects in the cache are set to, say, a 10 second 
lifetime. Defaulting to some fraction of the object lifetime would work 
and probably satisfy most people. The value could potentially be updated 
during a rebalancing operation. Another option is to make use of some 
sort of timer heap so that in the case of long-lived cached objects, you 
don't run unneeded checks for object expiration.


4) In addition to the CLI operations, I think equivalent AMI operations 
would be useful. I also considered the idea of being able to change 
cache configuration for an object type via AMI/CLI. I'm not sure how 
useful that would be, and it likely would just create extra contention 
points where they're really just not needed, so meh.


5) It may be useful to have C-level API calls for invalidating 
objects/caches. This way, we could implement behavior such as  a reload 
of res_pjsip.so resulting in invalidation of all cached objects owned by 
res_pjsip.so.


6) The tests are good for testing basic operation of the cache. However, 
there's not much being done so far with regards to off-nominal code 
paths (like a user creating a cache with a negative number of maximum 
objects). First, something needs to be decided regarding how such an 
error is treated. Do we still create the cache but with a default value 
configured instead, or do we completely fail to create the cache? Once 
this is decided, some off-nominal tests with bad configuration should be 
tested.


7) There doesn't seem to be any way for someone to state that they never 
want cached objects to automatically become invalidated (i.e. infinite 
object lifetime). For some installations where configuration is 
performed through web browsers, for instance, it may be that at the time 
the config is changed, the system would issue an AMI/CLI command to 
Asterisk to invalidate the old object. They would essentially always be 
in charge of telling Asterisk when to invalidate cached objects. I know 
this is treading close to the old sip prune realtime territory, but I 
feel like this isn't quite as bad since you would have control over 
individual objects and object types.


8) My final comment is in regards to the expiration of cached items. 
Based on wording in tests, it sounds like when the expiration interval 
arrives, the item is removed from the cache entirely. I wonder if there 
is a benefit to instead, updating the item when the expiration interval 
arrives. On the one hand, this has the benefit of meaning that a user of 
sorcery will always be hitting the cache and never have to actually fall 
back to a DB lookup. On the other hand, on an idle system, this can 
result in many pointless DB lookups. So there are tradeoffs, but it's 
still something to consider. Also, I think Scott pretty much brought up 
this same idea now that I look back at his response again.


On 04/28/2015 11:28 AM, Joshua Colp wrote:

Kia ora,

I've created a wiki page[1] which details the beginnings of a basic 
memory based caching wizard for sorcery. Right now while caching is 
possible using the existing memory wizard it's not possible to define 
object lifetimes, so once cached it's always pulled from the cache. 
This wiki page uses the memory wizard as a base but defines options 
which can tweak the behavior. Going forward this could serve as a 
basis for other wizards to be created for caching purposes.


Some things to consider:
1. How much control and flexibility should we allow?
2. Are there additional mechanisms that should be exposed to allow 
explicit object expiration?

3. Are the defaults sane?
4. Is there additional testing that should be done?
5. Does anything need additional explanation?

Cheers,

[1] https://wiki.asterisk.org/wiki/display/~jcolp/Sorcery+Caching




--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4490: astdb: Allow clustering of the Asterisk Database between multiple Asterisk servers

[Mark Michelson]

On 04/14/2015 12:11 PM, Matthew Jordan wrote:

snip
Yup.

SO!

The question is: is this change worth having, or should it be scrapped
in favour of some alternate approach that makes use of other
technology? My feelings won't be hurt if the answer is ditch it and
do something else - this was a fun piece of code to right on some
plane flights. On the other hand, I don't have any real interest in
writing an alternative approach, so if the expectation is that an
AstDB wrapper around RabbitMQ or Redis will magically appear if I hit
the delete key, that expectation is likely to be wrong.



Personally, I like the idea of either

1) Changing to a key-value store that can replicate data to remote instances
or
2) Allowing the AstDB to use a remote key-value store, thus allowing 
multiple Asterisk boxes to share the same store.


I don't have any mechanical findings with the code you've written. It'll 
work for what it's intended to do. However, I feel like it has a couple 
of flaws with it:


1) As I already pointed out, it has the problem that data consistency 
can be an issue.
2) For what it does, it's pretty heavy. It requires Stasis subscriptions 
and SIP publications. If the key-value store is doing the lifting 
itself, it's likely to be easier and the replication likely would run as 
a separate process.


--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[master]: res_pjsip: Add external PJSIP resolver implementation using ...

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: res_pjsip: Add external PJSIP resolver implementation using 
core DNS API.
..


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/#/c/75/1/main/dns_query_set.c
File main/dns_query_set.c:

Line 112:   if (query_set-in_progress) {
:   return -1;
:   }
I suggest having an error message here.

I'd actually go so far as to consider putting an assertion in here because it 
likely means there is some sort of programmer error occurring if we are 
attempting to add queries to a set after starting the resolution.


https://gerrit.asterisk.org/#/c/75/1/main/dns_recurring.c
File main/dns_recurring.c:

Line 36: #include asterisk/vector.h
   : #include asterisk/sched.h
   : #include asterisk/strings.h
   : #include asterisk/dns_core.h
   : #include asterisk/dns_recurring.h
   : #include asterisk/dns_query_set.h
On reviewboard, I asked about why these includes got added, and the the issue 
was marked as resolved, but they're still included now. Are they needed?


https://gerrit.asterisk.org/#/c/75/1/res/res_pjsip/pjsip_resolver.c
File res/res_pjsip/pjsip_resolver.c:

Line 544:   if ((type == PJSIP_TRANSPORT_UNSPECIFIED  
sip_transport_is_available(PJSIP_TRANSPORT_UDP6)) ||
:   sip_transport_is_available(type + 
PJSIP_TRANSPORT_IPV6)) {
:   res |= sip_resolve_add(resolve, host, ns_t_, 
ns_c_in, (type == PJSIP_TRANSPORT_UNSPECIFIED ? PJSIP_TRANSPORT_UDP6 : type + 
PJSIP_TRANSPORT_IPV6), target-addr.port);
:   }
: 
:   if ((type == PJSIP_TRANSPORT_UNSPECIFIED  
sip_transport_is_available(PJSIP_TRANSPORT_UDP)) ||
:   sip_transport_is_available(type)) {
:   res |= sip_resolve_add(resolve, host, ns_t_a, ns_c_in, 
(type == PJSIP_TRANSPORT_UNSPECIFIED ? PJSIP_TRANSPORT_UDP : type), 
target-addr.port);
:   }
After giving RFC 3263 another look, I don't think these A and  lookups 
should be done here.

Section 4.2 says If no SRV records were found, the client performs an A or 
 record lookup of the domain name.

Since you can't know if the SRV lookup(s) provided any records yet, performing 
this lookup is premature.

Now, I guess you could go ahead and perform this lookup early, but if the SRV 
lookup(s) provide any results, you can't actually use these A/ records.


-- 
To view, visit https://gerrit.asterisk.org/75
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I56cb03ce4f9d3d600776f36928e0b3e379b5d71e
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp jc...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[1.8]: .gitignore: Ignore tarballs (*.gz)

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: .gitignore: Ignore tarballs (*.gz)
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/86
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 1.8
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[12]: .gitignore: Ignore tarballs (*.gz)

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: .gitignore: Ignore tarballs (*.gz)
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/85
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 12
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[1.8]: .gitignore: Ignore tarballs (*.gz)

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: .gitignore: Ignore tarballs (*.gz)
..


.gitignore: Ignore tarballs (*.gz)

This patch updates the root .gitignore file to ignore files with a .gz
extension. This will cause git to ignore downloaded sound tarballs in
the the sounds/ directory.

Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
---
M .gitignore
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Matt Jordan: Looks good to me, but someone else must approve
  Jared K. Smith: Looks good to me, but someone else must approve
  Corey Farrell: Looks good to me, but someone else must approve



diff --git a/.gitignore b/.gitignore
index cf46873..33dd7cc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,6 +9,7 @@
 
 *~
 *.[oadi]
+*.gz
 *.ii
 *.oo
 *.eo

-- 
To view, visit https://gerrit.asterisk.org/86
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 1.8
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[1.8]: Add .gitignore and .gitreview files

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Add .gitignore and .gitreview files
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/82
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 1.8
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[1.8]: Add .gitignore and .gitreview files

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: Add .gitignore and .gitreview files
..


Add .gitignore and .gitreview files

Add the .gitignore and .gitreview files to the asterisk repo.

NB:  You can add local ignores to the .git/info/exclude file
without having to do a commit.

Common ignore patterns are in the top-level .gitignore file.
Subdirectory-specific ignore patterns are in their own .gitignore
files.

Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Tested-by: George Joseph
(cherry picked from commit b35e184d41c4e61e98b455d70321ba90118600a1)
---
A .gitignore
A .gitreview
A addons/.gitignore
A agi/.gitignore
A build_tools/.gitignore
A doc/.gitignore
A include/asterisk/.gitignore
A main/.gitignore
A menuselect/.gitignore
A res/ael/.gitignore
A utils/.gitignore
11 files changed, 72 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Matt Jordan: Looks good to me, but someone else must approve



diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..cf46873
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,26 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~
+
+# See .gitignore in subdirectories for more ignored files
+
+*~
+*.[oadi]
+*.ii
+*.oo
+*.eo
+*.so
+*.exports
+*.moduleinfo
+*.makeopts
+*.makedeps
+makeopts
+.lastclean
+config.log
+config.status
+defaults.h
+makeopts.embed_rules
+menuselect-tree
diff --git a/.gitreview b/.gitreview
new file mode 100644
index 000..f9ef050
--- /dev/null
+++ b/.gitreview
@@ -0,0 +1,4 @@
+[gerrit]
+host=gerrit.asterisk.org
+port=29418
+project=asterisk.git
diff --git a/addons/.gitignore b/addons/.gitignore
new file mode 100644
index 000..663e668
--- /dev/null
+++ b/addons/.gitignore
@@ -0,0 +1 @@
+mp3
diff --git a/agi/.gitignore b/agi/.gitignore
new file mode 100644
index 000..9b2a4e2
--- /dev/null
+++ b/agi/.gitignore
@@ -0,0 +1,3 @@
+eagi-sphinx-test
+eagi-test
+strcompat.c
diff --git a/build_tools/.gitignore b/build_tools/.gitignore
new file mode 100644
index 000..c60a0df
--- /dev/null
+++ b/build_tools/.gitignore
@@ -0,0 +1 @@
+menuselect-deps
diff --git a/doc/.gitignore b/doc/.gitignore
new file mode 100644
index 000..27acdb3
--- /dev/null
+++ b/doc/.gitignore
@@ -0,0 +1 @@
+core-en_US.xml
diff --git a/include/asterisk/.gitignore b/include/asterisk/.gitignore
new file mode 100644
index 000..ae33b3c
--- /dev/null
+++ b/include/asterisk/.gitignore
@@ -0,0 +1,3 @@
+autoconfig.h
+build.h
+buildopts.h
diff --git a/main/.gitignore b/main/.gitignore
new file mode 100644
index 000..23f5c58
--- /dev/null
+++ b/main/.gitignore
@@ -0,0 +1,3 @@
+asterisk
+libasteriskssl.so.1
+version.c
diff --git a/menuselect/.gitignore b/menuselect/.gitignore
new file mode 100644
index 000..38ea2d3
--- /dev/null
+++ b/menuselect/.gitignore
@@ -0,0 +1,5 @@
+autoconfig.h
+cmenuselect
+config.log
+config.status
+menuselect
diff --git a/res/ael/.gitignore b/res/ael/.gitignore
new file mode 100644
index 000..f39b612
--- /dev/null
+++ b/res/ael/.gitignore
@@ -0,0 +1 @@
+ael.output
diff --git a/utils/.gitignore b/utils/.gitignore
new file mode 100644
index 000..ed37a06
--- /dev/null
+++ b/utils/.gitignore
@@ -0,0 +1,24 @@
+aelbison.c
+aelparse
+aelparse.c
+ast_expr2.c
+ast_expr2f.c
+astman
+astcanary
+astdb2bdb
+astdb2sqlite3
+check_expr
+check_expr2
+conf2ael
+db1-ast/libdb1.a
+hashtab.c
+lock.c
+md5.c
+muted
+pbx_ael.c
+pval.c
+smsq
+stereorize
+strcompat.c
+streamplayer
+threadstorage.c

-- 
To view, visit https://gerrit.asterisk.org/82
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 1.8
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[1.8]: main/editline: Add .gitignore.

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: main/editline: Add .gitignore.
..


main/editline: Add .gitignore.

This patch adds a .gitignore for main/editline to ignore all build results.

Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
(cherry picked from commit 5d34bce635ad7f225d557fa226e6b79c6293dbe1)
---
A main/editline/.gitignore
1 file changed, 13 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Matt Jordan: Looks good to me, but someone else must approve



diff --git a/main/editline/.gitignore b/main/editline/.gitignore
new file mode 100644
index 000..d3bb06b
--- /dev/null
+++ b/main/editline/.gitignore
@@ -0,0 +1,13 @@
+*.o_a
+Makefile
+common.h
+config.cache
+config.h
+editline.c
+emacs.h
+fcns.c
+fcns.h
+help.c
+help.h
+makelist
+vi.h

-- 
To view, visit https://gerrit.asterisk.org/84
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 1.8
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[1.8]: main/editline: Add .gitignore.

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: main/editline: Add .gitignore.
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/84
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 1.8
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[12]: main/editline: Add .gitignore.

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: main/editline: Add .gitignore.
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/83
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 12
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[12]: main/editline: Add .gitignore.

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: main/editline: Add .gitignore.
..


main/editline: Add .gitignore.

This patch adds a .gitignore for main/editline to ignore all build results.

Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
(cherry picked from commit 5d34bce635ad7f225d557fa226e6b79c6293dbe1)
---
A main/editline/.gitignore
1 file changed, 13 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Matt Jordan: Looks good to me, but someone else must approve



diff --git a/main/editline/.gitignore b/main/editline/.gitignore
new file mode 100644
index 000..d3bb06b
--- /dev/null
+++ b/main/editline/.gitignore
@@ -0,0 +1,13 @@
+*.o_a
+Makefile
+common.h
+config.cache
+config.h
+editline.c
+emacs.h
+fcns.c
+fcns.h
+help.c
+help.h
+makelist
+vi.h

-- 
To view, visit https://gerrit.asterisk.org/83
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 12
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Review Request: DNS NAPTR/SRV Test plan for PJSIP

[Mark Michelson]

I have created a wiki page [1] with a test plan for DNS NAPTR/SRV for 
PJSIP. Please take a moment to have a look at the page and provide any 
feedback (on this list, not on the page please) about the test plan. Are 
there any test cases that I left out that should be covered? Are there 
any test cases that need further explanation?


Any sections on the page that are notes (meaning they are yellow with 
an exclamation point in the top-left corner) means that I have asserted 
some interpretation of the relevant RFCs but that my assertion may be up 
for debate. Please feel free to give counterarguments if you interpret 
passages differently from me or if you know of real-world scenarios 
where my interpretation will run into problems.


Thank you very much!

[1] https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=32375195

--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
  http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[12]: Add .gitignore and .gitreview files

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: Add .gitignore and .gitreview files
..


Add .gitignore and .gitreview files

Add the .gitignore and .gitreview files to the asterisk repo.

NB:  You can add local ignores to the .git/info/exclude file
without having to do a commit.

Common ignore patterns are in the top-level .gitignore file.
Subdirectory-specific ignore patterns are in their own .gitignore
files.

Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Tested-by: George Joseph
(cherry picked from commit b35e184d41c4e61e98b455d70321ba90118600a1)
---
A .gitignore
A .gitreview
A addons/.gitignore
A agi/.gitignore
A build_tools/.gitignore
A doc/.gitignore
A include/asterisk/.gitignore
A main/.gitignore
A menuselect/.gitignore
A res/ael/.gitignore
A utils/.gitignore
11 files changed, 72 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Matt Jordan: Looks good to me, but someone else must approve



diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..cf46873
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,26 @@
+# git ls-files --others --exclude-from=.git/info/exclude
+# Lines that start with '#' are comments.
+# For a project mostly in C, the following would be a good set of
+# exclude patterns (uncomment them if you want to use them):
+# *.[oa]
+# *~
+
+# See .gitignore in subdirectories for more ignored files
+
+*~
+*.[oadi]
+*.ii
+*.oo
+*.eo
+*.so
+*.exports
+*.moduleinfo
+*.makeopts
+*.makedeps
+makeopts
+.lastclean
+config.log
+config.status
+defaults.h
+makeopts.embed_rules
+menuselect-tree
diff --git a/.gitreview b/.gitreview
new file mode 100644
index 000..f9ef050
--- /dev/null
+++ b/.gitreview
@@ -0,0 +1,4 @@
+[gerrit]
+host=gerrit.asterisk.org
+port=29418
+project=asterisk.git
diff --git a/addons/.gitignore b/addons/.gitignore
new file mode 100644
index 000..663e668
--- /dev/null
+++ b/addons/.gitignore
@@ -0,0 +1 @@
+mp3
diff --git a/agi/.gitignore b/agi/.gitignore
new file mode 100644
index 000..9b2a4e2
--- /dev/null
+++ b/agi/.gitignore
@@ -0,0 +1,3 @@
+eagi-sphinx-test
+eagi-test
+strcompat.c
diff --git a/build_tools/.gitignore b/build_tools/.gitignore
new file mode 100644
index 000..c60a0df
--- /dev/null
+++ b/build_tools/.gitignore
@@ -0,0 +1 @@
+menuselect-deps
diff --git a/doc/.gitignore b/doc/.gitignore
new file mode 100644
index 000..27acdb3
--- /dev/null
+++ b/doc/.gitignore
@@ -0,0 +1 @@
+core-en_US.xml
diff --git a/include/asterisk/.gitignore b/include/asterisk/.gitignore
new file mode 100644
index 000..ae33b3c
--- /dev/null
+++ b/include/asterisk/.gitignore
@@ -0,0 +1,3 @@
+autoconfig.h
+build.h
+buildopts.h
diff --git a/main/.gitignore b/main/.gitignore
new file mode 100644
index 000..23f5c58
--- /dev/null
+++ b/main/.gitignore
@@ -0,0 +1,3 @@
+asterisk
+libasteriskssl.so.1
+version.c
diff --git a/menuselect/.gitignore b/menuselect/.gitignore
new file mode 100644
index 000..38ea2d3
--- /dev/null
+++ b/menuselect/.gitignore
@@ -0,0 +1,5 @@
+autoconfig.h
+cmenuselect
+config.log
+config.status
+menuselect
diff --git a/res/ael/.gitignore b/res/ael/.gitignore
new file mode 100644
index 000..f39b612
--- /dev/null
+++ b/res/ael/.gitignore
@@ -0,0 +1 @@
+ael.output
diff --git a/utils/.gitignore b/utils/.gitignore
new file mode 100644
index 000..ed37a06
--- /dev/null
+++ b/utils/.gitignore
@@ -0,0 +1,24 @@
+aelbison.c
+aelparse
+aelparse.c
+ast_expr2.c
+ast_expr2f.c
+astman
+astcanary
+astdb2bdb
+astdb2sqlite3
+check_expr
+check_expr2
+conf2ael
+db1-ast/libdb1.a
+hashtab.c
+lock.c
+md5.c
+muted
+pbx_ael.c
+pval.c
+smsq
+stereorize
+strcompat.c
+streamplayer
+threadstorage.c

-- 
To view, visit https://gerrit.asterisk.org/81
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 12
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[12]: Add .gitignore and .gitreview files

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Add .gitignore and .gitreview files
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/81
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 12
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in repotools[master]: mapmantis: Remove dependency on digium_jira

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: mapmantis: Remove dependency on digium_jira
..


Patch Set 2: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/68
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0ae337d24e3db7d552406a57a740d260d5c4d2d7
Gerrit-PatchSet: 2
Gerrit-Project: repotools
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in repotools[master]: digium_jira: Refactor module to wrap the Atlassian JIRA REST...

[Mark Michelson (Code Review)]

Hello Jared K. Smith,

I'd like you to reexamine a change.  Please visit

https://gerrit.asterisk.org/69

to look at the new patch set (#2).

Change subject: digium_jira: Refactor module to wrap the Atlassian JIRA REST 
client
..

digium_jira: Refactor module to wrap the Atlassian JIRA REST client

Many of our scripts that use the JIRA REST Python client first have
to obtain credentials from the .jira_login file, or else prompt for
them. The act of getting credentials and setting up the basic client is
done in a number of scripts.

Rather than reproduce that logic across many modules, this patch takes
the defunct digium_jira module and repurposes it to contain those
functions. Since Atlassian no longer supports its SOAP API and we no
longer use it in any of our modules, its reasonable to locate this
functionality in this module.

Change-Id: I69932dd472aef4290af97e809ce6b9ec9c25b39d
---
M digium_jira.py
1 file changed, 30 insertions(+), 173 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/repotools refs/changes/69/69/2
-- 
To view, visit https://gerrit.asterisk.org/69
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I69932dd472aef4290af97e809ce6b9ec9c25b39d
Gerrit-PatchSet: 2
Gerrit-Project: repotools
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[master]: Fixing extconf compile

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: Fixing extconf compile
..


Fixing extconf compile

During the mass code deletion for clang support, a stray backslash was
left behind that was causing utils to fail to compile.

Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
---
M utils/extconf.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Jared K. Smith: Looks good to me, but someone else must approve



diff --git a/utils/extconf.c b/utils/extconf.c
index 7ddbb67..53347b4 100644
--- a/utils/extconf.c
+++ b/utils/extconf.c
@@ -503,7 +503,7 @@
 static void  __attribute__((constructor)) init_##mutex(void) \
 { \
ast_mutex_init(mutex); \
-} \
+}
 #else /* !AST_MUTEX_INIT_W_CONSTRUCTORS */
 /* By default, use static initialization of mutexes. */ 
 #define __AST_MUTEX_DEFINE(scope, mutex) \

-- 
To view, visit https://gerrit.asterisk.org/90
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: David M. Lee d...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[13]: Fixing extconf compile

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: Fixing extconf compile
..


Fixing extconf compile

During the mass code deletion for clang support, a stray backslash was
left behind that was causing utils to fail to compile.

Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
---
M utils/extconf.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Jared K. Smith: Looks good to me, but someone else must approve



diff --git a/utils/extconf.c b/utils/extconf.c
index 89cf6dd..7989bcd 100644
--- a/utils/extconf.c
+++ b/utils/extconf.c
@@ -502,7 +502,7 @@
 static void  __attribute__((constructor)) init_##mutex(void) \
 { \
ast_mutex_init(mutex); \
-} \
+}
 #else /* !AST_MUTEX_INIT_W_CONSTRUCTORS */
 /* By default, use static initialization of mutexes. */ 
 #define __AST_MUTEX_DEFINE(scope, mutex) \

-- 
To view, visit https://gerrit.asterisk.org/89
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: David M. Lee d...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[11]: Fixing extconf compile

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: Fixing extconf compile
..


Fixing extconf compile

During the mass code deletion for clang support, a stray backslash was
left behind that was causing utils to fail to compile.

Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
---
M utils/extconf.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Jared K. Smith: Looks good to me, but someone else must approve



diff --git a/utils/extconf.c b/utils/extconf.c
index 2b1bc0c..bd2cc9a 100644
--- a/utils/extconf.c
+++ b/utils/extconf.c
@@ -487,7 +487,7 @@
 static void  __attribute__((constructor)) init_##mutex(void) \
 { \
ast_mutex_init(mutex); \
-} \
+}
 #else /* !AST_MUTEX_INIT_W_CONSTRUCTORS */
 /* By default, use static initialization of mutexes. */ 
 #define __AST_MUTEX_DEFINE(scope, mutex) \

-- 
To view, visit https://gerrit.asterisk.org/88
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: David M. Lee d...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[master]: Fixing extconf compile

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Fixing extconf compile
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/90
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: David M. Lee d...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[13]: Fixing extconf compile

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Fixing extconf compile
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/89
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: David M. Lee d...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[11]: Fixing extconf compile

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Fixing extconf compile
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/88
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I60e5fa58c9a5b248bde23aaada79ff663f87a2a1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: David M. Lee d...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in asterisk[12]: .gitignore: Ignore tarballs (*.gz)

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: .gitignore: Ignore tarballs (*.gz)
..


.gitignore: Ignore tarballs (*.gz)

This patch updates the root .gitignore file to ignore files with a .gz
extension. This will cause git to ignore downloaded sound tarballs in
the the sounds/ directory.

Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
---
M .gitignore
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Matt Jordan: Looks good to me, but someone else must approve
  Jared K. Smith: Looks good to me, but someone else must approve



diff --git a/.gitignore b/.gitignore
index cf46873..33dd7cc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,6 +9,7 @@
 
 *~
 *.[oadi]
+*.gz
 *.ii
 *.oo
 *.eo

-- 
To view, visit https://gerrit.asterisk.org/85
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 12
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in repotools[master]: digium_jira: Refactor module to wrap the Atlassian JIRA REST...

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: digium_jira: Refactor module to wrap the Atlassian JIRA REST 
client
..


Patch Set 2:

(2 comments)

https://gerrit.asterisk.org/#/c/69/2/digium_jira.py
File digium_jira.py:

Line 16: try:
   : jira_cache = open(os.path.expanduser('~') + /.jira_login, 
r)
   : jira_user = jira_cache.readline().strip()
   : jira_pw = jira_cache.readline().strip()
   : jira_cache.close()
   : return (jira_user, jira_pw)
   : except IOError:
   : pass
Python developers are encouraged to use the with keyword for file I/O since 
it automatically will close the file properly even if an exception occurs while 
operating on the file.


Line 25:# Didn't get auth deatils from file, try interactive instead.
s/deatils/details/


-- 
To view, visit https://gerrit.asterisk.org/69
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I69932dd472aef4290af97e809ce6b9ec9c25b39d
Gerrit-PatchSet: 2
Gerrit-Project: repotools
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Jared K. Smith jaredsm...@jaredsmith.net
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4609: chan_pjsip/res_pjsip/bridge_softmix/core: Improve translation path choices.

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4609/#review15184
---

Ship it!



/branches/13/main/channel.c
https://reviewboard.asterisk.org/r/4609/#comment25842

I figure I should point out why translate_via_slin is on by default. The 
big reason is for audiohooks or any other core audio manipulation features. 
Let's say that you do not have translate_via_slin enabled. Alice speaks g.722 
and calls Bob who speaks ulaw. Asterisk might set up a translation path like so:

Alice read path: g.722-slin-ulaw
Alice write path: ulaw-slin-g.722
Bob read path: ulaw
Bob write path: ulaw

Every frame that comes in from Alice or Bob ends up getting extra 
translations to slin if audiohooks are in use. Meanwhile, if you do have 
translate_via_slin enabled, you'd be guaranteed to have these translation paths:

Alice read path: g.722-slin
Alice write path: slin-g.722
Bob read path: ulaw-slin
Bob write path: slin-ulaw

By having slin as the read and write formats of both channels, frames that 
come into the core don't have to go through any extra translations in order to 
be used in audiohooks.


- Mark Michelson


On April 9, 2015, 8:50 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4609/
 ---
 
 (Updated April 9, 2015, 8:50 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24841
 https://issues.asterisk.org/jira/browse/ASTERISK-24841
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 With this patch, chan_pjsip/res_pjsip now sets the native formats to the
 codecs negotiated by a call.
 
 * The changes in chan_pjsip.c and res_pjsip_sdp_rtp.c set the native
 formats to include all the negotiated audio codecs instead of only the
 initial preferred audio codec and later the currently received audio
 codec.
 
 * The audio frame handling in channel.c:ast_read() is more streamlined and
 will automatically adjust to changes in received frame formats.  The new
 policy is to remove translation and pass the new frame format to the
 receiver except if the translation was to a signed linear format.  A more
 long winded version is commented in ast_read() along with some caveats.
 
 * The audio frame handling in channel.c:ast_write() is more streamlined
 and will automatically adjust any needed translation to changes in the
 frame formats sent.  Frame formats sent can change for many reasons such
 as a recording is being played back or the bridged peer changed the format
 it sends.  Since it is a normal expectation that sent formats can change,
 the codec mismatch warning message is demoted to a debug message.
 
 * Removed the short circuit check in
 channel.c:ast_channel_make_compatible_helper().  Two party bridges need to
 make channels compatible with each other.  However, transfers and moving
 channels among bridges can result in otherwise compatible channels having
 sub-optimal translation paths if the make compatible check is short
 circuited.  A result of forcing the reevaluation of channel compatibility
 is that the asterisk.conf:transcode_via_slin and codecs.conf:genericplc
 options take effect consistently now.  It is unfortunate that these two
 options are enabled by default and negate some of the benefits to the
 changes in channel.c:ast_read() by forcing translation through signed
 linear on a two party bridge.
 
 * Improved the softmix bridge technology to better control the translation
 of frames to the bridge.  All of the incoming translation is now normally
 handled by ast_read() instead of splitting any translation steps between
 ast_read() and the slin factory.  If any frame comes in with an unexpected
 format then the translation path in ast_read() is updated for the next
 frame and the slin factory handles the current frame translation.
 
 This is the final patch in a series of patches aimed at improving
 translation path choices.  The other patches are on the following reviews:
 https://reviewboard.asterisk.org/r/4600/
 https://reviewboard.asterisk.org/r/4605/
 
 
 Diffs
 -
 
   /branches/13/res/res_pjsip_sdp_rtp.c 434526 
   /branches/13/main/channel.c 434526 
   /branches/13/include/asterisk/channel.h 434526 
   /branches/13/channels/chan_pjsip.c 434526 
   /branches/13/bridges/bridge_softmix.c 434526 
 
 Diff: https://reviewboard.asterisk.org/r/4609/diff/
 
 
 Testing
 ---
 
 * The testsuite still passes as well as it ever has.
 
 * Manual SIP and DTMF attended transfers still function.  With all patches
 in the series applied, if a low speed party transfers a higher speed party
 to another high speed party then when the transfer completes the resulting
 call

Re: [asterisk-dev] [Code Review] 4604: loader/main: Don't set ast_fully_booted until deferred reloads are processed

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4604/#review15153
---

Ship it!


Ship It!

- Mark Michelson


On April 9, 2015, 2:57 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4604/
 ---
 
 (Updated April 9, 2015, 2:57 p.m.)
 
 
 Review request for Asterisk Developers and Corey Farrell.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Until we have a true module management facility it's sometimes necessary for 
 one module to force a reload on another before its own load is complete.  If 
 Asterisk isn't fully booted yet, these reloads are deferred.  The problem is 
 that asterisk reports fully booted before processing the deferred reloads 
 which means Asterisk really isn't quite ready when it says it is.
 
 This patch moves the report of fully booted after the processing of the 
 deferred reloads is complete.
 
 
 Diffs
 -
 
   branches/13/main/loader.c 434447 
   branches/13/main/asterisk.c 434447 
 
 Diff: https://reviewboard.asterisk.org/r/4604/diff/
 
 
 Testing
 ---
 
 Since the pjsip stack has the most number of related modules, I ran the 
 channels/pjsip testsuite to make sure there aren't any issues.  All tests 
 passed.
 
 
 Thanks,
 
 George Joseph
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: rest_api/applications/stasisstatus: Make run-test executable

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: rest_api/applications/stasisstatus: Make run-test executable
..


Patch Set 1: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/36
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf0647d37c9bba0318f251e1040c03372bce1b54
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4603: pjsip: Add external PJSIP resolver implementation using core DNS API

[Mark Michelson]

 the declaration that any NAPTR terminal flag 
other than s is invalid for SIP+D2X records.

Now, here's the part where things get really confusing. What happens if I 
do a NAPTR lookup for a domain in a SIP URI and I get back a record with empty 
flags and services (with either a regex or replacement domain)? Since no SIP 
records were returned, is that an error? Or does that mean I should do another 
NAPTR lookup on the returned record? After all, the next NAPTR lookup might 
give me back the SIP records I want. What if I get a mix of empty flags and 
empty service records AND SIP+D2X records with s flag? What then?


- Mark Michelson


On April 8, 2015, 7:37 p.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4603/
 ---
 
 (Updated April 8, 2015, 7:37 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24947
 https://issues.asterisk.org/jira/browse/ASTERISK-24947
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This change adds the following:
 
 1. A query set implementation. This is an API that allows queries to be 
 executed in parallel and once all have completed a callback is invoked.
 2. Unit tests for the query set implementation.
 3. An external PJSIP resolver which uses the DNS core API to do NAPTR, SRV, 
 , and A lookups.
 
 For the resolver it will do NAPTR, SRV, and /A lookups in parallel. If 
 NAPTR or SRV are available it will then do more queries. And so on. 
 Preference is NAPTR  SRV  /A, with IPv6 preferred over IPv4. For 
 transport it will prefer TLS  TCP  UDP if no explicit transport has been 
 provided. Configured transports on the system are taken into account to 
 eliminate resolved addresses which have no hope of completing.
 
 
 Diffs
 -
 
   /trunk/tests/test_dns_query_set.c PRE-CREATION 
   /trunk/res/res_pjsip_session.c 434446 
   /trunk/res/res_pjsip/pjsip_resolver.c PRE-CREATION 
   /trunk/res/res_pjsip/include/res_pjsip_private.h 434446 
   /trunk/res/res_pjsip.c 434446 
   /trunk/main/dns_srv.c 434446 
   /trunk/main/dns_recurring.c 434446 
   /trunk/main/dns_query_set.c 434446 
   /trunk/main/dns_naptr.c 434446 
   /trunk/main/dns_core.c 434446 
   /trunk/include/asterisk/dns_query_set.h 434446 
   /trunk/include/asterisk/dns_internal.h 434446 
   /trunk/include/asterisk/dns_core.h 434446 
   /trunk/include/asterisk/autoconfig.h.in 434446 
   /trunk/configure.ac 434446 
   /configure UNKNOWN 
 
 Diff: https://reviewboard.asterisk.org/r/4603/diff/
 
 
 Testing
 ---
 
 Ran unit tests, they pass. Ran testsuite tests, they pass. Did spot checking 
 using my own domains. They resolve as expected.
 
 
 Thanks,
 
 Joshua Colp
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: rest_api/applications/stasisstatus: Make run-test executable

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: rest_api/applications/stasisstatus: Make run-test executable
..


rest_api/applications/stasisstatus: Make run-test executable

If it isn't executable, it can't run!

Change-Id: Ibf0647d37c9bba0318f251e1040c03372bce1b54
---
M tests/rest_api/applications/stasisstatus/run-test
1 file changed, 0 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Ashley Sanders: Looks good to me, but someone else must approve



diff --git a/tests/rest_api/applications/stasisstatus/run-test 
b/tests/rest_api/applications/stasisstatus/run-test
old mode 100644
new mode 100755

-- 
To view, visit https://gerrit.asterisk.org/36
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibf0647d37c9bba0318f251e1040c03372bce1b54
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4605: translate.c: Only select audio codecs to determine the best translation choice.

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4605/#review15161
---

Ship it!


Ship It!

- Mark Michelson


On April 9, 2015, 4:50 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4605/
 ---
 
 (Updated April 9, 2015, 4:50 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-21777, ASTERISK-24380 and ASTERISK-24841
 https://issues.asterisk.org/jira/browse/ASTERISK-21777
 https://issues.asterisk.org/jira/browse/ASTERISK-24380
 https://issues.asterisk.org/jira/browse/ASTERISK-24841
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Given a source capability of h264 and ulaw, a destination capability of
 h264 and g722 then ast_translator_best_choice() would pick h264 as the
 best choice even though h264 is a video codec and Asterisk only supports
 translation of audio codecs.  When the audio starts flowing, there are
 warnings about a codec mismatch when the channel tries to write a frame to
 the peer.
 
 * Made ast_translator_best_choice() only select audio codecs.
 
 * Restore a check in channel.c:set_format() lost after v1.8 to prevent
 trying to set a non-audio codec.
 
 This is an intermediate patch for a series of patches aimed at improving
 translation path choices for ASTERISK-24841.
 
 This patch is a partial fix for ASTERISK-21777 as the v11 version of
 ast_translator_best_choice() does the same thing.  However, chan_sip
 somehow sets the nativeformats capabilities to just h264 or empty.
 
 
 This patch will be backported to v11 as a partial fix for ASTERISK-21777.
 
 
 Diffs
 -
 
   /branches/13/main/translate.c 434447 
   /branches/13/main/channel.c 434447 
 
 Diff: https://reviewboard.asterisk.org/r/4605/diff/
 
 
 Testing
 ---
 
 Modified chan_pjsip.c:chan_pjsip_new() to force inclusion of the h264
 video codec in the native capabilities.  Without the patch I get the path
 translation warnings.  With the patch I don't get the warnings.
 
 
 Thanks,
 
 rmudgett
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4598: Refactor duplicated DNS routines into common sections

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4598/
---

(Updated April 9, 2015, 10:50 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 434490


Repository: Asterisk


Description
---

Josh worked on SRV in one branch, and I worked on NAPTR in a separate branch. 
Independently we kept coming to realizations that something that one of us had 
developed independently would be needed by the other person. We decided to 
simply have copies of common functionality in our branches. After merging, we 
would perform a refactor to remove duplication.

This changeset introduces no new DNS functionality. Instead, it takes some 
duplicated code and places them into common areas of the DNS core.


Diffs
-

  /trunk/tests/test_dns_srv.c 434218 
  /trunk/tests/test_dns_naptr.c 434218 
  /trunk/main/dns_test.c PRE-CREATION 
  /trunk/main/dns_srv.c 434218 
  /trunk/main/dns_naptr.c 434218 
  /trunk/main/dns_core.c 434218 
  /trunk/include/asterisk/dns_test.h PRE-CREATION 
  /trunk/include/asterisk/dns_internal.h 434218 

Diff: https://reviewboard.asterisk.org/r/4598/diff/


Testing
---

All DNS unit tests continue to pass.


Thanks,

Mark Michelson

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4594: Asterisk manager output escape message control characters

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4594/#review15129
---


I have a few problems with this review:

* I don't see in this review or in the patch you uploaded to the linked issue 
where ast_escape_c() or astman_escape_output() are defined.
* This patch performs a lot of heap allocations and does not check for 
potential memory allocation failure.
* This patch places the responsibility of escaping strings on all callers of 
ast_manager_event(). If escaping strings is an essential part of sending 
manager events, then it should be built into the function that sends manager 
events. Otherwise, it's easy for a programmer to forget to perform the escaping 
before outputting strings. If escaping of manager events is in the same 
location where manager events are sent, you may also be able to get away with 
fewer heap allocations, too, which is another bonus.

- Mark Michelson


On April 7, 2015, 6:25 p.m., warren smith wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4594/
 ---
 
 (Updated April 7, 2015, 6:25 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: asterisk-24934
 https://issues.asterisk.org/jira/browse/asterisk-24934
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Asterisk manager output is created using printf formatting, like:
 
 manager_event(SOME_EVENT_FLAG, EventName,
 KeyOne: %s\r\nKeyTwo: %s\r\n, val1, val2);
 
 This causes problems when the values themselves contain control characters 
 like carriage return and newline, so that applications parsing the output 
 will interpret this as a new key, or the end of an event.  An example of this 
 is having a callerid contain \r\n\r\n.  This ends the event, and the keys 
 for the same event are interpreted as a new message, and any keys below are 
 missed for the real event.
 
 I've included a patch that provides a ast_escape_c() function which takes a 
 string, then returns a pointer to a new string that has the c characters 
 escaped (i.e., newline into \n).  I've modified the calls to the 
 manager_event functions (manager_event, ast_manager_event, 
 ast_manager_event_multichan) so that values that could be set by a user are 
 escaped.  The string values that as far as I know aren't user-created were 
 left as-is, like channel names and uniqueid.
 
 There are quite a few calls to the manager event functions and I've double 
 checked to make sure all memory allocations are freed after creating the 
 escaped string.  I also had added an ast_replace_string function which i 
 didn't end up using, and added an ast_escape_output function which just calls 
 ast_escape_c.  An alternative would be to replace the sequence \r\n with 
 the escaped version, rather than the individual characters.
 
 I'm testing on our asterisk 11 install and this fixes the parsing bugs we run 
 into from messed up callerids and things like agent names containing return + 
 newline.
 
 
 Diffs
 -
 
   http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_local.c 
 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_iax2.c 
 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_agent.c 
 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/cel/cel_manager.c 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/cdr/cdr_manager.c 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/apps/app_voicemail.c 
 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/apps/app_userevent.c 
 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/apps/app_stack.c 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/apps/app_queue.c 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/apps/app_meetme.c 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/apps/app_dial.c 433419 
   http://svn.asterisk.org/svn/asterisk/branches/11/apps/app_confbridge.c 
 433419 
 
 Diff: https://reviewboard.asterisk.org/r/4594/diff/
 
 
 Testing
 ---
 
 In our production environment I have been running the patch for about 5 days. 
  The parsing issues we have had in the past are now resolved.
 
 
 Thanks,
 
 warren smith
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
..


Patch Set 5: Verified+1

-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 5
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
..


Patch Set 5: Code-Review+2

-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 5
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
..


Patch Set 5: -Verified

Removing the Verified since there are still outstanding findings.

-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 5
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
..


Patch Set 5: Code-Review+1

(2 comments)

It's looking good by me!

https://gerrit.asterisk.org/#/c/18/5/tests/rest_api/applications/stasisstatus/ari_client.py
File tests/rest_api/applications/stasisstatus/ari_client.py:

Line 365:if run is 0:
: LOGGER.debug(msg + 'Tearing down active connections.')
: self.__delete_all_channels()
: reactor.callLater(2, wait_for_it, deferred, 1)
: elif run is 1:
: if len(self.__channels)  0:
: msg += 'Waiting for channels to be destroyed.'
: LOGGER.debug(msg)
: reactor.callLater(2, wait_for_it, deferred, 1)
: reactor.callLater(2, wait_for_it, deferred, 2)
: elif run is 2:
: LOGGER.debug(msg + 'Disconnecting web socket.')
: self.__ari = None
: self.__factory = None
: self.disconnect_websocket()
: reactor.callLater(2, wait_for_it, deferred, 3)
: elif run is 3:
This is another of those cases where I'm not 100% sure on this, but I think the 
use of the 'is' operator in these comparisons is iffy. The 'is' operator 
returns true if the operands on either side refer to the same object, not if 
their values are equal. I'm honestly not sure what the behavior is when an 
integer literal is used as an operand, though. If I were writing this, though, 
I'd default to the safe bet and use the '==' operator for these comparisons 
instead.


https://gerrit.asterisk.org/#/c/18/5/tests/rest_api/applications/stasisstatus/configs/ast1/sip.conf
File tests/rest_api/applications/stasisstatus/configs/ast1/sip.conf:

Line 9: [acme](!)
 I am not sure that this template or the sherman endpoint is being used any 
In fact, I don't think SIP is being used at all for your tests, so you should 
be able to just scrap this conf file entirely.


-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 5
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4490: astdb: Allow clustering of the Asterisk Database between multiple Asterisk servers

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4490/#review15135
---


The only things I can think of that are issues here are based on theoretical 
usage of shared DB families, especially global ones.

Say that someone is sharing /foo/key between Asterisk 1 and Asterisk 2. A user 
on Asterisk 1 sets /foo/key = bar at the same time that a user on Asterisk 2 
sets /foo/key = baz. What may happen is that the local Asterisk boxes will set 
the values locally and then publish the updated values near-simultaneously to 
the other Asterisk box. The result may be that they both have /foo/key = bar, 
they both might have /foo/key = baz, or Asterisk 1 may end up with /foo/key = 
baz and Asterisk 2 ends up with /foo/key = bar. Essentially, there is nothing 
in place to guarantee the synchronization of globally shared values.

So what's the best way to deal with this? I can think of several answers:
* The AstDB may not be the right tool for this use case and you should be using 
a key-value store that is built for this instead.
* You may be able to satisfy your use case with unique shared keys instead of 
global shared keys.
* We may want to make global shared keys one-way. In other words, there is an 
owner of the key and listeners for changes in the key's value. Listeners on 
a global shared key cannot update their local DB with new values; only the 
owner can push new values to it. Listeners on a global shared key cannot share 
the value with other listeners; only owners may push changes to listeners.



While I'm thinking about it, there's also the possibility that the underlying 
AstDB implementation could be replaced with a key-value store that has the 
concept of replication built into it. If that were done, then presumably, there 
would be no need to involve Stasis or SIP PUBLISH in the process of sharing 
values between Asterisk boxes.

- Mark Michelson


On April 6, 2015, 2:22 a.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4490/
 ---
 
 (Updated April 6, 2015, 2:22 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24909
 https://issues.asterisk.org/jira/browse/ASTERISK-24909
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 As described on the asterisk-dev mailing list [1], I got some inspiration 
 from seeing Kamailio's htable implementation, and thought a similar mechanism 
 would work for the Asterisk Database. This patch is the result.
 
 This patch provides a mechanism to mark families within the AstDB as 
 shared. The marking of a family as shared is independent of the existance 
 of the family, and is independent of the updates already made to the family. 
 Shared families are subject to distribution with other Asterisk instances, as 
 well as subject to updates from other Asterisk instances. Two strategies for 
 sharing are implemented:
 
 * Global: A 'global' shared family shares the family/key space with all other 
 Asterisk instances. Say we have shared family 'foo', and we have two Asterisk 
 instances. Say the first Asterisk instance (ast1) updates a key in family 
 'foo' to the following:
 
   ast1
 /foo/key = bar
 
 The second Asterisk instance (ast2) would then receive an update in its AstDB:
 
   ast2
 /foo/key = bar
 
 If ast2 later updates the same key in its local AstDB to 'foobar', ast1 will 
 receive a notification to update the same key in its AstDB:
 
   ast2
 /foo/key = foobar
   ast1
 /foo/key = foobar
 
 * Unique: A 'unique' shared family shares its values with other Asterisk 
 instances, however, updates from other Asterisk instances are placed in 
 unique families in the local AstDB for each Asterisk instance. Again, say we 
 have shared family 'foo', and we have two Asterisk instances - ast1 and ast2. 
 ast1 has an EID of 11:11:11:11:11:11, while ast2 has an EID of 
 ff:ff:ff:ff:ff:ff. Say ast1 updates a key in family 'foo':
 
   ast1
 /foo/key = bar
 
 ast2 would receive the update for family 'foo', but instead of updating its 
 local copy, it would instead store the value in a new family for ast1 
 corresponding to its EID:
 
   ast2
 /11:11:11:11:11:11/foo/key = bar
 
 If ast2 later updates the same ky in its local AstDB to 'foobar', the 
 received value from ast1 will not be updated. However, ast1 will receive the 
 update, and store the value in a new family for ast2 corresponding to its EID:
 
   ast2
 /foo/key = foobar
 /11:11:11:11:11:11/foo/key = bar
   ast1
 /foo/key = bar
 /ff:ff:ff:ff:ff:ff/foo/key = foobar
 
 In order to manipulate shared families, two new dialplan functions have been 
 added, DB_SHARED and DB_SHARED_EXISTS. DB_SHARED allows

[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
..


Patch Set 10: Code-Review+2 Verified+1

-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 10
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Mark Michelson (Code Review)]

Mark Michelson has submitted this change and it was merged.

Change subject: stasis: set a channel variable on websocket disconnect error
..


stasis: set a channel variable on websocket disconnect error

This test is to ensure Asterisk applies the correct state to the channel
variable, STASISSTATUS. STASISSTATUS was introduced as a means for Stasis
applications to have context when errors occur in Stasis that disrupt normal
processing.

The test scenarios:
1. The 'Babs' scenario:
a. A channel is originated through ARI referencing a subscribed app (Babs)
   that was registered in Stasis during startup.
b. After Stasis is started, the channel is then hungup.
c. A check is made to ensure that the value of STASISSTATUS is SUCCESS.
2. The 'Bugs' scenario:
   a. A channel is originated through ARI referencing a subscribed app
  (BugsAlt) that was never registered in Stasis.
   b. A check is then made to ensure that the value of STASISSTATUS is FAILED.
3. The 'Buster' scenario:
   a. A channel is originated through ARI referencing a subscribed app
  (Buster) that was registered in Stasis during startup.
   b. While the channel from step 'a' is still active, the websocket is then
  disconnected out from underneath ARI.
   c. A new channel is originated through ARI, also referencing the subscribed
  app (Buster) that was registered in Stasis during startup.
   d. A check is then made to ensure that the value of STASISSTATUS is FAILED.

ASTERISK-24802
Reported By: Kevin Harwell

Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
---
A tests/rest_api/applications/stasisstatus/__init__.py
A tests/rest_api/applications/stasisstatus/ari_client.py
A tests/rest_api/applications/stasisstatus/configs/ast1/extensions.conf
A tests/rest_api/applications/stasisstatus/monitor.py
A tests/rest_api/applications/stasisstatus/observable_object.py
A tests/rest_api/applications/stasisstatus/run-test
A tests/rest_api/applications/stasisstatus/test-config.yaml
A tests/rest_api/applications/stasisstatus/test_case.py
A tests/rest_api/applications/stasisstatus/test_scenario.py
M tests/rest_api/applications/tests.yaml
10 files changed, 1,447 insertions(+), 0 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved; Verified
  Matt Jordan: Looks good to me, but someone else must approve



diff --git a/tests/rest_api/applications/stasisstatus/__init__.py 
b/tests/rest_api/applications/stasisstatus/__init__.py
new file mode 100644
index 000..e69de29
--- /dev/null
+++ b/tests/rest_api/applications/stasisstatus/__init__.py
diff --git a/tests/rest_api/applications/stasisstatus/ari_client.py 
b/tests/rest_api/applications/stasisstatus/ari_client.py
new file mode 100644
index 000..e7e2eb3
--- /dev/null
+++ b/tests/rest_api/applications/stasisstatus/ari_client.py
@@ -0,0 +1,404 @@
+#!/usr/bin/env python
+
+Copyright (C) 2015, Digium, Inc.
+Ashley Sanders asand...@digium.com
+
+This program is free software, distributed under the terms of
+the GNU General Public License Version 2.
+
+
+import sys
+import logging
+import uuid
+
+sys.path.append(lib/python)
+sys.path.append(tests/rest_api/applications)
+
+from asterisk.ari import ARI, AriClientFactory
+from stasisstatus.observable_object import ObservableObject
+from twisted.internet import defer, reactor
+
+LOGGER = logging.getLogger(__name__)
+
+
+class AriClient(ObservableObject):
+The ARI client.
+
+ This class serves as a facade for ARI and AriClientFactory. It is
+ responsible for creating and persisting the connection state needed to
+ execute a test scenario.
+ 
+
+def __init__(self, host, port, credentials, name='testsuite'):
+Constructor.
+
+Keyword Arguments:
+host  -- The [bindaddr] of the Asterisk HTTP web
+ server.
+port  -- The [bindport] of the Asterisk HTTP web
+ server.
+credentials   -- User credentials for ARI. A tuple.
+ E.g.: ('username', 'password').
+name  -- The name of the app to register in Stasis via
+ ARI (optional) (default 'testsuite').
+
+
+
+super(AriClient, self).__init__(name, ['on_channelcreated',
+   'on_channeldestroyed',
+   'on_channelvarset',
+   'on_client_start',
+   'on_client_stop',
+   'on_stasisend',
+   'on_stasisstart',
+   'on_ws_open',
+   'on_ws_closed'])
+self.__ari = None
+self.__factory = None

Re: [asterisk-dev] [Code Review] 4592: chan_pjsip: 183 sent when inband_progress=no

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4592/#review15093
---


I think this change is widening the scope of the inband_progress option beyond 
what it was intended to do. The intended purpose of inband_progress is to 
determine whether AST_CONTROL_RINGING should be handled by sending a 180 
(out-of-band ringing) or 183 with early media (in-band ringing) to the caller. 
With this change, setting inband_progress to no also prevents any early media 
from being sent, which is likely not what people intend when they set the 
option.

However, the ability to prevent early media by transforming a progress 
indication into a 180 Ringing makes sense as something someone may want to do. 
I just think this behavior should be relegated to a separate option.

- Mark Michelson


On April 6, 2015, 6:32 p.m., Kevin Harwell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4592/
 ---
 
 (Updated April 6, 2015, 6:32 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24835
 https://issues.asterisk.org/jira/browse/ASTERISK-24835
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The issue started out as early media not playing in chan_sip. However, it was 
 then reported that once the 'progressinband' option was set to 'yes' it would 
 work, but that the default behavior was different than that of chan_pjsip. 
 chan_pjsip's 'inband_progress' option defaults to 'no' as well, but it still 
 sends a 183. It turns out chan_pjsip was always sending a 183 despite 
 whatever the 'inband_progress' option was set to. Fixed it so chan_pjsip only 
 sends a 183 if 'inband_progress' is set to 'yes'.
 
 
 Diffs
 -
 
   branches/13/channels/chan_pjsip.c 434021 
 
 Diff: https://reviewboard.asterisk.org/r/4592/diff/
 
 
 Testing
 ---
 
 Duplicated the issue with chan_pjsip always sending the 183. After the patch 
 it will only send it when 'inband_progress' is set to 'yes'
 
 
 Thanks,
 
 Kevin Harwell
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: sip_attended_transfer now supports pre-12 Asterisk versions.

[Mark Michelson (Code Review)]

Mark Michelson has uploaded a new patch set (#3).

Change subject: sip_attended_transfer now supports pre-12 Asterisk versions.
..

sip_attended_transfer now supports pre-12 Asterisk versions.

The sip_attended transfer test was recently rewritten to prevent it from
bouncing during automatic test runs. The rewrite attempted to break into
two tests in an attempt to separate the logic of different versions from
one another.

Ashley pointed out on my original Asterisk 11 version of the patch that
there are only small difference between the Asterisk 11 and 12 versions
of the test, resulting in a lot of repeated boilerplate code that could
otherwise be avoided.

This change alters the 12+ specific test by separating the bridge logic
for different versions into their own classes. I have verified that the
test passes using both Asterisk 11 and Asterisk 13.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d

Fix a typo where a function call was evaluated as a boolean.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d

Address review feedback from Ashley and Matt

* Make efforts to make pylint less angry
* Add debugging to the 11 version since it's weird
* Changed some variable names. I went with the suggested names in some
  case, but in others I did not.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
---
M tests/channels/SIP/sip_attended_transfer/attended_transfer.py
M tests/channels/SIP/sip_attended_transfer/test-config.yaml
2 files changed, 199 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/testsuite refs/changes/29/29/3
-- 
To view, visit https://gerrit.asterisk.org/29
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: sip_attended_transfer now supports pre-12 Asterisk versions.

[Mark Michelson (Code Review)]

Mark Michelson has uploaded a new patch set (#3).

Change subject: sip_attended_transfer now supports pre-12 Asterisk versions.
..

sip_attended_transfer now supports pre-12 Asterisk versions.

The sip_attended transfer test was recently rewritten to prevent it from
bouncing during automatic test runs. The rewrite attempted to break into
two tests in an attempt to separate the logic of different versions from
one another.

Ashley pointed out on my original Asterisk 11 version of the patch that
there are only small difference between the Asterisk 11 and 12 versions
of the test, resulting in a lot of repeated boilerplate code that could
otherwise be avoided.

This change alters the 12+ specific test by separating the bridge logic
for different versions into their own classes. I have verified that the
test passes using both Asterisk 11 and Asterisk 13.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d

Fix a typo where a function call was evaluated as a boolean.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d

Address review feedback from Ashley and Matt

* Make efforts to make pylint less angry
* Add debugging to the 11 version since it's weird
* Changed some variable names. I went with the suggested names in some
  case, but in others I did not.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
---
M tests/channels/SIP/sip_attended_transfer/attended_transfer.py
M tests/channels/SIP/sip_attended_transfer/test-config.yaml
2 files changed, 199 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/testsuite refs/changes/29/29/3
-- 
To view, visit https://gerrit.asterisk.org/29
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4597: res_pjsip: add CLI commands for global and system configuration

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4597/#review15095
---



branches/13/res/res_pjsip/config_global.c
https://reviewboard.asterisk.org/r/4597/#comment25775

I think for now this is fine, but there actually is the possibility to 
misrepresent to the user what is configured here. For instance, if you do not 
have any global configuration set, then you will have no user_agent option set. 
This means that Asterisk will add no User-Agent header to outgoing SIP requests 
at all. However, printing CLI output would show that the user_agent is set to 
Asterisk PBX Version 13.3 (or something similar).

When I evaluate this, to me the issue isn't with what you're showing in 
this CLI command, but rather the somewhat odd behavior associated with how 
global configuration works. If I have no global configuration at all, then 
there is no user_agent set. If I have a global configuration section, but I 
have not set a user_agent, then the user_agent gets set to Asterisk PBX 
version 13.3 even though I didn't specify anything.

So rather than raising an issue on this code review, I've opened 
https://issues.asterisk.org/jira/browse/ASTERISK-24945 to get this 
inconsistency rectified. When that change gets made, then this check for a NULL 
cfg can be changed to an assertion instead since it should be impossible to 
have a NULL global configuration.



branches/13/res/res_pjsip/config_system.c
https://reviewboard.asterisk.org/r/4597/#comment25774

This should be impossible to happen since default system configuration 
should automatically get set on start-up if no system section was present in 
pjsip.conf. If default system configuration setup on startup fails, then 
res_pjsip.so should fail to load.

I think an assertion here that cfg is non-NULL would do the trick instead 
of trying again to create default configuration settings


- Mark Michelson


On April 7, 2015, 4:05 p.m., Kevin Harwell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4597/
 ---
 
 (Updated April 7, 2015, 4:05 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24918
 https://issues.asterisk.org/jira/browse/ASTERISK-24918
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Added two new CLI commands for res_pjsip global and system configuration 
 settings:
 
 pjsip show global
 pjsip show system
 
 
 Diffs
 -
 
   branches/13/res/res_pjsip/config_system.c 434150 
   branches/13/res/res_pjsip/config_global.c 434150 
   branches/13/CHANGES 434150 
 
 Diff: https://reviewboard.asterisk.org/r/4597/diff/
 
 
 Testing
 ---
 
 Ran the commands and checked output. Changed some options and reloaded and 
 made sure global settings changed, but system ones did not. Changed some 
 settings again and restarted and made sure both global and system changes too 
 effect. Also removed the sections completely from the pjsip.conf file and 
 made sure the defaults were shown.
 
 
 Thanks,
 
 Kevin Harwell
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4589: res_pjsip_t38: Add missing initialization of udptl-far_max_datagram in t38_initialize_session()

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4589/#review15103
---


I want to give this a Ship It! but the Testing Done section is not filled 
in at all. What has been done to test this change?

- Mark Michelson


On April 6, 2015, 3:53 p.m., Juergen Spies wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4589/
 ---
 
 (Updated April 6, 2015, 3:53 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24928
 https://issues.asterisk.org/jira/browse/ASTERISK-24928
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Initialize udptl-far_max_datagram in t38_initialize_session() with a 
 default value or a value provided
 in pjsip.conf (t38_udptl_maxdatagram).
 Without this far_max_datagram remains -1 if the remote endpoint does not 
 provide the MediaAttribute T38FaxMaxDatagram
 in it's SIP INVITE SDP. This will result in the INVITE being rejected.
 
 
 Diffs
 -
 
   /branches/13/res/res_pjsip_t38.c 433967 
 
 Diff: https://reviewboard.asterisk.org/r/4589/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Juergen Spies
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4541: clang compiler warning: -Wpointer-bool-conversion

[Mark Michelson]

 On April 7, 2015, 1:05 a.m., rmudgett wrote:
  /branches/13/apps/app_minivm.c, line 1842
  https://reviewboard.asterisk.org/r/4541/diff/3/?file=73407#file73407line1842
 
  Missing the !
  
  if (!ast_strlen_zero())
 
 Diederik de Groot wrote:
 Thanks again for checking my stuff, and sorry for the mistakes.

Sorry for telling you wrong to begin with.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4541/#review15081
---


On April 7, 2015, 2:23 a.m., Diederik de Groot wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4541/
 ---
 
 (Updated April 7, 2015, 2:23 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24917
 https://issues.asterisk.org/jira/browse/ASTERISK-24917
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 clang's static analyzer will throw quite a number warnings / errors during 
 compilation, some of which can be very helpfull in finding corner-case bugs.
 
 clang compiler warning:-Wpointer-bool-conversion
 
 Issues:
 chan_pjsip.c:1898:13: warning: address of array 'data-text' will always 
 evaluate to 'true' [-Wpointer-bool-conversion]
 if (!data-text) {
 ~~~^~~~
 app_minivm.c:1841:18: warning: address of array 'etemplate-locale' will 
 always evaluate to 'true' [-Wpointer-bool-conversion]
 if (etemplate-locale) {
 ~~  ~~~^~
 app_minivm.c:1871:17: warning: address of array 'etemplate-locale' will 
 always evaluate to 'true' [-Wpointer-bool-conversion]
 if (etemplate-locale) {
 ~~  ~~~^~
 app_queue.c:6686:19: warning: address of array 'qe-parent-monfmt' will 
 always evaluate to 'true' [-Wpointer-bool-conversion]
 if (qe-parent-monfmt  *qe-parent-monfmt) {
 ^~ ~~
 app_queue.c:9090:15: warning: address of array 'mem-state_interface' will 
 always evaluate to 'true' [-Wpointer-bool-conversion]
 if (mem-state_interface) {
 ~~  ~^~~
 res_smdi.c:876:19: warning: address of array 'search_msg-mesg_desk_num' will 
 always evaluate to 'true' [-Wpointer-bool-conversion]
 if (search_msg-mesg_desk_num) {
 ~~  ^
 res_smdi.c:879:19: warning: address of array 'search_msg-mesg_desk_term' 
 will always evaluate to 'true' [-Wpointer-bool-conversion]
 if (search_msg-mesg_desk_term) {
 ~~  ^~
 
 Changed: removed the superfluous checks
 
 
 Diffs
 -
 
   /branches/13/res/res_smdi.c 433444 
   /branches/13/channels/chan_pjsip.c 433444 
   /branches/13/apps/app_queue.c 433444 
   /branches/13/apps/app_minivm.c 433444 
 
 Diff: https://reviewboard.asterisk.org/r/4541/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Diederik de Groot
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4598: Refactor duplicated DNS routines into common sections

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4598/
---

(Updated April 7, 2015, 5:49 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Kevin's comments.


Repository: Asterisk


Description
---

Josh worked on SRV in one branch, and I worked on NAPTR in a separate branch. 
Independently we kept coming to realizations that something that one of us had 
developed independently would be needed by the other person. We decided to 
simply have copies of common functionality in our branches. After merging, we 
would perform a refactor to remove duplication.

This changeset introduces no new DNS functionality. Instead, it takes some 
duplicated code and places them into common areas of the DNS core.


Diffs (updated)
-

  /trunk/tests/test_dns_srv.c 434218 
  /trunk/tests/test_dns_naptr.c 434218 
  /trunk/main/dns_test.c PRE-CREATION 
  /trunk/main/dns_srv.c 434218 
  /trunk/main/dns_naptr.c 434218 
  /trunk/main/dns_core.c 434218 
  /trunk/include/asterisk/dns_test.h PRE-CREATION 
  /trunk/include/asterisk/dns_internal.h 434218 

Diff: https://reviewboard.asterisk.org/r/4598/diff/


Testing
---

All DNS unit tests continue to pass.


Thanks,

Mark Michelson

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4595: Voicemail API: fix handling of mailbox full condition

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4595/#review15101
---

Ship it!


Ship It!

- Mark Michelson


On April 6, 2015, 8:20 p.m., Scott Griepentrog wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4595/
 ---
 
 (Updated April 6, 2015, 8:20 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24942
 https://issues.asterisk.org/jira/browse/ASTERISK-24942
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 In r115582 (2008), the ERROR_MAILBOX_FULL flag was removed, and a check of 
 the save_to_folder result no longer handled the mailbox full condition.  This 
 lead to the deletion of a new Inbox message, once played, that could not be 
 relocated to the Old mailbox because of the maxmsg limit.  This patch 
 restores the original functionality lost, which is to leave the message in 
 the Inbox without deleting it.
 
 
 Diffs
 -
 
   /branches/11/apps/app_voicemail.c 434135 
 
 Diff: https://reviewboard.asterisk.org/r/4595/diff/
 
 
 Testing
 ---
 
 Tested manually on my system under Asterisk 13.
 
 
 Thanks,
 
 Scott Griepentrog
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4598: Refactor duplicated DNS routines into common sections

[Mark Michelson]

 On April 7, 2015, 5:28 p.m., Kevin Harwell wrote:
  /trunk/main/dns_core.c, line 438
  https://reviewboard.asterisk.org/r/4598/diff/1/?file=73671#file73671line438
 
  No value is set on true.

This is intended. See

https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4598/#review15092
---


On April 6, 2015, 10:46 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4598/
 ---
 
 (Updated April 6, 2015, 10:46 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Josh worked on SRV in one branch, and I worked on NAPTR in a separate branch. 
 Independently we kept coming to realizations that something that one of us 
 had developed independently would be needed by the other person. We decided 
 to simply have copies of common functionality in our branches. After merging, 
 we would perform a refactor to remove duplication.
 
 This changeset introduces no new DNS functionality. Instead, it takes some 
 duplicated code and places them into common areas of the DNS core.
 
 
 Diffs
 -
 
   /trunk/tests/test_dns_srv.c 434186 
   /trunk/tests/test_dns_naptr.c 434186 
   /trunk/main/dns_srv.c 434186 
   /trunk/main/dns_naptr.c 434186 
   /trunk/main/dns_core.c 434186 
   /trunk/include/asterisk/dns_internal.h 434186 
   /team/group/dns/main/dns_test.c PRE-CREATION 
   /team/group/dns/include/asterisk/dns_test.h PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/4598/diff/
 
 
 Testing
 ---
 
 All DNS unit tests continue to pass.
 
 
 Thanks,
 
 Mark Michelson
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: Enable support for directory containing custom tests.

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Enable support for directory containing custom tests.
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/27
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iff45fe5574900b5c5c77e3132984659133baadfd
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: Testsuite: New test for FAX via PJSIP T38 with authentication

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Testsuite: New test for FAX via PJSIP T38 with authentication
..


Patch Set 4: Code-Review+2

-- 
To view, visit https://gerrit.asterisk.org/28
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If37cf20857ae3c0b35e0637a0a2cb7e7d6226df6
Gerrit-PatchSet: 4
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Jonathan Rose jr...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Jonathan Rose jr...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4522: segfault-queue-ASTERISK-23319

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4522/#review15105
---

Ship it!


Ship It!

- Mark Michelson


On March 25, 2015, 4:10 p.m., Stefan Engström wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4522/
 ---
 
 (Updated March 25, 2015, 4:10 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23319
 https://issues.asterisk.org/jira/browse/ASTERISK-23319
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 See comment in ASTERISK-23319 by Matt Jordan
 
 
 Diffs
 -
 
   /branches/13/apps/app_queue.c 433370 
 
 Diff: https://reviewboard.asterisk.org/r/4522/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Stefan Engström
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4542: DNS: Add NAPTR support and tests

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4542/
---

(Updated April 6, 2015, 12:06 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 434068


Repository: Asterisk


Description
---

This adds NAPTR support for DNS in Asterisk.

The main parts of this are the functions for allocating a DNS NAPTR record when 
a resolver wishes to add a NAPTR record, the sorting algorithm for sorting DNS 
NAPTR records, and the tests that use a mock DNS resolver.

NOTE: There is likely to be some overlap here in this review and Josh's SRV 
review (/r/4528). Our stance on this is that we will factor out the duplicated 
code once both SRV and NAPTR have been merged into the main DNS branch. The 
factoring out of common functions will be placed in its own review.


Diffs
-

  /team/group/dns/tests/test_dns_naptr.c PRE-CREATION 
  /team/group/dns/res/res_resolver_unbound.c 433885 
  /team/group/dns/main/dns_naptr.c 433885 
  /team/group/dns/main/dns_core.c 433885 
  /team/group/dns/include/asterisk/dns_internal.h 433885 

Diff: https://reviewboard.asterisk.org/r/4542/diff/


Testing
---

All previous DNS tests continue to pass, and all new tests added in this review 
pass as well.


Thanks,

Mark Michelson

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] [Code Review] 4579: res_pjsip_messaging: Serialize outgoing MESSAGEs

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4579/
---

Review request for Asterisk Developers.


Bugs: ASTERISK-24937
https://issues.asterisk.org/jira/browse/ASTERISK-24937


Repository: Asterisk


Description
---

Because res_pjsip_messaging throws all messages to send into the threadpool, 
there is no guarantee that consecutive outgoing messages from the same dialplan 
execution will be sent in the same order.
So for instance, if you had the following dialplan:

exten = hello,1,NoOp()
same = n,SendMessage(hello)
same = n,SendMessage(world)

It would be expected that the hello message would be sent before the world 
message. However, it cannot be guaranteed this will happen with the current 
threadpool usage.

The patch on this review introduces a serializer for outgoing MESSAGE requests 
from Asterisk. This ensures that all MESSAGE requests are sent in the same 
order that they are processed in the dialplan.


Diffs
-

  /branches/13/res/res_pjsip_messaging.c 433838 

Diff: https://reviewboard.asterisk.org/r/4579/diff/


Testing
---

The bug itself is incredibly difficult to have happen under normal 
circumstances, but I have confirmed that this patch has not hindered operations 
any.


Thanks,

Mark Michelson

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4571: Add usegmtime option to cel_pgsql

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4571/#review15047
---


When I try to view the review, Review Board complains that it cannot display 
the changes because the diff uploaded did not apply cleanly. Please ensure that 
your working copy of the files being altered is up to date.

Also, if you're not already using it, there are some very useful command line 
tools you can use for uploading reviews to review board. See the Posting Code 
to Review Board section on this wiki page: 
https://wiki.asterisk.org/wiki/display/AST/Review+Board+Usage

- Mark Michelson


On April 3, 2015, 4:56 p.m., Rodrigo Ramirez Norambuena wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4571/
 ---
 
 (Updated April 3, 2015, 4:56 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: https://issues.asterisk.org/jira/browse/ASTERISK-23186
 
 https://issues.asterisk.org/jira/browse/https://issues.asterisk.org/jira/browse/ASTERISK-23186
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Feature for added an option that lets you specify that the timestamps going 
 into PostgreSQL from CEL log should be in GMT instead of local time.
 
 
 Diffs
 -
 
   /trunk/configs/samples/cel_pgsql.conf.sample 433966 
   /trunk/cel/cel_pgsql.c 433966 
   /trunk/CHANGES 433966 
 
 Diff: https://reviewboard.asterisk.org/r/4571/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Rodrigo Ramirez Norambuena
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: Add SIP attended transfer for Asterisk 11.

[Mark Michelson (Code Review)]

Mark Michelson has abandoned this change.

Change subject: Add SIP attended transfer for Asterisk 11.
..


Abandoned

-- 
To view, visit https://gerrit.asterisk.org/20
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I48c7b6a9298552aa756d0c2f26afbd6a96d553b5
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: Add SIP attended transfer for Asterisk 11.

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: Add SIP attended transfer for Asterisk 11.
..


Patch Set 1:

I am abandoning this change in favor of change /c/29/

-- 
To view, visit https://gerrit.asterisk.org/20
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I48c7b6a9298552aa756d0c2f26afbd6a96d553b5
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: sip_attended_transfer now supports pre-12 Asterisk versions.

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: sip_attended_transfer now supports pre-12 Asterisk versions.
..


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/29/1/tests/channels/SIP/sip_attended_transfer/attended_transfer.py
File tests/channels/SIP/sip_attended_transfer/attended_transfer.py:

Line 235: if self.bridge_state.bridge2_bridged and 
self.carol_call_answered:
Got a typo here. This should be self.bridge_staet.bridge2_bridged() (note the 
added parentheses)

As it turns out my test runs must have been passing because it just so happened 
that bridge 2 was actually bridged before Alice's call to Carol had been 
answered.


-- 
To view, visit https://gerrit.asterisk.org/29
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: sip_attended_transfer now supports pre-12 Asterisk versions.

[Mark Michelson (Code Review)]

Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/29

Change subject: sip_attended_transfer now supports pre-12 Asterisk versions.
..

sip_attended_transfer now supports pre-12 Asterisk versions.

The sip_attended transfer test was recently rewritten to prevent it from
bouncing during automatic test runs. The rewrite attempted to break into
two tests in an attempt to separate the logic of different versions from
one another.

Ashley pointed out on my original Asterisk 11 version of the patch that
there are only small difference between the Asterisk 11 and 12 versions
of the test, resulting in a lot of repeated boilerplate code that could
otherwise be avoided.

This change alters the 12+ specific test by separating the bridge logic
for different versions into their own classes. I have verified that the
test passes using both Asterisk 11 and Asterisk 13.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
---
M tests/channels/SIP/sip_attended_transfer/attended_transfer.py
M tests/channels/SIP/sip_attended_transfer/test-config.yaml
2 files changed, 125 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/testsuite refs/changes/29/29/1

diff --git a/tests/channels/SIP/sip_attended_transfer/attended_transfer.py 
b/tests/channels/SIP/sip_attended_transfer/attended_transfer.py
index cb133f3..ef9f314 100644
--- a/tests/channels/SIP/sip_attended_transfer/attended_transfer.py
+++ b/tests/channels/SIP/sip_attended_transfer/attended_transfer.py
@@ -8,7 +8,11 @@
 
 import logging
 import pjsua as pj
+import sys
 
+sys.path.append('lib/python/asterisk')
+
+from version import AsteriskVersion
 from twisted.internet import reactor
 
 LOGGER = logging.getLogger(__name__)
@@ -52,8 +56,8 @@
 reactor.callFromThread(self.on_answered)
 
 
-class BridgeState(object):
-'''Object for tracking state of a bridge
+class BridgeTwelve(object):
+'''Object for tracking attributes of an Asterisk 12+ bridge
 
 The main data the test cares about is the bridge's unique id and whether 
two
 channels have been bridged together by the bridge.
@@ -61,6 +65,118 @@
 def __init__(self):
 self.unique_id = None
 self.bridged = False
+
+
+class BridgeStateTwelve(object):
+'''Tracker of Bridge State for Asterisk 12+
+
+Since Asterisk 12+ has the concept of Bridge objects, this tracks the
+bridges by detecting when they are created. Once bridges are created, we
+determine that channels are bridged when BridgeEnter events indicate that
+two channels are present.
+'''
+def __init__(self, test_object, controller, ami):
+self.test_object = test_object
+self.controller = controller
+self.ami = ami
+self.bridge1 = BridgeTwelve()
+self.bridge2 = BridgeTwelve()
+
+self.ami.registerEvent('BridgeCreate', self.bridge_create)
+self.ami.registerEvent('BridgeEnter', self.bridge_enter)
+
+def bridge_create(self, ami, event):
+if not self.bridge1.unique_id:
+self.bridge1.unique_id = event.get('bridgeuniqueid')
+elif not self.bridge2.unique_id:
+self.bridge2.unique_id = event.get('bridgeuniqueid')
+else:
+LOGGER.error(Unexpected third bridge created)
+self.test_object.set_passed(False)
+self.test_object.stop_reactor()
+
+def bridge_enter(self, ami, event):
+if (event.get('bridgeuniqueid') == self.bridge1.unique_id and
+event.get('bridgenumchannels') == '2'):
+self.bridge1.bridged = True
+if self.controller.state == BOB_CALLED:
+self.controller.call_carol()
+elif self.controller.state == TRANSFERRED:
+self.controller.hangup_calls()
+else:
+LOGGER.error(Unexpected BridgeEnter event)
+self.test_object.set_passed(False)
+self.test_object.stop_reactor()
+elif (event.get('bridgeuniqueid') == self.bridge2.unique_id and
+  event.get('bridgenumchannels') == '2'):
+self.bridge2.bridged = True
+if self.controller.state == CAROL_CALLED:
+self.controller.transfer_call()
+elif self.controller.state == TRANSFERRED:
+self.controller.hangup_calls()
+else:
+LOGGER.error(Unexpected BridgeEnter event)
+self.test_object.set_passed(False)
+self.test_object.stop_reactor()
+
+def bridge1_bridged(self):
+return self.bridge1.bridged
+
+def bridge2_bridged(self):
+return self.bridge2.bridged
+
+
+class BridgeStateEleven(object):
+'''Tracker of bridge state for Asterisk 11-
+
+Since in Asterisk versions prior to 12, there are no bridge objects, the
+only way we can track the state of bridges in Asterisk is via Bridge

[asterisk-dev] Change in testsuite[master]: sip_attended_transfer now supports pre-12 Asterisk versions.

[Mark Michelson (Code Review)]

Mark Michelson has uploaded a new patch set (#2).

Change subject: sip_attended_transfer now supports pre-12 Asterisk versions.
..

sip_attended_transfer now supports pre-12 Asterisk versions.

The sip_attended transfer test was recently rewritten to prevent it from
bouncing during automatic test runs. The rewrite attempted to break into
two tests in an attempt to separate the logic of different versions from
one another.

Ashley pointed out on my original Asterisk 11 version of the patch that
there are only small difference between the Asterisk 11 and 12 versions
of the test, resulting in a lot of repeated boilerplate code that could
otherwise be avoided.

This change alters the 12+ specific test by separating the bridge logic
for different versions into their own classes. I have verified that the
test passes using both Asterisk 11 and Asterisk 13.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d

Fix a typo where a function call was evaluated as a boolean.

Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
---
M tests/channels/SIP/sip_attended_transfer/attended_transfer.py
M tests/channels/SIP/sip_attended_transfer/test-config.yaml
2 files changed, 125 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/testsuite refs/changes/29/29/2
-- 
To view, visit https://gerrit.asterisk.org/29
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I958c52cebb94f9cfc8dc8ed81311ae62efb2679d
Gerrit-PatchSet: 2
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4542: DNS: Add NAPTR support and tests

[Mark Michelson]

 On March 31, 2015, 3:25 p.m., Matt Jordan wrote:
  /team/group/dns/main/dns_naptr.c, lines 420-421
  https://reviewboard.asterisk.org/r/4542/diff/2/?file=73012#file73012line420
 
  The asserts here are appropriate.
  
  However, if there is an error in the record, such that the memcmp never 
  returns true, we'll get stuck in this loop.
  
  It may be good to have a 'fail safe' break out in the loop, after the 
  asserts. That way in dev-mode we'll catch issues, but in production 
  systems, the allocation of the NAPTR record will fail and we can 
  (hopefully) gracefully handle it.

I'm not sure I agree with this.

The reason why these are assertions instead of if statements is because the 
record we have been given came from the DNS answer in the first place, so it 
HAS to be present in the answer. Even if the answer is malformed in some way, 
the record we've been given comes from that answer, so it needs to be present.

On the other hand, it's not difficult to add a fail-safe if check here just to 
be certain.


 On March 31, 2015, 3:25 p.m., Matt Jordan wrote:
  /team/group/dns/main/dns_naptr.c, lines 447-449
  https://reviewboard.asterisk.org/r/4542/diff/2/?file=73012#file73012line447
 
  Suggestion: since this is repeated after each check, you may want to 
  macro-tize it:
  
  #define CHECK_END_OF_RECORD do { \
  if (ptr = end_of_record) { \
  return NULL; \
  } } while (0)
  
  
  Then you can just put:
  
  ptr += 2;
  CHECK_END_OF_RECORD;
  
  Or something like that.
 
 rmudgett wrote:
 Doing this hides return points.  Which adds a potential for memory and 
 ref leaks.
 
 Matt Jordan wrote:
 True, right now all of these do not perform cleanup. In fact, the entire 
 exercise here is mostly to figure out how long everything is so that things 
 can be allocated.
 
 This is a long routine; sometimes it's nice to take repetitive code and 
 squish it down. If Mark feels like that injects too much risk, I have no 
 problem with the finding being dropped.


Yeah, I'm not proud of this routine's length, and I wanted to break it up more 
than it already is, but unfortunately everything I came up with ended up either 
being uglier or questionable.

The macro idea has some merit to it because it prevents fat-fingering and 
accidentally comparing the wrong values or typing a  instead of a =. But as 
Richard pointed out, this hides return points, which in my opinion ultimately 
made the code less readable.

I could make a compromise and have

#define CHECK_END_OF_RECORD (ptr = end_of_record)
...
if (CHECK_END_OF_RECORD) {
return NULL
}

Readability is a bit compromised, but the typo possibilities are eliminated. 
I'll just go with that.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4542/#review14972
---


On March 27, 2015, 2:45 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4542/
 ---
 
 (Updated March 27, 2015, 2:45 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This adds NAPTR support for DNS in Asterisk.
 
 The main parts of this are the functions for allocating a DNS NAPTR record 
 when a resolver wishes to add a NAPTR record, the sorting algorithm for 
 sorting DNS NAPTR records, and the tests that use a mock DNS resolver.
 
 NOTE: There is likely to be some overlap here in this review and Josh's SRV 
 review (/r/4528). Our stance on this is that we will factor out the 
 duplicated code once both SRV and NAPTR have been merged into the main DNS 
 branch. The factoring out of common functions will be placed in its own 
 review.
 
 
 Diffs
 -
 
   /team/group/dns/tests/test_dns_naptr.c PRE-CREATION 
   /team/group/dns/res/res_resolver_unbound.c 433573 
   /team/group/dns/main/dns_naptr.c 433573 
   /team/group/dns/main/dns_core.c 433573 
   /team/group/dns/include/asterisk/dns_internal.h 433573 
 
 Diff: https://reviewboard.asterisk.org/r/4542/diff/
 
 
 Testing
 ---
 
 All previous DNS tests continue to pass, and all new tests added in this 
 review pass as well.
 
 
 Thanks,
 
 Mark Michelson
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4542: DNS: Add NAPTR support and tests

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4542/
---

(Updated April 1, 2015, 2:51 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed review feedback from Matt and Kevin.

* Added Doxygen comments for NAPTR data and DNS record data_ptr
* Added safety checks to return NULL if NAPTR record not found in DNS result.
* Macrotized the check if we are past the end of the NAPTR record when parsing.
* Short-circuit the sort operation if fewer than two NAPTR records are returned
* Removed ast_ prefix from internal DNS routines.
* Error messages for nominal tests are more descriptive


Repository: Asterisk


Description
---

This adds NAPTR support for DNS in Asterisk.

The main parts of this are the functions for allocating a DNS NAPTR record when 
a resolver wishes to add a NAPTR record, the sorting algorithm for sorting DNS 
NAPTR records, and the tests that use a mock DNS resolver.

NOTE: There is likely to be some overlap here in this review and Josh's SRV 
review (/r/4528). Our stance on this is that we will factor out the duplicated 
code once both SRV and NAPTR have been merged into the main DNS branch. The 
factoring out of common functions will be placed in its own review.


Diffs (updated)
-

  /team/group/dns/tests/test_dns_naptr.c PRE-CREATION 
  /team/group/dns/res/res_resolver_unbound.c 433885 
  /team/group/dns/main/dns_naptr.c 433885 
  /team/group/dns/main/dns_core.c 433885 
  /team/group/dns/include/asterisk/dns_internal.h 433885 

Diff: https://reviewboard.asterisk.org/r/4542/diff/


Testing
---

All previous DNS tests continue to pass, and all new tests added in this review 
pass as well.


Thanks,

Mark Michelson

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4536: iax2_poke_noanswer expiration timer too short

[Mark Michelson]

 On April 1, 2015, 1:07 a.m., Matt Jordan wrote:
  trunk/channels/chan_iax2.c, line 12370
  https://reviewboard.asterisk.org/r/4536/diff/1/?file=72980#file72980line12370
 
  The usage of max_retries here feels arbitrary. I'm not against this 
  being controlled more dynamically based on the last known qualify time, but 
  I'd rather just see that be 4 or 8 here, as appropriate.

Just going to chime in with my agreement here. I'm not sure how max_retries 
fits into this.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4536/#review15007
---


On March 26, 2015, 11:42 p.m., Y Ateya wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4536/
 ---
 
 (Updated March 26, 2015, 11:42 p.m.)
 
 
 Review request for Asterisk Developers and rnewton.
 
 
 Bugs: ASTERISK-24894
 https://issues.asterisk.org/jira/browse/ASTERISK-24894
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Increase POKE retry window from DEFAULT_MAXMS * 2 (4 seconds) to bigger 
 number (derived from qualify time; which control POKE retry time).
 
 
 Diffs
 -
 
   trunk/channels/chan_iax2.c 432806 
 
 Diff: https://reviewboard.asterisk.org/r/4536/diff/
 
 
 Testing
 ---
 
 - Tried test with multiple qualify values (2 and 10 seconds).
 - Tried test with 100% packets loss to ensure that when a POKE packet is 
 dropped it will be retried couple of time before declaring client 
 disconnected.
 
 
 Thanks,
 
 Y Ateya
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4572: RFC: Refactor Qualify and res_pjsip/endpt_send_request

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4572/#review15026
---


As a concept, this feels like a suitable stop-gap solution. Better solutions 
would be alterations to PJSIP itself. Either

1) Make pjsip_endpt_send_request() not ignore the timeout parameter passed to 
it.
2) Make pjsip transaction timers configurable per transaction.

The solution provided here has the issue that it has an Asterisk scheduler and 
a PJLIB timer competing with each other, when it would be less wasteful to have 
the PJLIB timer do everything. Because of the competing timers, it's possible 
for the transaction state callback to be called into from multiple threads at 
the same time. If the transaction timers could be altered per transaction, then 
there would only ever be a single timer running, and the possibility of races 
is eliminated.

- Mark Michelson


On March 31, 2015, 5:57 a.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4572/
 ---
 
 (Updated March 31, 2015, 5:57 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24863
 https://issues.asterisk.org/jira/browse/ASTERISK-24863
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This review is based on the discussion here
 http://lists.digium.com/pipermail/asterisk-dev/2015-March/073921.html
 
 -
 Right now when a contact is qualified, it's immediately marked as
 UNAVAILABLE.  If a response to the OPTIONS message comes back, it's
 marked as AVAILABLE again and the round trip time calculated.   The
 status flapping even in normal operation makes generating events
 tricky because the subscriber will see up/down events even if the
 contact is really AVAILABLE.   Now the pjsip transaction will
 eventually time out at an unconfigurable 32 seconds and call the
 callback but 32 seconds is a long time.  This is also an issue if the
 qualify_frequency is less than 32 seconds since they'll be multiple
 pjsip transactions in progress for the same contact.
 
 snip
 
 So, I'm proposing pulling libpjsip/pjsip_endpt_send_request up into
 res_pjsip.   It's 2 short function and a pjsip_module structure with
 no access to any private pjsip stuff.   Now we'll have the ability to
 terminate the transaction AND it seems that there's a timeout member
 of pjsip_transaction which I'm hoping (but haven't tested) will
 eliminate the need to add timeout processing in pjsip_options.
 
 The result of all of this is that we'll be able to generate events for
 contact status (ASTERISK-24863) which in turn will help is provide
 functionality like marking an endpoint unavailable when all of its
 contacts are unavailable.
 -
 
 Both Josh and Mark had comments in the thread so this is an RFC ONLY review.
 It covers the pull up of libpjsip/pjsip_endpt_send_request but I've also 
 added the
 use case which is creating and processing a new aor/contact option called 
 'qualify_timeout'.  I'll split the final submission into 2 pieces.
 
 
 Diffs
 -
 
   branches/13/res/res_pjsip/pjsip_options.c 433794 
   branches/13/res/res_pjsip/location.c 433794 
   branches/13/res/res_pjsip.c 433794 
   branches/13/include/asterisk/res_pjsip.h 433794 
 
 Diff: https://reviewboard.asterisk.org/r/4572/diff/
 
 
 Testing
 ---
 
 All existing pjsip related tests complete.  I'll also have to add event 
 generation on contact status change and the tests based on it.
 
 
 Thanks,
 
 George Joseph
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: stasis: set a channel variable on websocket disconnect error

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: stasis: set a channel variable on websocket disconnect error
..


Patch Set 1: Code-Review+1

(4 comments)

I noticed that I gave the Code-Review a 0 last time. I figure that a +1 is 
actually in order since I noticed that my comments last time weren't about 
functional deficiencies so much as style and nitpicks. So here's my +1.

https://gerrit.asterisk.org/#/c/18/1/tests/rest_api/applications/stasisstatus/ari_client.py
File tests/rest_api/applications/stasisstatus/ari_client.py:

Line 148: def on_channelcreated(self, message):
 This function is what builds up the local container for channels created by
Ah, I missed the getattr() call in on_ws_event(). I'm going to blame gerrit's 
hijacking of my browser's search :-P


https://gerrit.asterisk.org/#/c/18/1/tests/rest_api/applications/stasisstatus/observable_object.py
File tests/rest_api/applications/stasisstatus/observable_object.py:

Line 84: del self.__registrar[event][:]
 From the python documentation https://docs.python.org/3/tutorial/datastruct
Ah, thanks for the enlightenment!


Line 132: if self.__suspended  0:
: self.__suspended = 0
 Not including the = sign excludes this redundant case:
Correct, but my point was that if self.__suspended is 0 when entering this 
function, the result will be that it ends up being set to -1. My assumption was 
that you wanted self.__suspended to have a lower bound of 0. My suggestion was 
made in the spirit of brevity, but you could just as easily have

if self.__suspended == 0:
return
elif self.__suspended  0:
self.__suspended -= 1
else:
# WTF, how did it get to be  0?


Line 143: def __validate(self, **kwargs):
 It is so that there are more meaningful messages in the logfile when debugg
I wasn't suggesting getting rid of the method, just rewriting its body.

While DRY is fantastic, I'm also a firm believer in YAGNI, especially for tests 
that are concentrated on a specific case, where the data that is to be used is 
self-generated and known in advance. In this case, you've written a generic 
validation function that can take any number of named parameters, and based on 
the name of the parameter, perform some specific action. In actual use, the 
only parameter ever passed to this function is a single parameter called 
event (never multiple parameters), and it will never be NoneType (since the 
data being passed to this function is generated by your own test cases), and I 
don't actually think that any of the operations you perform could throw an 
IndexError anway. There's no reason to build a trans-oceanic cruise liner to 
cross a creek.

The error message argument makes sense. If you want to keep an error message in 
here, that's cool.


-- 
To view, visit https://gerrit.asterisk.org/18
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0f7dadfd429bd30e9f07a531f47884d8c923fc13
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4556: dns: Add res_resolver_system module.

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4556/#review15024
---


In general, I'm going to say that while I understand that this code was adapted 
from old code in Asterisk, this is is dire need of comments, both in the form 
of doxygen and comments over individual sections in functions like 
resolver_parse_answer().

This could use some off-nominal tests, specifically to ensure that malformed 
DNS responses do not cause crashes or other unexpected behaviors.


/trunk/res/res_resolver_system.c
https://reviewboard.asterisk.org/r/4556/#comment25695

Curly braces on if statements



/trunk/res/res_resolver_system.c
https://reviewboard.asterisk.org/r/4556/#comment25696

This has the same alignment concerns that I brought up in the SRV review.



/trunk/res/res_resolver_system.c
https://reviewboard.asterisk.org/r/4556/#comment25697

Unless there's something I've missed, this appears to be passing the TTL in 
network byte order instead of host byte order.

Add a check of the TTL in the nominal unit tests to be sure about this.



/trunk/res/res_resolver_system.c
https://reviewboard.asterisk.org/r/4556/#comment25698

This isn't used.

(yes, this is present in the NAPTR and SRV tests as well. I just didn't 
catch it until now)



/trunk/res/res_resolver_system.c
https://reviewboard.asterisk.org/r/4556/#comment25699

The v4_buf isn't really needed here. You could just do:

inet_pton(AF_INET, record-ip, ptr);
ptr += V4_BUFSIZE;



/trunk/res/res_resolver_system.c
https://reviewboard.asterisk.org/r/4556/#comment25693

There is a red blob at the end of this line.



/trunk/res/res_resolver_system.c
https://reviewboard.asterisk.org/r/4556/#comment25692

Based on recent anecdotal experience, I think it's a good idea to impose a 
low maximum threadpool size for new threadpools we introduce. Something like 10 
is probably a good default maximum (though even lower than that is probably 
fine).


- Mark Michelson


On March 29, 2015, 6:05 p.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4556/
 ---
 
 (Updated March 29, 2015, 6:05 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This change adds a res_resolver_system module which is a fallback for when 
 libunbound is not available. It's a port of the dns.c code into the new core 
 DNS API. While queries can be executed in an async fashion you can't cancel a 
 query that is in progress. There are also unit tests which cover the parsing 
 aspect of the code to make sure it works as expected.
 
 
 Diffs
 -
 
   /trunk/res/res_resolver_system.c PRE-CREATION 
   /trunk/configs/samples/resolver_system.conf.sample PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/4556/diff/
 
 
 Testing
 ---
 
 Ran unit tests, they are happy.
 
 
 Thanks,
 
 Joshua Colp
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: Rewrite sip_attended_transfer test to stop failing.

[Mark Michelson (Code Review)]

Hello Ashley Sanders,

I'd like you to reexamine a change.  Please visit

https://gerrit.asterisk.org/19

to look at the new patch set (#3).

Change subject: Rewrite sip_attended_transfer test to stop failing.
..

Rewrite sip_attended_transfer test to stop failing.

The sip_attended_transfer test has been bouncing for a while. There
are two major fixes introduced here to prevent the bouncing.

First, by converting to using the testsuite's PJSUA module, we no
longer are using the native Python threading library. Instead, we
are using a method that works better with the Twisted framework.

Second, the test is more strict about when the transfer may be
performed. The previous test would attempt the transfer when Asterisk
reported that the call was bridged. The problem is that Asterisk may
report the call as bridged before PJSUA has properly processed the
200 OK that Asterisk has sent to it. By waiting, we can be sure that
all parties are prepared when the transfer is attempted.

The test has also been rewritten to only work with Asterisk 12+. A
new separate test will be written to work on Asterisk 11. This helps
the code to be a little less cluttered.

Change-Id: I1676801d90bcafc28ba25e8b6889f40ab08cc90e

Address review feedback from Ashley

* Two CallCallback classes have been combined into one
* Bridge state has been factored into a minimal class
* Unnecessary checks of test state have been removed.

Change-Id: I1676801d90bcafc28ba25e8b6889f40ab08cc90e
---
A tests/channels/SIP/sip_attended_transfer/attended_transfer.py
M tests/channels/SIP/sip_attended_transfer/configs/ast1/extensions.conf
M tests/channels/SIP/sip_attended_transfer/configs/ast1/sip.conf
D tests/channels/SIP/sip_attended_transfer/run-test
M tests/channels/SIP/sip_attended_transfer/test-config.yaml
5 files changed, 234 insertions(+), 216 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/testsuite refs/changes/19/19/3
-- 
To view, visit https://gerrit.asterisk.org/19
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1676801d90bcafc28ba25e8b6889f40ab08cc90e
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: Rewrite sip_attended_transfer test to stop failing.

[Mark Michelson (Code Review)]

Mark Michelson has uploaded a new patch set (#3).

Change subject: Rewrite sip_attended_transfer test to stop failing.
..

Rewrite sip_attended_transfer test to stop failing.

The sip_attended_transfer test has been bouncing for a while. There
are two major fixes introduced here to prevent the bouncing.

First, by converting to using the testsuite's PJSUA module, we no
longer are using the native Python threading library. Instead, we
are using a method that works better with the Twisted framework.

Second, the test is more strict about when the transfer may be
performed. The previous test would attempt the transfer when Asterisk
reported that the call was bridged. The problem is that Asterisk may
report the call as bridged before PJSUA has properly processed the
200 OK that Asterisk has sent to it. By waiting, we can be sure that
all parties are prepared when the transfer is attempted.

The test has also been rewritten to only work with Asterisk 12+. A
new separate test will be written to work on Asterisk 11. This helps
the code to be a little less cluttered.

Change-Id: I1676801d90bcafc28ba25e8b6889f40ab08cc90e

Address review feedback from Ashley

* Two CallCallback classes have been combined into one
* Bridge state has been factored into a minimal class
* Unnecessary checks of test state have been removed.

Change-Id: I1676801d90bcafc28ba25e8b6889f40ab08cc90e
---
A tests/channels/SIP/sip_attended_transfer/attended_transfer.py
M tests/channels/SIP/sip_attended_transfer/configs/ast1/extensions.conf
M tests/channels/SIP/sip_attended_transfer/configs/ast1/sip.conf
D tests/channels/SIP/sip_attended_transfer/run-test
M tests/channels/SIP/sip_attended_transfer/test-config.yaml
5 files changed, 234 insertions(+), 216 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/testsuite refs/changes/19/19/3
-- 
To view, visit https://gerrit.asterisk.org/19
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1676801d90bcafc28ba25e8b6889f40ab08cc90e
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson mmichel...@digium.com
Gerrit-Reviewer: Ashley Sanders asand...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] Change in testsuite[master]: rest_api/channels/snoop_spy: Stop test on bridge destruction

[Mark Michelson (Code Review)]

Mark Michelson has posted comments on this change.

Change subject: rest_api/channels/snoop_spy: Stop test on bridge destruction
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/21
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I373981f81ca455743bbf2371f07b380d013cd12b
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Joshua Colp jc...@digium.com
Gerrit-Reviewer: Mark Michelson mmichel...@digium.com
Gerrit-HasComments: No

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4574: pjsip: resolve compatibility problem with ast_sip_session struct

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4574/#review15021
---

Ship it!


Ship It!

- Mark Michelson


On April 1, 2015, 9:20 p.m., Scott Griepentrog wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4574/
 ---
 
 (Updated April 1, 2015, 9:20 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 A compatibility problem was introduced in r430179 
 (https://reviewboard.asterisk.org/r/4308) when a new variable was inserted 
 into the struct ast_sip_session.  This change puts that new variable at the 
 end of the structure to avoid misalignment of the values.
 
 This also affects releases 13.2 and 13.3.
 
 
 Diffs
 -
 
   /branches/13/include/asterisk/res_pjsip_session.h 433918 
 
 Diff: https://reviewboard.asterisk.org/r/4574/diff/
 
 
 Testing
 ---
 
 With this change, a DPMA crash has been eliminated when transmitting messages 
 via PJSIP.
 
 
 Thanks,
 
 Scott Griepentrog
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev