Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-04-16 Thread gareth
/manager.h 434448 /trunk/UPGRADE.txt 434448 Diff: https://reviewboard.asterisk.org/r/4391/diff/ Testing --- Connected to manager, issued 'core show uptime' command and verified that there was a blank line between the headers and output. Thanks, gareth -- ___

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-04-13 Thread gareth
ile73873line4904> > > > > Nit pick: "Success" or "Error" is already provided through a standard > > field, so this could be static - "Message: Command Output Follows\r\n". > > This would give a single value f

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-04-12 Thread gareth
34448 /trunk/main/cli.c 434448 /trunk/include/asterisk/manager.h 434448 /trunk/UPGRADE.txt 434448 Diff: https://reviewboard.asterisk.org/r/4391/diff/ Testing --- Connected to manager, issued 'core show uptime' command and verifie

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-04-08 Thread gareth
how ...' commands then they likely care about the output. In this case, I think it would be better to provide a more descriptive error message to the caller so they can detect if the command was executed. Yes, ast_cli_commmand_full should indicate whether the command failed, I wi

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-04-08 Thread gareth
/diff/ Testing --- Connected to manager, issued 'core show uptime' command and verified that there was a blank line between the headers and output. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http:/

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-03-24 Thread gareth
/manager.h 433198 /trunk/UPGRADE.txt 433198 Diff: https://reviewboard.asterisk.org/r/4391/diff/ Testing --- Connected to manager, issued 'core show uptime' command and verified that there was a blank line between the headers and output. Thanks, gareth --

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-03-24 Thread gareth
r character occurs in the middle of the line, eg: "Hello\rWorld\n", splitting that into two output headers does not seem like the expected behaviour. And by splitting on more than one character you could end up with a difference when reassembling the output: join('

Re: [asterisk-dev] [Code Review] 4515: build_peer peer mailbox management bug

2015-03-20 Thread gareth
table (rtcachefriends=yes) and the state of the peer is being checked via sip_devicestate [1]. So firstpass=false should only happen for realtime peers that had been semi-built (peer exists and peer->the_mark=false) as the result of a sip_devicestate call. [1] sip_unregister also calls wi

[asterisk-dev] [Code Review] 4515: build_peer peer mailbox management bug

2015-03-19 Thread gareth
that those mailboxes were no longer assigned to peer. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-03-19 Thread gareth
to manager, issued 'core show uptime' command and verified that there was a blank line between the headers and output. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.c

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-03-19 Thread gareth
> On Jan. 30, 2015, 1:22 a.m., George Joseph wrote: > > If you run the Testsuite, you'll get a good indication of whether this is > > truly backwards compatible. > > gareth wrote: > I ran the test apps/queue/set_penalty which makes use of ami.command and >

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-03-15 Thread gareth
> On Jan. 30, 2015, 1:22 a.m., George Joseph wrote: > > If you run the Testsuite, you'll get a good indication of whether this is > > truly backwards compatible. > > gareth wrote: > I ran the test apps/queue/set_penalty which makes use of ami.command and >

Re: [asterisk-dev] [Code Review] 4473: chan_pjsip: Add "rpid_immediate" option to prevent unnecessary "180 Ringing" messages.

2015-03-13 Thread gareth
hat the REDIRECTING information is supposed to be doing. > > gareth wrote: > I disagree, providing connected line updates pre-answer makes perfect > sense. Why would I want to know the connected-line name only after the call > has been answered? > > That assume

Re: [asterisk-dev] [Code Review] 4473: chan_pjsip: Add "rpid_immediate" option to prevent unnecessary "180 Ringing" messages.

2015-03-13 Thread gareth
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4473/#review14690 --- Ship it! Ship It! - gareth On March 13, 2015, 5:13 p.m

Re: [asterisk-dev] [Code Review] 4473: chan_pjsip: Add "rpid_immediate" option to prevent unnecessary "180 Ringing" messages.

2015-03-12 Thread gareth
immediately allows the phone to include the name in it's call history. If no call-forwarding and/or re-addressing has taken place why would REDIRECTING be used? - gareth --- This is an automatically generated e-mail. To reply, visit

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-01-29 Thread gareth
be a bug with starpy, it is treading the "\r\n" as the end of message. Changing it to output just "\n" results in a successful test. Breaking an existing client library isn't ideal, but the correct delimiter for command output is "--

[asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

2015-01-29 Thread gareth
r, issued 'core show uptime' command and verified that there was a blank line between the headers and output. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- ast

Re: [asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-12-22 Thread gareth
ing --- Code is originally written as part of ASTERISK-13145 which has undergone extensive testing. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list

Re: [asterisk-dev] [Code Review] 4023: Allow passing options and command to MixMonitor when recording in ConfBridge

2014-12-21 Thread gareth
/diff/ Testing --- Set record_options to m(${MAILBOX}) and verified that a recording was delivered to ${MAILBOX}. Set record_command to /bin/rm ^{MIXMONITOR_FILENAME} and checked that recording was deleted on ending the conference. Thanks, g

Re: [asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-12-03 Thread gareth
ment) were copied from device_state_cb(). The NULL check may be unnecessary now as modification to hintdevices is protected by context_merge_lock, so device->hint can't be set to NULL by hintdevice_destroy during a dialplan reload. I could remove the che

Re: [asterisk-dev] [Code Review] 4023: Allow passing options and command to MixMonitor when recording in ConfBridge

2014-12-03 Thread gareth
https://reviewboard.asterisk.org/r/4023/diff/ Testing --- Set record_options to m(${MAILBOX}) and verified that a recording was delivered to ${MAILBOX}. Set record_command to /bin/rm ^{MIXMONITOR_FILENAME} and checked that recording was deleted on ending the conference. Thanks, g

Re: [asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-12-02 Thread gareth
050/diff/ Testing --- Code is originally written as part of ASTERISK-13145 which has undergone extensive testing. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev

Re: [asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-12-02 Thread gareth
On Dec. 1, 2014, 11:46 p.m., gareth wrote: > > This could probably use a unit test, especially since you didn't add any > > support for this feature to any of the existing channel drivers in this > > patch. > > > > I would imagine a unit test to do the follo

Re: [asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-10-16 Thread gareth
d on that in device_state_cb(). The overall presence state is whichever provider is most-unavailable, eg: if SIP/alice is DND and CustomPresence:alice is CHAT then the presence state is DND. - gareth --- This is an automatically generated e-

Re: [asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-10-16 Thread gareth
Testing --- Code is originally written as part of ASTERISK-13145 which has undergone extensive testing. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing

[asterisk-dev] [Code Review] 4050: Add ability for Channel Drivers to provide Presence State information

2014-10-05 Thread gareth
unk/main/channel.c 424055 /trunk/include/asterisk/channel.h 424055 Diff: https://reviewboard.asterisk.org/r/4050/diff/ Testing --- Code is originally written as part of ASTERISK-13145 which has undergone extensive testing. Thanks, gar

Re: [asterisk-dev] [Code Review] 4023: Allow passing options and command to MixMonitor when recording in ConfBridge

2014-09-26 Thread gareth
iff: https://reviewboard.asterisk.org/r/4023/diff/ Testing --- Set record_options to m(${MAILBOX}) and verified that a recording was delivered to ${MAILBOX}. Set record_command to /bin/rm ^{MIXMONITOR_FILENAME} and checked that recording was deleted on ending the conference. Thank

Re: [asterisk-dev] [Code Review] 4023: Allow passing options and command to MixMonitor when recording in ConfBridge

2014-09-26 Thread gareth
en > > asterisk.conf has live_dangerously=no. Agreed, also current write access to record_file allows a user to overwrite any audio file Asterisk has write access to. eg: setting CONFBRIDGE(brigde,record_file) to ${ASTSPOOLDIR}/voicemail/default/${MA

Re: [asterisk-dev] [Code Review] 4023: Allow passing options and command to MixMonitor when recording in ConfBridge

2014-09-25 Thread gareth
ILBOX}) and verified that a recording was delivered to ${MAILBOX}. Set record_command to /bin/rm ^{MIXMONITOR_FILENAME} and checked that recording was deleted on ending the conference. Thanks, gareth -- _ -- Bandwidth and Coloc

[asterisk-dev] [Code Review] 4023: Allow passing options and command to MixMonitor when recording in ConfBridge

2014-09-25 Thread gareth
recording was deleted on ending the conference. Thanks, gareth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com