Summary: | ASTERISK-04644: [patch] Manager action "AgentCallbackLogin" and "AgentLogoff" added | ||
Reporter: | Stefan Reuter (srt) | Labels: | |
Date Opened: | 2005-07-21 11:46:24 | Date Closed: | 2008-01-15 15:45:49.000-0600 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | 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 |