From 3960ced4638fdb24ddf904fcb6734dae0959671e Mon Sep 17 00:00:00 2001 From: jsh Date: Tue, 23 Jun 2009 09:50:44 -0700 Subject: Fix SMS Ack. There were a few places where we were not sending SMS ack. This change fixes that, and helps ensure that we don't miss any others. Also fix a bug I introduced in RIL.java for CDMA SMS ACK (it uses RIL_CDMA_SMS_Ack and not int[]). --- .../java/com/android/internal/telephony/RIL.java | 1 - .../android/internal/telephony/SMSDispatcher.java | 27 ++++++++---- .../android/internal/telephony/WapPushOverSms.java | 18 +++++--- .../internal/telephony/cdma/CdmaSMSDispatcher.java | 48 +++++++++++----------- .../internal/telephony/gsm/GsmSMSDispatcher.java | 30 ++++++-------- 5 files changed, 69 insertions(+), 55 deletions(-) diff --git a/telephony/java/com/android/internal/telephony/RIL.java b/telephony/java/com/android/internal/telephony/RIL.java index 9f780c9..4db3e5b 100644 --- a/telephony/java/com/android/internal/telephony/RIL.java +++ b/telephony/java/com/android/internal/telephony/RIL.java @@ -1365,7 +1365,6 @@ public final class RIL extends BaseCommands implements CommandsInterface { RILRequest rr = RILRequest.obtain(RIL_REQUEST_CDMA_SMS_ACKNOWLEDGE, result); - rr.mp.writeInt(2); rr.mp.writeInt(success ? 0 : 1); //RIL_CDMA_SMS_ErrorClass // cause code according to X.S004-550E rr.mp.writeInt(cause); diff --git a/telephony/java/com/android/internal/telephony/SMSDispatcher.java b/telephony/java/com/android/internal/telephony/SMSDispatcher.java index a3016fa..62a5d65 100644 --- a/telephony/java/com/android/internal/telephony/SMSDispatcher.java +++ b/telephony/java/com/android/internal/telephony/SMSDispatcher.java @@ -289,7 +289,13 @@ public abstract class SMSDispatcher extends Handler { sms = (SmsMessage) ar.result; try { if (mStorageAvailable) { - dispatchMessage(sms.mWrappedSmsMessage); + int result = dispatchMessage(sms.mWrappedSmsMessage); + if (result != Activity.RESULT_OK) { + // RESULT_OK means that message was broadcast for app(s) to handle. + // Any other result, we should ack here. + boolean handled = (result == Intents.RESULT_SMS_HANDLED); + acknowledgeLastIncomingSms(handled, result, null); + } } else { acknowledgeLastIncomingSms(false, Intents.RESULT_SMS_OUT_OF_MEMORY, null); } @@ -469,8 +475,11 @@ public abstract class SMSDispatcher extends Handler { * Dispatches an incoming SMS messages. * * @param sms the incoming message from the phone + * @return a result code from {@link Telephony.Sms.Intents}, or + * {@link Activity#RESULT_OK} if the message has been broadcast + * to applications */ - protected abstract void dispatchMessage(SmsMessageBase sms); + protected abstract int dispatchMessage(SmsMessageBase sms); /** @@ -478,8 +487,11 @@ public abstract class SMSDispatcher extends Handler { * the part is stored for later processing. * * NOTE: concatRef (naturally) needs to be non-null, but portAddrs can be null. + * @return a result code from {@link Telephony.Sms.Intents}, or + * {@link Activity#RESULT_OK} if the message has been broadcast + * to applications */ - protected void processMessagePart(SmsMessageBase sms, + protected int processMessagePart(SmsMessageBase sms, SmsHeader.ConcatRef concatRef, SmsHeader.PortAddrs portAddrs) { // Lookup all other related parts @@ -506,8 +518,7 @@ public abstract class SMSDispatcher extends Handler { values.put("destination_port", portAddrs.destPort); } mResolver.insert(mRawUri, values); - acknowledgeLastIncomingSms(true, Intents.RESULT_SMS_HANDLED, null); - return; + return Intents.RESULT_SMS_HANDLED; } // All the parts are in place, deal with them @@ -529,8 +540,7 @@ public abstract class SMSDispatcher extends Handler { } catch (SQLException e) { Log.e(TAG, "Can't access multipart SMS database", e); // TODO: Would OUT_OF_MEMORY be more appropriate? - acknowledgeLastIncomingSms(false, Intents.RESULT_SMS_GENERIC_ERROR, null); - return; + return Intents.RESULT_SMS_GENERIC_ERROR; } finally { if (cursor != null) cursor.close(); } @@ -555,7 +565,7 @@ public abstract class SMSDispatcher extends Handler { output.write(data, 0, data.length); } // Handle the PUSH - mWapPush.dispatchWapPdu(output.toByteArray()); + return mWapPush.dispatchWapPdu(output.toByteArray()); } else { // The messages were sent to a port, so concoct a URI for it dispatchPortAddressedPdus(pdus, portAddrs.destPort); @@ -564,6 +574,7 @@ public abstract class SMSDispatcher extends Handler { // The messages were not sent to a port dispatchPdus(pdus); } + return Activity.RESULT_OK; } /** diff --git a/telephony/java/com/android/internal/telephony/WapPushOverSms.java b/telephony/java/com/android/internal/telephony/WapPushOverSms.java index a9aacda..9970940 100644 --- a/telephony/java/com/android/internal/telephony/WapPushOverSms.java +++ b/telephony/java/com/android/internal/telephony/WapPushOverSms.java @@ -16,8 +16,10 @@ package com.android.internal.telephony; +import android.app.Activity; import android.content.Context; import android.content.Intent; +import android.provider.Telephony; import android.provider.Telephony.Sms.Intents; import android.util.Config; import android.util.Log; @@ -51,8 +53,11 @@ public class WapPushOverSms { * wap-230-wsp-20010705-a section 8 for details on the WAP PDU format. * * @param pdu The WAP PDU, made up of one or more SMS PDUs + * @return a result code from {@link Telephony.Sms.Intents}, or + * {@link Activity#RESULT_OK} if the message has been broadcast + * to applications */ - public void dispatchWapPdu(byte[] pdu) { + public int dispatchWapPdu(byte[] pdu) { if (Config.LOGD) Log.d(LOG_TAG, "Rx: " + IccUtils.bytesToHexString(pdu)); @@ -64,7 +69,7 @@ public class WapPushOverSms { if ((pduType != WspTypeDecoder.PDU_TYPE_PUSH) && (pduType != WspTypeDecoder.PDU_TYPE_CONFIRMED_PUSH)) { if (Config.LOGD) Log.w(LOG_TAG, "Received non-PUSH WAP PDU. Type = " + pduType); - return; + return Intents.RESULT_SMS_HANDLED; } pduDecoder = new WspTypeDecoder(pdu); @@ -77,7 +82,7 @@ public class WapPushOverSms { */ if (pduDecoder.decodeUintvarInteger(index) == false) { if (Config.LOGD) Log.w(LOG_TAG, "Received PDU. Header Length error."); - return; + return Intents.RESULT_SMS_GENERIC_ERROR; } headerLength = (int)pduDecoder.getValue32(); index += pduDecoder.getDecodedDataLength(); @@ -98,7 +103,7 @@ public class WapPushOverSms { */ if (pduDecoder.decodeContentType(index) == false) { if (Config.LOGD) Log.w(LOG_TAG, "Received PDU. Header Content-Type error."); - return; + return Intents.RESULT_SMS_GENERIC_ERROR; } int binaryContentType; String mimeType = pduDecoder.getValueString(); @@ -128,7 +133,7 @@ public class WapPushOverSms { Log.w(LOG_TAG, "Received PDU. Unsupported Content-Type = " + binaryContentType); } - return; + return Intents.RESULT_SMS_HANDLED; } } else { if (mimeType.equals(WspTypeDecoder.CONTENT_MIME_TYPE_B_DRM_RIGHTS_XML)) { @@ -145,7 +150,7 @@ public class WapPushOverSms { binaryContentType = WspTypeDecoder.CONTENT_TYPE_B_MMS; } else { if (Config.LOGD) Log.w(LOG_TAG, "Received PDU. Unknown Content-Type = " + mimeType); - return; + return Intents.RESULT_SMS_HANDLED; } } index += pduDecoder.getDecodedDataLength(); @@ -167,6 +172,7 @@ public class WapPushOverSms { if (dispatchedByApplication == false) { dispatchWapPdu_default(pdu, transactionId, pduType, mimeType, dataIndex); } + return Activity.RESULT_OK; } private void dispatchWapPdu_default( diff --git a/telephony/java/com/android/internal/telephony/cdma/CdmaSMSDispatcher.java b/telephony/java/com/android/internal/telephony/cdma/CdmaSMSDispatcher.java index 8a0070d..2d43e0d 100644 --- a/telephony/java/com/android/internal/telephony/cdma/CdmaSMSDispatcher.java +++ b/telephony/java/com/android/internal/telephony/cdma/CdmaSMSDispatcher.java @@ -25,6 +25,7 @@ import android.database.Cursor; import android.database.SQLException; import android.os.AsyncResult; import android.os.Message; +import android.provider.Telephony; import android.provider.Telephony.Sms.Intents; import android.preference.PreferenceManager; import android.util.Config; @@ -66,17 +67,12 @@ final class CdmaSMSDispatcher extends SMSDispatcher { Log.d(TAG, "handleStatusReport is a special GSM function, should never be called in CDMA!"); } - /** - * Dispatches an incoming SMS messages. - * - * @param smsb the incoming message from the phone - */ - protected void dispatchMessage(SmsMessageBase smsb) { + /** {@inheritDoc} */ + protected int dispatchMessage(SmsMessageBase smsb) { // If sms is null, means there was a parsing error. - // TODO: Should NAK this. if (smsb == null) { - return; + return Intents.RESULT_SMS_GENERIC_ERROR; } // Decode BD stream and set sms variables. @@ -92,11 +88,13 @@ final class CdmaSMSDispatcher extends SMSDispatcher { handled = true; } - if (handled) return; + if (handled) { + return Intents.RESULT_SMS_HANDLED; + } if (SmsEnvelope.TELESERVICE_WAP == teleService){ - processCdmaWapPdu(sms.getUserData(), sms.messageRef, sms.getOriginatingAddress()); - return; + return processCdmaWapPdu(sms.getUserData(), sms.messageRef, + sms.getOriginatingAddress()); } else if (SmsEnvelope.TELESERVICE_VMN == teleService) { // handling Voicemail int voicemailCount = sms.getNumOfVoicemails(); @@ -108,7 +106,7 @@ final class CdmaSMSDispatcher extends SMSDispatcher { editor.putInt(CDMAPhone.VM_COUNT_CDMA, voicemailCount); editor.commit(); ((CDMAPhone) mPhone).updateMessageWaitingIndicator(voicemailCount); - return; + return Intents.RESULT_SMS_HANDLED; } /** @@ -138,17 +136,19 @@ final class CdmaSMSDispatcher extends SMSDispatcher { if (smsHeader != null && smsHeader.portAddrs != null) { if (smsHeader.portAddrs.destPort == SmsHeader.PORT_WAP_PUSH) { // GSM-style WAP indication - mWapPush.dispatchWapPdu(sms.getUserData()); + return mWapPush.dispatchWapPdu(sms.getUserData()); + } else { + // The message was sent to a port, so concoct a URI for it. + dispatchPortAddressedPdus(pdus, smsHeader.portAddrs.destPort); } - // The message was sent to a port, so concoct a URI for it. - dispatchPortAddressedPdus(pdus, smsHeader.portAddrs.destPort); } else { // Normal short and non-port-addressed message, dispatch it. dispatchPdus(pdus); } + return Activity.RESULT_OK; } else { // Process the message part. - processMessagePart(sms, smsHeader.concatRef, smsHeader.portAddrs); + return processMessagePart(sms, smsHeader.concatRef, smsHeader.portAddrs); } } @@ -158,8 +158,11 @@ final class CdmaSMSDispatcher extends SMSDispatcher { * WDP segments are gathered until a datagram completes and gets dispatched. * * @param pdu The WAP-WDP PDU segment + * @return a result code from {@link Telephony.Sms.Intents}, or + * {@link Activity#RESULT_OK} if the message has been broadcast + * to applications */ - protected void processCdmaWapPdu(byte[] pdu, int referenceNumber, String address) { + protected int processCdmaWapPdu(byte[] pdu, int referenceNumber, String address) { int segment; int totalSegments; int index = 0; @@ -171,7 +174,7 @@ final class CdmaSMSDispatcher extends SMSDispatcher { msgType = pdu[index++]; if (msgType != 0){ Log.w(TAG, "Received a WAP SMS which is not WDP. Discard."); - return; + return Intents.RESULT_SMS_HANDLED; } totalSegments = pdu[index++]; // >=1 segment = pdu[index++]; // >=0 @@ -210,7 +213,7 @@ final class CdmaSMSDispatcher extends SMSDispatcher { mResolver.insert(mRawUri, values); - return; + return Intents.RESULT_SMS_HANDLED; } // All the parts are in place, deal with them @@ -230,7 +233,7 @@ final class CdmaSMSDispatcher extends SMSDispatcher { mResolver.delete(mRawUri, where.toString(), whereArgs); } catch (SQLException e) { Log.e(TAG, "Can't access multipart SMS database", e); - return; // TODO: NACK the message or something, don't just discard. + return Intents.RESULT_SMS_GENERIC_ERROR; } finally { if (cursor != null) cursor.close(); } @@ -250,15 +253,14 @@ final class CdmaSMSDispatcher extends SMSDispatcher { switch (destinationPort) { case SmsHeader.PORT_WAP_PUSH: // Handle the PUSH - mWapPush.dispatchWapPdu(datagram); - break; + return mWapPush.dispatchWapPdu(datagram); default:{ pdus = new byte[1][]; pdus[0] = datagram; // The messages were sent to any other WAP port dispatchPortAddressedPdus(pdus, destinationPort); - break; + return Activity.RESULT_OK; } } } diff --git a/telephony/java/com/android/internal/telephony/gsm/GsmSMSDispatcher.java b/telephony/java/com/android/internal/telephony/gsm/GsmSMSDispatcher.java index f87392a..2770ddc 100644 --- a/telephony/java/com/android/internal/telephony/gsm/GsmSMSDispatcher.java +++ b/telephony/java/com/android/internal/telephony/gsm/GsmSMSDispatcher.java @@ -78,24 +78,16 @@ final class GsmSMSDispatcher extends SMSDispatcher { } } } - - if (mCm != null) { - mCm.acknowledgeLastIncomingGsmSms(true, Intents.RESULT_SMS_HANDLED, null); - } + acknowledgeLastIncomingSms(true, Intents.RESULT_SMS_HANDLED, null); } - /** - * Dispatches an incoming SMS messages. - * - * @param sms the incoming message from the phone - */ - protected void dispatchMessage(SmsMessageBase smsb) { + /** {@inheritDoc} */ + protected int dispatchMessage(SmsMessageBase smsb) { // If sms is null, means there was a parsing error. - // TODO: Should NAK this. if (smsb == null) { - return; + return Intents.RESULT_SMS_GENERIC_ERROR; } SmsMessage sms = (SmsMessage) smsb; boolean handled = false; @@ -115,7 +107,9 @@ final class GsmSMSDispatcher extends SMSDispatcher { } } - if (handled) return; + if (handled) { + return Intents.RESULT_SMS_HANDLED; + } SmsHeader smsHeader = sms.getUserDataHeader(); // See if message is partial or port addressed. @@ -126,17 +120,19 @@ final class GsmSMSDispatcher extends SMSDispatcher { if (smsHeader != null && smsHeader.portAddrs != null) { if (smsHeader.portAddrs.destPort == SmsHeader.PORT_WAP_PUSH) { - mWapPush.dispatchWapPdu(sms.getUserData()); + return mWapPush.dispatchWapPdu(sms.getUserData()); + } else { + // The message was sent to a port, so concoct a URI for it. + dispatchPortAddressedPdus(pdus, smsHeader.portAddrs.destPort); } - // The message was sent to a port, so concoct a URI for it. - dispatchPortAddressedPdus(pdus, smsHeader.portAddrs.destPort); } else { // Normal short and non-port-addressed message, dispatch it. dispatchPdus(pdus); } + return Activity.RESULT_OK; } else { // Process the message part. - processMessagePart(sms, smsHeader.concatRef, smsHeader.portAddrs); + return processMessagePart(sms, smsHeader.concatRef, smsHeader.portAddrs); } } -- cgit v1.1