Bug 24898

(In reply to comment #0)
> Maemo 5 uses various extended interfaces beyond what's in telepathy-spec,
> including those in:
> 
> http://git.collabora.co.uk/?p=telepathy-mission-control.git;a=tree;f=xml;hb=master
> 
> One of those is Account.Interface.Compat, which includes the
> "SecondaryVCardFields" property. We should work out why this property exists
> and decide whether it makes sense to store it.

I vaguely recall this was used for SIP and Skype, where the primary vCard field was X-SIP/X-SKYPE and the secondary was TEL (as Will mentioned in a recent mailing list post).

As implemented in MC, SecondaryVCardFields can be set, too. I believe this is so the accounts-ui can flag whether the address book should (offer to) use the SIP and Skype accounts for telephony, or not.

It's slightly awkward to edit this property to configure whether or not to use this account for TEL; but as mentioned on bug 26866 we might want to configure whether or not to use an XMPP account for SIP calls. (Gabble supports this.)

So maybe to make it easier to update:

  Account.UseForFields: as
  Account.SetUseFor(s: Field, b: yes or no)

would do? (Though in practice it would probably mostly be called with 'TEL'.)

So I think that, given this plus bug 26866 (which is merged as a draft), TelepathyQt4 could pretty easily have some sensible convenience API to show the library user a list which is basically the intersection of this property (the vCard fields we'd *like* to use this account for) and Protocol.I.Addressing.AddressableVCardFields.

The story so far (spec branch to follow)

Spec branch also prepared: http://git.collabora.co.uk/?p=user/vivek/telepathy-spec;a=shortlog;h=refs/heads/account-interface-addressing

keyword, not whiteboard.

Spec:

> +    <tp:added version="0.21.3">(draft 1)</tp:added>

0.21.UNRELEASED please; you're assuming that this'll be merged as a draft before the next spec release, which isn't necessarily true.

> +      <p>The Compat interface holds properties necessary for maintaining the
> +        libmissioncontrol compatible layer.</p>

This isn't the Compat interface, and "remain compatible with libmissioncontrol" isn't the reason for this functionality. Why did libmissioncontrol have it? You may be able to derive some useful wording from the Conn.I.Addressing interface.

Something like this, perhaps:

    Some accounts can be used for multiple protocols; for instance, SIP
    and Skype accounts can often be used to contact the PSTN, MSN and
    Yahoo accounts can contact each other, and XMPP accounts can potentially
    contact many protocols via a transport.

    However, if the user does not intend to make use of this functionality,
    user interfaces can improve clarity by not displaying it: for instance,
    if a user prefers to call phone numbers via a particular SIP account,
    when an address book displays a contact with a phone number, it is
    desirable to display a "call with SIP" button for that account, but
    avoid displaying similar buttons for any other configured SIP or
    Skype accounts.

I'd assumed that this interface would have parallel API for vCard fields and for URI schemes, like Conn.I.Addressing does, but I can see that that raises the possibility that they get out of sync: if my account has UseForFields = [] and UseForSchemes = ['tel'], a vCard-field-based UI would not offer it for telephony, but a URI-scheme-based UI would.

If you have any thoughts on whether vCard fields, URI schemes, or both make most sense, I'd like to hear them!

> +        <p>A list of fields indicating the type of URI addressing scheme
> +          the the account can be used for (eg 'tel') indicating the
> +          account is suitable for use by apps offering a telephony UI,
> +          or 'sip' or 'xmpp' for those protocols</p>

Please make it clearer that this is a matter of user preference rather than technical capability - every Skype account can potentially contact the PSTN, but it's up to the user whether they want to use it as such or not.

> +      <arg name="Old_URI_Association" direction="out" type="b">
> +        <tp:docstring><p>The old URI association value</p></tp:docstring>
> +      </arg>

We don't normally do this. Is there a special reason to do so?

> +      <arg name="Association" direction="in" type="b">
> +        <tp:docstring>
> +          <p>Whether to associate or disassociate with/from the URI scheme</p>
> +        </tp:docstring>
> +      </arg>

I prefer wording of the form "True if this account should be offered as a possible way to contact this URI scheme; False if it should not".

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

MC:

Your branch seems to be against telepathy-mission-control-5.6, which seems rather inappropriate for a new feature.

> +  <interface name="org.freedesktop.Telepathy.Account.Interface.Addressing">

You need a .DRAFT in there (and the draft used in a trial implementation should generally be byte-for-byte identical to your latest spec proposal). I haven't reviewed the implementation in detail.

(In reply to comment #8)
> If you have any thoughts on whether vCard fields, URI schemes, or both make
> most sense, I'd like to hear them!

Bear in mind that:

- clients (UIs) can be assumed to understand either specific vCard fields, or specific URI schemes, or both

- for Conn.I.Addressing, connection managers must understand specific vCard fields *and* specific URI schemes

- MC has no particular knowledge of either of those things

Argh, this does not appear to be the right version. Obviously not had enough coffee. Fixing...

> > + <tp:docstring><p>The old URI association value</p></tp:docstring>
> We don't normally do this. Is there a special reason to do so?

No particular reason, it just seemed like a useful bit of info to return:

   The state you asked for is already the case (and maybe you therefore 
   don't need to take some other action)

Can the MC implementation branch also bin the property and code from Account.Interface.Compat that it replaces?

I wasn't sure if that was how we rolled, but if it is, then I don't see why not.

+  if (schemes == NULL)
+    {
+      gchar *empty[] = { NULL };
+      schemes = empty;
+    }

Forgive my C ignorance: is dereferencing 'schemes' actually kosher once we leave the if {} block? Wouldn't it be more obvious to just not run the following loop...

+  for(scheme = schemes[pos++]; scheme != NULL; scheme = schemes[pos++])
+    {
+      if (!tp_strdiff (scheme, uri_scheme))
+        old_association = TRUE;
+    }

...which is secret code for old_association = tp_strv_contains (schemes, uri_scheme);. tp_strv_contains() does the right thing if you pass it a NULL strv, so the previous block becomes unnecessary.

And then actually I found the pointer arithmetic that followed very hard to read, so I rewrote the whole function. :) The result is shorter, and (IMHO) more readable. Top two patches of <http://git.collabora.co.uk/?p=user/wjt/telepathy-mission-control-wjt.git;a=shortlog;h=refs/heads/account-interface-addressing>.

+    uri_schemes = get_schemes (account_props)
+    assertContains ('scheme-a', uri_schemes)
+    assertContains ('scheme-b', uri_schemes)
+    assertContains ('scheme-c', uri_schemes)
+    assertEquals (len(uri_schemes), 3)

How about assertEquals(set(['scheme-a', 'scheme-b', 'scheme-c']), set(uri_schemes))? In fact Gabble's copy of servicetest.py has:

def assertSameSets(expected, value):
    exp_set = set(expected)
    val_set = set(value)

    if exp_set != val_set:
        raise AssertionError(
            "expected contents:\n%s\ngot:\n%s" % (
                pretty(exp_set), pretty(val_set)))

to make this easier. Maybe import this to MC's, just after assertEquals() in servicetest.py (to avoid divergence).

(I don't really like the Association naming, but I can't think of a better one off-hand.)

Let's use this bug for the spec and its clone for the MC implementation.

Spec review:

This looks good enough to be a draft if someone is working on pushing it forward. However, I'd like to move towards a policy of "stable-branch releases never contain draft API", so to have this in MC 5.8 it needs finishing and undrafting.

I'd always assumed that this interface would be in terms of either vCard fields as per Comment #3, or both vCard fields and URI schemes together as per Conn.I.Addressing. Is there a reason to prefer URI schemes over vCard fields? Is there a reason not to have both?

Undrafted in git for 0.21.5