Hi,
> 2009/7/31 Denis Kenzior <[email protected]>:
> >> + if (mw->messages[i] != value) {
> >> + mw->messages[i] = value;
> >> +
> >> + if (!mw->pending)
> >> + mw->pending = g_timeout_add(0, mw_mwis_update,
> >> modem); +
> >> + dbus_gsm_signal_property_changed(conn, modem->path,
> >> + MESSAGE_WAITING_INTERFACE,
> >> + mw_messages_property_name[i],
> >> + DBUS_TYPE_BYTE, &value);
> >> + }
> >> +
> >> + return dbus_message_new_method_return(msg);
> >> +}
> >
> > Ok, I'm a bit confused here. Are you sure that MWIs should have the
> > ability to be cleared out by the user? The way I understood it, the user
> > dials the voice mail system and the provider sends another MWI message
> > which clears out the status.
>
> You're right, there's probably no point to provide this method. I
> haven't found the exact statement in the docs saying that the provider
> should send a new MWI but it's logical. Otherwise I thought the D-Bus
> client might be able to tell somehow that the mailbox should now have
> 1 message less and want to update us.
Ok, now you've taken out the method entirely. We still want to set the MBDN
:) So just add SetProperty that can set the MBDN.
>
> >> +
> >> +enum mwi_information_type {
> >> + MWI_UNSPECIFIED = -1,
> >> + MWI_MESSAGES_WAITING = -2,
> >> + MWI_NO_MESSAGES_WAITING = -3,
> >> +};
> >
> > Is there a reason these are negative?
>
> So the caller can give the exact number of messages (zero or positive)
> or partial information using one of the constants.
>
I'm still not sure what the point is, since you just set messages to 1 if
MWI_MESSAGES_WAITING and to 0 if MWI_NO_MESSAGES_WAITING. And MWI_UNSPECIFIED
is just simply ignored. Perhaps you want to break up the attributes into two:
FooMailWaiting -> True / False
FooMessages -> Number, with 0 in case we're not sure
Also, please move all the MWI parsing stuff to message-waiting.c since this
will allow us to extend the interface more readily. In particular I'd like to
eventually support Message ID, Message Calling Line Identity by exposing an
additional EnhancedMessageInfo interface.
Names are for illustration only at this point:
EnhancedMessages -> "/modem1/mwi/4344", "/modem1/mwi/2255"
EnhancedMessageInfo Properties:
RetentionDays
Priority
CallingParty
However, this one's lower priority.
> Yes. :-)
>
> I'm thinking it's better to try to implement what's in the text so
> when someone needs to know how ofono will react to something, you can
> point at the specification, barring any TODOs in the code.
>
Sure, it might be a good idea to quote the relevant parts of the spec.
> > Can we instead process the different sources from highest to lowest
> > instead and bail out early? No sense in calling message_waiting_notify
> > several times (and possibly emitting spurious signals) when only once is
> > required.
>
> We can but then we won't comply with that 23.040 9.2.3.24.2 above.
> There's also a tiny possibility someone might send us updates for the
> different mailboxes (out of the 5 types defined) one using DCS and
> another one in UDH. In the attached patch I left this as is and made
> sure PropertyChanged is only sent from the time callback (bottom half)
> which was my original intention. I can still change this.
I still don't understand why we can't do something like:
if sms_contains_enhanced_vm_iei(sms):
handle_enhanced_vm_info
return
if sms_contains_special_vm_iei(sms):
discard = extract_discard_from_iei_data
// Quote relevant part of the spec here
if mwi_dcs_decode(sms, ... ,dcsdiscard)
discard = discard || dcsdiscard
return
if mwi_dcs_decode:
handle simple DCS MWI
return
if "Return Call Message"
return
> >> +final:
> >> + /* Only bother the Message Waiting interface with textual
> >> + * notifications that are not to be stored together with other
> >> + * Short Messages in ME. The other messages will be delivered
> >> + * to the user through normal SMS store. */
> >
> > It sounds to me like we can completely ignore the text of such messages.
> > The network is basically saying: "the text isn't important, the contents
> > are."
>
> Probably, although with the Return Call type of notification there's
> no other information beside the text. I left it in the updated patch.
So I still say lets get rid of LastNotificationText entirely. Remember, most
likely scenario is that carriers will send something like:
.pid = Return Call
.dcs = mwi dcs
.uhdi = true
.udh = contains special iei, contains enhanced iei.
to cover all phase mobiles in some way.
The only case you're really covering here is if we get a pure Return Call
Message. This should not happen on any modern network, and we should not
give it to the MessageWaiting interface anyway, since there's nothing useful
it can do with it. Instead, we process it like any regular text SMS.
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
