GNOME Bugzilla – Bug 601697
SIP account widget is incomplete
Last modified: 2011-08-29 10:12:28 UTC
The SIP account widget used to have a lot of (advanced) options but most of them have been removed because no one was understanding them (nobody has been able to explain some fields to translatiors asking for info..). As a result, the SIP widget is now very simple but it's now impossible to make Empathy work with some SIP servers. The params currently configurable in the UI are: param-account=s required register param-password=s [advanced] param-discover-stun=b (default: true) param-stun-server=s param-stun-port=q (default: 3478) The remaining params available in tp-sofiasip 0.5.18 are: param-auth-user=s param-alias=s param-registrar=s param-proxy-host=s param-port=q (default: 5060) param-transport=s (default: auto) param-loose-routing=b (default: false) param-discover-binding=b (default: true) param-keepalive-mechanism=s param-keepalive-interval=u param-local-ip-address=s param-local-port=q param-extra-auth-user=s param-extra-auth-password=s For each of those we should decide if it makes sense to display it in the UI. And if it does we should have a small description of it (that will be displayed in a tooltip).
According to bug #601666, param-registrar should be displayed. *** This bug has been marked as a duplicate of bug 601666 ***
*** Bug 601666 has been marked as a duplicate of this bug. ***
Some explanation for parameters: (In reply to comment #0) > param-auth-user=s This is the username for SIP authentication, if different from the SIP URI username (as set by the account parameter). > param-alias=s The display name for the From: header, also available through the Aliasing interface. > param-registrar=s URI of the registrar proxy. Never needed, I'm not sure if it works at all. > param-proxy-host=s Host name of the proxy for outbound requests. > param-port=q (default: 5060) Port of the proxy for outbound requests. > param-transport=s (default: auto) Transport protocol for SIP messages. Can be "udp", "tcp", or "tls". "auto" means whatever the DNS records suggest, defaulting to UDP. > param-loose-routing=b (default: false) Use the loose routing behavior and the Route header as recommended in RFC 3261. Should be enabled, but some proxies have problems with it. > param-discover-binding=b (default: true) Update the registration binding if the external address for the client is discovered to be different from the local binding. > param-keepalive-mechanism=s Mechanism of keep-alive maintenance for the SIP proxy connection/flow. "auto" - implementation-selected "register" - use REGISTER requests "options" - use OPTIONS requests "none" - disable keep-alive > param-keepalive-interval=u Interval for keep-alive pings, in seconds. > param-local-ip-address=s > param-local-port=q These two parameters provide a way to bind the SIP sockets to a specific local address and/or port. > param-extra-auth-user=s > param-extra-auth-password=s A pair of extra credentials to use when an outbound request is challenged by a proxy other than the one that authenticated client registration. This is a lame workaround, it will not be needed when we have proper challenge-response authentication in Telepathy.
Nothing to add to Mikhail Zabaluev comment; If you feel that UI is too complicated with all options visible, you could "hide" them under Advanced. One thing for sure, those should be available somewhere as at the moment I can not connect to number of SIP registrars because I can not set param-port to a different value from 5060.
I added the following params in the advanced section: - auth-user - proxy-server - port We should use the alias/avatar UI to set the 'alias' param. I ignored param-registrar as you suggested. param-transport, param-loose-routing, param-discover-binding, param-keepalive-*: they all seem very technical. Do you think we should expose them in Empathy? param-local-ip-address and param-local-port: Do them make sense for a desktop use? param-extra-auth-*: how often are needed these params? I'm wondering if we should include them or wait for the proper way to do challenge/response auth.
(In reply to comment #5) > param-transport, param-loose-routing, param-discover-binding, > param-keepalive-*: they all seem very technical. Do you think we should expose > them in Empathy? Yes, unfortunately, each of them can make or break interoperability with a SIP service and/or local network setup. > param-local-ip-address and param-local-port: Do them make sense for a desktop > use? Probably not, at least not in the UI. > param-extra-auth-*: how often are needed these params? I'm wondering if we > should include them or wait for the proper way to do challenge/response auth. They are of dubious utility; better wait for proper auth, and connect it to gnome-keyring or something like that.
I added support for "transport", "loose-routine", "discover-binding" and "keepalive-*". Shouldn't param-keepalive-mechanism have "auto" as default? There is no default atm so my UI can't display anything by default. Also, shoudn't param-keepalive-interval have a default as well?
(In reply to comment #7) > Shouldn't param-keepalive-mechanism have "auto" as default? There is no default > atm so my UI can't display anything by default. > Also, shoudn't param-keepalive-interval have a default as well? Good points. I have added defaults on a branch: http://git.collabora.co.uk/?p=user/zabaluev/telepathy-sofiasip.git;a=shortlog;h=refs/heads/param-defaults
Created attachment 147962 [details] [review] http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/sip-widget-601697 libempathy-gtk/empathy-account-widget-private.h | 4 + libempathy-gtk/empathy-account-widget-sip.c | 120 +++++++++++++++- libempathy-gtk/empathy-account-widget-sip.ui | 173 ++++++++++++++++++++++- libempathy-gtk/empathy-account-widget.c | 74 ++++++++++ 4 files changed, 367 insertions(+), 4 deletions(-)
I looks like http://people.collabora.co.uk/~cassidy/sip.png. What do you think about having subsections in the advanced section? We could have "STUN", "Proxy" and "Keep-Alive". Though I'm not sure which subsection to use for the remaining params...
(In reply to comment #10) > I looks like http://people.collabora.co.uk/~cassidy/sip.png. > > What do you think about having subsections in the advanced section? We could > have "STUN", "Proxy" and "Keep-Alive". This breakdown sounds good to me. Alternatively, you could have "NAT traversal" which would join STUN settings and "discover-binding". > Though I'm not sure which subsection to > use for the remaining params... "Misc. SIP arcana"? :)
It now looks like http://people.collabora.co.uk/~cassidy/sip.png Suggestion for a better name than "Misc" are welcome.
Created attachment 147980 [details] [review] http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/sip-widget-601697 libempathy-gtk/empathy-account-widget-private.h | 4 + libempathy-gtk/empathy-account-widget-sip.c | 120 +++++++++++- libempathy-gtk/empathy-account-widget-sip.ui | 248 +++++++++++++++++++++- libempathy-gtk/empathy-account-widget.c | 74 +++++++ 4 files changed, 431 insertions(+), 15 deletions(-)
(In reply to comment #12) > It now looks like http://people.collabora.co.uk/~cassidy/sip.png > > Suggestion for a better name than "Misc" are welcome. Nitpick: I don't think "NAT" is a proper noun, it's an acronym.
Good point; I changed to "NAT".
(In reply to comment #12) > It now looks like http://people.collabora.co.uk/~cassidy/sip.png > > Suggestion for a better name than "Misc" are welcome. How about "Additional Options"?
(In reply to comment #6) > (In reply to comment #5) > > param-local-ip-address and param-local-port: Do them make sense for a desktop > > use? > > Probably not, at least not in the UI. > I do think that those should be exposed as well. In many cases, those parameters could be useful when NAT traversal does not work.
Created attachment 148054 [details] [review] updated UI http://people.collabora.co.uk/~cassidy/sip.png libempathy-gtk/empathy-account-widget-private.h | 4 + libempathy-gtk/empathy-account-widget-sip.c | 120 ++++++++- libempathy-gtk/empathy-account-widget-sip.ui | 339 +++++++++++++++++++++-- libempathy-gtk/empathy-account-widget.c | 74 +++++ 4 files changed, 509 insertions(+), 28 deletions(-)
Review of attachment 148054 [details] [review]: It looks fine overall, I inlined some comments. ::: libempathy-gtk/empathy-account-widget-private.h @@ +46,3 @@ + GtkWidget *widget, + const gchar *param_name); + Could you make the indentation of the declarations in this file uniform? The other one does not seem to follow the tp style (probably my fault). ::: libempathy-gtk/empathy-account-widget-sip.c @@ +222,3 @@ + gtk_list_store_append (store, &iter); + gtk_list_store_set (store, &iter, 0, "options", 1, "Options", -1); + Why these strings (Options and Register) are not marked for translation? ::: libempathy-gtk/empathy-account-widget.c @@ +339,1 @@ account_widget_setup_widget (EmpathyAccountWidget *self, If you're going to export this in empathy-account-widget-private.h, please rename it to empathy_account_widget_setup_widget()
(In reply to comment #19) > Review of attachment 148054 [details] [review]: > > It looks fine overall, I inlined some comments. > > ::: libempathy-gtk/empathy-account-widget-private.h > @@ +46,3 @@ > + GtkWidget *widget, > + const gchar *param_name); > + > > Could you make the indentation of the declarations in this file uniform? The > other one does not seem to follow the tp style (probably my fault). done. > ::: libempathy-gtk/empathy-account-widget-sip.c > @@ +222,3 @@ > + gtk_list_store_append (store, &iter); > + gtk_list_store_set (store, &iter, 0, "options", 1, "Options", -1); > + > > Why these strings (Options and Register) are not marked for translation? They are related to SIP's internal and though that translating them would just make things more confusing to users. > ::: libempathy-gtk/empathy-account-widget.c > @@ +339,1 @@ > account_widget_setup_widget (EmpathyAccountWidget *self, > > If you're going to export this in empathy-account-widget-private.h, please > rename it to empathy_account_widget_setup_widget() Good point; done.
(In reply to comment #20) > They are related to SIP's internal and though that translating them would just > make things more confusing to users. Well, if they're exposed in the UI, they should be marked for translation anyway, no? Maybe together with a comment for translators, explaining how the strings should be taken care of. The rest of the branch looks good to me.
(In reply to comment #21) > (In reply to comment #20) > > > They are related to SIP's internal and though that translating them would just > > make things more confusing to users. > > Well, if they're exposed in the UI, they should be marked for translation > anyway, no? Maybe together with a comment for translators, explaining how the > strings should be taken care of. Good idea. I've done that.
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
(In reply to comment #3) > > param-registrar=s > > URI of the registrar proxy. Never needed, I'm not sure if it works at all. It seems to work actually. I wasn't able to send SMS because the registrar wasn't configured ( http://bugs.freedesktop.org/show_bug.cgi?id=25504 ). Shouldn't we add it to the UI then?
(In reply to comment #24) > It seems to work actually. I wasn't able to send SMS because the registrar > wasn't configured ( http://bugs.freedesktop.org/show_bug.cgi?id=25504 ). Can you confirm that setting the proxy-host parameter does not help in this case?