On 28/01/13 10:24, Julien Puydt wrote:
Hi,
I just had a deeper look at what the code in this directory does (and
commit something which shouldn't change things much, we just use
Ekiga::ServiceCore only at startup).
In Opal::Sip::EndPoint::update_bank, I see that the endpoint is
connecting to Opal::Bank's signals to know what happens ; which means
it's "above" it from an abstract point of view. This is confirmed by
inspection of methods Opal::Sip::EndPoint::menu_builder_add_actions and
Opal::Sip::EndPoint::account_updated_or_removed, which ping the bank to
know what to do. The endpoints also asks things to Opal::SIP::Dialect!
On the other hand, Opal::Bank delegates everything to Opal::Account. And
what does Opal::Account do? Whenver it has something to do, it asks
Opal::Sip::EndPoint!
In short, the code in this directory is structurally bound to give
problems.
I suggest the following organization:
- Opal::Sip::EndPoint does everything low-level (register/unregister
accounts, call and presence management) ; when something happens, it
emits signals. It shouldn't ping the bank for anything, as it's not its
business!
- Opal::Account is connected on the endpoint, and gives orders
(registration, presence subscription/unsubscription), and watches the
signals to know when something happens (registration, mwi, presence) (it
is "above" abstractly).
- Opal::SIP::Dialect should also be "above" the endpoint : when it wants
to send a message, it asks the endpoint, and when a message comes, it
knows about it through a signal.
- Opal::Bank should become
Ekiga::PresentityDecorator+Ekiga::ContactDecorator, and when one of the
populate_menu methods gets called, loop on the account, and call their
own populate_menu methods, which will then call a method in the
endpoint, which will actually add actions (to each its own business!)
The big picture is that I would like to replace the Gordian knot that we
now have with a nice tree (ie: no dependency cycle) :
Opal::CallManager -----------\
\
Opal:: Bank -> Opal::Account -->- Opal::Sip::EndPoint
/
Opal::SIP::Dialect -----------/
The current memory handling is also unsatisfying (cf opal-main.cpp),
since we put the endpoints in boost shared pointers, but with a
null_deleter, which means we don't really manage them. This hits the
current ekiga master : as Damien noted (in a private email), the
endpoints get freed before the accounts, so the accounts call some
endpoint and we get a crash! Somewhere else in ekiga, there must be some
code which says to dispose of the endpoints ; that is wrong.
The good solution is to remove that other code, and put our endpoints in
the boost shared pointers with a deleter which will shut them down
gently, then call delete. This way, we'll be sure that the endpoints
will live as long as something in the ekiga needs them, and that they
are properly closed whenever they're not needed anymore. It's the code
which creates the objets, which should take care of disposing of the
objects!
I cannot say anything on all this, except: can we use more meaningful
words than bank and dialect? Is bank a registrar? For me, bank is for
money. What is a dialect (a version of a spoken language?)?
You propose PresentityDecorator and ContactDecorator? What do these
words mean? Decorator is for GUI things, is that the case?
I would also like to change roster (who in this list knows what it
means?) to local contacts or something like this. No software I know
uses the word "roster".
--
Eugen
_______________________________________________
ekiga-devel-list mailing list
ekiga-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/ekiga-devel-list
