diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a535f99..6d32e779 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## 3.1.1 * Apple: a read, write or subscription the peripheral refuses now reports its ATT error as the matching `UniversalBleErrorCode` (`insufficientEncryption`, `insufficientAuthentication`, `readNotPermitted`, ...), as Android does, instead of `unknownError`. The error code is still in `details`. Codes in `CBErrorDomain` are unchanged: its numbers overlap ATT's with other meanings (#348). +* Windows: a read, write or subscription the peripheral refuses now reports its ATT error as the matching `UniversalBleErrorCode` (`insufficientEncryption`, `insufficientAuthentication`, `readNotPermitted`, ...) instead of the generic `failed`, as Android already does. Characteristic/descriptor reads and writes and `SetNotifiable` are covered; service/characteristic discovery at connect time is unchanged. Application-defined codes `0x80`-`0xFF` now report `unknownError`. The raw ATT error code is still in `details`. +* Linux: take the ATT error code from the BlueZ failure message only when it follows `ATT error:`, so an unrelated hex value in the message is no longer mistaken for the code. * Bump `bluez` to `^0.9.0`. * Bump the `pigeon` dev dependency to `^29.0.0`. * Regenerate the Pigeon bindings with Pigeon 29. The async platform channels now use `@asyncCallback`, so the existing callback-based native implementations (Swift completion handlers, Kotlin callbacks) are kept as-is rather than migrated to native `async`/`suspend`. No public API change. diff --git a/README.md b/README.md index b4edbfbf..5c6f158e 100644 --- a/README.md +++ b/README.md @@ -944,6 +944,8 @@ try { The error parser automatically converts platform-specific error formats (strings, numeric codes, PlatformExceptions) into the unified `UniversalBleErrorCode` enum, ensuring consistent error handling across all platforms. +When a peripheral refuses a characteristic or descriptor read/write, or a subscription, Android, Apple and Windows report the ATT error as the matching `UniversalBleErrorCode` (`insufficientEncryption`, `insufficientAuthentication`, `readNotPermitted`, ...) instead of a generic `failed`/`unknownError`. The raw native code is available in `UniversalBleException.details`. Service/characteristic discovery at connect time, and the Linux and Web backends, may still report a generic code. + ## Peripheral Mode `universal_ble` provides peripheral mode through `UniversalBlePeripheral`, so your app can advertise as a peripheral "server" in addition to client mode. diff --git a/android/src/test/kotlin/com/navideck/universal_ble/UniversalBleAttErrorConformanceTest.kt b/android/src/test/kotlin/com/navideck/universal_ble/UniversalBleAttErrorConformanceTest.kt new file mode 100644 index 00000000..78917b77 --- /dev/null +++ b/android/src/test/kotlin/com/navideck/universal_ble/UniversalBleAttErrorConformanceTest.kt @@ -0,0 +1,36 @@ +package com.navideck.universal_ble + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Drives [gattStatusToUniversalBleErrorCode] from the shared conformance fixture + * (`test/fixtures/att_error_cases.tsv`, exposed as a test resource by + * `android/build.gradle`), so the ATT error mapping stays identical to the Dart + * and Windows implementations. + */ +internal class UniversalBleAttErrorConformanceTest { + + @Test + fun matchesTheSharedConformanceFixture() { + val stream = javaClass.getResourceAsStream("/att_error_cases.tsv") + ?: error("Fixture not found on the test classpath") + val rows = stream.bufferedReader().readLines() + .filter { it.isNotBlank() && !it.startsWith("#") } + .map { it.split("|") } + .drop(1) // header row + assertTrue(rows.isNotEmpty(), "Fixture should contain cases") + + for (columns in rows) { + assertEquals(3, columns.size, "Bad fixture row: ${columns.joinToString("|")}") + val attCode = columns[0].removePrefix("0x").toInt(16) + val expected = columns[1].toInt() + assertEquals( + expected, + gattStatusToUniversalBleErrorCode(attCode).raw, + "ATT ${columns[0]} (${columns[2]})", + ) + } + } +} diff --git a/darwin/universal_ble/Sources/universal_ble/UniversalBleHelper.swift b/darwin/universal_ble/Sources/universal_ble/UniversalBleHelper.swift index 2c5031f2..2ff7d89b 100644 --- a/darwin/universal_ble/Sources/universal_ble/UniversalBleHelper.swift +++ b/darwin/universal_ble/Sources/universal_ble/UniversalBleHelper.swift @@ -149,13 +149,14 @@ func attErrorToUniversalBleErrorCode(_ code: Int) -> UniversalBleErrorCode { } } -/// Creates a PigeonError with the error code enum in details +/// Creates a PigeonError carrying the error code enum's raw value func createFlutterError( code: UniversalBleErrorCode, message: String? = nil, details: String? = nil ) -> PigeonError { - // Pass the enum's rawValue (Int) in code as string, and enum name in details + // `code` and `details` are both numeric strings: the enum's raw value, unless + // the caller passes a native code (e.g. the ATT error) in details. return PigeonError( code: code.rawValue.description, message: message, diff --git a/lib/src/universal_ble_linux/universal_ble_linux.dart b/lib/src/universal_ble_linux/universal_ble_linux.dart index 639f6d30..d0ea18ec 100644 --- a/lib/src/universal_ble_linux/universal_ble_linux.dart +++ b/lib/src/universal_ble_linux/universal_ble_linux.dart @@ -798,20 +798,10 @@ extension on BlueZFailedException { ); } - /// Extract error code from message and parse into decimal - /// example: 'Operation failed with ATT error: 0x90' => 144 - String? get errorCode { - try { - RegExp regExp = RegExp(r'0x\w+'); - Match? match = regExp.firstMatch(message); - String? code = match?.group(0); - if (code == null) return null; - int? decimalValue = int.tryParse(code.replaceFirst('0x', ''), radix: 16); - return decimalValue?.toString() ?? code; - } catch (e) { - return null; - } - } + /// The ATT error code parsed out of the BlueZ failure message as a decimal + /// string, e.g. 'Operation failed with ATT error: 0x90' => '144'. + String? get errorCode => + UniversalBleErrorParser.extractBlueZAttErrorCode(message); } extension BlueZDeviceExtension on BlueZDevice { diff --git a/lib/src/utils/universal_ble_error_parser.dart b/lib/src/utils/universal_ble_error_parser.dart index a99080b0..9d92fe9a 100644 --- a/lib/src/utils/universal_ble_error_parser.dart +++ b/lib/src/utils/universal_ble_error_parser.dart @@ -32,6 +32,28 @@ class UniversalBleErrorParser { return UniversalBleErrorCode.unknownError; } + /// Extracts the ATT error code from a BlueZ GATT failure message as a decimal + /// string, e.g. `'Operation failed with ATT error: 0x90'` => `'144'`, or + /// `null` when the message carries no code. + /// + /// BlueZ reports a GATT protocol error as `org.bluez.Error.Failed` with an + /// "ATT error: 0xNN" message. The phrase is matched first (case-insensitively), + /// so an unrelated hex value elsewhere in the message is not mistaken for the + /// ATT code. Only when the phrase is absent is the first `0x`-prefixed hex token + /// (lower-case prefix) used as a best-effort fallback; a message with no such + /// token returns `null`. + static String? extractBlueZAttErrorCode(String message) { + final attCode = RegExp( + r'ATT error:\s*0x([0-9a-fA-F]+)', + caseSensitive: false, + ).firstMatch(message)?.group(1); + final hex = + attCode ?? RegExp(r'0x([0-9a-fA-F]+)').firstMatch(message)?.group(1); + if (hex == null) return null; + final decimalValue = int.tryParse(hex, radix: 16); + return decimalValue?.toString() ?? '0x$hex'; + } + static UniversalBleErrorCode? _parseStringErrorCode(String code) { switch (code.toLowerCase()) { case 'failed to get device': diff --git a/test/att_error_conformance_test.dart b/test/att_error_conformance_test.dart new file mode 100644 index 00000000..fa9fbd30 --- /dev/null +++ b/test/att_error_conformance_test.dart @@ -0,0 +1,52 @@ +// Drives the Dart ATT error mapping from the shared conformance fixture +// (test/fixtures/att_error_cases.tsv), so its semantics stay identical to the +// Android and Windows implementations. +// +// Skipped on the web test run: the fixture is read from disk. +@TestOn('vm') +library; + +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:universal_ble/src/utils/universal_ble_error_parser.dart'; + +const _fixturePath = 'test/fixtures/att_error_cases.tsv'; + +void main() { + final file = File(_fixturePath); + if (!file.existsSync()) { + throw StateError( + 'Fixture not found at $_fixturePath (run tests from the package root)', + ); + } + + final rows = file + .readAsLinesSync() + .where((line) => line.isNotEmpty && !line.startsWith('#')) + .toList(); + rows.removeAt(0); // header + + test('maps every shared ATT error code like the other platforms', () { + expect(rows, isNotEmpty); + + for (final row in rows) { + final columns = row.split('|'); + expect( + columns.length, + 3, + reason: 'Fixture row "$row" must have 3 columns', + ); + + final attCode = int.parse(columns[0].replaceFirst('0x', ''), radix: 16); + final expected = int.parse(columns[1]); + + final code = UniversalBleErrorParser.getCode(attCode); + expect( + code.index, + expected, + reason: 'ATT ${columns[0]} (${columns[2]})', + ); + } + }); +} diff --git a/test/fixtures/att_error_cases.tsv b/test/fixtures/att_error_cases.tsv new file mode 100644 index 00000000..62f13aa3 --- /dev/null +++ b/test/fixtures/att_error_cases.tsv @@ -0,0 +1,42 @@ +# Shared ATT error-mapping conformance cases. +# +# Consumed by: +# * test/att_error_conformance_test.dart (Dart) +# * android/src/test/.../UniversalBleAttErrorConformanceTest.kt (Kotlin) +# * windows/test/att_error_mapper_test.cpp (C++) +# so the Android, Dart and Windows ATT mappings are held to one table instead of +# drifting apart. Apple's `attErrorToUniversalBleErrorCode` maps the same way but +# its XCTest (example/ios/RunnerTests) still asserts a hand-written list; it can +# be pointed at this fixture too. +# +# Columns, '|'-separated (exactly two separators per row): +# att_code | expected | expected_name +# att_code ATT error code in hex, as the platform delivers it +# expected the UniversalBleErrorCode raw value the code maps to +# expected_name the UniversalBleErrorCode name (documentation only) +# +# Codes 0x80-0xFF are application-defined: they are not protocol errors, so the +# mappings driven by this fixture (Dart, Android, Windows) report them as +# unknownError. Linux is not driven by this fixture: its wrapper falls back to the +# caller's default code (readFailed / writeFailed / ...) whenever the ATT code +# maps to unknownError. +att_code|expected|expected_name +0x01|26|invalidHandle +0x02|28|readNotPermitted +0x03|30|writeNotPermitted +0x04|25|invalidPdu +0x05|50|insufficientAuthentication +0x06|33|operationNotSupported +0x07|23|invalidOffset +0x08|51|insufficientAuthorization +0x09|36|operationInProgress +0x0A|19|serviceNotFound +0x0B|24|invalidAttributeLength +0x0C|53|insufficientKeySize +0x0D|24|invalidAttributeLength +0x0E|1|failed +0x0F|52|insufficientEncryption +0x10|33|operationNotSupported +0x11|1|failed +0x80|0|unknownError +0xFF|0|unknownError diff --git a/test/universal_ble_error_parser_test.dart b/test/universal_ble_error_parser_test.dart index 2496eabf..1d823b32 100644 --- a/test/universal_ble_error_parser_test.dart +++ b/test/universal_ble_error_parser_test.dart @@ -63,5 +63,40 @@ void main() { ); }); }); + + group('extractBlueZAttErrorCode', () { + test('reads the ATT code that follows the phrase', () { + expect( + UniversalBleErrorParser.extractBlueZAttErrorCode( + 'GDBus.Error:org.bluez.Error.Failed: Operation failed with ATT ' + 'error: 0x0F', + ), + '15', + ); + }); + + test('prefers the ATT code over an unrelated hex value', () { + expect( + UniversalBleErrorParser.extractBlueZAttErrorCode( + 'write failed (handle 0xdead) with ATT error: 0x0f', + ), + '15', + ); + }); + + test('falls back to a bare hex value when the phrase is absent', () { + expect( + UniversalBleErrorParser.extractBlueZAttErrorCode('error code 0x1A'), + '26', + ); + }); + + test('returns null when the message has no code', () { + expect( + UniversalBleErrorParser.extractBlueZAttErrorCode('not connected'), + isNull, + ); + }); + }); }); } diff --git a/windows/CMakeLists.txt b/windows/CMakeLists.txt index 5c5d2f75..bdfb2cc3 100644 --- a/windows/CMakeLists.txt +++ b/windows/CMakeLists.txt @@ -47,6 +47,8 @@ list(APPEND PLUGIN_SOURCES "src/scan_result_merge.h" "src/scan_filter_matcher.cpp" "src/scan_filter_matcher.h" + "src/att_error_mapper.cpp" + "src/att_error_mapper.h" "src/universal_ble_filter_util.cpp" "src/universal_ble_filter_util.h" "src/universal_ble_thread_safe.h" diff --git a/windows/src/att_error_mapper.cpp b/windows/src/att_error_mapper.cpp new file mode 100644 index 00000000..eb05150a --- /dev/null +++ b/windows/src/att_error_mapper.cpp @@ -0,0 +1,62 @@ +#include "att_error_mapper.h" + +namespace universal_ble { + +// ATT error codes (Bluetooth Core Spec, Vol 3, Part F, 3.4.1.1). +// +// The right-hand values are the `UniversalBleErrorCode` integer values from +// windows/src/generated/universal_ble.g.h. They are pinned there (the enum uses +// explicit `= N` values) and shared with the Dart and Android layers, so they +// do not shift. Kept in lockstep with Android's +// `gattStatusToUniversalBleErrorCode` and Apple's +// `attErrorToUniversalBleErrorCode`: +// +// 1 kFailed 19 kServiceNotFound +// 23 kInvalidOffset 24 kInvalidAttributeLength +// 25 kInvalidPdu 26 kInvalidHandle +// 28 kReadNotPermitted 30 kWriteNotPermitted +// 33 kOperationNotSupported 36 kOperationInProgress +// 50 kInsufficientAuthentication 51 kInsufficientAuthorization +// 52 kInsufficientEncryption 53 kInsufficientKeySize +int AttErrorToUniversalBleErrorCodeValue(uint8_t att_error) { + switch (att_error) { + case 0x01: + return 26; // kInvalidHandle + case 0x02: + return 28; // kReadNotPermitted + case 0x03: + return 30; // kWriteNotPermitted + case 0x04: + return 25; // kInvalidPdu + case 0x05: + return 50; // kInsufficientAuthentication + case 0x06: + return 33; // kOperationNotSupported (request not supported) + case 0x07: + return 23; // kInvalidOffset + case 0x08: + return 51; // kInsufficientAuthorization + case 0x09: + return 36; // kOperationInProgress (prepare queue full) + case 0x0A: + return 19; // kServiceNotFound (attribute not found) + case 0x0B: + return 24; // kInvalidAttributeLength (attribute not long) + case 0x0C: + return 53; // kInsufficientKeySize + case 0x0D: + return 24; // kInvalidAttributeLength + case 0x0E: + return 1; // kFailed (unlikely error) + case 0x0F: + return 52; // kInsufficientEncryption + case 0x10: + return 33; // kOperationNotSupported (unsupported group type) + case 0x11: + return 1; // kFailed (insufficient resources) + default: + return 0; // kUnknownError (incl. application-defined 0x80-0xFF) + } +} + +} // namespace universal_ble diff --git a/windows/src/att_error_mapper.h b/windows/src/att_error_mapper.h new file mode 100644 index 00000000..08cfb14a --- /dev/null +++ b/windows/src/att_error_mapper.h @@ -0,0 +1,21 @@ +#pragma once + +#include + +namespace universal_ble { + +/// Maps a Bluetooth ATT error code (the `ProtocolError` byte of a GATT read, +/// write or descriptor-discovery result) to the raw value of the matching +/// `UniversalBleErrorCode`. +/// +/// Mirrors Android's `gattStatusToUniversalBleErrorCode` and Apple's +/// `attErrorToUniversalBleErrorCode`, so a peripheral refusing a read, write or +/// subscription reports the same code on every platform. The result is the +/// `UniversalBleErrorCode` integer rather than the enum itself, which keeps +/// this free of flutter/winrt types so it can be unit tested without a Flutter +/// engine (see windows/test/att_error_mapper_test.cpp). Application-defined +/// codes (`0x80`-`0xFF`) carry no protocol meaning and map to `0` +/// (`kUnknownError`). +int AttErrorToUniversalBleErrorCodeValue(uint8_t att_error); + +} // namespace universal_ble diff --git a/windows/src/helper/utils.cpp b/windows/src/helper/utils.cpp index b428c5dc..3450a78b 100644 --- a/windows/src/helper/utils.cpp +++ b/windows/src/helper/utils.cpp @@ -1,4 +1,5 @@ #include "utils.h" +#include "../att_error_mapper.h" #include "../generated/universal_ble.g.h" #include "../enum_parser.h" @@ -158,8 +159,8 @@ bool is_windows11_or_greater() { FlutterError create_flutter_error(UniversalBleErrorCode code, const std::string &message, const std::string &details) { - // Pass the enum's underlying integer value as string in code, and enum name - // or details in details + // code and details both carry the enum's integer value as a string; a caller + // may pass a native code (e.g. the ATT error) in details instead. std::string code_str = std::to_string(static_cast(code)); std::string details_str = details.empty() ? std::to_string(static_cast(code)) : details; @@ -191,6 +192,28 @@ FlutterError create_flutter_error_from_gatt_communication_status( return create_flutter_error(error_code, final_message); } +FlutterError create_flutter_error_from_gatt_result( + GattCommunicationStatus status, const IReference &protocol_error, + const std::string &message) { + // A refused GATT operation is reported as ProtocolError; Windows then fills + // in the ATT error code, which names the reason (read not permitted, + // insufficient encryption, ...). Only ProtocolError carries it: Unreachable + // and AccessDenied have no protocol error, so they keep the generic mapping. + if (status == GattCommunicationStatus::ProtocolError && protocol_error) { + const uint8_t att_error = protocol_error.Value(); + const auto error_code = static_cast( + AttErrorToUniversalBleErrorCodeValue(att_error)); + auto error_message = gatt_communication_status_to_error(status); + std::string final_message = message.empty() && error_message.has_value() + ? error_message.value() + : message; + // Keep the raw ATT code in details, as Android and Apple do. + return create_flutter_error(error_code, final_message, + std::to_string(att_error)); + } + return create_flutter_error_from_gatt_communication_status(status, message); +} + FlutterError create_flutter_error_from_pairing_status( DevicePairingResultStatus status, const std::string &message) { if (status == DevicePairingResultStatus::Paired) { diff --git a/windows/src/helper/utils.h b/windows/src/helper/utils.h index 2698886c..6f9af7d2 100644 --- a/windows/src/helper/utils.h +++ b/windows/src/helper/utils.h @@ -40,6 +40,15 @@ FlutterError create_flutter_error(UniversalBleErrorCode code, FlutterError create_flutter_error_from_gatt_communication_status( GattCommunicationStatus status, const std::string &message = ""); +/// Creates a FlutterError from a GATT read/write result. When the peripheral +/// refused the operation (`ProtocolError`) and Windows supplies the ATT error, +/// the matching UniversalBleErrorCode is reported instead of the generic failed +/// code. `status` and `protocol_error` come from the result object's +/// `Status()` and `ProtocolError()`. +FlutterError create_flutter_error_from_gatt_result( + GattCommunicationStatus status, const IReference &protocol_error, + const std::string &message = ""); + /// Creates a FlutterError from device pairing result status FlutterError create_flutter_error_from_pairing_status( DevicePairingResultStatus status, const std::string &message = ""); diff --git a/windows/src/universal_ble_plugin.cpp b/windows/src/universal_ble_plugin.cpp index 35a5dab2..405e3c18 100644 --- a/windows/src/universal_ble_plugin.cpp +++ b/windows/src/universal_ble_plugin.cpp @@ -572,9 +572,9 @@ void UniversalBlePlugin::ReadValue( "READ_FAILED <- " + device_id + " " + service + " " + characteristic + " status=" + std::to_string(static_cast(status))); - safe_reply( - "ReadValue", result, - create_flutter_error_from_gatt_communication_status(status)); + safe_reply("ReadValue", result, + create_flutter_error_from_gatt_result( + status, read_value_result.ProtocolError())); } else { if (!bluetooth_agent->IsActive()) { safe_reply("ReadValue", result, @@ -650,11 +650,12 @@ void UniversalBlePlugin::WriteValue( } } - gatt_characteristic.WriteValueAsync(from_bytevc(value), write_option) + gatt_characteristic + .WriteValueWithResultAsync(from_bytevc(value), write_option) .Completed([bluetooth_agent, gatt_characteristic_lease, device_id, - service, characteristic, result]( - IAsyncOperation const &sender, - AsyncStatus const args) { + service, characteristic, + result](IAsyncOperation const &sender, + AsyncStatus const args) { try { if (!bluetooth_agent->IsActive()) { safe_reply("WriteValue", result, @@ -672,15 +673,16 @@ void UniversalBlePlugin::WriteValue( return; } - const auto status = sender.GetResults(); + const auto write_result = sender.GetResults(); + const auto status = write_result.Status(); if (status != GattCommunicationStatus::Success) { UniversalBleLogger::LogError( "WRITE_FAILED <- " + device_id + " " + service + " " + characteristic + " status=" + std::to_string(static_cast(status))); - safe_reply( - "WriteValue", result, - create_flutter_error_from_gatt_communication_status(status)); + safe_reply("WriteValue", result, + create_flutter_error_from_gatt_result( + status, write_result.ProtocolError())); } else { if (!bluetooth_agent->IsActive()) { safe_reply("WriteValue", result, @@ -759,9 +761,10 @@ void UniversalBlePlugin::ReadDescriptorValue( } const auto desc_result = desc_sender.GetResults(); if (desc_result.Status() != GattCommunicationStatus::Success) { - safe_reply("ReadDescriptorValue", result, - create_flutter_error_from_gatt_communication_status( - desc_result.Status())); + safe_reply( + "ReadDescriptorValue", result, + create_flutter_error_from_gatt_result( + desc_result.Status(), desc_result.ProtocolError())); return; } if (desc_result.Descriptors().Size() == 0) { @@ -801,8 +804,8 @@ void UniversalBlePlugin::ReadDescriptorValue( if (status != GattCommunicationStatus::Success) { safe_reply( "ReadDescriptorValue", result, - create_flutter_error_from_gatt_communication_status( - status)); + create_flutter_error_from_gatt_result( + status, read_value_result.ProtocolError())); } else { if (!bluetooth_agent->IsActive()) { safe_reply( @@ -893,9 +896,10 @@ void UniversalBlePlugin::WriteDescriptorValue( } const auto desc_result = desc_sender.GetResults(); if (desc_result.Status() != GattCommunicationStatus::Success) { - safe_reply("WriteDescriptorValue", result, - create_flutter_error_from_gatt_communication_status( - desc_result.Status())); + safe_reply( + "WriteDescriptorValue", result, + create_flutter_error_from_gatt_result( + desc_result.Status(), desc_result.ProtocolError())); return; } if (desc_result.Descriptors().Size() == 0) { @@ -906,12 +910,11 @@ void UniversalBlePlugin::WriteDescriptorValue( return; } const auto gatt_descriptor = desc_result.Descriptors().GetAt(0); - gatt_descriptor.WriteValueAsync(from_bytevc(value)) + gatt_descriptor.WriteValueWithResultAsync(from_bytevc(value)) .Completed([bluetooth_agent, gatt_characteristic_lease, device_id, service, characteristic, descriptor, result]( - IAsyncOperation const - &sender, + IAsyncOperation const &sender, AsyncStatus const args) { try { if (!bluetooth_agent->IsActive()) { @@ -932,12 +935,12 @@ void UniversalBlePlugin::WriteDescriptorValue( return; } - const auto status = sender.GetResults(); + const auto write_result = sender.GetResults(); + const auto status = write_result.Status(); if (status != GattCommunicationStatus::Success) { - safe_reply( - "WriteDescriptorValue", result, - create_flutter_error_from_gatt_communication_status( - status)); + safe_reply("WriteDescriptorValue", result, + create_flutter_error_from_gatt_result( + status, write_result.ProtocolError())); } else { if (!bluetooth_agent->IsActive()) { safe_reply( @@ -2067,6 +2070,11 @@ fire_and_forget UniversalBlePlugin::ConnectAsync(uint64_t bluetooth_address, services_result = co_await device.GetGattServicesAsync(BluetoothCacheMode::Uncached); } + // Discovery failures report the generic status message rather than the ATT + // code in `services_result.ProtocolError()` (unlike the read/write paths, + // which use create_flutter_error_from_gatt_result): this runs during connect + // and is surfaced as a connection failure. The same applies to the + // characteristics query below. auto services_result_error = gatt_communication_status_to_error(services_result.Status()); if (services_result_error.has_value()) { @@ -2997,12 +3005,13 @@ fire_and_forget UniversalBlePlugin::SetNotifiableAsync( } // Write to the descriptor. - GattCommunicationStatus status{}; + GattWriteResult write_result{nullptr}; try { stage = "write-descriptor"; - status = co_await gatt_characteristic - .WriteClientCharacteristicConfigurationDescriptorAsync( - descriptor_value); + write_result = + co_await gatt_characteristic + .WriteClientCharacteristicConfigurationDescriptorWithResultAsync( + descriptor_value); } catch (const hresult_error &err) { UniversalBleLogger::LogError( "SET_NOTIFY exception hr=" + std::to_string(err.code()) + @@ -3025,6 +3034,7 @@ fire_and_forget UniversalBlePlugin::SetNotifiableAsync( co_return; } stage = "descriptor-completed"; + const auto status = write_result.Status(); if (status != GattCommunicationStatus::Success) { UniversalBleLogger::LogError("SET_NOTIFY_FAILED <- " + device_id + " " + service + " " + characteristic + " status=" + @@ -3039,7 +3049,8 @@ fire_and_forget UniversalBlePlugin::SetNotifiableAsync( device_agent->FinishNotificationOperation(service, characteristic, previous_subscription_token); notification_operation_started = false; - reply(create_flutter_error_from_gatt_communication_status(status)); + reply(create_flutter_error_from_gatt_result( + status, write_result.ProtocolError())); co_return; } diff --git a/windows/test/CMakeLists.txt b/windows/test/CMakeLists.txt index 6a4fbcc3..16177ede 100644 --- a/windows/test/CMakeLists.txt +++ b/windows/test/CMakeLists.txt @@ -29,6 +29,20 @@ target_compile_definitions(scan_filter_matcher_test PRIVATE add_test(NAME scan_filter_matcher_test COMMAND scan_filter_matcher_test) +# Drives the ATT error mapping used when a peripheral refuses a read, write or +# subscription. Platform-agnostic so it runs on every host. +add_executable(att_error_mapper_test + att_error_mapper_test.cpp + ../src/att_error_mapper.cpp +) +target_compile_features(att_error_mapper_test PRIVATE cxx_std_20) +target_include_directories(att_error_mapper_test PRIVATE ../src) +target_compile_definitions(att_error_mapper_test PRIVATE + ATT_ERROR_FIXTURE_PATH="${CMAKE_CURRENT_SOURCE_DIR}/../../test/fixtures/att_error_cases.tsv" +) + +add_test(NAME att_error_mapper_test COMMAND att_error_mapper_test) + if(WIN32) add_executable(cleanup_drain_test cleanup_drain_test.cpp) target_compile_features(cleanup_drain_test PRIVATE cxx_std_20) diff --git a/windows/test/att_error_mapper_test.cpp b/windows/test/att_error_mapper_test.cpp new file mode 100644 index 00000000..8b75ded9 --- /dev/null +++ b/windows/test/att_error_mapper_test.cpp @@ -0,0 +1,83 @@ +// Drives the ATT error mapping used when a peripheral refuses a read, write or +// subscription from the shared conformance fixture +// (test/fixtures/att_error_cases.tsv), so the Windows table stays identical to +// the Android and Dart ones. Builds and runs without a Flutter engine or WinRT: +// see windows/src/att_error_mapper.h. +#include +#include +#include +#include +#include +#include +#include +#include + +#include "att_error_mapper.h" + +namespace { + +using universal_ble::AttErrorToUniversalBleErrorCodeValue; + +#ifndef ATT_ERROR_FIXTURE_PATH +#error "ATT_ERROR_FIXTURE_PATH must point at test/fixtures/att_error_cases.tsv" +#endif + +struct Case { + uint8_t att_code = 0; + int expected = 0; + std::string expected_name; +}; + +std::vector LoadCases(const std::string &path) { + std::ifstream file(path); + if (!file.is_open()) { + std::cerr << "Failed to open fixture: " << path << "\n"; + std::exit(2); + } + + std::vector cases; + std::string line; + while (std::getline(file, line)) { + if (line.empty() || line[0] == '#' || line.rfind("att_code|", 0) == 0) { + continue; + } + std::stringstream row(line); + std::string att_code; + std::string expected; + std::string expected_name; + if (!std::getline(row, att_code, '|') || + !std::getline(row, expected, '|') || + !std::getline(row, expected_name, '|')) { + std::cerr << "Bad fixture row: " << line << "\n"; + std::exit(2); + } + cases.push_back(Case{ + static_cast(std::stoul(att_code, nullptr, 16)), + std::stoi(expected), + expected_name, + }); + } + return cases; +} + +} // namespace + +int main() { + const auto cases = LoadCases(ATT_ERROR_FIXTURE_PATH); + if (cases.empty()) { + std::cerr << "Fixture contained no cases\n"; + return 1; + } + + for (const auto &test_case : cases) { + const int actual = AttErrorToUniversalBleErrorCodeValue(test_case.att_code); + if (actual != test_case.expected) { + std::cerr << "ATT code 0x" << std::hex + << static_cast(test_case.att_code) << std::dec << " (" + << test_case.expected_name << "): expected " + << test_case.expected << ", got " << actual << std::endl; + return 1; + } + } + return 0; +}