Version 1.23.0 alexa-client-sdk

Changes in this update:

Feature enhancements, updates, and resolved issues from all releases are available on the [Amazon developer portal](https://developer.amazon.com/docs/alexa/avs-device-sdk/release-notes.html)

Author: caleighatamazon

ACL/CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 project(ACL LANGUAGES CXX)
 
-include(../build/BuildDefaults.cmake)
+include(${AVS_CMAKE_BUILD}/BuildDefaults.cmake)
 
 add_subdirectory("src")
 add_subdirectory("test")

ACL/include/ACL/AVSConnectionManager.h

@@ -176,9 +176,14 @@ class AVSConnectionManager
 
     void receive(const std::string& contextId, const std::string& message) override;
 
+    std::shared_ptr<MessageRouterInterface> getMessageRouter() const;
+
     /// Mutex to serialize access to @c m_isEnabled
     std::mutex m_isEnabledMutex;
 
+    /// Mutex to serialize access to @c m_messageRouter
+    mutable std::mutex m_messageRouterMutex;
+
     /// Internal state to indicate if the Connection object is enabled for making an AVS connection.
     bool m_isEnabled;
 

ACL/include/ACL/Transport/MessageRequestHandler.h

@@ -21,6 +21,7 @@
 #include <AVSCommon/AVS/Attachment/AttachmentManagerInterface.h>
 #include <AVSCommon/AVS/MessageRequest.h>
 #include <AVSCommon/SDKInterfaces/EventTracerInterface.h>
+#include <AVSCommon/SDKInterfaces/MessageRequestObserverInterface.h>
 #include <AVSCommon/Utils/Power/PowerResource.h>
 #include <AVSCommon/Utils/HTTP2/HTTP2MimeRequestSourceInterface.h>
 #include <AVSCommon/Utils/Metrics/MetricRecorderInterface.h>
@@ -142,6 +143,9 @@ class MessageRequestHandler
 
     /// The reference to @c PowerResource to prevent device from going into LPM.
     std::shared_ptr<avsCommon::utils::power::PowerResource> m_powerResource;
+
+    /// Status to be reported back to the @c MessageRequest.
+    avsCommon::sdkInterfaces::MessageRequestObserverInterface::Status m_resultStatus;
 };
 
 }  // namespace acl

ACL/src/AVSConnectionManager.cpp

@@ -122,21 +122,37 @@ void AVSConnectionManager::doShutdown() {
         std::lock_guard<std::mutex> lock{m_messageObserverMutex};
         m_messageObservers.clear();
     }
+    std::unique_lock<std::mutex> lock{m_messageRouterMutex};
+    /* There is still a potential deadlock if the reset of m_messageRouter triggers a delete of the message router,
+    and that delete blocks on code that could call back into AVSConnectionManager and try to acquire that new mutex.
+    We can get around this by having doShutdown() swap m_messageRouter with a local variable while holding the lock,
+    and then resetting the local variable after new mutex is released.
+     */
+    auto messageRouter = m_messageRouter;
     m_messageRouter.reset();
+    lock.unlock();
+
+    messageRouter.reset();
 }
 
 void AVSConnectionManager::enable() {
     std::lock_guard<std::mutex> lock(m_isEnabledMutex);
     ACSDK_DEBUG5(LX(__func__));
     m_isEnabled = true;
-    m_messageRouter->enable();
+    auto messageRouter = getMessageRouter();
+    if (messageRouter) {
+        messageRouter->enable();
+    }
 }
 
 void AVSConnectionManager::disable() {
     std::lock_guard<std::mutex> lock(m_isEnabledMutex);
     ACSDK_DEBUG5(LX(__func__));
     m_isEnabled = false;
-    m_messageRouter->disable();
+    auto messageRouter = getMessageRouter();
+    if (messageRouter) {
+        messageRouter->disable();
+    }
 }
 
 bool AVSConnectionManager::isEnabled() {
@@ -148,39 +164,76 @@ void AVSConnectionManager::reconnect() {
     std::lock_guard<std::mutex> lock(m_isEnabledMutex);
     ACSDK_DEBUG5(LX(__func__).d("isEnabled", m_isEnabled));
     if (m_isEnabled) {
-        m_messageRouter->disable();
-        m_messageRouter->enable();
+        auto messageRouter = getMessageRouter();
+        if (messageRouter) {
+            messageRouter->disable();
+            messageRouter->enable();
+        }
     }
 }
 
 void AVSConnectionManager::sendMessage(std::shared_ptr<avsCommon::avs::MessageRequest> request) {
-    m_messageRouter->sendMessage(request);
+    auto messageRouter = getMessageRouter();
+    if (messageRouter) {
+        messageRouter->sendMessage(request);
+    } else {
+        ACSDK_WARN(LX("sendMessageFailed")
+                       .d("reason", "nullMessageRouter")
+                       .m("setting status for request to NOT_CONNECTED")
+                       .d("request", request->getJsonContent()));
+        request->sendCompleted(MessageRequestObserverInterface::Status::NOT_CONNECTED);
+    }
 }
 
 bool AVSConnectionManager::isConnected() const {
-    return m_messageRouter->getConnectionStatus().first == ConnectionStatusObserverInterface::Status::CONNECTED;
+    auto messageRouter = getMessageRouter();
+    if (messageRouter) {
+        return messageRouter->getConnectionStatus().first == ConnectionStatusObserverInterface::Status::CONNECTED;
+    }
+    return false;
 }
 
 void AVSConnectionManager::onWakeConnectionRetry() {
     ACSDK_DEBUG9(LX(__func__));
-    m_messageRouter->onWakeConnectionRetry();
+    auto messageRouter = getMessageRouter();
+    if (messageRouter) {
+        messageRouter->onWakeConnectionRetry();
+    } else {
+        ACSDK_WARN(LX("onWakeConnectionRetryFailed").d("reason", "nullMessageRouter"));
+    }
 }
 
 void AVSConnectionManager::setAVSGateway(const std::string& avsGateway) {
-    m_messageRouter->setAVSGateway(avsGateway);
+    auto messageRouter = getMessageRouter();
+    if (messageRouter) {
+        messageRouter->setAVSGateway(avsGateway);
+    } else {
+        ACSDK_WARN(LX("setAVSGatewayFailed").d("reason", "nullMessageRouter"));
+    }
 }
 
 std::string AVSConnectionManager::getAVSGateway() const {
-    return m_messageRouter->getAVSGateway();
+    auto messageRouter = getMessageRouter();
+    if (messageRouter) {
+        return messageRouter->getAVSGateway();
+    } else {
+        ACSDK_WARN(LX("getAVSGatewayFailed").d("reason", "nullMessageRouter"));
+    }
+    return "";
 }
 
 void AVSConnectionManager::onConnectionStatusChanged(bool connected) {
     ACSDK_DEBUG5(LX(__func__).d("connected", connected).d("isEnabled", m_isEnabled));
     if (m_isEnabled) {
-        if (connected) {
-            m_messageRouter->onWakeConnectionRetry();
+        auto messageRouter = getMessageRouter();
+        if (messageRouter) {
+            if (connected) {
+                messageRouter->onWakeConnectionRetry();
+            } else {
+                messageRouter->onWakeVerifyConnectivity();
+            }
         } else {
-            m_messageRouter->onWakeVerifyConnectivity();
+            ACSDK_WARN(LX("onConnectionStatusChangedFailed").d("reason", "nullMessageRouter"));
         }
     }
 }
@@ -228,5 +281,10 @@ void AVSConnectionManager::receive(const std::string& contextId, const std::stri
     }
 }
 
+std::shared_ptr<MessageRouterInterface> AVSConnectionManager::getMessageRouter() const {
+    std::lock_guard<std::mutex> lock{m_messageRouterMutex};
+    return m_messageRouter;
+}
+
 }  // namespace acl
 }  // namespace alexaClientSDK

ACL/src/Transport/MessageRequestHandler.cpp

@@ -43,7 +43,7 @@ using namespace avsCommon::utils::power;
 const static std::string AVS_EVENT_URL_PATH_EXTENSION = "/v20160207/events";
 
 /// Boundary for mime encoded requests
-const static std::string MIME_BOUNDARY = "WhooHooZeerOoonie!";
+const static std::string MIME_BOUNDARY = "WhooHooZeerOoonie=";
 
 /// Timeout for transmission of data on a given stream
 static const std::chrono::seconds STREAM_PROGRESS_TIMEOUT = std::chrono::seconds(15);
@@ -200,7 +200,8 @@ MessageRequestHandler::MessageRequestHandler(
         m_wasMessageRequestAcknowledgeReported{false},
         m_wasMessageRequestFinishedReported{false},
         m_responseCode{0},
-        m_powerResource{powerResource} {
+        m_powerResource{powerResource},
+        m_resultStatus{MessageRequestObserverInterface::Status::PENDING} {
     ACSDK_DEBUG7(LX(__func__).d("context", context.get()).d("messageRequest", messageRequest.get()));
 
     if (m_powerResource) {
@@ -324,15 +325,37 @@ void MessageRequestHandler::onActivity() {
 bool MessageRequestHandler::onReceiveResponseCode(long responseCode) {
     ACSDK_DEBUG7(LX(__func__).d("responseCode", responseCode));
 
-    // TODO ACSDK-1839: Provide MessageRequestObserverInterface immediate notification of receipt of response code.
-
     reportMessageRequestAcknowledged();
 
     if (HTTPResponseCode::CLIENT_ERROR_FORBIDDEN == intToHTTPResponseCode(responseCode)) {
         m_context->onForbidden(m_authToken);
     }
 
     m_responseCode = responseCode;
+
+    // Map HTTPResponseCode values to MessageRequestObserverInterface::Status values.
+    static const std::unordered_map<long, MessageRequestObserverInterface::Status> responseToResult = {
+        {HTTPResponseCode::HTTP_RESPONSE_CODE_UNDEFINED, MessageRequestObserverInterface::Status::INTERNAL_ERROR},
+        {HTTPResponseCode::SUCCESS_OK, MessageRequestObserverInterface::Status::SUCCESS},
+        {HTTPResponseCode::SUCCESS_ACCEPTED, MessageRequestObserverInterface::Status::SUCCESS_ACCEPTED},
+        {HTTPResponseCode::SUCCESS_NO_CONTENT, MessageRequestObserverInterface::Status::SUCCESS_NO_CONTENT},
+        {HTTPResponseCode::CLIENT_ERROR_BAD_REQUEST, MessageRequestObserverInterface::Status::BAD_REQUEST},
+        {HTTPResponseCode::CLIENT_ERROR_FORBIDDEN, MessageRequestObserverInterface::Status::INVALID_AUTH},
+        {HTTPResponseCode::CLIENT_ERROR_THROTTLING_EXCEPTION, MessageRequestObserverInterface::Status::THROTTLED},
+        {HTTPResponseCode::SERVER_ERROR_INTERNAL, MessageRequestObserverInterface::Status::SERVER_INTERNAL_ERROR_V2},
+        {HTTPResponseCode::SERVER_UNAVAILABLE, MessageRequestObserverInterface::Status::REFUSED}};
+
+    auto responseIterator = responseToResult.find(m_responseCode);
+    if (responseIterator != responseToResult.end()) {
+        m_resultStatus = responseIterator->second;
+    } else {
+        m_resultStatus = MessageRequestObserverInterface::Status::SERVER_OTHER_ERROR;
+    }
+
+    ACSDK_DEBUG7(LX("responseCodeTranslated").d("responseStatus", m_resultStatus));
+
+    m_messageRequest->responseStatusReceived(m_resultStatus);
+
     return true;
 }
 
@@ -350,51 +373,35 @@ void MessageRequestHandler::onResponseFinished(HTTP2ResponseFinishedStatus statu
         m_messageRequest->exceptionReceived(nonMimeBody);
     }
 
-    // Hash to allow use of HTTP2ResponseFinishedStatus as the key in an unordered_map.
-    struct statusHash {
-        size_t operator()(const HTTP2ResponseFinishedStatus& key) const {
-            return static_cast<size_t>(key);
-        }
-    };
-
-    // Mapping HTTP2ResponseFinishedStatus to a MessageRequestObserverInterface::Status.  Note that no mapping is
-    // provided from the COMPLETE status so that the logic below falls through to map the HTTPResponseCode value
-    // from the completed requests to the appropriate MessageRequestObserverInterface value.
-    static const std::unordered_map<HTTP2ResponseFinishedStatus, MessageRequestObserverInterface::Status, statusHash>
-        statusToResult = {
-            {HTTP2ResponseFinishedStatus::INTERNAL_ERROR, MessageRequestObserverInterface::Status::INTERNAL_ERROR},
-            {HTTP2ResponseFinishedStatus::CANCELLED, MessageRequestObserverInterface::Status::CANCELED},
-            {HTTP2ResponseFinishedStatus::TIMEOUT, MessageRequestObserverInterface::Status::TIMEDOUT}};
-
-    // Map HTTPResponseCode values to MessageRequestObserverInterface::Status values.
-    static const std::unordered_map<long, MessageRequestObserverInterface::Status> responseToResult = {
-        {HTTPResponseCode::HTTP_RESPONSE_CODE_UNDEFINED, MessageRequestObserverInterface::Status::INTERNAL_ERROR},
-        {HTTPResponseCode::SUCCESS_OK, MessageRequestObserverInterface::Status::SUCCESS},
-        {HTTPResponseCode::SUCCESS_ACCEPTED, MessageRequestObserverInterface::Status::SUCCESS_ACCEPTED},
-        {HTTPResponseCode::SUCCESS_NO_CONTENT, MessageRequestObserverInterface::Status::SUCCESS_NO_CONTENT},
-        {HTTPResponseCode::CLIENT_ERROR_BAD_REQUEST, MessageRequestObserverInterface::Status::BAD_REQUEST},
-        {HTTPResponseCode::CLIENT_ERROR_FORBIDDEN, MessageRequestObserverInterface::Status::INVALID_AUTH},
-        {HTTPResponseCode::CLIENT_ERROR_THROTTLING_EXCEPTION, MessageRequestObserverInterface::Status::THROTTLED},
-        {HTTPResponseCode::SERVER_ERROR_INTERNAL, MessageRequestObserverInterface::Status::SERVER_INTERNAL_ERROR_V2},
-        {HTTPResponseCode::SERVER_UNAVAILABLE, MessageRequestObserverInterface::Status::REFUSED}};
-
-    auto result = MessageRequestObserverInterface::Status::INTERNAL_ERROR;
+    bool receivedResponseCode = MessageRequestObserverInterface::Status::PENDING != m_resultStatus;
+
+    // Map HTTP2ResponseFinishedStatus to a MessageRequestObserverInterface::Status.
+
+    switch (status) {
+        case HTTP2ResponseFinishedStatus::COMPLETE:
+            if (!receivedResponseCode) {
+                m_resultStatus = MessageRequestObserverInterface::Status::INTERNAL_ERROR;
+            }
+            break;
+        case HTTP2ResponseFinishedStatus::TIMEOUT:
+            m_resultStatus = MessageRequestObserverInterface::Status::TIMEDOUT;
+            break;
+        case HTTP2ResponseFinishedStatus::CANCELLED:
+            m_resultStatus = MessageRequestObserverInterface::Status::CANCELED;
+            break;
+        case HTTP2ResponseFinishedStatus::INTERNAL_ERROR:
+            m_resultStatus = MessageRequestObserverInterface::Status::INTERNAL_ERROR;
+            break;
+        default:
+            ACSDK_ERROR(LX("unhandledHTTP2ResponseFinishedStatus").d("status", status));
+            m_resultStatus = MessageRequestObserverInterface::Status::INTERNAL_ERROR;
+    }
 
-    if (HTTP2ResponseFinishedStatus::COMPLETE == status) {
-        auto responseIterator = responseToResult.find(m_responseCode);
-        if (responseIterator != responseToResult.end()) {
-            result = responseIterator->second;
-        } else {
-            result = MessageRequestObserverInterface::Status::SERVER_OTHER_ERROR;
-        }
-    } else {
-        auto statusIterator = statusToResult.find(status);
-        if (statusIterator != statusToResult.end()) {
-            result = statusIterator->second;
-        }
+    if (!receivedResponseCode) {
+        m_messageRequest->responseStatusReceived(m_resultStatus);
     }
 
-    m_messageRequest->sendCompleted(result);
+    m_messageRequest->sendCompleted(m_resultStatus);
 }
 
 }  // namespace acl

ACL/src/Transport/MessageRouter.cpp

@@ -150,6 +150,13 @@ void MessageRouter::sendMessage(std::shared_ptr<MessageRequest> request) {
         ACSDK_ERROR(LX("sendFailed").d("reason", "nullRequest"));
         return;
     }
+
+    if (!request->isResolved()) {
+        ACSDK_ERROR(LX("sendFailed").d("reason", "requestNotReady"));
+        request->sendCompleted(MessageRequestObserverInterface::Status::BAD_REQUEST);
+        return;
+    }
+
     std::unique_lock<std::mutex> lock{m_connectionMutex};
     if (m_activeTransport) {
         m_requestQueue->enqueueRequest(request);

ACL/test/Transport/HTTP2TransportTest.cpp

@@ -975,8 +975,14 @@ TEST_F(HTTP2TransportTest, test_onSendCompletedNotification) {
     // Check that we got the right onSendCompleted notifications.
     for (unsigned messageNum = 0; messageNum < messagesCount; messageNum++) {
         if (messageObservers[messageNum]->m_status.waitFor(RESPONSE_TIMEOUT)) {
-            auto expectedMessageObserverStatus = std::get<2>(messageResponseMap[messageNum]);
-            ASSERT_EQ(messageObservers[messageNum]->m_status.getValue(), expectedMessageObserverStatus);
+            auto& item = messageResponseMap[messageNum];
+            auto responseCode = std::get<0>(item);
+            auto responseFinished = std::get<1>(item);
+            auto expectedMessageObserverStatus = std::get<2>(item);
+            ASSERT_EQ(messageObservers[messageNum]->m_status.getValue(), expectedMessageObserverStatus)
+                << "messageNum=" << messageNum << " responseCode=" << responseCode
+                << " responseFinished=" << responseFinished
+                << " expectedMessageObserverStatus=" << expectedMessageObserverStatus;
         }
     }
 }

ADSL/CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 project(ADSL LANGUAGES CXX)
 
-include(../build/BuildDefaults.cmake)
+include(${AVS_CMAKE_BUILD}/BuildDefaults.cmake)
 
 add_subdirectory("src")
 add_subdirectory("test")

AFML/CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 project(AFML LANGUAGES CXX)
 
-include(../build/BuildDefaults.cmake)
+include(${AVS_CMAKE_BUILD}/BuildDefaults.cmake)
 
 add_subdirectory("src")
 add_subdirectory("test")