Bug 13351

(In reply to comment #0)
> Tobias Hunger has some suggestions in
> http://monkey.collabora.co.uk/telepathy-spec-rob-roundup/.

That branch has long gone.
I guess a modern interface for simple call forwarding would, as a bare minimum, have a property for call forwarding handle, and a signal for changes of that handle.
A settable timeout would be nice to have, with a possibility that it is not supported (indicated by a flag).
A more advanced interface would have a list of (handle, timeout) pairs, with the semantic that the handles are tried out in the given sequence. An immutable property would define the maximum number of entries allowed.

This interface exists in Maemo 5's extensions:

http://git.collabora.co.uk/?p=rtcom-telepathy-glib.git;a=blob;f=rtcom-telepathy-glib/Connection_Interface_Forwarding.xml

However, I don't think it's actually implemented anywhere.

From some preliminary research, I infer that SIP's Forwarding model is much less elaborate than GSM's, so for generality we should probably base our API on GSM?

<sjoerd> mikhailz: hey, about call forwarding, is the timeout before trying another contact something that's in the protocol in sip or just a convenience thing ?
<mikhailz> sjoerd: the forwarding timeout is settable in Skype
<sjoerd> mikhailz: how does that work you give it a list of contacts and a timeout or ?
<mikhailz> sjoerd: it's even hairier: they have a list of contacts and configurable pause and timeout values for each contact.
<sjoerd> mikhailz: what's pause vs. timeout?
<mikhailz> sjoerd: I can't imagine the use for pause between calling attempts, so decided not to make it more complicated
<sjoerd> oh right
<sjoerd> so you say, try to forward to smcv for 10s, then pause for 2, try to forward to cassidy for 5 s then pause for 5 etc etc ?
<mikhailz> sjoerd: maybe it's part of a more generic call script language
<sjoerd> could be
<Robot101> lets embed lua...! :)
<sjoerd> it sounds overly complex indeed
*** Robot101 ducks and runs
<sjoerd> mikhailz: what happens if they all fail? the person forwarding gets the call back or it pokes you or ?
<sjoerd> i was imagining the forwarding api would leave the what to do if the forward fails up to the client
<mikhailz> sjoerd: I need to check about that. Probably the original callee will get a missed call notification
<sjoerd> hrm
<sjoerd> i was hoping that kind of stuff could be controlled by a client
<sjoerd> As in just tell the CM forward to A, if that fails you have a chance to say then forward to B etc
<sjoerd> without needing the complexity of giving the CM a queue with timeouts
<mikhailz> sjoerd: wrong, in Skype it's fire and forget
<sjoerd> ah, so once you say forward, you're no longer part of the calling process
<sjoerd> good to know ;)
<sjoerd> mikhailz: i guess it makes sense to do it that way, do you know how forwarding works in SIP ?
<mikhailz> sjoerd: You mean REFER? It's far messier affair, for sure
<sjoerd> mikhailz: i guess i mean REFER, yes
<mikhailz> Pekka the Great may know all the details, but AFAIK the referer gets an implicit event subscription (think wiring a whole new slab of sofia-sip functionality) to get notified about the forwarded call's progress
<sjoerd> sounds like lots of fun....
<mikhailz> Thinking of turning Empathy into an executive phone?
<mikhailz> empathy --secretary-kiosk-mode &
<sjoerd> Heh
<sjoerd> Well while working on the new media spec we should try to take these things into account as well
<sjoerd> it'd be cool if empathy could do that kind of stuff as well indeed
<mikhailz> I guess... but forwarding most likely won't mix media between the different call attempts
<sjoerd> mikhailz: how do you mean?
<mikhailz> sjoerd: so if you've forwarded an incoming call, the streams might close or be put on hold and there's only some signalling to report on the forwarded call's progress, if any
<sjoerd> ah right
<sjoerd> yeah, the one-shot vs. the referrer being in the process models are quite different
<mikhailz> sjoerd: if the CM supports bouncing the call back, the streams should get reestablished, probably with local pending send so the client could resume without losing privacy
<sjoerd> mikhailz: yeah
<sjoerd> you'd have to accept it again indeed
<sjoerd> which is a nice challenge
<mikhailz> sjoerd: A forwarded call's Group could remove the self handle for the duration of the forwarding, and add it back to local pending if the call gets back
<sjoerd> mikhailz: the new media spec doesn't have the Group interface
<mikhailz> the Call channel will not have a group at all?
<sjoerd> 1-to-1 won't indeed
<mikhailz> sjoerd: then it's perhaps about extending CallState
<sjoerd> yeah
<sjoerd> the tricky bit is which streams you move from pending to sending when the call comes back
<sjoerd> anyway, that's pondering for another day :)
<sjoerd> I now at least have a better idea of the situations that could arise
<mikhailz> sjoerd: OK, but do regard that other case with a call script and no reporting
<sjoerd> i will
<mikhailz> sjoerd: actually, these are different even on Telepathy level. The call script would belong to a Connection, and the refer action is for a Call
<mikhailz> so might as well be orthogonal
<sjoerd> mikhailz: surely you'd get the incoming call and then give it a forwarding call script ?
<mikhailz> sjoerd: not necessarily, the script can be sent to the server side to act while you are offline
<mikhailz> I'm dimly aware of some cellular network services for that
<sjoerd> mikhailz: right, that's different indeed
<sjoerd> mikhailz: but i assume you can tell some server/service, if there is a call try calling these guys instead
<mikhailz> again, grab pessi to get things I might know nothing about
<sjoerd> but also when there is an incoming call you tell it, please try forwarding this call to X, Y, Z
<mikhailz> sjoerd: yes, in some CMs you cannot manage it manually
<mikhailz> sjoerd: you just tell the server or network "please forward this call using the script I have set before"
<sjoerd> right

Additional ramifications for server-side call handling rules: they could be adorned with conditions, such as "use this script when the user is offline" or "use this script when an incoming call times out"; there could be also "use this script when the client tells you to forward the call" (should be supported by the call channel forwarding interface). A catch-all rule would be the only one supported by simple protocols.

The way GSM (and oFono's API exposes this) handles this is with rules for the following scenarios:

 - Unconditional
 - Busy
 - No Reply
 - Not Reachable (ie, phone turned off)

There's also a timeout attached to NoReply.  Multiple rules can be set at the same time, with some overriding others (ie, if NoReply is set to forward to 555-555-5555, and then Unconditional is set to forward to 333-333-3333, any calls will immediately be forwarded to 333-333-3333).

The following is supported by GSM and is for toggling multiple rules at once, but is not exposed by oFono:
 - All Call Forwarding
 - All Conditional Call Forwarding

Here's an initial spec based upon some of Mikhail's comments -

http://git.collabora.co.uk/?p=user/dilinger/telepathy-spec;a=shortlog;h=refs/heads/forwarding

I'm still not overly pleased with the way that multiple forwarding entries are handled.  I describe some of the inconsistencies involved w/ forwarding to multiple contacts in commit    e06fc6b166b67e12d4f7e154aa762cde935e74d3 .

(In reply to comment #6)
> Here's an initial spec based upon some of Mikhail's comments -
> 
> http://git.collabora.co.uk/?p=user/dilinger/telepathy-spec;a=shortlog;h=refs/heads/forwarding

Thanks. Some more comments:

MaxForwards can be confused with a header in SIP protocol, which has a different purpose. MaxForwardingContacts, perhaps?

Timeout in Forwarding_Rule_Entry: is it correctly documented as the timeout of the forwarding attempt itself prescribed by this entry, or did you mean it to be the timeout before the attempt is taken, as I meant it to? If it's the first, we need some other way to specify the initial timeout, and I think it's unnecessary.

Type Forwarding_Rule_Entry is not referenced in the places that actually use it.

ForwardingRulesChanged: either rename to ForwardingRuleChanged because it has data for one rule only, or make it able to emit multiple rules (maybe the entire set currently active?); the latter change will remove the atomicity issue which is mentioned in the documentation to SetForwardingRule.

(In reply to comment #7)
> (In reply to comment #6)
> > Here's an initial spec based upon some of Mikhail's comments -
> > 
> > http://git.collabora.co.uk/?p=user/dilinger/telepathy-spec;a=shortlog;h=refs/heads/forwarding
> 
> Thanks. Some more comments:
> 
> MaxForwards can be confused with a header in SIP protocol, which has a
> different purpose. MaxForwardingContacts, perhaps?

I went w/ MaxForwardingEntries.


> 
> Timeout in Forwarding_Rule_Entry: is it correctly documented as the timeout of
> the forwarding attempt itself prescribed by this entry, or did you mean it to
> be the timeout before the attempt is taken, as I meant it to? If it's the
> first, we need some other way to specify the initial timeout, and I think it's
> unnecessary.
> 

The wording is kind of confusing; here's an example.

  ForwardingRules = [
    Busy = [   [Handle:3, Timeout:30]   ],
    NoReply = [   [Handle:5, Timeout:30], [Handle:3, Timeout:20]   ],
    NotReachable = []
  ]

Assuming a CM which is limited to only 1 media stream at a time, is capable of chaining forwards, and can determine whether a remote handle has accepted a channel or not.

Scenario 1) An incoming channel is detected; the media stream is already in use.  The CM determines that the result should be Busy.  It looks at the forwarding rule; the CM ignores the timeout for Busy, so it immediately forwards the incoming channel to  Handle #3.

Scenario 2) An incoming channel is detected.  The media stream is available, and the local user is online.  While the CM is waiting for the local user to accept the channel, it looks at NoReply's first timeout value.  After 30s if the local user hasn't accepted, the CM forwards the channel to Handle #5.  The CM then waits 20s for Handle #5 to accept the channel.  If after 20s it does not, the CM forwards the incoming channel to Handle #3.



> Type Forwarding_Rule_Entry is not referenced in the places that actually use
> it.


Yeah, I'm planning to update all of the references once we're happy w/ the API.


> 
> ForwardingRulesChanged: either rename to ForwardingRuleChanged because it has
> data for one rule only, or make it able to emit multiple rules (maybe the
> entire set currently active?); the latter change will remove the atomicity
> issue which is mentioned in the documentation to SetForwardingRule.
> 

(In reply to comment #7)
> ForwardingRulesChanged: either rename to ForwardingRuleChanged because it has
> data for one rule only, or make it able to emit multiple rules (maybe the
> entire set currently active?); the latter change will remove the atomicity
> issue which is mentioned in the documentation to SetForwardingRule.
> 

BTW, the reason this only emits one rule change at a time is to make dependency changes explicit for clients.  A client changes a rule; if the CM proceeds to modify multiple rules due to that, we have two possibilities:

1) ForwardingRulesChanged is emitted (once) and supplies the new list of active rules.  A client might only check that the signal was emitted before updating the UI, or might only check that the rule it was looking for was changed.  In order to catch these kinds of changes, the client/UI would need to go through the entire list of rules and update the UI accordingly.  Failure to do this could result in a mismatch between the CM and the UI. 

2) ForwardingRuleChanged is emitted for each rule change.  A client should notice multiple signals and update the UI for each accordingly.  A UI might simply be watching for a signal, which should result in a bug as the UI sees multiple signals (a bug which is less subtle than "hm, the UI and what's actually happening when I get a phone call don't actually match up").

(In reply to comment #8)
> Scenario 1) An incoming channel is detected; the media stream is already in
> use.  The CM determines that the result should be Busy.  It looks at the
> forwarding rule; the CM ignores the timeout for Busy, so it immediately
> forwards the incoming channel to  Handle #3.
> 
> Scenario 2) An incoming channel is detected.  The media stream is available,
> and the local user is online.  While the CM is waiting for the local user to
> accept the channel, it looks at NoReply's first timeout value.  After 30s if
> the local user hasn't accepted, the CM forwards the channel to Handle #5.  The
> CM then waits 20s for Handle #5 to accept the channel.  If after 20s it does
> not, the CM forwards the incoming channel to Handle #3.

Thanks, this is how I intended it to be. It may also be the network/service that implements this logic (for Not_Available rule, it's the only way).

(In reply to comment #10)
> (In reply to comment #8)
> > Scenario 1) An incoming channel is detected; the media stream is already in
> > use.  The CM determines that the result should be Busy.  It looks at the
> > forwarding rule; the CM ignores the timeout for Busy, so it immediately
> > forwards the incoming channel to  Handle #3.
> > 
> > Scenario 2) An incoming channel is detected.  The media stream is available,
> > and the local user is online.  While the CM is waiting for the local user to
> > accept the channel, it looks at NoReply's first timeout value.  After 30s if
> > the local user hasn't accepted, the CM forwards the channel to Handle #5.  The
> > CM then waits 20s for Handle #5 to accept the channel.  If after 20s it does
> > not, the CM forwards the incoming channel to Handle #3.
> 
> Thanks, this is how I intended it to be. It may also be the network/service
> that implements this logic (for Not_Available rule, it's the only way).
> 

I added an example to the spec; hopefully that makes it clearer.

(In reply to comment #11)
> I added an example to the spec; hopefully that makes it clearer.

Thank you. Please use formatting-preservation tags like <code/> for such pseudocode examples.

(In reply to comment #12)
> (In reply to comment #11)
> > I added an example to the spec; hopefully that makes it clearer.
> 
> Thank you. Please use formatting-preservation tags like <code/> for such
> pseudocode examples.
> 

Done.

AFAICS, there are no open issues on this one and the Cabal is happy.
Can we proceed to merging it?

From http://lists.freedesktop.org/archives/telepathy/2010-April/004452.html:


Our consensus was that this interface is the right shape, but some types 
are not ideal, poorly named, etc. In particular:

The Forwarding_Rule enum isn't the type of a rule, it's the type of a 
condition to apply rules in. So it should be called Forwarding_Condition 
or similar.

The ForwardingRules property maps conditions to what to do in those 
situations, so it should be a dict not an array. So we get:

   ForwardingRules: a{ u — Forwarding_Condition
                  → a(uu) — Forwarding_Rule_Entry[]
                     }

There's ambiguity as to what the timeout in a forwarding rule entry 
means. According to the example, it means "the time to take before 
redirecting to this handle"; according to the struct definition, it 
means "the time to wait for this handle to answer before failing over to 
the next rule".

We could bike-shed which way round it should be... In the first case, we 
have no way to specify how long to wait for the handle in the last rule 
to answer before giving up; in the second case, we have no way to 
specify how long to wait for the local user to answer before trying the 
first rule. Perhaps we should take the second definition, and change the 
type of ForwardingRules further into:

   a{ u — Forwarding_Condition
    → ( u — initial timeout,
        a( u — handle,
           u — timeout for this action
         )
      )
    }

Then for example, the following value of ForwardingRules:

   { Busy: (30 seconds, [ (handle 3, 15 seconds)
                        , (handle 5, 20 seconds)
                        ]
           ),
     ...
   }

would mean: “if I'm on another call, keep the call waiting for 30 
seconds. If I don't answer the new call, redirect it to handle 3. If 
they don't answer within 15 seconds, redirect it to handle 5. Finally, 
if they don't answer within 20 seconds, give up.”

We should say what happens if your provider doesn't support call 
waiting, but you specify a Busy action with a non-zero time: the time is 
ignored.

In ‘a user's status is set to "Don't Bother Me"’, link to simple presence.

0 shouldn't mean default. 0 should mean "immediately", and MAX_UINT32 
should be the default. Or something (maybe make it signed so we can use 
-1? Maybe introduce maybe types to D-Bus? :þ).

SupportedForwardingRules should be renamed SupportedForwardingConditions.

How would we deal with Busy being handled locally, and NotReachable 
being on the server? Specifically: the server may only support one 
contact for NotReachable, but the local impl. can support >1. Maybe 
SupportedForwardingConditions should be an a{u: Condition → u: 
max_count} mapping supported conditions to how supported they are.

Side point: we need an error condition which says "this call ended 
because it was forwarded". (Do we want to say to whom?) Add an element 
to Call_State_Change_Reason.

How do we represent a call being forwarded immediately? It's easy with 
Call: we just announce a channel with the state already set to Ended. 
Call channels don't close themselves, so it'll be dispatched as normal. 
Hooray! Dancing in the streets, baked treats all round. Maybe we can 
clone this pattern onto the Group interface so it can deal with being 
booted immediately from a chat room.

(In reply to comment #15)
> The Forwarding_Rule enum isn't the type of a rule, it's the type of a 
> condition to apply rules in. So it should be called Forwarding_Condition 
> or similar.

Changed. This also means I've had to do a few changes in the wording of the spec, to use "condition" instead of "rule" - this distinction (a condition being one component in the pattern that can trigger a rule, the other being initial timeout) is somewhat blured by the fact that a condition can be used in at most one rule.

> The ForwardingRules property maps conditions to what to do in those 
> situations, so it should be a dict not an array.
> [...]
> first rule. Perhaps we should take the second definition, and change the 
> type of ForwardingRules further into:
> 
>    a{ u — Forwarding_Condition
>     → ( u — initial timeout,
>         a( u — handle,
>            u — timeout for this action
>          )
>       )
>     }
> 
> Then for example, the following value of ForwardingRules:
> 
>    { Busy: (30 seconds, [ (handle 3, 15 seconds)
>                         , (handle 5, 20 seconds)
>                         ]
>            ),
>      ...
>    }
> 
> would mean: “if I'm on another call, keep the call waiting for 30 
> seconds. If I don't answer the new call, redirect it to handle 3. If 
> they don't answer within 15 seconds, redirect it to handle 5. Finally, 
> if they don't answer within 20 seconds, give up.”

Agreed, that seems the best to me.

> We should say what happens if your provider doesn't support call 
> waiting, but you specify a Busy action with a non-zero time: the time is 
> ignored.

Added.

> In ‘a user's status is set to "Don't Bother Me"’, link to simple presence.

Added.

> 0 shouldn't mean default. 0 should mean "immediately", and MAX_UINT32 
> should be the default. Or something (maybe make it signed so we can use 
> -1? Maybe introduce maybe types to D-Bus? :þ).

Changed so that 0 means immediately and MAX_UINT32 is the default, with an option to change this once Maybe is added to D-Bus :)

> SupportedForwardingRules should be renamed SupportedForwardingConditions.
> 
> How would we deal with Busy being handled locally, and NotReachable 
> being on the server? Specifically: the server may only support one 
> contact for NotReachable, but the local impl. can support >1. Maybe 
> SupportedForwardingConditions should be an a{u: Condition → u: 
> max_count} mapping supported conditions to how supported they are.

Makes sense. This also removes the need for MaxForwardingEntries.

> Side point: we need an error condition which says "this call ended 
> because it was forwarded". (Do we want to say to whom?) Add an element 
> to Call_State_Change_Reason.

Added. Although the forwardee handle is not actually a subject in this case (it's more of an object), I think it makes sense to include their handle in Actor, if it's known.

> Hooray! Dancing in the streets, baked treats all round. Maybe we can 
> clone this pattern onto the Group interface so it can deal with being 
> booted immediately from a chat room.

Wouldn't such change be backwards-incompatible with the current Group behaviour? Anyways, we should probably have a new bug for discussing that so the idea doesn't get lost in this one.

Updated branch available at:
http://git.collabora.co.uk/?p=user/ptlo/tp-spec-senko/.git;a=shortlog;h=refs/heads/forwarding

I think this looks good to merge as a draft.

Editorial changes, none of which are merge blockers:

I'd like blank lines between <p> in future.

> +      <p>In other cases, the CM will handling the forwarding itself.  When an

"will handle the"

> +        forwarding entries in the list).</p> 

Trailing whitespace

> +            <tp:type>Connection_Presence_Type</tp:type>?).</p>

I'd omit the question mark here.

> +          The contact to forward an incoming channel to.  If the handle
> +          is invalid, the entry SHOULD be skipped.

Invalid handles are always considered to be programmer error (the client should be holding the handle), so I think instead of this wording, SetForwardingHandle should be allowed to raise InvalidHandle.

As an implementation detail, the Connection must hold a reference to each handle mentioned in the current forwarding details.

It would be OK to have similar wording saying that entries with a handle that is valid, but turns out not to point to anything (e.g. a phone number that doesn't exist), are skipped.

> +            behavior), the CM MUST raise multiple <tp:member-ref>ForwardingRuleChanged</tp:member-ref>

"MUST emit multiple". Exceptions are raised, but signals are emitted.

> +            been changed (ie, Unconditional automatically modifies

"e.g. if Unconditional..."

> +          <p>Each forwarding condition will occur exactly once or less in

"occur no more than once"

> +          <tp:docstring>
> +            A Handle that has been supplied is invalid.

This doesn't necessarily mean handle 0; it could mean handle 31337 where the CM has only allocated handles from 1 up to 666, for instance.

Most of my editorial changes were applied in 0.19.6. See smcv/trivia for a few more; review would be appreciated.

Thanks, editorial changes merged for 0.19.7. To get it undrafted, someone who's involved in implementing this will need to pick it up.

Nobody has implemented this since the draft was last touched in 2010, so I'm probably going to delete it. We can bring it back from git history if anyone cares at some point in the future, but carrying around unimplemented interfaces in the spec doesn't really help anyone.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-spec/issues/3.