Bug 828798 - [Bluetooth] [HFP] No redundant +CIEV commands are sent to bluetooth headset, r=echou
authorGina Yeh <gyeh@mozilla.com>
Mon, 28 Jan 2013 19:28:51 +0800
changeset 129939 b0db5caaeb0ee7f9b6ee6a61c686c85e778dd6d5
parent 129938 55ab9f026dc41a62e6b6af3a901eac79f1b6df7b
child 129940 3985cb4da6b6c4d4d5dd936b9ec55dd0a19d3e29
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersechou
bugs828798
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 828798 - [Bluetooth] [HFP] No redundant +CIEV commands are sent to bluetooth headset, r=echou
dom/bluetooth/BluetoothHfpManager.cpp
dom/bluetooth/BluetoothHfpManager.h
dom/bluetooth/BluetoothRilListener.cpp
--- a/dom/bluetooth/BluetoothHfpManager.cpp
+++ b/dom/bluetooth/BluetoothHfpManager.cpp
@@ -671,18 +671,16 @@ BluetoothHfpManager::ReceiveSocketData(U
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   nsAutoCString msg((const char*)aMessage->mData.get());
   msg.StripWhitespace();
 
   nsTArray<nsCString> atCommandValues;
 
-  uint16_t currentCallState = mCurrentCallArray[mCurrentCallIndex].mState;
-
   // For more information, please refer to 4.34.1 "Bluetooth Defined AT
   // Capabilities" in Bluetooth hands-free profile 1.6
   if (msg.Find("AT+BRSF=") != -1) {
     SendCommand("+BRSF: ", 33);
   } else if (msg.Find("AT+CIND=?") != -1) {
     // Asking for CIND range
     SendCommand("+CIND: ", 0);
   } else if (msg.Find("AT+CIND?") != -1) {
@@ -883,50 +881,46 @@ BluetoothHfpManager::ReceiveSocketData(U
     if (atCommandValues.IsEmpty()) {
       NS_WARNING("Could't get the value of command [AT+CCWA=]");
       goto respond_with_ok;
     }
 
     mCCWA = atCommandValues[0].EqualsLiteral("1");
   } else if (msg.Find("AT+CKPD") != -1) {
     // For Headset Profile (HSP)
-    switch (currentCallState) {
+    switch (mCurrentCallArray[mCurrentCallIndex].mState) {
       case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
         NotifyDialer(NS_LITERAL_STRING("ATA"));
         break;
       case nsIRadioInterfaceLayer::CALL_STATE_CONNECTED:
       case nsIRadioInterfaceLayer::CALL_STATE_DIALING:
       case nsIRadioInterfaceLayer::CALL_STATE_ALERTING:
         NotifyDialer(NS_LITERAL_STRING("CHUP"));
         break;
       case nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED:
         NotifyDialer(NS_LITERAL_STRING("BLDN"));
         break;
       default:
-#ifdef DEBUG
         NS_WARNING("Not handling state changed");
-#endif
         break;
     }
   } else if (msg.Find("AT+CNUM") != -1) {
     if (!mMsisdn.IsEmpty()) {
       nsAutoCString message("+CNUM: ,\"");
       message += NS_ConvertUTF16toUTF8(mMsisdn).get();
       message += "\",";
       message.AppendInt(TOA_UNKNOWN);
       message += ",,4";
       SendLine(message.get());
     }
   } else {
-#ifdef DEBUG
     nsCString warningMsg;
     warningMsg.AssignLiteral("Not handling HFP message, reply ok: ");
     warningMsg.Append(msg);
     NS_WARNING(warningMsg.get());
-#endif
   }
 
 respond_with_ok:
   // We always respond to remote device with "OK" in general cases.
   SendLine("OK");
 }
 
 bool
@@ -1007,17 +1001,16 @@ BluetoothHfpManager::Listen()
 void
 BluetoothHfpManager::Disconnect()
 {
   if (GetConnectionStatus() == SocketConnectionStatus::SOCKET_DISCONNECTED) {
     NS_WARNING("BluetoothHfpManager has been disconnected!");
     return;
   }
 
-  CloseScoSocket();
   CloseSocket();
 }
 
 bool
 BluetoothHfpManager::SendLine(const char* aMessage)
 {
   const char* kHfpCrlf = "\xd\xa";
   nsAutoCString msg;
@@ -1025,17 +1018,17 @@ BluetoothHfpManager::SendLine(const char
   msg += kHfpCrlf;
   msg += aMessage;
   msg += kHfpCrlf;
 
   return SendSocketData(msg);
 }
 
 bool
-BluetoothHfpManager::SendCommand(const char* aCommand, const uint16_t aValue)
+BluetoothHfpManager::SendCommand(const char* aCommand, uint8_t aValue)
 {
   if (mSocketStatus != SocketConnectionStatus::SOCKET_CONNECTED) {
     return false;
   }
 
   nsAutoCString message;
   int value = aValue;
   message += aCommand;
@@ -1125,246 +1118,221 @@ BluetoothHfpManager::SendCommand(const c
   } else {
     message.AppendInt(value);
   }
 
   return SendLine(message.get());
 }
 
 void
-BluetoothHfpManager::SetupCIND(uint32_t aCallIndex, uint16_t aCallState,
-                               const nsAString& aNumber, bool aInitial)
+BluetoothHfpManager::UpdateCIND(uint8_t aType, uint8_t aValue, bool aSend)
 {
-  nsRefPtr<nsRunnable> sendRingTask;
-  nsString address;
+  if (sCINDItems[aType].value != aValue) {
+    sCINDItems[aType].value = aValue;
+    // Indicator status update is enabled
+    if (aSend && mCMER) {
+      SendCommand("+CIEV: ", aType);
+    }
+  }
+}
+
+void
+BluetoothHfpManager::HandleCallStateChanged(uint32_t aCallIndex,
+                                            uint16_t aCallState,
+                                            const nsAString& aNumber,
+                                            bool aSend)
+{
+  if (GetConnectionStatus() != SocketConnectionStatus::SOCKET_CONNECTED) {
+    return;
+  }
 
   while (aCallIndex >= mCurrentCallArray.Length()) {
     Call call;
     mCurrentCallArray.AppendElement(call);
   }
 
   // Same logic as implementation in ril_worker.js
   if (aNumber.Length() && aNumber[0] == '+') {
     mCurrentCallArray[aCallIndex].mType = TOA_INTERNATIONAL;
   }
   mCurrentCallArray[aCallIndex].mNumber = aNumber;
 
-  uint16_t currentCallState = mCurrentCallArray[aCallIndex].mState;
+  nsRefPtr<nsRunnable> sendRingTask;
+  nsString address;
+  uint16_t prevCallState = mCurrentCallArray[aCallIndex].mState;
+  uint32_t callArrayLength = mCurrentCallArray.Length();
+  uint32_t index = 1;
 
   switch (aCallState) {
+    case nsIRadioInterfaceLayer::CALL_STATE_HELD:
+      sCINDItems[CINDType::CALLHELD].value = CallHeldState::ONHOLD_ACTIVE;
+      SendCommand("+CIEV: ", CINDType::CALLHELD);
+      break;
     case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
       mCurrentCallArray[aCallIndex].mDirection = true;
-      sCINDItems[CINDType::CALLSETUP].value = CallSetupState::INCOMING;
 
       if (mCurrentCallIndex) {
         if (mCCWA) {
           nsAutoCString ccwaMsg("+CCWA: \"");
           ccwaMsg += NS_ConvertUTF16toUTF8(aNumber).get();
           ccwaMsg += "\",";
           ccwaMsg.AppendInt(mCurrentCallArray[aCallIndex].mType);
           SendLine(ccwaMsg.get());
         }
-
-        if (!aInitial) {
-          SendCommand("+CIEV: ", CINDType::CALLSETUP);
-        }
+        UpdateCIND(CINDType::CALLSETUP, CallSetupState::INCOMING, aSend);
       } else {
         // Start sending RING indicator to HF
         sStopSendingRingFlag = false;
-
-        if (!aInitial) {
-          SendCommand("+CIEV: ", CINDType::CALLSETUP);
-        }
+        UpdateCIND(CINDType::CALLSETUP, CallSetupState::INCOMING, aSend);
 
         nsAutoString number(aNumber);
         if (!mCLIP) {
           number.AssignLiteral("");
         }
 
         MessageLoop::current()->PostDelayedTask(FROM_HERE,
           new SendRingIndicatorTask(number, mCurrentCallArray[aCallIndex].mType),
           sRingInterval);
       }
       break;
     case nsIRadioInterfaceLayer::CALL_STATE_DIALING:
       mCurrentCallArray[aCallIndex].mDirection = false;
-      sCINDItems[CINDType::CALLSETUP].value = CallSetupState::OUTGOING;
-      if (!aInitial) {
-        SendCommand("+CIEV: ", CINDType::CALLSETUP);
+      UpdateCIND(CINDType::CALLSETUP, CallSetupState::OUTGOING, aSend);
 
-        GetSocketAddr(address);
-        OpenScoSocket(address);
-      }
+      GetSocketAddr(address);
+      OpenScoSocket(address);
       break;
     case nsIRadioInterfaceLayer::CALL_STATE_ALERTING:
       mCurrentCallArray[aCallIndex].mDirection = false;
-      sCINDItems[CINDType::CALLSETUP].value = CallSetupState::OUTGOING_ALERTING;
-      if (!aInitial) {
-        SendCommand("+CIEV: ", CINDType::CALLSETUP);
-      }
+      UpdateCIND(CINDType::CALLSETUP, CallSetupState::OUTGOING_ALERTING, aSend);
+
+      // If there's an ongoing call when the headset is just connected, we have
+      // to open a sco socket here.
+      GetSocketAddr(address);
+      OpenScoSocket(address);
       break;
     case nsIRadioInterfaceLayer::CALL_STATE_CONNECTED:
       mCurrentCallIndex = aCallIndex;
-      if (aInitial) {
-        sCINDItems[CINDType::CALL].value = CallState::IN_PROGRESS;
-        sCINDItems[CINDType::CALLSETUP].value = CallSetupState::NO_CALLSETUP;
-      } else {
-        switch (currentCallState) {
-          case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
-            // Incoming call, no break
-            sStopSendingRingFlag = true;
+      switch (prevCallState) {
+        case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
+        case nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED:
+          // Incoming call, no break
+          sStopSendingRingFlag = true;
 
-            GetSocketAddr(address);
-            OpenScoSocket(address);
-          case nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED:
-          case nsIRadioInterfaceLayer::CALL_STATE_ALERTING:
-            // Outgoing call
-            sCINDItems[CINDType::CALL].value = CallState::IN_PROGRESS;
-            SendCommand("+CIEV: ", CINDType::CALL);
-            sCINDItems[CINDType::CALLSETUP].value = CallSetupState::NO_CALLSETUP;
-            SendCommand("+CIEV: ", CINDType::CALLSETUP);
-            break;
-          default:
-#ifdef DEBUG
-            NS_WARNING("Not handling state changed");
-#endif
-            break;
-        }
+          GetSocketAddr(address);
+          OpenScoSocket(address);
+        case nsIRadioInterfaceLayer::CALL_STATE_ALERTING:
+          // Outgoing call
+          UpdateCIND(CINDType::CALL, CallState::IN_PROGRESS, aSend);
+          UpdateCIND(CINDType::CALLSETUP, CallSetupState::NO_CALLSETUP, aSend);
+          break;
+        case nsIRadioInterfaceLayer::CALL_STATE_HELD:
+          // Check whether to update CINDType::CALLHELD or not
+          while (index < callArrayLength) {
+            if (index == mCurrentCallIndex) {
+              index++;
+              continue;
+            }
+
+            uint16_t state = mCurrentCallArray[index].mState;
+            // If there's another call on hold or other calls exist, no need to
+            // update CINDType::CALLHELD
+            if (state != nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED) {
+              break;
+            }
+            index++;
+          }
+
+          if (index == callArrayLength) {
+            UpdateCIND(CINDType::CALLHELD, CallHeldState::NO_CALLHELD, aSend);
+          }
+          break;
+        default:
+          NS_WARNING("Not handling state changed");
       }
       break;
     case nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED:
-      if (!aInitial) {
-        switch (currentCallState) {
-          case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
-          case nsIRadioInterfaceLayer::CALL_STATE_BUSY:
-            // Incoming call, no break
-            sStopSendingRingFlag = true;
-          case nsIRadioInterfaceLayer::CALL_STATE_DIALING:
-          case nsIRadioInterfaceLayer::CALL_STATE_ALERTING:
-            // Outgoing call
-            sCINDItems[CINDType::CALLSETUP].value = CallSetupState::NO_CALLSETUP;
-            SendCommand("+CIEV: ", CINDType::CALLSETUP);
+      switch (prevCallState) {
+        case nsIRadioInterfaceLayer::CALL_STATE_INCOMING:
+        case nsIRadioInterfaceLayer::CALL_STATE_BUSY:
+          // Incoming call, no break
+          sStopSendingRingFlag = true;
+        case nsIRadioInterfaceLayer::CALL_STATE_DIALING:
+        case nsIRadioInterfaceLayer::CALL_STATE_ALERTING:
+          // Outgoing call
+          UpdateCIND(CINDType::CALLSETUP, CallSetupState::NO_CALLSETUP, aSend);
+          break;
+        case nsIRadioInterfaceLayer::CALL_STATE_CONNECTED:
+          // No call is ongoing
+          if (sCINDItems[CINDType::CALLHELD].value == CallHeldState::NO_CALLHELD) {
+            UpdateCIND(CINDType::CALL, CallState::NO_CALL, aSend);
+          }
+          break;
+        case nsIRadioInterfaceLayer::CALL_STATE_HELD:
+          UpdateCIND(CINDType::CALLHELD, CallHeldState::NO_CALLHELD, aSend);
+          break;
+        default:
+          NS_WARNING("Not handling state changed");
+      }
+
+      if (aCallIndex == mCurrentCallIndex) {
+        NS_ASSERTION(mCurrentCallArray.Length() > aCallIndex,
+          "Call index out of bounds!");
+        mCurrentCallArray[aCallIndex].mState = aCallState;
+
+        // Find the first non-disconnected call (like connected, held),
+        // and update mCurrentCallIndex
+        while (index < callArrayLength) {
+          if (mCurrentCallArray[index].mState !=
+              nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED) {
+            mCurrentCallIndex = index;
             break;
-          case nsIRadioInterfaceLayer::CALL_STATE_CONNECTED:
-            sCINDItems[CINDType::CALL].value = CallState::NO_CALL;
-            SendCommand("+CIEV: ", CINDType::CALL);
-            break;
-          case nsIRadioInterfaceLayer::CALL_STATE_HELD:
-            sCINDItems[CINDType::CALLHELD].value = NO_CALLHELD;
-            SendCommand("+CIEV: ", CINDType::CALLHELD);
-          default:
-#ifdef DEBUG
-            NS_WARNING("Not handling state changed");
-#endif
-            break;
+          }
+          index++;
         }
 
-        if (aCallIndex == mCurrentCallIndex) {
-          NS_ASSERTION(mCurrentCallArray.Length() > aCallIndex,
-            "Call index out of bounds!");
-          mCurrentCallArray[aCallIndex].mState = aCallState;
-
-          // Find the first non-disconnected call (like connected, held),
-          // and update mCurrentCallIndex
-          uint32_t c = 1;
-          uint32_t length = mCurrentCallArray.Length();
-          while (c < length) {
-            if (mCurrentCallArray[c].mState !=
-                nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED) {
-              mCurrentCallIndex = c;
-              break;
-            }
-            c++;
-          }
-
-          // There is no call, close Sco and clear mCurrentCallArray
-          if (c == length) {
-            CloseScoSocket();
-            ResetCallArray();
-          }
+        // There is no call, close Sco and clear mCurrentCallArray
+        if (index == callArrayLength) {
+          CloseScoSocket();
+          ResetCallArray();
         }
       }
       break;
-    case nsIRadioInterfaceLayer::CALL_STATE_HELD:
-      sCINDItems[CINDType::CALLHELD].value = CallHeldState::ONHOLD_ACTIVE;
-
-      if (!aInitial) {
-        SendCommand("+CIEV: ", CINDType::CALLHELD);
-      }
-
-      break;
     default:
-#ifdef DEBUG
       NS_WARNING("Not handling state changed");
       sCINDItems[CINDType::CALL].value = CallState::NO_CALL;
       sCINDItems[CINDType::CALLSETUP].value = CallSetupState::NO_CALLSETUP;
       sCINDItems[CINDType::CALLHELD].value = CallHeldState::NO_CALLHELD;
-#endif
-      break;
   }
 
   mCurrentCallArray[aCallIndex].mState = aCallState;
 }
 
-/*
- * EnumerateCallState will be called for each call
- */
-void
-BluetoothHfpManager::EnumerateCallState(uint32_t aCallIndex, uint16_t aCallState,
-                                        const nsAString& aNumber, bool aIsActive)
-{
-  SetupCIND(aCallIndex, aCallState, aNumber, true);
-
-  if (sCINDItems[CINDType::CALL].value == CallState::IN_PROGRESS ||
-      sCINDItems[CINDType::CALLSETUP].value == CallSetupState::OUTGOING ||
-      sCINDItems[CINDType::CALLSETUP].value == CallSetupState::OUTGOING_ALERTING) {
-    nsString address;
-    GetSocketAddr(address);
-    OpenScoSocket(address);
-  }
-}
-
-/*
- * CallStateChanged will be called whenever call status is changed, and it
- * also means we need to notify HS about the change. For more information,
- * please refer to 4.13 ~ 4.15 in Bluetooth hands-free profile 1.6.
- */
-void
-BluetoothHfpManager::CallStateChanged(uint32_t aCallIndex, uint16_t aCallState,
-                                      const nsAString& aNumber, bool aIsActive)
-{
-  if (GetConnectionStatus() != SocketConnectionStatus::SOCKET_CONNECTED) {
-    return;
-  }
-
-  SetupCIND(aCallIndex, aCallState, aNumber, false);
-}
-
 void
 BluetoothHfpManager::OnConnectSuccess()
 {
+  nsCOMPtr<nsIRILContentHelper> ril =
+    do_GetService(NS_RILCONTENTHELPER_CONTRACTID);
+  NS_ENSURE_TRUE_VOID(ril);
+  ril->EnumerateCalls(mListener->GetCallback());
+
   // For active connection request, we need to reply the DOMRequest
   if (mRunnable) {
     BluetoothValue v = true;
     nsString errorStr;
     DispatchBluetoothReply(mRunnable, v, errorStr);
 
     mRunnable.forget();
   }
 
   // Cache device path for NotifySettings() since we can't get socket address
   // when a headset disconnect with us
   GetSocketAddr(mDevicePath);
   mSocketStatus = GetConnectionStatus();
 
-  nsCOMPtr<nsIRILContentHelper> ril =
-    do_GetService(NS_RILCONTENTHELPER_CONTRACTID);
-  NS_ENSURE_TRUE_VOID(ril);
-  ril->EnumerateCalls(mListener->GetCallback());
-
   NotifySettings();
 }
 
 void
 BluetoothHfpManager::OnConnectError()
 {
   // For active connection request, we need to reply the DOMRequest
   if (mRunnable) {
@@ -1389,10 +1357,11 @@ BluetoothHfpManager::OnDisconnect()
   // notify Settings app.
   if (mSocketStatus == SocketConnectionStatus::SOCKET_CONNECTED) {
     Listen();
     NotifySettings();
   } else if (mSocketStatus == SocketConnectionStatus::SOCKET_CONNECTING) {
     NS_WARNING("BluetoothHfpManager got unexpected socket status!");
   }
 
+  CloseScoSocket();
   Reset();
 }
--- a/dom/bluetooth/BluetoothHfpManager.h
+++ b/dom/bluetooth/BluetoothHfpManager.h
@@ -56,43 +56,46 @@ public:
     MOZ_OVERRIDE;
 
   bool Connect(const nsAString& aDeviceObjectPath,
                const bool aIsHandsfree,
                BluetoothReplyRunnable* aRunnable);
   void Disconnect();
   bool Listen();
 
-  void CallStateChanged(uint32_t aCallIndex, uint16_t aCallState,
-                        const nsAString& aNumber, bool aIsActive);
-  void EnumerateCallState(uint32_t aCallIndex, uint16_t aCallState,
-                          const nsAString& aNumber, bool aIsActive);
+  /**
+   * @param aSend A boolean indicates whether we need to notify headset or not
+   */
+  void HandleCallStateChanged(uint32_t aCallIndex, uint16_t aCallState,
+                              const nsAString& aNumber, bool aSend);
 
 private:
   class GetVolumeTask;
   friend class GetVolumeTask;
   friend class BluetoothHfpManagerObserver;
 
   BluetoothHfpManager();
   ~BluetoothHfpManager();
   nsresult HandleIccInfoChanged();
   nsresult HandleShutdown();
   nsresult HandleVolumeChanged(const nsAString& aData);
   nsresult HandleVoiceConnectionChanged();
 
   bool Init();
   void Cleanup();
-  void NotifyDialer(const nsAString& aCommand);
-  void NotifySettings();
   void Reset();
   void ResetCallArray();
-  bool SendCommand(const char* aCommand, const uint16_t aValue = 0);
+
+  void NotifyDialer(const nsAString& aCommand);
+  void NotifySettings();
+
+  bool SendCommand(const char* aCommand, uint8_t aValue = 0);
   bool SendLine(const char* aMessage);
-  void SetupCIND(uint32_t aCallIndex, uint16_t aCallState,
-                 const nsAString& aNumber, bool aInitial);
+  void UpdateCIND(uint8_t aType, uint8_t aValue, bool aSend);
+
   virtual void OnConnectSuccess() MOZ_OVERRIDE;
   virtual void OnConnectError() MOZ_OVERRIDE;
   virtual void OnDisconnect() MOZ_OVERRIDE;
 
   int mCurrentVgs;
   int mCurrentVgm;
   uint32_t mCurrentCallIndex;
   bool mCCWA;
--- a/dom/bluetooth/BluetoothRilListener.cpp
+++ b/dom/bluetooth/BluetoothRilListener.cpp
@@ -26,30 +26,30 @@ NS_IMPL_ISUPPORTS1(BluetoothRILTelephony
 
 NS_IMETHODIMP
 BluetoothRILTelephonyCallback::CallStateChanged(uint32_t aCallIndex,
                                                 uint16_t aCallState,
                                                 const nsAString& aNumber,
                                                 bool aIsActive)
 {
   BluetoothHfpManager* hfp = BluetoothHfpManager::Get();
-  hfp->CallStateChanged(aCallIndex, aCallState, aNumber, aIsActive);
+  hfp->HandleCallStateChanged(aCallIndex, aCallState, aNumber, true);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BluetoothRILTelephonyCallback::EnumerateCallState(uint32_t aCallIndex,
                                                   uint16_t aCallState,
                                                   const nsAString_internal& aNumber,
                                                   bool aIsActive,
                                                   bool* aResult)
 {
   BluetoothHfpManager* hfp = BluetoothHfpManager::Get();
-  hfp->EnumerateCallState(aCallIndex, aCallState, aNumber, aIsActive);
+  hfp->HandleCallStateChanged(aCallIndex, aCallState, aNumber, false);
   *aResult = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BluetoothRILTelephonyCallback::NotifyError(int32_t aCallIndex,
                                            const nsAString& aError)
 {