From 544d6cbe52015a73f036bc2f40789dfaf3ab137e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 15:43:33 -0500 Subject: [PATCH 1/6] InputCommon/DualShockUDPClient: Add missing header guard Prevents potential inclusion issues from occurring. --- .../ControllerInterface/DualShockUDPClient/DualShockUDPClient.h | 2 ++ .../ControllerInterface/DualShockUDPClient/DualShockUDPProto.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.h b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.h index b4bb1210c6..e694cb622d 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.h +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.h @@ -2,6 +2,8 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#pragma once + #include "Common/Config/Config.h" namespace ciface::DualShockUDPClient diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h index 45980bcd5d..475cf98966 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h @@ -2,6 +2,8 @@ // Licensed under GPLv2+ // Refer to the license.txt file included. +#pragma once + #include #include From 4488719a761243bf6de95b6608d6ef6a780b7bd0 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 15:51:25 -0500 Subject: [PATCH 2/6] InputCommon/DualShockUDPClient: In-class initialize members where applicable Deduplicates members within the constructor's initializer list. --- .../DualShockUDPClient/DualShockUDPClient.cpp | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp index 9ed62e5285..b98e9d42bd 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp @@ -96,16 +96,17 @@ public: private: const Proto::DsModel m_model; const int m_index; - u32 m_client_uid; + u32 m_client_uid = Common::Random::GenerateValue(); sf::UdpSocket m_socket; - Common::DVec3 m_accel; - Common::DVec3 m_gyro; - std::chrono::steady_clock::time_point m_next_reregister; - Proto::MessageType::PadDataResponse m_pad_data; - Proto::Touch m_prev_touch; - bool m_prev_touch_valid; - int m_touch_x; - int m_touch_y; + Common::DVec3 m_accel{}; + Common::DVec3 m_gyro{}; + std::chrono::steady_clock::time_point m_next_reregister = + std::chrono::steady_clock::time_point::min(); + Proto::MessageType::PadDataResponse m_pad_data{}; + Proto::Touch m_prev_touch{}; + bool m_prev_touch_valid = false; + int m_touch_x = 0; + int m_touch_y = 0; }; using MathUtil::GRAVITY_ACCELERATION; @@ -297,11 +298,7 @@ void DeInit() StopHotplugThread(); } -Device::Device(Proto::DsModel model, int index) - : m_model(model), m_index(index), - m_client_uid(Common::Random::GenerateValue()), m_accel{}, m_gyro{}, - m_next_reregister(std::chrono::steady_clock::time_point::min()), m_pad_data{}, m_prev_touch{}, - m_prev_touch_valid(false), m_touch_x(0), m_touch_y(0) +Device::Device(Proto::DsModel model, int index) : m_model{model}, m_index{index} { m_socket.setBlocking(false); From 67097b45742b08faf9b3440f70e2871b5aecd95e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 15:55:11 -0500 Subject: [PATCH 3/6] InputCommon/DualShockUDPClient: Relocate settings to top of source file This is a small namespace, so we can move it to the top of the file to get it out of the way of everything else. --- .../DualShockUDPClient/DualShockUDPClient.cpp | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp index b98e9d42bd..7af3bc17ae 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp @@ -24,6 +24,19 @@ namespace ciface::DualShockUDPClient { +namespace Settings +{ +constexpr char DEFAULT_SERVER_ADDRESS[] = "127.0.0.1"; +constexpr u16 DEFAULT_SERVER_PORT = 26760; + +const Config::ConfigInfo SERVER_ENABLED{ + {Config::System::DualShockUDPClient, "Server", "Enabled"}, false}; +const Config::ConfigInfo SERVER_ADDRESS{ + {Config::System::DualShockUDPClient, "Server", "IPAddress"}, DEFAULT_SERVER_ADDRESS}; +const Config::ConfigInfo SERVER_PORT{{Config::System::DualShockUDPClient, "Server", "Port"}, + DEFAULT_SERVER_PORT}; +} // namespace Settings + class Device : public Core::Device { private: @@ -110,22 +123,10 @@ private: }; using MathUtil::GRAVITY_ACCELERATION; -static constexpr char DEFAULT_SERVER_ADDRESS[] = "127.0.0.1"; -static constexpr u16 DEFAULT_SERVER_PORT = 26760; -static constexpr auto SERVER_REREGISTER_INTERVAL = std::chrono::seconds{1}; -static constexpr auto SERVER_LISTPORTS_INTERVAL = std::chrono::seconds{1}; -static constexpr int TOUCH_X_AXIS_MAX = 1000; -static constexpr int TOUCH_Y_AXIS_MAX = 500; - -namespace Settings -{ -const Config::ConfigInfo SERVER_ENABLED{ - {Config::System::DualShockUDPClient, "Server", "Enabled"}, false}; -const Config::ConfigInfo SERVER_ADDRESS{ - {Config::System::DualShockUDPClient, "Server", "IPAddress"}, DEFAULT_SERVER_ADDRESS}; -const Config::ConfigInfo SERVER_PORT{{Config::System::DualShockUDPClient, "Server", "Port"}, - DEFAULT_SERVER_PORT}; -} // namespace Settings +constexpr auto SERVER_REREGISTER_INTERVAL = std::chrono::seconds{1}; +constexpr auto SERVER_LISTPORTS_INTERVAL = std::chrono::seconds{1}; +constexpr int TOUCH_X_AXIS_MAX = 1000; +constexpr int TOUCH_Y_AXIS_MAX = 500; static bool s_server_enabled; static std::string s_server_address; From 278d03f737fe3bf1e4ca2b9b1ea33bbd7e94aa8f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 16:07:12 -0500 Subject: [PATCH 4/6] InputCommon/DualShockUDPClient: Make use of std::array where applicable Provides the same semantics of a C array, but is much nicer to work with. Notably, it makes all cases of performing comparisons with said arrays significantly less reading-involved. --- .../DualShockUDPClient/DualShockUDPClient.cpp | 33 ++++++++++--------- .../DualShockUDPClient/DualShockUDPProto.h | 32 +++++++++--------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp index 7af3bc17ae..6183b69049 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp @@ -4,8 +4,11 @@ #include "InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.h" +#include +#include #include -#include +#include +#include #include #include @@ -18,7 +21,6 @@ #include "Common/Random.h" #include "Common/Thread.h" #include "Core/CoreTiming.h" -#include "Core/HW/SystemTimers.h" #include "InputCommon/ControllerInterface/ControllerInterface.h" #include "InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h" @@ -136,16 +138,15 @@ static std::chrono::steady_clock::time_point s_next_listports; static std::thread s_hotplug_thread; static Common::Flag s_hotplug_thread_running; static std::mutex s_port_info_mutex; -static Proto::MessageType::PortInfo s_port_info[Proto::PORT_COUNT]; +static std::array s_port_info; static sf::UdpSocket s_socket; static bool IsSameController(const Proto::MessageType::PortInfo& a, const Proto::MessageType::PortInfo& b) { // compare everything but battery_status - return a.pad_id == b.pad_id && a.pad_state == b.pad_state && a.model == b.model && - a.connection_type == b.connection_type && - memcmp(a.pad_mac_address, b.pad_mac_address, sizeof a.pad_mac_address) == 0; + return std::tie(a.pad_id, a.pad_state, a.model, a.connection_type, a.pad_mac_address) == + std::tie(b.pad_id, b.pad_state, b.model, b.connection_type, b.pad_mac_address); } static sf::Socket::Status ReceiveWithTimeout(sf::UdpSocket& socket, void* data, std::size_t size, @@ -176,10 +177,7 @@ static void HotplugThreadFunc() Proto::Message msg(s_client_uid); auto& list_ports = msg.m_message; list_ports.pad_request_count = 4; - list_ports.pad_id[0] = 0; - list_ports.pad_id[1] = 1; - list_ports.pad_id[2] = 2; - list_ports.pad_id[3] = 3; + list_ports.pad_id = {0, 1, 2, 3}; msg.Finish(); if (s_socket.send(&list_ports, sizeof list_ports, s_server_address, s_server_port) != sf::Socket::Status::Done) @@ -246,10 +244,10 @@ static void Restart() s_client_uid = Common::Random::GenerateValue(); s_next_listports = std::chrono::steady_clock::time_point::min(); - for (int port_index = 0; port_index < Proto::PORT_COUNT; port_index++) + for (size_t port_index = 0; port_index < s_port_info.size(); port_index++) { s_port_info[port_index] = {}; - s_port_info[port_index].pad_id = port_index; + s_port_info[port_index].pad_id = static_cast(port_index); } PopulateDevices(); // remove devices @@ -286,11 +284,14 @@ void PopulateDevices() [](const auto* dev) { return dev->GetSource() == "DSUClient"; }); std::lock_guard lock(s_port_info_mutex); - for (int port_index = 0; port_index < Proto::PORT_COUNT; port_index++) + for (size_t port_index = 0; port_index < s_port_info.size(); port_index++) { - Proto::MessageType::PortInfo port_info = s_port_info[port_index]; - if (port_info.pad_state == Proto::DsState::Connected) - g_controller_interface.AddDevice(std::make_shared(port_info.model, port_index)); + const Proto::MessageType::PortInfo& port_info = s_port_info[port_index]; + if (port_info.pad_state != Proto::DsState::Connected) + continue; + + g_controller_interface.AddDevice( + std::make_shared(port_info.model, static_cast(port_index))); } } diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h index 475cf98966..4932c22049 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPProto.h @@ -4,6 +4,8 @@ #pragma once +#include +#include #include #include @@ -17,12 +19,12 @@ namespace ciface::DualShockUDPClient::Proto // // WARNING: Little endian host assumed -static constexpr u16 CEMUHOOK_PROTOCOL_VERSION = 1001; +constexpr u16 CEMUHOOK_PROTOCOL_VERSION = 1001; -static constexpr int PORT_COUNT = 4; +constexpr int PORT_COUNT = 4; -static constexpr char CLIENT[] = "DSUC"; -static constexpr char SERVER[] = "DSUS"; +constexpr std::array CLIENT{'D', 'S', 'U', 'C'}; +constexpr std::array SERVER{'D', 'S', 'U', 'S'}; #pragma pack(push, 1) @@ -69,7 +71,7 @@ enum RegisterFlags : u8 struct MessageHeader { - u8 source[4]; + std::array source; u16 protocol_version; u16 message_length; // actually message size minus header size u32 crc32; @@ -101,7 +103,7 @@ struct VersionResponse MessageHeader header; u32 message_type; u16 max_protocol_version; - u8 padding[2]; + std::array padding; }; struct ListPorts @@ -111,7 +113,7 @@ struct ListPorts MessageHeader header; u32 message_type; u32 pad_request_count; - u8 pad_id[4]; + std::array pad_id; }; struct PortInfo @@ -124,7 +126,7 @@ struct PortInfo DsState pad_state; DsModel model; DsConnection connection_type; - u8 pad_mac_address[6]; + std::array pad_mac_address; DsBattery battery_status; u8 padding; }; @@ -137,7 +139,7 @@ struct PadDataRequest u32 message_type; RegisterFlags register_flags; u8 pad_id_to_register; - u8 mac_address_to_register[6]; + std::array mac_address_to_register; }; struct PadDataResponse @@ -150,7 +152,7 @@ struct PadDataResponse DsState pad_state; DsModel model; DsConnection connection_type; - u8 pad_mac_address[6]; + std::array pad_mac_address; DsBattery battery_status; u8 active; u32 hid_packet_counter; @@ -224,11 +226,11 @@ static inline u32 CRC32(const void* buffer, unsigned length) template struct Message { - Message() : m_message{} {} + Message() = default; - explicit Message(u32 source_uid) : m_message{} + explicit Message(u32 source_uid) { - memcpy((char*)m_message.header.source, MsgType::FROM, sizeof(m_message.header.source)); + m_message.header.source = MsgType::FROM; m_message.header.protocol_version = CEMUHOOK_PROTOCOL_VERSION; m_message.header.message_length = sizeof(*this) - sizeof(m_message.header); m_message.header.source_uid = source_uid; @@ -253,7 +255,7 @@ struct Message } if (m_message.header.protocol_version > CEMUHOOK_PROTOCOL_VERSION) return std::nullopt; - if (memcmp(m_message.header.source, ToMsgType::FROM, sizeof(m_message.header.source))) + if (m_message.header.source != ToMsgType::FROM) return std::nullopt; if (m_message.message_type != ToMsgType::TYPE) return std::nullopt; @@ -265,7 +267,7 @@ struct Message return tomsg; } - MsgType m_message; + MsgType m_message{}; }; #pragma pack(pop) From db9e59276556d1797d4a9b939434e4dbd8f86e40 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 16:20:48 -0500 Subject: [PATCH 5/6] InputCommon/DualShockUDPClient: Use deduction guides for lock_guard With C++17, we can use template deduction guides provided by the standard library. This allows the omission of the mutex type itself. --- .../DualShockUDPClient/DualShockUDPClient.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp index 6183b69049..6595a0193b 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp @@ -200,7 +200,7 @@ static void HotplugThreadFunc() { const bool port_changed = !IsSameController(*port_info, s_port_info[port_info->pad_id]); { - std::lock_guard lock(s_port_info_mutex); + std::lock_guard lock{s_port_info_mutex}; s_port_info[port_info->pad_id] = *port_info; } if (port_changed) @@ -283,7 +283,7 @@ void PopulateDevices() g_controller_interface.RemoveDevice( [](const auto* dev) { return dev->GetSource() == "DSUClient"; }); - std::lock_guard lock(s_port_info_mutex); + std::lock_guard lock{s_port_info_mutex}; for (size_t port_index = 0; port_index < s_port_info.size(); port_index++) { const Proto::MessageType::PortInfo& port_info = s_port_info[port_index]; From 334e2768f58f7076dc9517f7200de90fbe47b5b4 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Fri, 22 Nov 2019 16:49:43 -0500 Subject: [PATCH 6/6] InputCommon/DualShockUDPClient: Use an alias for the clock type Makes code slightly less verbose without exposing the whole chrono header to the current source file. --- .../DualShockUDPClient/DualShockUDPClient.cpp | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp index 6595a0193b..3651deeada 100644 --- a/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp +++ b/Source/Core/InputCommon/ControllerInterface/DualShockUDPClient/DualShockUDPClient.cpp @@ -39,6 +39,9 @@ const Config::ConfigInfo SERVER_PORT{{Config::System::DualShockUDPClient, " DEFAULT_SERVER_PORT}; } // namespace Settings +// Clock type used for querying timeframes +using SteadyClock = std::chrono::steady_clock; + class Device : public Core::Device { private: @@ -115,8 +118,7 @@ private: sf::UdpSocket m_socket; Common::DVec3 m_accel{}; Common::DVec3 m_gyro{}; - std::chrono::steady_clock::time_point m_next_reregister = - std::chrono::steady_clock::time_point::min(); + SteadyClock::time_point m_next_reregister = SteadyClock::time_point::min(); Proto::MessageType::PadDataResponse m_pad_data{}; Proto::Touch m_prev_touch{}; bool m_prev_touch_valid = false; @@ -134,7 +136,7 @@ static bool s_server_enabled; static std::string s_server_address; static u16 s_server_port; static u32 s_client_uid; -static std::chrono::steady_clock::time_point s_next_listports; +static SteadyClock::time_point s_next_listports; static std::thread s_hotplug_thread; static Common::Flag s_hotplug_thread_running; static std::mutex s_port_info_mutex; @@ -168,7 +170,7 @@ static void HotplugThreadFunc() while (s_hotplug_thread_running.IsSet()) { - const auto now = std::chrono::steady_clock::now(); + const auto now = SteadyClock::now(); if (now >= s_next_listports) { s_next_listports = now + SERVER_LISTPORTS_INTERVAL; @@ -185,16 +187,17 @@ static void HotplugThreadFunc() } // Receive controller port info + using namespace std::chrono; + using namespace std::chrono_literals; Proto::Message msg; - const auto timeout = s_next_listports - std::chrono::steady_clock::now(); + const auto timeout = s_next_listports - SteadyClock::now(); // ReceiveWithTimeout treats a timeout of zero as infinite timeout, which we don't want - auto timeout_ms = std::chrono::duration_cast(timeout).count(); - timeout_ms = std::max(timeout_ms, 1); + const auto timeout_ms = std::max(duration_cast(timeout), 1ms); std::size_t received_bytes; sf::IpAddress sender; u16 port; if (ReceiveWithTimeout(s_socket, &msg, sizeof(msg), received_bytes, sender, port, - sf::milliseconds(timeout_ms)) == sf::Socket::Status::Done) + sf::milliseconds(timeout_ms.count())) == sf::Socket::Status::Done) { if (auto port_info = msg.CheckAndCastTo()) { @@ -379,7 +382,7 @@ std::string Device::GetSource() const void Device::UpdateInput() { // Regularly tell the UDP server to feed us controller data - const auto now = std::chrono::steady_clock::now(); + const auto now = SteadyClock::now(); if (now >= m_next_reregister) { m_next_reregister = now + SERVER_REREGISTER_INTERVAL;