From 72946a40f6734dd7ecd7f3cd418b5c649cd492e0 Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Sat, 25 Mar 2017 16:50:54 -0700 Subject: [PATCH 01/10] SI_DeviceGBA: clarify request-response state machine Inspired by "#5147: GBASockServer: remove m_device_number (fixes warning)," trying to wrap my head around how this file works. --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 92 +++++++++++++------------ Source/Core/Core/HW/SI/SI_DeviceGBA.h | 15 ++-- 2 files changed, 56 insertions(+), 51 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index e2e3fe0223..302be66d00 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -239,31 +239,28 @@ void GBASockServer::Send(const u8* si_buffer) if (!GetAvailableSock(m_client)) return; - for (size_t i = 0; i < m_send_data.size(); i++) - m_send_data[i] = si_buffer[i ^ 3]; - - m_cmd = (u8)m_send_data[0]; + std::array send_data; + for (size_t i = 0; i < send_data.size(); i++) + send_data[i] = si_buffer[i ^ 3]; + m_cmd = send_data[0]; #ifdef _DEBUG - NOTICE_LOG(SERIALINTERFACE, "%01d cmd %02x [> %02x%02x%02x%02x]", m_device_number, - (u8)m_send_data[0], (u8)m_send_data[1], (u8)m_send_data[2], (u8)m_send_data[3], - (u8)m_send_data[4]); + NOTICE_LOG(SERIALINTERFACE, "%01d cmd %02x [> %02x%02x%02x%02x]", m_device_number, send_data[0], + send_data[1], send_data[2], send_data[3], send_data[4]); #endif m_client->setBlocking(false); sf::Socket::Status status; if (m_cmd == CMD_WRITE) - status = m_client->send(m_send_data.data(), m_send_data.size()); + status = m_client->send(send_data.data(), send_data.size()); else - status = m_client->send(m_send_data.data(), 1); + status = m_client->send(send_data.data(), 1); if (m_cmd != CMD_STATUS) m_booted = true; if (status == sf::Socket::Disconnected) Disconnect(); - - m_time_cmd_sent = CoreTiming::GetTicks(); } int GBASockServer::Receive(u8* si_buffer) @@ -272,21 +269,17 @@ int GBASockServer::Receive(u8* si_buffer) if (!GetAvailableSock(m_client)) return 5; - size_t num_received = 0; - u64 transfer_time = GetTransferTime((u8)m_send_data[0]); - bool block = (CoreTiming::GetTicks() - m_time_cmd_sent) > transfer_time; - if (m_cmd == CMD_STATUS && !m_booted) - block = false; - - if (block) + if (m_booted) { sf::SocketSelector selector; selector.add(*m_client); selector.wait(sf::milliseconds(1000)); } + size_t num_received = 0; + std::array recv_data; sf::Socket::Status recv_stat = - m_client->receive(m_recv_data.data(), m_recv_data.size(), num_received); + m_client->receive(recv_data.data(), recv_data.size(), num_received); if (recv_stat == sf::Socket::Disconnected) { Disconnect(); @@ -296,31 +289,33 @@ int GBASockServer::Receive(u8* si_buffer) if (recv_stat == sf::Socket::NotReady) num_received = 0; - if (num_received > m_recv_data.size()) - num_received = m_recv_data.size(); + if (num_received > recv_data.size()) + num_received = recv_data.size(); if (num_received > 0) { #ifdef _DEBUG - if ((u8)m_send_data[0] == 0x00 || (u8)m_send_data[0] == 0xff) + if (m_cmd == CMD_STATUS || m_cmd == CMD_RESET) { WARN_LOG(SERIALINTERFACE, "%01d [< %02x%02x%02x%02x%02x] (%zu)", - m_device_number, (u8)m_recv_data[0], (u8)m_recv_data[1], (u8)m_recv_data[2], - (u8)m_recv_data[3], (u8)m_recv_data[4], num_received); + m_device_number, recv_data[0], recv_data[1], recv_data[2], recv_data[3], + recv_data[4], num_received); } else { ERROR_LOG(SERIALINTERFACE, "%01d [< %02x%02x%02x%02x%02x] (%zu)", - m_device_number, (u8)m_recv_data[0], (u8)m_recv_data[1], (u8)m_recv_data[2], - (u8)m_recv_data[3], (u8)m_recv_data[4], num_received); + m_device_number, recv_data[0], recv_data[1], recv_data[2], recv_data[3], + recv_data[4], num_received); } #endif - for (size_t i = 0; i < m_recv_data.size(); i++) - si_buffer[i ^ 3] = m_recv_data[i]; + for (size_t i = 0; i < recv_data.size(); i++) + { + si_buffer[i ^ 3] = recv_data[i]; + } } - return (int)num_received; + return static_cast(num_received); } CSIDevice_GBA::CSIDevice_GBA(SIDevices device, int device_number) @@ -335,38 +330,45 @@ CSIDevice_GBA::~CSIDevice_GBA() int CSIDevice_GBA::RunBuffer(u8* buffer, int length) { - if (!m_waiting_for_response) + switch (m_next_action) + { + case NextAction::SendCommand: { - for (size_t i = 0; i < m_send_data.size(); i++) - m_send_data[i] = buffer[i ^ 3]; - - m_num_data_received = 0; ClockSync(); Send(buffer); + m_last_cmd = buffer[3]; m_timestamp_sent = CoreTiming::GetTicks(); - m_waiting_for_response = true; + m_next_action = NextAction::WaitTransferTime; } - if (m_waiting_for_response && m_num_data_received == 0) + // [[fallthrough]] + case NextAction::WaitTransferTime: { - m_num_data_received = Receive(buffer); + int elapsed_time = static_cast(CoreTiming::GetTicks() - m_timestamp_sent); + // Tell SI to ask again after TransferInterval() cycles + if (GetTransferTime(m_last_cmd) > elapsed_time) + return 0; + m_next_action = NextAction::ReceiveResponse; } - if ((GetTransferTime(m_send_data[0])) > (int)(CoreTiming::GetTicks() - m_timestamp_sent)) + // [[fallthrough]] + case NextAction::ReceiveResponse: { - return 0; + int num_data_received = Receive(buffer); + if (num_data_received > 0) + m_next_action = NextAction::SendCommand; + return num_data_received; } - else - { - if (m_num_data_received != 0) - m_waiting_for_response = false; - return m_num_data_received; } + + // This should never happen, but appease MSVC which thinks it might. + ERROR_LOG(SERIALINTERFACE, "Unknown state %i\n", static_cast(m_next_action)); + return 0; } int CSIDevice_GBA::TransferInterval() { - return GetTransferTime(m_send_data[0]); + return GetTransferTime(m_last_cmd); } bool CSIDevice_GBA::GetData(u32& hi, u32& low) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.h b/Source/Core/Core/HW/SI/SI_DeviceGBA.h index fd0cc5251c..6c27d6956d 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.h +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.h @@ -36,10 +36,7 @@ public: private: std::unique_ptr m_client; std::unique_ptr m_clock_sync; - std::array m_send_data{}; - std::array m_recv_data{}; - u64 m_time_cmd_sent = 0; u64 m_last_time_slice = 0; int m_device_number; u8 m_cmd = 0; @@ -58,9 +55,15 @@ public: void SendCommand(u32 command, u8 poll) override; private: - std::array m_send_data{}; - int m_num_data_received = 0; + enum class NextAction + { + SendCommand, + WaitTransferTime, + ReceiveResponse + }; + + NextAction m_next_action = NextAction::SendCommand; + u8 m_last_cmd; u64 m_timestamp_sent = 0; - bool m_waiting_for_response = false; }; } // namespace SerialInterface From 751377256b475d54e25c8cc23672b43052f5e534 Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Fri, 31 Mar 2017 14:06:26 -0700 Subject: [PATCH 02/10] Move GBASockServer logging into SI_DeviceGBA --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 73 ++++++++++++++----------- Source/Core/Core/HW/SI/SI_DeviceGBA.h | 7 +-- 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index 302be66d00..b9949f274d 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -168,7 +168,7 @@ static bool GetNextClock(std::unique_ptr& sock_to_fill) return sock_filled; } -GBASockServer::GBASockServer(int device_number) : m_device_number{device_number} +GBASockServer::GBASockServer() { if (!s_connection_thread.joinable()) s_connection_thread = std::thread(GBAConnectionWaiter); @@ -233,32 +233,38 @@ void GBASockServer::ClockSync() } } +bool GBASockServer::Connect() +{ + if (!IsConnected()) + GetAvailableSock(m_client); + return IsConnected(); +} + +bool GBASockServer::IsConnected() +{ + return static_cast(m_client); +} + void GBASockServer::Send(const u8* si_buffer) { - if (!m_client) - if (!GetAvailableSock(m_client)) - return; + if (!Connect()) + return; std::array send_data; for (size_t i = 0; i < send_data.size(); i++) send_data[i] = si_buffer[i ^ 3]; - m_cmd = send_data[0]; -#ifdef _DEBUG - NOTICE_LOG(SERIALINTERFACE, "%01d cmd %02x [> %02x%02x%02x%02x]", m_device_number, send_data[0], - send_data[1], send_data[2], send_data[3], send_data[4]); -#endif + u8 cmd = send_data[0]; + if (cmd != CMD_STATUS) + m_booted = true; m_client->setBlocking(false); sf::Socket::Status status; - if (m_cmd == CMD_WRITE) + if (cmd == CMD_WRITE) status = m_client->send(send_data.data(), send_data.size()); else status = m_client->send(send_data.data(), 1); - if (m_cmd != CMD_STATUS) - m_booted = true; - if (status == sf::Socket::Disconnected) Disconnect(); } @@ -294,32 +300,14 @@ int GBASockServer::Receive(u8* si_buffer) if (num_received > 0) { -#ifdef _DEBUG - if (m_cmd == CMD_STATUS || m_cmd == CMD_RESET) - { - WARN_LOG(SERIALINTERFACE, "%01d [< %02x%02x%02x%02x%02x] (%zu)", - m_device_number, recv_data[0], recv_data[1], recv_data[2], recv_data[3], - recv_data[4], num_received); - } - else - { - ERROR_LOG(SERIALINTERFACE, "%01d [< %02x%02x%02x%02x%02x] (%zu)", - m_device_number, recv_data[0], recv_data[1], recv_data[2], recv_data[3], - recv_data[4], num_received); - } -#endif - for (size_t i = 0; i < recv_data.size(); i++) - { si_buffer[i ^ 3] = recv_data[i]; - } } return static_cast(num_received); } -CSIDevice_GBA::CSIDevice_GBA(SIDevices device, int device_number) - : ISIDevice(device, device_number), GBASockServer(device_number) +CSIDevice_GBA::CSIDevice_GBA(SIDevices device, int device_number) : ISIDevice(device, device_number) { } @@ -335,7 +323,14 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) case NextAction::SendCommand: { ClockSync(); - Send(buffer); + if (Connect()) + { +#ifdef _DEBUG + NOTICE_LOG(SERIALINTERFACE, "%01d cmd %02x [> %02x%02x%02x%02x]", m_device_number, buffer[3], + buffer[2], buffer[1], buffer[0], buffer[7]); +#endif + Send(buffer); + } m_last_cmd = buffer[3]; m_timestamp_sent = CoreTiming::GetTicks(); m_next_action = NextAction::WaitTransferTime; @@ -355,6 +350,18 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) case NextAction::ReceiveResponse: { int num_data_received = Receive(buffer); + if (IsConnected()) + { +#ifdef _DEBUG + LogTypes::LOG_LEVELS log_level = (m_last_cmd == CMD_STATUS || m_last_cmd == CMD_RESET) ? + LogTypes::LERROR : + LogTypes::LWARNING; + GENERIC_LOG(LogTypes::SERIALINTERFACE, log_level, + "%01d [< %02x%02x%02x%02x%02x] (%i)", + m_device_number, buffer[3], buffer[2], buffer[1], buffer[0], buffer[7], + num_data_received); +#endif + } if (num_data_received > 0) m_next_action = NextAction::SendCommand; return num_data_received; diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.h b/Source/Core/Core/HW/SI/SI_DeviceGBA.h index 6c27d6956d..446684f3cd 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.h +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.h @@ -23,13 +23,14 @@ void GBAConnectionWaiter_Shutdown(); class GBASockServer { public: - explicit GBASockServer(int device_number); + GBASockServer(); ~GBASockServer(); void Disconnect(); + bool Connect(); + bool IsConnected(); void ClockSync(); - void Send(const u8* si_buffer); int Receive(u8* si_buffer); @@ -38,8 +39,6 @@ private: std::unique_ptr m_clock_sync; u64 m_last_time_slice = 0; - int m_device_number; - u8 m_cmd = 0; bool m_booted = false; }; From 89ca32daa62033c1fd88169d6be6897b15b354cd Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Fri, 31 Mar 2017 14:07:28 -0700 Subject: [PATCH 03/10] GBASockServer: cleanup GetNextSock and GetNextClock --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 42 ++++++++++--------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index b9949f274d..382fadd749 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -136,36 +136,26 @@ void GBAConnectionWaiter_Shutdown() s_connection_thread.join(); } -static bool GetAvailableSock(std::unique_ptr& sock_to_fill) +template +static std::unique_ptr MoveFromFront(std::queue>& ptrs) { - bool sock_filled = false; - - std::lock_guard lk(s_cs_gba); - - if (!s_waiting_socks.empty()) - { - sock_to_fill = std::move(s_waiting_socks.front()); - s_waiting_socks.pop(); - sock_filled = true; - } - - return sock_filled; + if (ptrs.empty()) + return nullptr; + std::unique_ptr ptr = std::move(ptrs.front()); + ptrs.pop(); + return ptr; } -static bool GetNextClock(std::unique_ptr& sock_to_fill) +static std::unique_ptr GetNextSock() { - bool sock_filled = false; + std::lock_guard lk(s_cs_gba); + return MoveFromFront(s_waiting_socks); +} +static std::unique_ptr GetNextClock() +{ std::lock_guard lk(s_cs_gba_clk); - - if (!s_waiting_clocks.empty()) - { - sock_to_fill = std::move(s_waiting_clocks.front()); - s_waiting_clocks.pop(); - sock_filled = true; - } - - return sock_filled; + return MoveFromFront(s_waiting_clocks); } GBASockServer::GBASockServer() @@ -201,7 +191,7 @@ void GBASockServer::Disconnect() void GBASockServer::ClockSync() { if (!m_clock_sync) - if (!GetNextClock(m_clock_sync)) + if (!(m_clock_sync = GetNextClock())) return; u32 time_slice = 0; @@ -236,7 +226,7 @@ void GBASockServer::ClockSync() bool GBASockServer::Connect() { if (!IsConnected()) - GetAvailableSock(m_client); + m_client = GetNextSock(); return IsConnected(); } From d30e0ea28e286805afeec650bc3370dc96c10b2c Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Fri, 31 Mar 2017 14:09:13 -0700 Subject: [PATCH 04/10] GBASockServer: clean up Receive() return value --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index 382fadd749..83676f39a5 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -262,8 +262,7 @@ void GBASockServer::Send(const u8* si_buffer) int GBASockServer::Receive(u8* si_buffer) { if (!m_client) - if (!GetAvailableSock(m_client)) - return 5; + return 0; if (m_booted) { @@ -279,22 +278,18 @@ int GBASockServer::Receive(u8* si_buffer) if (recv_stat == sf::Socket::Disconnected) { Disconnect(); - return 5; + return 0; } if (recv_stat == sf::Socket::NotReady) num_received = 0; - if (num_received > recv_data.size()) - num_received = recv_data.size(); - if (num_received > 0) { for (size_t i = 0; i < recv_data.size(); i++) si_buffer[i ^ 3] = recv_data[i]; } - - return static_cast(num_received); + return static_cast(std::min(num_received, recv_data.size())); } CSIDevice_GBA::CSIDevice_GBA(SIDevices device, int device_number) : ISIDevice(device, device_number) @@ -352,6 +347,10 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) num_data_received); #endif } + else + { + num_data_received = 5; + } if (num_data_received > 0) m_next_action = NextAction::SendCommand; return num_data_received; From e6cfc3a75b75b875fedec2c5715cc79f21f44965 Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Fri, 31 Mar 2017 14:21:39 -0700 Subject: [PATCH 05/10] SI_DeviceGBA: clean up GetTransferTime() --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 21 ++++++--------------- Source/Core/Core/HW/SI/SI_DeviceGBA.h | 2 -- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index 83676f39a5..bdfcd39d6e 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -29,7 +29,7 @@ std::queue> s_waiting_socks; std::queue> s_waiting_clocks; std::mutex s_cs_gba; std::mutex s_cs_gba_clk; -u8 s_num_connected; +int s_num_connected; Common::Flag s_server_running; } @@ -41,21 +41,12 @@ enum EJoybusCmds CMD_WRITE = 0x15 }; -const u64 BITS_PER_SECOND = 115200; -const u64 BYTES_PER_SECOND = BITS_PER_SECOND / 8; - -u8 GetNumConnected() -{ - int num_ports_connected = s_num_connected; - if (num_ports_connected == 0) - num_ports_connected = 1; - - return num_ports_connected; -} +constexpr auto BITS_PER_SECOND = 115200; +constexpr auto BYTES_PER_SECOND = BITS_PER_SECOND / 8; // --- GameBoy Advance "Link Cable" --- -int GetTransferTime(u8 cmd) +static int GetTransferTime(u8 cmd) { u64 bytes_transferred = 0; @@ -83,8 +74,8 @@ int GetTransferTime(u8 cmd) break; } } - return (int)(bytes_transferred * SystemTimers::GetTicksPerSecond() / - (GetNumConnected() * BYTES_PER_SECOND)); + return static_cast(bytes_transferred * SystemTimers::GetTicksPerSecond() / + (std::max(s_num_connected, 1) * BYTES_PER_SECOND)); } static void GBAConnectionWaiter() diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.h b/Source/Core/Core/HW/SI/SI_DeviceGBA.h index 446684f3cd..e3819edf88 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.h +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.h @@ -16,8 +16,6 @@ namespace SerialInterface { -u8 GetNumConnected(); -int GetTransferTime(u8 cmd); void GBAConnectionWaiter_Shutdown(); class GBASockServer From ff78327643920a0592d2322cba7ecc700166e636 Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Fri, 31 Mar 2017 14:12:38 -0700 Subject: [PATCH 06/10] SI_DeviceGBA: remove duplication of GBASockServer destructor logic --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 5 ----- Source/Core/Core/HW/SI/SI_DeviceGBA.h | 5 ++--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index bdfcd39d6e..8e13b03d04 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -287,11 +287,6 @@ CSIDevice_GBA::CSIDevice_GBA(SIDevices device, int device_number) : ISIDevice(de { } -CSIDevice_GBA::~CSIDevice_GBA() -{ - GBASockServer::Disconnect(); -} - int CSIDevice_GBA::RunBuffer(u8* buffer, int length) { switch (m_next_action) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.h b/Source/Core/Core/HW/SI/SI_DeviceGBA.h index e3819edf88..92d7637ed5 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.h +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.h @@ -24,8 +24,6 @@ public: GBASockServer(); ~GBASockServer(); - void Disconnect(); - bool Connect(); bool IsConnected(); void ClockSync(); @@ -33,6 +31,8 @@ public: int Receive(u8* si_buffer); private: + void Disconnect(); + std::unique_ptr m_client; std::unique_ptr m_clock_sync; @@ -44,7 +44,6 @@ class CSIDevice_GBA : public ISIDevice, private GBASockServer { public: CSIDevice_GBA(SIDevices device, int device_number); - ~CSIDevice_GBA(); int RunBuffer(u8* buffer, int length) override; int TransferInterval() override; From 05ab03a5515c97eff89368963170bfaa0cc9a90b Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Fri, 31 Mar 2017 14:13:48 -0700 Subject: [PATCH 07/10] SI_DeviceGBA: make GBASockServer a member instead of parent --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 10 +++++----- Source/Core/Core/HW/SI/SI_DeviceGBA.h | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index 8e13b03d04..add3b11f7c 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -293,14 +293,14 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) { case NextAction::SendCommand: { - ClockSync(); - if (Connect()) + m_sock_server.ClockSync(); + if (m_sock_server.Connect()) { #ifdef _DEBUG NOTICE_LOG(SERIALINTERFACE, "%01d cmd %02x [> %02x%02x%02x%02x]", m_device_number, buffer[3], buffer[2], buffer[1], buffer[0], buffer[7]); #endif - Send(buffer); + m_sock_server.Send(buffer); } m_last_cmd = buffer[3]; m_timestamp_sent = CoreTiming::GetTicks(); @@ -320,8 +320,8 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) // [[fallthrough]] case NextAction::ReceiveResponse: { - int num_data_received = Receive(buffer); - if (IsConnected()) + int num_data_received = m_sock_server.Receive(buffer); + if (m_sock_server.IsConnected()) { #ifdef _DEBUG LogTypes::LOG_LEVELS log_level = (m_last_cmd == CMD_STATUS || m_last_cmd == CMD_RESET) ? diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.h b/Source/Core/Core/HW/SI/SI_DeviceGBA.h index 92d7637ed5..6e1467b88d 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.h +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.h @@ -40,7 +40,7 @@ private: bool m_booted = false; }; -class CSIDevice_GBA : public ISIDevice, private GBASockServer +class CSIDevice_GBA : public ISIDevice { public: CSIDevice_GBA(SIDevices device, int device_number); @@ -58,6 +58,7 @@ private: ReceiveResponse }; + GBASockServer m_sock_server; NextAction m_next_action = NextAction::SendCommand; u8 m_last_cmd; u64 m_timestamp_sent = 0; From 8dee8e6494fd19f9b92a91076cf35643d948f855 Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Sat, 1 Apr 2017 14:18:58 -0700 Subject: [PATCH 08/10] SI_DeviceGBA: extract magic numbers SEND_MAX_SIZE, RECV_MAX_SIZE --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index add3b11f7c..1222f88aa4 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -43,6 +43,7 @@ enum EJoybusCmds constexpr auto BITS_PER_SECOND = 115200; constexpr auto BYTES_PER_SECOND = BITS_PER_SECOND / 8; +constexpr auto SEND_MAX_SIZE = 5, RECV_MAX_SIZE = 5; // --- GameBoy Advance "Link Cable" --- @@ -231,7 +232,7 @@ void GBASockServer::Send(const u8* si_buffer) if (!Connect()) return; - std::array send_data; + std::array send_data; for (size_t i = 0; i < send_data.size(); i++) send_data[i] = si_buffer[i ^ 3]; @@ -263,7 +264,7 @@ int GBASockServer::Receive(u8* si_buffer) } size_t num_received = 0; - std::array recv_data; + std::array recv_data; sf::Socket::Status recv_stat = m_client->receive(recv_data.data(), recv_data.size(), num_received); if (recv_stat == sf::Socket::Disconnected) @@ -335,7 +336,7 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) } else { - num_data_received = 5; + num_data_received = RECV_MAX_SIZE; } if (num_data_received > 0) m_next_action = NextAction::SendCommand; From f004dfa92be2801514437670ec81e99bbc50580a Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Wed, 17 May 2017 14:39:41 -0700 Subject: [PATCH 09/10] SI_DeviceGBA: use SI_ERROR_NO_RESPONSE when client isn't connected Slight behavior change, but fills a gaping hole in the state logic. --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 32 +++++++++++++++---------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index 1222f88aa4..c771a7556b 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -5,6 +5,7 @@ #include "Core/HW/SI/SI_DeviceGBA.h" #include +#include #include #include #include @@ -303,6 +304,12 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) #endif m_sock_server.Send(buffer); } + else + { + constexpr u32 reply = SI_ERROR_NO_RESPONSE; + std::memcpy(buffer, &reply, sizeof(reply)); + return sizeof(reply); + } m_last_cmd = buffer[3]; m_timestamp_sent = CoreTiming::GetTicks(); m_next_action = NextAction::WaitTransferTime; @@ -322,22 +329,21 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) case NextAction::ReceiveResponse: { int num_data_received = m_sock_server.Receive(buffer); - if (m_sock_server.IsConnected()) + if (!m_sock_server.IsConnected()) { + m_next_action = NextAction::SendCommand; + constexpr u32 reply = SI_ERROR_NO_RESPONSE; + std::memcpy(buffer, &reply, sizeof(reply)); + return sizeof(reply); + } #ifdef _DEBUG - LogTypes::LOG_LEVELS log_level = (m_last_cmd == CMD_STATUS || m_last_cmd == CMD_RESET) ? - LogTypes::LERROR : - LogTypes::LWARNING; - GENERIC_LOG(LogTypes::SERIALINTERFACE, log_level, - "%01d [< %02x%02x%02x%02x%02x] (%i)", - m_device_number, buffer[3], buffer[2], buffer[1], buffer[0], buffer[7], - num_data_received); + LogTypes::LOG_LEVELS log_level = (m_last_cmd == CMD_STATUS || m_last_cmd == CMD_RESET) ? + LogTypes::LERROR : + LogTypes::LWARNING; + GENERIC_LOG(LogTypes::SERIALINTERFACE, log_level, + "%01d [< %02x%02x%02x%02x%02x] (%i)", m_device_number, + buffer[3], buffer[2], buffer[1], buffer[0], buffer[7], num_data_received); #endif - } - else - { - num_data_received = RECV_MAX_SIZE; - } if (num_data_received > 0) m_next_action = NextAction::SendCommand; return num_data_received; From becb1a744b762c5ee166b46ab8904e3236c7127b Mon Sep 17 00:00:00 2001 From: Michael Maltese Date: Thu, 13 Jul 2017 17:06:38 -0700 Subject: [PATCH 10/10] SI_DeviceGBA: if a client doesn't respond within 1s, disconnect them Rather than returning 0 / not creating an expected SI interrupt. You can test this by running VBA-M in a debugger and stopping it while it's connected to Dolphin: on current master, Dolphin will freeze-up until it gets a response. With this PR, Dolphin will gracefully disconnect the device, and reconnect if it starts responding again. --- Source/Core/Core/HW/SI/SI_DeviceGBA.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp index c771a7556b..e7e20a9fc2 100644 --- a/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp +++ b/Source/Core/Core/HW/SI/SI_DeviceGBA.cpp @@ -274,14 +274,14 @@ int GBASockServer::Receive(u8* si_buffer) return 0; } - if (recv_stat == sf::Socket::NotReady) - num_received = 0; - - if (num_received > 0) + if (recv_stat == sf::Socket::NotReady || num_received == 0) { - for (size_t i = 0; i < recv_data.size(); i++) - si_buffer[i ^ 3] = recv_data[i]; + m_booted = false; + return 0; } + + for (size_t i = 0; i < recv_data.size(); i++) + si_buffer[i ^ 3] = recv_data[i]; return static_cast(std::min(num_received, recv_data.size())); } @@ -329,9 +329,9 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) case NextAction::ReceiveResponse: { int num_data_received = m_sock_server.Receive(buffer); - if (!m_sock_server.IsConnected()) + m_next_action = NextAction::SendCommand; + if (num_data_received == 0) { - m_next_action = NextAction::SendCommand; constexpr u32 reply = SI_ERROR_NO_RESPONSE; std::memcpy(buffer, &reply, sizeof(reply)); return sizeof(reply); @@ -344,8 +344,6 @@ int CSIDevice_GBA::RunBuffer(u8* buffer, int length) "%01d [< %02x%02x%02x%02x%02x] (%i)", m_device_number, buffer[3], buffer[2], buffer[1], buffer[0], buffer[7], num_data_received); #endif - if (num_data_received > 0) - m_next_action = NextAction::SendCommand; return num_data_received; } }