From 849fd7a975bb42421b075807a0ad94d4a4cf2ee3 Mon Sep 17 00:00:00 2001 From: Glenn Maynard Date: Tue, 1 Jan 2019 18:00:07 -0600 Subject: [PATCH] Simplify overlapped I/O handling, and add a command timeout. If we don't get a response from a command in a while, resend it. This doesn't normally happen (it only happened during firmware development), but it makes command sending more robust, so let's keep it. This also uses a single OVERLAPPED for a whole command, which is simpler. --- sdk/Windows/SMXDeviceConnection.cpp | 59 ++++++++++++++++++++++------- sdk/Windows/SMXDeviceConnection.h | 11 +++++- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/sdk/Windows/SMXDeviceConnection.cpp b/sdk/Windows/SMXDeviceConnection.cpp index 1c1f228..b36d11c 100644 --- a/sdk/Windows/SMXDeviceConnection.cpp +++ b/sdk/Windows/SMXDeviceConnection.cpp @@ -11,7 +11,11 @@ using namespace SMX; SMX::SMXDeviceConnection::PendingCommandPacket::PendingCommandPacket() { - memset(&m_OverlappedWrite, 0, sizeof(m_OverlappedWrite)); +} + +SMXDeviceConnection::PendingCommand::PendingCommand() +{ + memset(&m_Overlapped, 0, sizeof(m_Overlapped)); } shared_ptr SMXDeviceConnection::Create() @@ -114,6 +118,32 @@ bool SMX::SMXDeviceConnection::ReadPacket(string &out) void SMX::SMXDeviceConnection::CheckReads(wstring &error) { + if(m_pCurrentCommand) + { + // See if this command timed out. This doesn't happen often, so this is + // mostly just a failsafe. The controller takes a moment to initialize on + // startup, so we use a large enough timeout that this doesn't trigger on + // every connection. + double fSecondsAgo = SMX::GetMonotonicTime() - m_pCurrentCommand->m_fSentAt; + if(fSecondsAgo > 2.0f) + { + // If we didn't get a response in this long, we're not going to. Retry the + // command by cancelling its I/O and moving it back to the command queue. + // + // if we were delayed and the response is in the queue, we'll get out of sync + Log("Command timed out. Retrying..."); + CancelIoEx(m_hDevice->value(), &m_pCurrentCommand->m_Overlapped); + + // Block until the cancellation completes. This should happen quickly. + DWORD unused; + GetOverlappedResultEx(m_hDevice->value(), &m_pCurrentCommand->m_Overlapped, &unused, INFINITE, false); + + m_aPendingCommands.push_front(m_pCurrentCommand); + m_pCurrentCommand = nullptr; + Log("Command requeued"); + } + } + DWORD bytes; int result = GetOverlappedResult(m_hDevice->value(), &overlapped_read, &bytes, FALSE); if(result == 0) @@ -280,15 +310,13 @@ void SMX::SMXDeviceConnection::BeginAsyncRead(wstring &error) void SMX::SMXDeviceConnection::CheckWrites(wstring &error) { - if(m_pCurrentCommand && !m_pCurrentCommand->m_Packets.empty()) + if(m_pCurrentCommand) { - // A command is in progress. See if any writes have completed. - while(!m_pCurrentCommand->m_Packets.empty()) + // A command is in progress. See if its writes have completed. + if(m_pCurrentCommand->m_bWriting) { - shared_ptr pFirstPacket = m_pCurrentCommand->m_Packets.front(); - DWORD bytes; - int iResult = GetOverlappedResult(m_hDevice->value(), &pFirstPacket->m_OverlappedWrite, &bytes, FALSE); + int iResult = GetOverlappedResult(m_hDevice->value(), &m_pCurrentCommand->m_Overlapped, &bytes, FALSE); if(iResult == 0) { int windows_error = GetLastError(); @@ -297,17 +325,15 @@ void SMX::SMXDeviceConnection::CheckWrites(wstring &error) return; } - m_pCurrentCommand->m_Packets.pop_front(); + m_pCurrentCommand->m_bWriting = false; } - // Don't clear m_pCurrentCommand here. It'll stay set until we get a PACKET_FLAG_HOST_CMD_FINISHED // packet from the device, which tells us it's ready to receive another command. - } - - // Don't send packets if there's a command in progress. - if(m_pCurrentCommand) + // + // Don't send packets if there's a command in progress. return; + } // Stop if we have nothing to do. if(m_aPendingCommands.empty()) @@ -316,6 +342,9 @@ void SMX::SMXDeviceConnection::CheckWrites(wstring &error) // Send the next command. shared_ptr pPendingCommand = m_aPendingCommands.front(); + // Record the time. We can use this for timeouts. + pPendingCommand->m_fSentAt = SMX::GetMonotonicTime(); + for(shared_ptr &pPacket: pPendingCommand->m_Packets) { // In theory the API allows this to return success if the write completed successfully without needing to @@ -324,7 +353,7 @@ void SMX::SMXDeviceConnection::CheckWrites(wstring &error) // so this assumes all writes are async. DWORD unused; // Log(ssprintf("Write: %s", BinaryToHex(pPacket->sData).c_str())); - if(!WriteFile(m_hDevice->value(), pPacket->sData.data(), pPacket->sData.size(), &unused, &pPacket->m_OverlappedWrite)) + if(!WriteFile(m_hDevice->value(), pPacket->sData.data(), pPacket->sData.size(), &unused, &pPendingCommand->m_Overlapped)) { int windows_error = GetLastError(); if(windows_error != ERROR_IO_PENDING && windows_error != ERROR_IO_INCOMPLETE) @@ -335,6 +364,8 @@ void SMX::SMXDeviceConnection::CheckWrites(wstring &error) } } + pPendingCommand->m_bWriting = true; + // Remove this command and store it in m_pCurrentCommand, and we'll stop sending data until the command finishes. m_pCurrentCommand = pPendingCommand; m_aPendingCommands.pop_front(); diff --git a/sdk/Windows/SMXDeviceConnection.h b/sdk/Windows/SMXDeviceConnection.h index c096863..75a6452 100644 --- a/sdk/Windows/SMXDeviceConnection.h +++ b/sdk/Windows/SMXDeviceConnection.h @@ -87,13 +87,19 @@ private: PendingCommandPacket(); string sData; - OVERLAPPED m_OverlappedWrite; }; // Commands that are waiting to be sent: struct PendingCommand { + PendingCommand(); + list> m_Packets; + // The overlapped struct for writing this command's packets. m_bWriting is true + // if we're waiting for the write to complete. + OVERLAPPED m_Overlapped; + bool m_bWriting = false; + // This is only called if m_bWaitForResponse if true. Otherwise, we send the command // and forget about it. If the command has a response, it'll be in buf. function m_pComplete; @@ -101,6 +107,9 @@ private: // If true, once we send this command we won't send any other commands until we get // a response. bool m_bIsDeviceInfoCommand = false; + + // The SMX::GetMonotonicTime when we started sending this command. + double m_fSentAt = 0; }; list> m_aPendingCommands;