Fix TWS battery, rfcomm crash, quality/speed selector, equalizer fetch and macOS icon

Client/BluetoothWrapper.cpp

@@ -1,5 +1,10 @@
 #include "BluetoothWrapper.h"
 
+// How many frames one transaction will read before giving up. The device interleaves ACKs and
+// unsolicited notifications with the reply we're after, so a handful of unrelated frames is normal;
+// this only bounds a device that never sends what we asked for.
+static constexpr int MAX_FRAMES_PER_TRANSACTION = 16;
+
 BluetoothWrapper::BluetoothWrapper(std::unique_ptr<IBluetoothConnector> connector)
 {
 	this->_connector.swap(connector);
@@ -72,7 +77,7 @@ Buffer BluetoothWrapper::sendCommandAndReadResponse(const std::vector<char>& byt
 	this->_connector->send(data.data(), data.size());
 
 	// The device replies with an ACK and then the RET frame; unrelated notifications may interleave.
-	for (int i = 0; i < 16; i++)
+	for (int i = 0; i < MAX_FRAMES_PER_TRANSACTION; i++)
 	{
 		auto msg = this->_readMessage();
 		if (msg.dataType == DATA_TYPE::ACK)
@@ -92,8 +97,22 @@ Buffer BluetoothWrapper::sendCommandAndReadResponse(const std::vector<char>& byt
 
 void BluetoothWrapper::_waitForAck()
 {
-	auto msg = this->_readMessage();
-	this->_seqNumber = msg.seqNumber;
+	// The device answers a command with an ACK, but it also pushes unsolicited notifications of its own
+	// (a state change, a press of the headset's own button) that can arrive first. Reading exactly one
+	// frame and assuming it was the ACK left the response stream one frame behind for the rest of the
+	// session, and took _seqNumber from the wrong frame - the device dedupes on that sequence number, so
+	// it then silently ignored the next command while the UI snapped back to the old value.
+	// Skipped notifications are still acked back to the device by _readMessage().
+	for (int i = 0; i < MAX_FRAMES_PER_TRANSACTION; i++)
+	{
+		auto msg = this->_readMessage();
+		if (msg.dataType == DATA_TYPE::ACK)
+		{
+			this->_seqNumber = msg.seqNumber;
+			return;
+		}
+	}
+	throw RecoverableException("No ack received from device", true);
 }
 
 CommandSerializer::Message BluetoothWrapper::_readMessage()
@@ -119,18 +138,24 @@ CommandSerializer::Message BluetoothWrapper::_readMessage()
 			numRecvd = this->_connector->recv(buf, sizeof(buf));
 		}
 
-		size_t messageStart = 0;
-		size_t messageEnd = numRecvd;
-
-		for (size_t i = 0; i < numRecvd; i++)
+		// Every 60/61/62 inside a frame is escaped, so a START_MARKER can only ever begin one. That makes
+		// resyncing unambiguous, which is what matters after a brief link glitch garbles or truncates a
+		// chunk: bytes before the first START_MARKER are the tail of a frame we can no longer parse, and
+		// a second START_MARKER means the frame we were collecting never finished. Both cases resync on
+		// the new marker here. Previously the first silently concatenated garbage into the message and
+		// the second threw, in either case leaving the parser stranded mid-stream for the rest of the
+		// session - every command after that failed and the UI kept snapping back to the old value.
+		size_t messageStart = ongoingMessage ? 0 : static_cast<size_t>(numRecvd);
+		size_t messageEnd = static_cast<size_t>(numRecvd);
+
+		for (size_t i = 0; i < static_cast<size_t>(numRecvd); i++)
 		{
 			if (buf[i] == START_MARKER)
 			{
-				if (ongoingMessage)
-				{
-					throw RecoverableException("Invalid: Multiple start markers without an end marker", true);
-				}
+				// Whatever we had collected was a truncated frame, not the start of this one.
+				msgBytes.clear();
 				messageStart = i + 1;
+				messageEnd = static_cast<size_t>(numRecvd);
 				ongoingMessage = true;
 			}
 			else if (ongoingMessage && buf[i] == END_MARKER)
@@ -139,13 +164,23 @@ CommandSerializer::Message BluetoothWrapper::_readMessage()
 				ongoingMessage = false;
 				messageFinished = true;
 				// A single recv() can return more than one framed message back-to-back; keep whatever's
-				// past this message's END_MARKER for the next _waitForAck() call instead of dropping it.
+				// past this message's END_MARKER for the next read instead of dropping it.
 				this->_leftoverBytes.assign(buf + i + 1, buf + numRecvd);
 				break;
 			}
 		}
 
-		msgBytes.insert(msgBytes.end(), buf + messageStart, buf + messageEnd);
+		if (messageStart < messageEnd)
+		{
+			msgBytes.insert(msgBytes.end(), buf + messageStart, buf + messageEnd);
+		}
+
+		// Bail out on a stream that keeps delivering bytes but never an END_MARKER, so the next read can
+		// resync on the next START_MARKER instead of growing this buffer forever.
+		if (msgBytes.size() > MAX_BLUETOOTH_MESSAGE_SIZE)
+		{
+			throw RecoverableException("Invalid: message exceeded the maximum size without an end marker", true);
+		}
 	} while (!messageFinished);
 
 	auto msg = CommandSerializer::unpackBtMessage(msgBytes);

Client/Constants.h

@@ -33,15 +33,48 @@ namespace V2Command
 {
 	inline constexpr unsigned char INIT_REQUEST = 0x00; // payload: 00 00
 	inline constexpr unsigned char INIT_REPLY   = 0x01; // reply: 01 ... (8 bytes total => v2 device)
-	inline constexpr unsigned char BATTERY_GET  = 0x22; // payload: 22 <type: 00=single>
-	inline constexpr unsigned char BATTERY_RET  = 0x23; // reply: 23 <type> <level 0-100> <charging 0/1>
-	inline constexpr unsigned char BATTERY_NTFY = 0x25; // notify: 25 <type> <level> <charging>
+	inline constexpr unsigned char BATTERY_GET  = 0x22; // payload: 22 <sub-type>
+	inline constexpr unsigned char BATTERY_RET  = 0x23; // reply: 23 <sub-type> <level 0-100> <charging 0/1>
+	inline constexpr unsigned char BATTERY_NTFY = 0x25; // notify: 25 <sub-type> <level> <charging>
+	// Battery inquiry sub-types (payload byte 1). A device answers only the ones it actually has, so
+	// these have to be probed. TWS earbuds report DUAL (or DUAL2 on the WF-C5xx/C700N line) plus CASE;
+	// everything else reports SINGLE. Values and the DUAL byte layout match GadgetBridge's
+	// SonyProtocolImplV2.encodeBatteryType() / handleBattery().
+	inline constexpr unsigned char BATTERY_SUB_SINGLE = 0x00; // RET 23 00 <level> <charging>
+	inline constexpr unsigned char BATTERY_SUB_DUAL2  = 0x01; // WF-C500 / WF-C510 / WF-C700N
+	inline constexpr unsigned char BATTERY_SUB_DUAL   = 0x09; // WF-1000XM4 / WF-1000XM5 / LinkBuds
+	inline constexpr unsigned char BATTERY_SUB_CASE   = 0x0a; // RET 23 0a <level> <charging>
+	// DUAL/DUAL2 reply: 23 <sub> <Llvl> <Lchg> <Rlvl> <Rchg>. A level of 0 means that earbud isn't
+	// reporting (docked in the case / powered down) rather than "empty" - treat it as unknown.
+	//
+	// The reply can be LONGER than the reference layout, so size checks must be lower bounds, never
+	// equality. Captured from a WF-1000XM5 (fw 6.1.0), both earbuds out of the case and full:
+	//   send 22 09  ->  23 09 64 00 64 00 64 64   (8 bytes; documented layout is 6)
+	//   send 22 0a  ->  23 0a 64 00 1e            (5 bytes; documented layout is 4)
+	// Confirmed on hardware by docking one earbud and watching the frames change:
+	//   right earbud in the case  ->  23 09 64 00 00 00 64 64   (byte [4] = R went 0x64 -> 0x00)
+	//   right earbud back out     ->  23 09 64 00 64 00 64 64
+	// So bytes [2]/[4] really are the L/R levels, as in the reference layout. The trailing bytes stayed
+	// fixed (0x64 0x64 on 22 09, 0x1e on 22 0a) across that change, which rules them out as per-earbud
+	// levels but leaves their meaning unknown - don't reuse them without a sample that moves them.
 	inline constexpr unsigned char EQ_GET       = 0x56; // payload: 56 00
 	inline constexpr unsigned char EQ_RET       = 0x57; // reply: 57 00 <preset> 06 <bass+10> <b1..b5 +10>
 	inline constexpr unsigned char EQ_SET       = 0x58; // preset: 58 00 <preset> 00 ; custom: 58 00 A0 06 <bass+10> <b1..b5 +10>
-	inline constexpr unsigned char DSEE_GET     = 0xe6; // payload: e6 01
-	inline constexpr unsigned char DSEE_RET     = 0xe7; // reply: e7 01 <enabled 0/1>
-	inline constexpr unsigned char DSEE_SET     = 0xe8; // payload: e8 01 <enabled 0/1>
+	// AUDIO parameter family. The opcode trio is shared by every audio parameter; the sub-type byte
+	// (payload[1]) picks which one, so DSEE and the sound-quality mode differ only in that byte.
+	inline constexpr unsigned char AUDIO_GET    = 0xe6; // GET e6 <sub>      -> RET e7 <sub> <value>
+	inline constexpr unsigned char AUDIO_RET    = 0xe7;
+	inline constexpr unsigned char AUDIO_SET    = 0xe8; // SET e8 <sub> <value>
+	inline constexpr unsigned char DSEE_GET     = AUDIO_GET; // payload: e6 01
+	inline constexpr unsigned char DSEE_RET     = AUDIO_RET; // reply: e7 01 <enabled 0/1>
+	inline constexpr unsigned char DSEE_SET     = AUDIO_SET; // payload: e8 01 <enabled 0/1>
+	// AUDIO sub-types (Sony's AudioInquiredType). Only the ones we actually send are listed.
+	inline constexpr unsigned char SUB_CONNECTION_MODE      = 0x00; // sound quality mode, plain
+	inline constexpr unsigned char SUB_UPSCALING            = 0x01; // DSEE
+	inline constexpr unsigned char SUB_CONNECTION_MODE_LDAC = 0x02; // same, on LDAC-capable models
+	// Deliberately NOT sent: 0x05 (CONNECTION_MODE_CLASSIC_AUDIO_LE_AUDIO). Its SET carries a fourth byte
+	// that also switches the device between Classic and LE Audio, which would drop the link for reasons
+	// the user didn't ask for. Devices that only answer 0x05 simply report the feature as unsupported.
 	inline constexpr unsigned char FW_GET       = 0x04; // payload: 04 02  -> RET 05 02 <ascii version...>
 	inline constexpr unsigned char FW_RET       = 0x05;
 	inline constexpr unsigned char CODEC_GET    = 0x12; // payload: 12 02  -> RET 13 02 <codec>
@@ -57,6 +90,15 @@ namespace V2Command
 	inline constexpr unsigned char SUB_SPEAK_TO_CHAT   = 0x0c;
 }
 
+// Sound quality mode ("Bluetooth connection quality" in the Sony app) - the value byte of the
+// AUDIO CONNECTION_MODE sub-types. Sony's PriorMode enum also has LOW_LATENCY_PRIOR_BETA (0x02), which
+// no shipping model exposes in its app, so we don't offer it.
+enum class PRIOR_MODE : unsigned char
+{
+	SOUND_QUALITY = 0x00,
+	STABLE_CONNECTION = 0x01
+};
+
 // v2 equalizer preset ids (byte value sent/received at EQ payload[2]).
 enum class EQ_PRESET : unsigned char
 {

Client/CrossPlatformGUI.cpp

@@ -4,6 +4,7 @@
 #include <vector>
 #include <algorithm>
 #include <cctype>
+#include <cstdio>
 
 #define STB_IMAGE_IMPLEMENTATION
 #define STBI_ONLY_PNG
@@ -50,6 +51,7 @@ bool CrossPlatformGUI::performGUIPass()
 				this->_drawEqualizer();
 				this->_drawDsee();
 			}
+			this->_drawSoundQualityMode();
 			this->_drawOptionalFeatures();
 			if (!this->_isV2())
 				this->_drawSurroundControls();
@@ -192,6 +194,8 @@ void CrossPlatformGUI::_pumpConnectionState()
 			try { this->_headphones.requestAmbientState(); } catch (const std::exception&) {}
 			if (this->_isV2())
 			{
+				// Lets requestBattery() probe the per-earbud layout first on TWS models (WF-*/LinkBuds).
+				this->_headphones.setDeviceName(this->_connectedDevice.name);
 				try { this->_headphones.requestBattery(); } catch (const std::exception&) {}
 				try { this->_headphones.requestEqualizer(); } catch (const std::exception&) {}
 				try { this->_headphones.requestDsee(); } catch (const std::exception&) {}
@@ -223,6 +227,7 @@ void CrossPlatformGUI::_pumpConnectionState()
 		this->_uiAutoPowerOff = this->_headphones.getAutoPowerOff();
 		this->_uiSpeakToChat = this->_headphones.getSpeakToChat();
 		this->_uiAdaptiveVolume = this->_headphones.getAdaptiveVolume();
+		this->_uiPrioritizeSoundQuality = this->_headphones.getSoundQualityMode() == PRIOR_MODE::SOUND_QUALITY;
 	}
 
 	// Poll the button-changeable ASM state so the app reflects changes made on the headphone itself.
@@ -262,6 +267,7 @@ void CrossPlatformGUI::_syncUIFromHeadphones()
 	this->_uiAutoPowerOff = this->_headphones.getAutoPowerOff();
 	this->_uiSpeakToChat = this->_headphones.getSpeakToChat();
 	this->_uiAdaptiveVolume = this->_headphones.getAdaptiveVolume();
+	this->_uiPrioritizeSoundQuality = this->_headphones.getSoundQualityMode() == PRIOR_MODE::SOUND_QUALITY;
 }
 
 std::string CrossPlatformGUI::_resourceBase()
@@ -357,9 +363,19 @@ void CrossPlatformGUI::_drawStatusHeader()
 
 	if (this->_headphones.hasDualBattery())
 	{
-		ImGui::Text("Battery   L %d%%    R %d%%", this->_headphones.getBatteryLeft(), this->_headphones.getBatteryRight());
+		// An earbud that isn't reporting (docked in the case / powered down) reads -1; show a dash.
+		auto budText = [](int level, bool charging) {
+			char buf[32];
+			if (level < 0) snprintf(buf, sizeof(buf), "--");
+			else snprintf(buf, sizeof(buf), "%d%%%s", level, charging ? "+" : "");
+			return std::string(buf);
+		};
+		ImGui::Text("Battery   L %s    R %s",
+			budText(this->_headphones.getBatteryLeft(), this->_headphones.isBatteryLeftCharging()).c_str(),
+			budText(this->_headphones.getBatteryRight(), this->_headphones.isBatteryRightCharging()).c_str());
 		if (this->_headphones.getBatteryCase() >= 0)
-			ImGui::Text("Case      %d%%", this->_headphones.getBatteryCase());
+			ImGui::Text("Case      %d%%%s", this->_headphones.getBatteryCase(),
+				this->_headphones.isBatteryCaseCharging() ? "  (charging)" : "");
 	}
 	else
 	{
@@ -447,6 +463,9 @@ void CrossPlatformGUI::_drawEqualizer()
 		{
 			this->_uiEqPreset = presets[i].val;
 			int p = presets[i].val;
+			// setEqualizerPreset() re-reads the bands that go with the new preset (the device restores the
+			// stored custom curve for Manual), so pull them into the sliders once the command lands.
+			this->_eqBandsNeedSync = true;
 			this->_sendFeatureCommand([this, p]() { this->_headphones.setEqualizerPreset((EQ_PRESET)p); });
 		}
 		if (sel) ImGui::PopStyleColor();
@@ -492,6 +511,31 @@ void CrossPlatformGUI::_drawDsee()
 	this->_endCard();
 }
 
+void CrossPlatformGUI::_drawSoundQualityMode()
+{
+	if (!this->_headphones.hasSoundQualityMode())
+		return;
+
+	if (!this->_beginCard("##sqmode")) { this->_endCard(); return; }
+	this->_cardTitle("Bluetooth Connection");
+
+	if (this->_headphones.hasCodec())
+		ImGui::TextDisabled("Current codec: %s", this->_headphones.getCodec().c_str());
+
+	int mode = this->_uiPrioritizeSoundQuality ? 0 : 1;
+	ImGui::SetNextItemWidth(ImGui::GetContentRegionAvail().x);
+	if (ImGui::Combo("##sqprio", &mode, "Prioritize Sound Quality\0Prioritize Stable Connection\0\0"))
+	{
+		this->_uiPrioritizeSoundQuality = (mode == 0);
+		PRIOR_MODE v = mode == 0 ? PRIOR_MODE::SOUND_QUALITY : PRIOR_MODE::STABLE_CONNECTION;
+		this->_sendFeatureCommand([this, v]() { this->_headphones.setSoundQualityMode(v); });
+	}
+
+	ImGui::TextDisabled("The headphones reconnect to apply this.");
+
+	this->_endCard();
+}
+
 void CrossPlatformGUI::_drawOptionalFeatures()
 {
 	const bool any = this->_headphones.hasAutoPowerOff() || this->_headphones.hasSpeakToChat() ||
@@ -565,6 +609,15 @@ void CrossPlatformGUI::_sendPendingASMChanges()
 		try { this->_featureCommandFuture.get(); }
 		catch (const RecoverableException& e) { if (e.shouldDisconnect) this->_bt.disconnect(); this->_mq.addMessage(e.what()); }
 		catch (const std::exception& e) { this->_mq.addMessage(e.what()); }
+
+		if (this->_eqBandsNeedSync)
+		{
+			this->_eqBandsNeedSync = false;
+			this->_uiEqPreset = (int)(unsigned char)this->_headphones.getEqualizerPreset();
+			for (int i = 0; i < 5; ++i)
+				this->_uiEqBands[i] = this->_headphones.getEqualizerBand(i);
+			this->_uiClearBass = this->_headphones.getClearBass();
+		}
 	}
 
 	if (this->_sendCommandFuture.valid() && this->_sendCommandFuture.ready())

Client/CrossPlatformGUI.h

@@ -44,6 +44,7 @@ class CrossPlatformGUI
 	void _drawASMControls();
 	void _drawEqualizer();
 	void _drawDsee();
+	void _drawSoundQualityMode();
 	void _drawOptionalFeatures();
 	void _drawSurroundControls();
 
@@ -90,10 +91,14 @@ class CrossPlatformGUI
 	int _uiEqPreset = 0;
 	std::array<int, 5> _uiEqBands = { 0, 0, 0, 0, 0 };
 	int _uiClearBass = 0;
+	// Set when a preset write is in flight, so the band sliders take the values the device reports for
+	// that preset once it completes instead of keeping the previous preset's.
+	bool _eqBandsNeedSync = false;
 	bool _uiDsee = false;
 	int _uiAutoPowerOff = 0;
 	bool _uiSpeakToChat = false;
 	bool _uiAdaptiveVolume = false;
+	bool _uiPrioritizeSoundQuality = true;
 	int _uiSoundPosition = 0;
 	int _uiVptType = 0;