Hello Dan, Initially, I created a separate callback for this job, but in the end I was just replicating code, and therefore, I merged them. Maybe we can create two wrappers around the generic dialog callback: - one would be the existing run_dlg_callback (and keep the existing signature), - the other one would be run_mi_callback.
Once again Dan, thank you for your time. Also, I would like to hear other opinions with respect to this issue. Bogdan, Henning? Another thing that I would like to know, what would the users prefer: a separate mi command for listing the extra information from all the modules sitting on top of the dialog module, or by default list all the available data? I would prefer to list everything under the existing mi dlg_list command, but that's just me ... Regards, Ovidiu Sas On Fri, Apr 18, 2008 at 6:52 AM, Dan Pascu <[EMAIL PROTECTED]> wrote: > > Hi Ovidiu, > > In the end I went through the patches and together with the detailed > explanations I now understand what you try to do. Since this is only a > means for the dialog module to display information about modules sitting > on top of dialog for MI purposes I think it is fine the way it is done. > Please disregard my previous comments as they were made for a different > context that I now see it doesn't apply to this case. > > There is only one thing I'd do differently. I would not use a dialog > callback for this as they are meant to inform the modules on top of the > dialog module that some event has happened, as opposed to a means for the > dialog module to get feedback from the module on top of it. I would > rather have a module on top of the dialog module register a MI query > function into the dialog module if it wishes to provide information that > is to be displayed in a dialog context as a result of an MI request. This > is a minor difference, but I think it makes for a cleaner and less > confusing interface and the change is minimal. In this case it is obvious > that the MI query function provides a means for the dialog module to > obtain some information from the modules on top of it, while this intent > is not obvious if a standard dialog callback is used. > > I believe that keeping them separate is for the better because we do not > intermix operations which have different directions for the information > flows between the modules under the same umbrella (the callbacks move > information from the dialog module to the modules on top of it to inform > them of events, while this query function moves information from the > modules on top to the dialog module to provide MI feedback). > > > > On Friday 18 April 2008, Ovidiu Sas wrote: > > Forgot to mention, the callback is generic and it can be used by any > > module sitting on top of the dialog module. > > In the extra pointer from the callback structure I expect to find a > > pointer to a mi_node structure (which is not module specific). Each > > module is responsible of providing it's data in a well known format, > > in this particular case, an mi_node structure. > > > > As an example, I'm working on a module that will track the sdp for > > both caller and callee (using the openser integrated sdp parser). > > Here's the output of the dlg_list mi command with both sst and qos > > (the module that I'm working on) loaded (see both sst and qos nodes > > attached to the mi tree structure): > > > > dialog:: hash=364:1735939007 > > state:: 2 > > timestart:: 0 > > timeout:: 0 > > callid:: [EMAIL PROTECTED] > > from_uri:: sip:[EMAIL PROTECTED]:5050 > > from_tag:: 1 > > caller_contact:: sip:[EMAIL PROTECTED]:5050 > > caller_cseq:: 1 > > caller_route_set:: > > caller_bind_addr:: udp:10.11.10.63:5060 > > to_uri:: sip:[EMAIL PROTECTED]:5060 > > to_tag:: > > callee_contact:: > > callee_cseq:: > > callee_route_set:: > > callee_bind_addr:: > > sst:: requester_flags=4 supported_flags=0 interval=2400 > > qos:: negotiated_sdp > > sdp:: m_dir=1 m_id=1 method=INVITE cseq=1 > > negotiation=1 session:: callee cnt_disp= streams=1 > > stream:: 0 media=audio port=18074 > > transport=RTP/AVP payloads_num=2 > > payload:: 100 codec=X-NSE > > sendrecv= ptime= > > payload:: 0 codec=PCMU > > sendrecv= ptime= session:: caller cnt_disp=session streams=1 stream:: 0 > > media=audio port=6002 transport=RTP/AVP payloads_num=3 > > payload:: 18 codec= sendrecv= > > ptime= payload:: 1 codec= sendrecv= ptime= payload:: 0 codec=PCMU > > sendrecv=inactive ptime= > > > > > > > > Regards, > > Ovidiu Sas > > > > On Thu, Apr 17, 2008 at 6:23 PM, Ovidiu Sas <[EMAIL PROTECTED]> > wrote: > > > Hello Dan, > > > > > > The problem that I'm trying to solve was described at the beginning > > > of this thread, maybe with not enough clarity (please follow also > > > http://sourceforge.net/tracker/index.php?func=detail&aid=1933630&grou > > >p_id=139143&atid=743022). > > > > > > I will use as an example the sst module. The sst module has a call > > > specific context: > > > > > > typedef struct sst_info_st { > > > enum sst_flags requester; > > > enum sst_flags supported; > > > unsigned int interval; > > > } sst_info_t; > > > > > > The pointer to the sst call specific context is saved inside the > > > dialog callbacks. > > > I would like to be able to interrogate - via the mi interface - the > > > content of the sst call specific context, but the only way to do it > > > is via the dialog module. Therefore, I need a callback from the > > > dialog module into the sst module in order to be able to retrieve the > > > sst call specific context. > > > > > > As an example, here's the output of the dlg_list mi command, with > > > the content of the sst call specific context (see the last line: > > > "sst:: requester_flags=4 supported_flags=0 interval=2400"): > > > dialog:: hash=898:913256572 > > > state:: 2 > > > timestart:: 0 > > > timeout:: 0 > > > callid:: [EMAIL PROTECTED] > > > from_uri:: sip:[EMAIL PROTECTED]:5050 > > > from_tag:: 1 > > > caller_contact:: sip:[EMAIL PROTECTED]:5050 > > > caller_cseq:: 1 > > > caller_route_set:: > > > caller_bind_addr:: udp:10.11.10.63:5060 > > > to_uri:: sip:[EMAIL PROTECTED]:5060 > > > to_tag:: > > > callee_contact:: > > > callee_cseq:: > > > callee_route_set:: > > > callee_bind_addr:: > > > sst:: requester_flags=4 supported_flags=0 interval=2400 > > > > > > > > > Hope this answer your question. > > > > > > > > > Regards, > > > Ovidiu Sas > > > > > > On Thu, Apr 17, 2008 at 5:31 PM, Dan Pascu <[EMAIL PROTECTED]> > wrote: > > > > On Thursday 17 April 2008, Ovidiu Sas wrote: > > > > > Well ... define generic. > > > > > > > > > > This particular callback is meant to be used via the MI > > > > > interface. You can define other type of callbacks and reuse the > > > > > extra pointer that was defined (see void **x_param inside > > > > > struct dlg_cb_params). > > > > > > > > That's exactly my point. I was asking the question with a > > > > purpose, that is to highlight that the proposed change would > > > > define a dialog callback that can only be used by one module and > > > > in a very specific context, callback that is on the other hand > > > > made public. What is the use of a callback that is advertised as > > > > public, but can only be used by one module for a very specific > > > > task? > > > > > > > > I mean, can't that module export a public API that the dialog > > > > module can use to get the information it needs? That would be more > > > > clean, yet it would still be an awkward reverse dependency. > > > > > > > > IMO, I still find such a dependency to be very problematic to be > > > > forged into any kind of generic interface. The direct dependency > > > > is fine. Any module built on top of the dialog module can use the > > > > dialog's public API to access the dialog state/events. But when > > > > you have it the other way around, the dialog module cannot > > > > possibly know what modules will be built on top of it to be able > > > > to generically extract information from them. This kind of > > > > dependency is not only awkward, but it is very narrow in its > > > > scope, which hardly justifies it being put in a public API that > > > > none uses, except that module. > > > > > > > > I'm sure that if we would know in more detail what problem you > > > > are trying to solve (as opposed to how you try to solve it), many > > > > people here could offer good suggestions that may turn into a > > > > better design. > > > > > > > > -- > > > > Dan > > > > -- > Dan > _______________________________________________ Devel mailing list Devel@lists.openser.org http://lists.openser.org/cgi-bin/mailman/listinfo/devel
