Merge pull request #17 from Plutoberth/feat/gentleErrors

Feat/gentle errors

Client/BluetoothWrapper.cpp

@@ -24,7 +24,7 @@ BluetoothWrapper& BluetoothWrapper::operator=(BluetoothWrapper&& other) noexcept
 
 int BluetoothWrapper::sendCommand(const std::vector<char>& bytes)
 {
-	std::lock_guard guard(this->_wrapperMtx);
+	std::lock_guard guard(this->_connectorMtx);
 	auto data = CommandSerializer::packageDataForBt(bytes, DATA_TYPE::DATA_MDR, this->_seqNumber++);
 	auto bytesSent = this->_connector->send(data.data(), data.size());
 
@@ -33,21 +33,20 @@ int BluetoothWrapper::sendCommand(const std::vector<char>& bytes)
 	return bytesSent;
 }
 
-bool BluetoothWrapper::isConnected()
+bool BluetoothWrapper::isConnected() noexcept
 {
-	std::lock_guard guard(this->_wrapperMtx);
 	return this->_connector->isConnected();
 }
 
 void BluetoothWrapper::connect(const std::string& addr)
 {
-	std::lock_guard guard(this->_wrapperMtx);
+	std::lock_guard guard(this->_connectorMtx);
 	this->_connector->connect(addr);
 }
 
-void BluetoothWrapper::disconnect()
+void BluetoothWrapper::disconnect() noexcept
 {
-	std::lock_guard guard(this->_wrapperMtx);
+	std::lock_guard guard(this->_connectorMtx);
 	this->_seqNumber = 0;
 	this->_connector->disconnect();
 }
@@ -77,7 +76,7 @@ void BluetoothWrapper::_waitForAck()
 			{
 				if (ongoingMessage)
 				{
-					throw std::runtime_error("Invalid: Multiple start markers without an end marker");
+					throw RecoverableException("Invalid: Multiple start markers without an end marker", true);
 				}
 				messageStart = i + 1;
 				ongoingMessage = true;

Client/BluetoothWrapper.h

@@ -23,17 +23,17 @@ class BluetoothWrapper
 
 	int sendCommand(const std::vector<char>& bytes);
 
-	bool isConnected();
+	bool isConnected() noexcept;
 	//Try to connect to the headphones
 	void connect(const std::string& addr);
-	void disconnect();
+	void disconnect() noexcept;
 
 	std::vector<BluetoothDevice> getConnectedDevices();
 
 private:
 	void _waitForAck();
 
 	std::unique_ptr<IBluetoothConnector> _connector;
-	std::mutex _wrapperMtx;
+	std::mutex _connectorMtx;
 	unsigned int _seqNumber = 0;
 };

Client/Client.vcxproj

@@ -213,7 +213,7 @@
     <ClInclude Include="imgui\imstb_rectpack.h" />
     <ClInclude Include="imgui\imstb_textedit.h" />
     <ClInclude Include="imgui\imstb_truetype.h" />
-    <ClInclude Include="RecoverableException.h" />
+    <ClInclude Include="Exceptions.h" />
     <ClInclude Include="SingleInstanceFuture.h" />
     <ClInclude Include="TimedMessageQueue.h" />
     <ClInclude Include="WindowsBluetoothConnector.h" />

Client/Client.vcxproj.filters

@@ -63,6 +63,9 @@
     <ClCompile Include="TimedMessageQueue.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="CascadiaCodeFont.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="WindowsBluetoothConnector.h">
@@ -113,14 +116,17 @@
     <ClInclude Include="imgui\imconfig.h">
       <Filter>Header Files\IMGUI</Filter>
     </ClInclude>
-    <ClInclude Include="RecoverableException.h">
-      <Filter>Header Files</Filter>
-    </ClInclude>
     <ClInclude Include="TimedMessageQueue.h">
       <Filter>Header Files</Filter>
     </ClInclude>
     <ClInclude Include="SingleInstanceFuture.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="CascadiaCodeFont.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
+    <ClInclude Include="Exceptions.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>
 </Project>

Client/CommandSerializer.cpp

@@ -108,7 +108,7 @@ namespace CommandSerializer
 		ret.reserve(toEscape.capacity());
 		toEscape.push_back(static_cast<unsigned char>(dataType));
 		toEscape.push_back(seqNumber);
-		auto retSize = intToBytesBE(src.size());
+		auto retSize = intToBytesBE(static_cast<unsigned int>(src.size()));
 		//Insert data size
 		toEscape.insert(toEscape.end(), retSize.begin(), retSize.end());
 		//Insert command data
@@ -148,7 +148,7 @@ namespace CommandSerializer
 		ret.seqNumber = src[1];
 		if (src[src.size() - 1] != _sumChecksum(src.data(), src.size() - 1))
 		{
-			throw std::runtime_error("Invalid Checksum!");
+			throw RecoverableException("Invalid checksum!", true);
 		}
 		return ret;
 	}

Client/CommandSerializer.h

@@ -4,6 +4,7 @@
 #include <cstddef>
 #include <vector>
 #include <stdexcept>
+#include "Exceptions.h"
 
 constexpr unsigned int MINIMUM_VOICE_FOCUS_STEP = 2;
 

Client/RecoverableException.h → Client/Exceptions.h

@@ -7,5 +7,6 @@
 class RecoverableException : public std::runtime_error {
 public:
 	//I: what - This string WILL BE SHOWN TO THE USER.
-	RecoverableException(const std::string& what) : std::runtime_error(what) {}
+	RecoverableException(const std::string& what, bool shouldDisconnect = false) : std::runtime_error(what), shouldDisconnect(shouldDisconnect) {}
+	bool shouldDisconnect = false;
 };

Client/GUI_Impls/CrossPlatformGUI.cpp

@@ -41,7 +41,8 @@ void CrossPlatformGUI::_drawErrors()
 	//There's a slight race condition here but I don't care, it'd only be for one frame.
 	if (this->_mq.begin() != this->_mq.end())
 	{
-		ImGui::Text("Errors:");
+		ImGui::Text("Errors");
+		ImGui::Text("----------------------------");
 		for (auto&& message : this->_mq)
 		{
 			ImGui::Text(message.message.c_str());
@@ -89,6 +90,10 @@ void CrossPlatformGUI::_drawDeviceDiscovery()
 					}
 					catch (const RecoverableException& exc)
 					{
+						if (exc.shouldDisconnect)
+						{
+							this->_bt.disconnect();
+						}
 						this->_mq.addMessage(exc.what());
 					}
 				}
@@ -121,6 +126,10 @@ void CrossPlatformGUI::_drawDeviceDiscovery()
 					}
 					catch (const RecoverableException& exc)
 					{
+						if (exc.shouldDisconnect)
+						{
+							this->_bt.disconnect();
+						}
 						this->_mq.addMessage(exc.what());
 					}
 				}
@@ -133,6 +142,7 @@ void CrossPlatformGUI::_drawDeviceDiscovery()
 			{
 				if (ImGui::Button("Refresh devices"))
 				{
+					selectedDevice = -1;
 					this->_connectedDevicesFuture.setFromAsync([this]() { return this->_bt.getConnectedDevices(); });
 				}
 			}
@@ -146,6 +156,8 @@ void CrossPlatformGUI::_drawASMControls()
 	static bool sentFocusOnVoice = focusOnVoice;
 	static int asmLevel = 0;
 	static int sentAsmLevel = asmLevel;
+	//Don't show if the command only takes a few frames to send
+	static int commandLinger = 0;
 
 	if (ImGui::CollapsingHeader("Ambient Sound Mode   ", ImGuiTreeNodeFlags_DefaultOpen))
 	{
@@ -163,39 +175,53 @@ void CrossPlatformGUI::_drawASMControls()
 		{
 			ImGui::Text("Focus on Voice isn't enabled on this level.");
 		}
-
 
-		if (sentAsmLevel != asmLevel || sentFocusOnVoice != focusOnVoice)
+		if (this->_sendCommandFuture.ready())
 		{
-			if (!this->_sendCommandFuture.valid())
+			commandLinger = 0;
+			try
 			{
-				auto ncAsmEffect = sliderActive ? NC_ASM_EFFECT::ADJUSTMENT_IN_PROGRESS : NC_ASM_EFFECT::ADJUSTMENT_COMPLETION;
-				auto asmId = focusOnVoice ? ASM_ID::VOICE : ASM_ID::NORMAL;
-
-				this->_sendCommandFuture.setFromAsync([=]() {
-					return this->_bt.sendCommand(CommandSerializer::serializeNcAndAsmSetting(
-						ncAsmEffect,
-						NC_ASM_SETTING_TYPE::LEVEL_ADJUSTMENT,
-						ASM_SETTING_TYPE::LEVEL_ADJUSTMENT,
-						asmId,
-						asmLevel
-					));
-				});
-				sentAsmLevel = asmLevel;
-				sentFocusOnVoice = focusOnVoice;
+				this->_sendCommandFuture.get();
 			}
-			else if (this->_sendCommandFuture.ready())
+			catch (const RecoverableException& exc)
 			{
-				try
-				{
-					this->_sendCommandFuture.get();
-				}
-				catch (const RecoverableException& exc)
+				std::string excString;
+				//We kinda have to do it here and not in the wrapper, due to async causing timing issues. To fix it, the messagequeue can be made
+				//static, but I'm not sure if I wanna do that.
+				if (exc.shouldDisconnect)
 				{
-					this->_mq.addMessage(exc.what());
+					this->_bt.disconnect();
+					excString = "Disconnected due to: ";
 				}
+				this->_mq.addMessage(excString + exc.what());
 			}
 		}
+		//This means that we're waiting
+		else if (this->_sendCommandFuture.valid()) 
+		{
+			if (commandLinger++ > FPS / 10)
+			{
+				ImGui::Text("Sending command %c", "|/-\\"[(int)(ImGui::GetTime() / 0.05f) & 3]);
+			}
+		}
+		//We're not waiting, and there's no command in the air, so we can evaluate sending a new command
+		else if (sentAsmLevel != asmLevel || sentFocusOnVoice != focusOnVoice) 
+		{
+			auto ncAsmEffect = sliderActive ? NC_ASM_EFFECT::ADJUSTMENT_IN_PROGRESS : NC_ASM_EFFECT::ADJUSTMENT_COMPLETION;
+			auto asmId = focusOnVoice ? ASM_ID::VOICE : ASM_ID::NORMAL;
+
+			this->_sendCommandFuture.setFromAsync([=]() {
+				return this->_bt.sendCommand(CommandSerializer::serializeNcAndAsmSetting(
+					ncAsmEffect,
+					NC_ASM_SETTING_TYPE::LEVEL_ADJUSTMENT,
+					ASM_SETTING_TYPE::LEVEL_ADJUSTMENT,
+					asmId,
+					asmLevel
+				));
+			});
+			sentAsmLevel = asmLevel;
+			sentFocusOnVoice = focusOnVoice;
+		}
 	}
 }
 

Client/GUI_Impls/CrossPlatformGUI.h

@@ -5,14 +5,17 @@
 #include "IBluetoothConnector.h"
 #include "BluetoothWrapper.h"
 #include "CommandSerializer.h"
-#include "RecoverableException.h"
+#include "Exceptions.h"
 #include "TimedMessageQueue.h"
 #include "SingleInstanceFuture.h"
 #include "CascadiaCodeFont.h"
 
 #include <future>
 
 constexpr auto GUI_MAX_MESSAGES = 10;
+constexpr auto FPS = 60;
+constexpr auto GUI_HEIGHT = 350;
+constexpr auto GUI_WIDTH = 540;
 
 //This class should be constructed after AFTER the Dear ImGUI context is initialized.
 class CrossPlatformGUI

Client/GUI_Impls/WindowsGUI.cpp

@@ -23,7 +23,7 @@ void EnterGUIMainLoop(BluetoothWrapper bt)
 	WNDCLASSEX wc = { sizeof(WNDCLASSEX), CS_CLASSDC, WindowsGUIInternal::WndProc, 0L, 0L, GetModuleHandle(NULL), NULL, NULL, NULL, NULL, APP_NAME_W, NULL };
 	::RegisterClassEx(&wc);
 	//TODO: pass window data (size, name, etc) as params and autoscale
-	HWND hwnd = ::CreateWindowW(wc.lpszClassName, APP_NAME_W, WS_OVERLAPPEDWINDOW, 100, 100, 540, 335, NULL, NULL, wc.hInstance, NULL);
+	HWND hwnd = ::CreateWindowW(wc.lpszClassName, APP_NAME_W, WS_OVERLAPPEDWINDOW, 100, 100, GUI_WIDTH, GUI_HEIGHT, NULL, NULL, wc.hInstance, NULL);
 
 	// Initialize Direct3D
 	if (!WindowsGUIInternal::CreateDeviceD3D(hwnd))

Client/IBluetoothConnector.h

@@ -1,7 +1,7 @@
 #pragma once
 #include <string>
 #include <vector>
-#include "RecoverableException.h"
+#include "Exceptions.h"
 
 inline constexpr auto SONY_UUID = "96CC203E-5068-46ad-B32D-E316F5E069BA";
 
@@ -14,6 +14,14 @@ struct BluetoothDevice
 	std::string mac;
 };
 
+/*
+General notes: Please look at the implementation of WindowsBluetoothConnector.
+* Functions should throw RecoverableExceptions if they're indeed recoverable, and throw std::runtime_error otherwise.
+* RecoverableException can force a disconnection of the socket with an additional param (a call to disconnect()).
+* See notes below
+*/
+
+//Thread-safety: IBluetoothConnector implementations don't need to be thread safe, except calls to isConnected;
 class IBluetoothConnector
 {
 public:
@@ -23,14 +31,19 @@ class IBluetoothConnector
 	IBluetoothConnector(const IBluetoothConnector&) = delete;
 	IBluetoothConnector& operator=(const IBluetoothConnector&) = delete;
 
+	//send, recv and connect can block.
 	//O: The number of bytes sent.
 	virtual int send(char* buf, size_t length) noexcept(false) = 0;
 	virtual int recv(char* buf, size_t length) noexcept(false) = 0;
 	virtual void connect(const std::string& addrStr) noexcept(false) = 0;
-	virtual void disconnect() noexcept(false) = 0;
+
+	//This function should not block.
+	virtual void disconnect() noexcept = 0;
 
 	//Cost directive: This function must be as cheap as possible.
+	//!!! Thread-safety: This function must be thread safe. !!!
 	virtual bool isConnected() noexcept = 0;
 
+	//getConnectedDevices can block.
 	virtual std::vector<BluetoothDevice> getConnectedDevices() = 0;
 };