Add a filtering API to VoiceCAllHandler

[dcaliste]

Following @abranson comment from #14 , I'm proposing an API extension allowing to filter out incoming calls. This API is adding a new method to AbstractVoiceCallHandler interface. Besides the existing answer() for an incoming call, I propose to add filter(action). Calling the later, allow the providers (telepathy or oFono) to deal with the call (by hanging up or letting it ring) and change its status. It is thus adding two new status: REJECTED and IGNORED. Introducing these two new status allow not to change much from voicecall-ui. Indeed, in case of filtering, an incoming status will be changed for rejected or ignored, so the ui won't show up and it will not call playRingTone(), which is called only for the incoming status.

This is a clean solution that avoid to call playRingTone() and immediately after to call silenceRingTone() or stop it because the call has been ignored or hang up. It still has the following drawback compared to the initial #14 proposition:

• the ending call dialog is still shown. But this can be now tuned in the UI easily since we have the new status that tells that it's not an incoming call. Some slight adjustements should be done in voicecall-ui not to show the ending call dialog for these new status.
• the calls are still seen as missed call or plain call in the call history. Indeed, commhistory-daemon is listening via Telepathy, which cannot flag calls (as far as I know). I'm wondering if it could be a good idea to remove the telepathy code from commhistory-daemon and use voicecallmanager instead. It would simplify the code in my opinion and avoid some duplicates between commhistory-daemon and voicecallmanager.

Regarding the API itself, it is proposing the same filtering capabilities as the ones exposed by oFono and the ones from Android call screening response API (as pointed out by @abranson). Compared to Android, I didn't add the possibility to mess up with the call log and notifications. I think that the first is not a good idea (I don't want to have calls filtered out without having the possibility to supervise what happened), and I think that the second belongs to voicecall-ui, based on the new status. About the name, I remember that @pvuorela pointed out that filter might not be a good name, since one can confuse the action with filtering / sorting results. Android is using "screening" as a term. What do you think ? Should the new method be called "screenIncomingCall()" or something like that ?

I'm keeping the second commit implementing a filtering plugin based on this API, to show how it can be used. Depending on the discussion, I will close #14 if this PR is found to be a better approach.

[attah]

Not that you should care very much about what i think: but i definitelyprefer filtering over screening. The latter, as a synonym "filtering", is usually not well understood for non-native speakers. Screening tends to be understood as a synonym to investigate, survey etc. How about something based around "blocking"?

[abranson]

This looks good. The distinction between rejecting or just muting calls is important. I wonder if muting a call and letting it go to voicemail counts as a positive connection and encourages spammers, but I don't think we have a way to block that for the current call. I also wonder if that's still a thing, or they're just spamming hundreds of numbers regardless.

I sent a message to the Phonehook developer about this. They've been dealing with this for years and if anyone would know exactly what this API needs, it's them.

[dcaliste]

Thank you @attah and @abranson for your comment. About the naming, I'm afraid "blocking" is too restrictive as a name, since one can also simply ignore the call.

Thank you @abranson for contacting Phonehook developers. I several times looked at their code to see how they are doing things. I cannot say that I read it all though : / They are using an SQlite backend for filtering storage, they can filter on contact or on number, as far as I understand. I'm wondering if the usage of an SQlite storage is required, if it ends up with so many numbers that going through all of them becomes a bootleneck. The disavantage I see to it is that, there is no automatic signaling when the database is changed. So a frontend adding a filtered number for instance as no simple mechanism to notify a voicecallmanager plugin of the change. This is not an issue in Phonehook case, but it could be one if we separate UI and filtering. That's why I chose to use a DConf entry. But I'm wondering if it can become too slow... The blocking in Phonehook is then done via DBus, calling hangup on the voicecallmanager object over the bus. That's why, in this PR, I've also added the "filter(action)" API to DBus (and not only internally for sync plugin usage). So like that, Phonehook can be updated to use this new API, if they desire so. Last thing about Phonehook, I'm still wondering why they are patching Lipstick, but that's because I've not read it carefully enough yet !

[pvuorela]

Going through again the existing code in this voicecall projects there are details that feel suspicious, like registered handlers reported by voiceCallAdded. Then the actions done on the handler, not a phone call, which itself doesn't have any specific handle to use for referring.

The changes here slightly continuing on that, filtering the handler instead of the call, though granted that anything better would need further changes. Then again by such I'm not sure how much the handler should be needing information on filtered calls instead of just being asked to hang up etc. At the moment there doesn't seem to be much other actions.

Another thing I'm pondering is whether this is racy. Now the order of d-bus signalling for the incoming call to voicecall-ui can first get the incoming call notification and slightly after that message for "nevermind, we ignore this"(?)

With these, still thinking would it be better or worse handling this in the ui app level.

the calls are still seen as missed call or plain call in the call history. Indeed, commhistory-daemon is listening via Telepathy, which cannot flag calls (as far as I know). I'm wondering if it could be a good idea to remove the telepathy code from commhistory-daemon and use voicecallmanager instead. It would simplify the code in my opinion and avoid some duplicates between commhistory-daemon and voicecallmanager.

That seems indeed problematic. There might be some chance to adjust the status of the commhistory items but generally would indeed sound better if there was some middle layer feeding the call items and commhistoryd not listening them itself.

[pvuorela]

Intentionally added to d-bus interface but missing from the declarative side that does the d-bus calls?

[dcaliste]

Yes, the addition to the voicecall-manager daemon D-Bus API is to be able to use the filtering capability from an external program like phone-hook. Currently, I created two ways to filter a call:

• via a plugin, then D-Bus is not involved,
• via a D-Bus call on "/calls/xxx" object path of the voicecall-manager daemon.

If one wants to do it via QML, like in the voice-call-ui for instance, then, the code will have to be added in the declarative plugin, you're right.

[pvuorela]

Repeats on earlier, but these should be all "it's", right?

[dcaliste]

Fixed.

[dcaliste]

I apologise for taking such a long time to answer your review. As you've seen, I've tested various things with libcommhistory and here at voicecall, making related but independant commits on DBus related parts or call handler workflow. Now, I'm coming back to this PR.

Going through again the existing code in this voicecall projects there are details that feel suspicious, like registered handlers reported by voiceCallAdded. Then the actions done on the handler, not a phone call, which itself doesn't have any specific handle to use for referring.

I'm sorry, I don't get what you mean here :/ The implementations of the voicecallhandler interface (in ofono or telepathy providers) are phone calls and have ways to refer to the actual provider call (either via telepathy or directly via QOfono).

Then again by such I'm not sure how much the handler should be needing information on filtered calls instead of just being asked to hang up etc. At the moment there doesn't seem to be much other actions.

The call handler have several actions like hangup(), hold(), answer(), split() or merge(). I'm introducing a new one filter() that takes a parameter with three possible actions : ring (normal case), reject (= hangup) or ignore (= let the call continue but don't advertise it to the device user). Reject must be different from hangup() since one will later need to log the call differently. I agree this is not visible in PR at the moment because I'm still wondering how to convey this information to the comm history. But at least, it's changing the status of the call handler.

Another thing I'm pondering is whether this is racy.

That's correct. I've reworked the PR to avoid it. To do so, I've updated the providers not to change the status to STATUS_INCOMING immediately for a newly created voicecallhandler. It relies on calling filter()on it later to set the status to INCOMING, REJECTED or IGNORED. I've added a setCallFiltering() method in the manager also. When not set, the manager is always calling filter(CONTINUE) on any new call handler, keeping the old behaviour. But when setCallFilter() is set, the call is added to the manager and advertised over D-Bus and so on, but it stays in the NULL state as long as no one called filter() on it.

It's actually a similar behaviour than the one used by Slava in its filtering implementation in oFono: a call is not in an active state is there is a possible filtering, waiting for the filtering decision.