Code review comment for lp:~alfonsosanchezbeato/powerd/ofono-fixes

Revision history for this message
Ricardo Salveti (rsalveti) wrote :

> New commit takes state of the call into consideration for activating the
> proximity sensor. However, I have changed the logic a bit: previously, the
> sensor was activated when calls moved to either "alerting" or "active" states.
> Now I do it when the call reaches "dialing" or "active" state.
> The rationale is as follows. Taking into account that the usual sequence of
> states of the calls is (not all transitions are considered):
> Outgoing: dialing -> alerting -> active -> disconnected
> Incoming: incoming -> active -> disconnected
> then I think that it makes sense to activate the sensor when the state of an
> outgoing call is "dialing" instead of waiting for it to be "alerting", as most
> people once they press the call button they move their phone near to their ear
> immediately (while we are still dialing) so they can hear the other party
> ringing (move to alerting).
> So the change makes sure the proximity sensor is off only when the state is
> "incoming": the user is supposed to be looking at the screen at that moment to
> decide whether to accept the call or not. We can discuss this further if you
> thing there is some problem with this.

Makes sense, and it now behaves similarly as done in android.

Just a minor comment:
235 +ofono_get_modems_cb(GObject *source_object,
236 + GAsyncResult *res,
237 + gpointer user_data)
238 +{
239 + GDBusProxy *client = G_DBUS_PROXY(source_object);

Indentation here is different from the rest of the file, please change it to be similar to the rest.

Building and testing.

review: Needs Fixing

« Back to merge proposal