[Home]

Summary:ASTERISK-04644: [patch] Manager action "AgentCallbackLogin" and "AgentLogoff" added
Reporter:Stefan Reuter (srt)Labels:
Date Opened:2005-07-21 11:46:24Date Closed:2008-01-15 15:45:49.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/ManagerInterface
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chan_agent-3.patch
( 1) chan_agent-4.patch
Description:This patch adds two new actions
- AgentCallbackLogin
- AgentLogoff
to channels/chan_agent.c

It allows Agent login/logoff via the Manager API.
Comments:By: Michael Jerris (mikej) 2005-07-21 11:50:10

Why not add cli commands instead of actions, so they can be used in both cli and manager?
By: Clod Patry (junky) 2005-07-21 11:53:10

Do you mind using ast_string_copy instead of strncpy ?

Thanks for contribution.
By: Stefan Reuter (srt) 2005-07-21 11:58:21

no problem I can change that, but I don't find that function.

find . -type f -exec grep 'ast_string_copy' \;

in asterisk's source tree returns no result :(
By: Michael Jerris (mikej) 2005-07-21 11:59:25

ast_copy_string, not ast_string_copy
By: Stefan Reuter (srt) 2005-07-21 12:03:04

using CLI functions instead via AMI is a hassle plus it doesn't allow returning qualified errors.
If you have a look at the patch I actually refactored the CLI "agent logoff" so there is one function that implements the feature and one small wrapper for CLI and one for AMI. I think that way is fine, so both worlds get what they deserve.
By: Stefan Reuter (srt) 2005-07-21 12:06:39

ok did a s/strncpy/ast_copy_string/ - it was used quiete a lot in chan_agent.c ;)


By: Michael Jerris (mikej) 2005-07-21 12:12:21

ast_copy_string is not a duplicate of strncpy, at least you don't need the -1 but look at the doxygen for details as there are other differnces.
By: Olle Johansson (oej) 2005-07-21 12:19:56

...you need to support the Actionid
By: Stefan Reuter (srt) 2005-07-21 12:26:16

read the docs and fixed the usage of ast_copy_string
By: Stefan Reuter (srt) 2005-07-21 12:28:49

where do I need the ActionID?
It gets automatically generated for the responses and the events are not response events but general events that are also triggered when using the Login/Logoff from the dialplan.
I think adding an ActionId there would lead to confusion as the event is of general interest and not only to the Login/Logoff action.
By: Olle Johansson (oej) 2005-07-21 12:53:06

Are you adding events or actions? Events does not have to support actionid, actions has. This sure looks like actions.
By: Stefan Reuter (srt) 2005-07-21 15:48:17

oej, i don't understand what you mean.
The actions support the ActionId:

Action: AgentCallbackLogin
ActionId: hello1234
Agent: 1001
Exten: 1310

Event: Agentcallbacklogin
Privilege: agent,all
Agent: 1001
Loginchan: 1310

Response: Success
ActionID: hello1234
Message: Agent logged in

--

Action: AgentLogoff
ActionId: hello2345
Agent: 1001

Event: Agentcallbacklogoff
Privilege: agent,all
Agent: 1001
Loginchan: 1310
Logintime: 12044

Response: Success
ActionID: hello2345
Message: Agent logged out


There is no ActionId in the Agentcallbacklogin- and Agentcallbacklogoff-Events. And why I didn't include them there was described above.
Please detail what I should change!


By: Olle Johansson (oej) 2005-07-23 06:20:00

Thanks for the example, I misunderstood the interface. Sorry. Everything seems to be ok.

Thanks for your contributions to Asterisk!
By: Stefan Reuter (srt) 2005-07-26 17:29:51

is there anything i can do to speed up things regarding this patch?
chan_agent-3.patch addresses the correct use of ast_copy_string pointed out by junky and MikeJ.

Would be really cool to get this patch included in 1.2...

thanks
By: Richard Hamnett (riksta) 2005-07-30 07:18:04

I'd also appreciate if you could ensure this bugfix makes it into Asterisk 1.2, as it adds important functionality to the manager api. Thanks
By: Richard Hamnett (riksta) 2005-08-09 09:54:25

Hi, since this patch was added before the freeze, please can we get it included in 1.2

Many thanks
Rick
By: Kevin P. Fleming (kpfleming) 2005-08-22 21:19:14

This will go into 1.2, if it can be cleaned up.

Review notes:

1) Don't change calls to strncat; they still need 1 subtracted from the buffer size.

2) Don't use ast_copy_string() to copy constant strings into buffers that you know are large enough (I know the code was already using strncpy(), but since you are changing it...)

3) Use ast_true() for parsing the 'soft' parameter, so you can support true/yes/y/1 etc. You can pass soft_s to it directly, it knows how to handle NULL and empty strings.

4) Don't put spaces between function names and their argument lists.

5) Don't use unnecessary indentation. In action_agent_callback_login(), use:

if (strcmp(p->agent, agent) || p->pending)
    continue;

instead of indenting the entire block following it. Same goes for 'if (!p->chan)'... see the coding guidelines for more clarification.
By: Stefan Reuter (srt) 2005-08-25 19:29:39

ok i updated the patch to include your suggestions
By: Kevin P. Fleming (kpfleming) 2005-08-26 16:22:48

Nice work! Committed to CVS HEAD.
By: Digium Subversion (svnbot) 2008-01-15 15:45:49.000-0600

Repository: asterisk
Revision: 6427

U   trunk/channels/chan_agent.c

------------------------------------------------------------------------
r6427 | kpfleming | 2008-01-15 15:45:49 -0600 (Tue, 15 Jan 2008) | 2 lines

add AgentCallbackLogin and AgentLogoff manager actions (issue ASTERISK-4644)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=6427