Skip to content

Emit device ids in lower-case on every platform (breaking, next major) - #270

Closed
postmaxin wants to merge 9 commits into
Navideck:mainfrom
postmaxin:lowercase-device-ids
Closed

postmaxin wants to merge 9 commits into
Navideck:mainfrom
postmaxin:lowercase-device-ids

Conversation

@postmaxin

Copy link
Copy Markdown
Contributor

Follow-up to #269 (merged in 2.1.1): make the emitted device-id case consistent too — the fully-universal, breaking version discussed there for the next major.

What changes

Device ids are now canonicalised to lower-case throughout the Dart layer and emitted lower-case on every platform (scan results + connection / value / pairing / connection-parameter callbacks and streams). Previously each platform reported its native case — Android upper-cased MACs, Windows/WinRT lower-cased them — so a caller holding the id in the "wrong" case could split state or (before #269) miss events.

How — Dart-only, no Kotlin / Swift / C++ changes

  • The update* handlers lower-case on ingestion, so every event, callback and per-device map key is lower-case. This also lets Match device ids case-insensitively across event streams #269's dual-case stream matching collapse to a single lower-case compare.
  • Native BLE calls still need the platform's case (Android's getRemoteDevice requires upper-case and throws otherwise; Apple's peripheral cache is keyed by the upper-case uuidString, Windows parses the address either way, Linux's BlueZ address is upper-case), so the platform implementations convert back at their boundary: a single _nativeId() helper at the pigeon-channel native-op sites, and one line in the Linux instance's device lookup (the single point every Linux op funnels through).

Note the native side receives the same upper-case id it always has — it's just reconstructed at the Dart boundary now instead of being supplied by the caller — so native behaviour is unchanged.

Breaking

Callers that stored or compared an emitted id by exact case (e.g. an Android upper-case MAC) must now lower-case it, or compare case-insensitively. CHANGELOG updated under "next major".

Testing

  • 97 Dart tests pass (flutter test) and flutter analyze is clean — including 5 new tests asserting lower-case emission from every update* handler, with the existing Match device ids case-insensitively across event streams #269 case-insensitive-matching / dedup / cache tests still green.
  • Verified on a real Android device. I pointed a consumer app (a BLE pet-tracker gateway that round-trips scanned ids straight back into connect()) at this branch and confirmed the whole path: scan → connect() → auth → characteristic streaming → and the reconnect/self-heal path. That's exactly the round-trip this change relies on — Android's getRemoteDevice throws on a lower-case MAC, so a successful connect proves the boundary conversion is doing its job.

Developed with AI assistance (noted via the commit's Co-Authored-By trailer).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR standardizes BLE device IDs to a single canonical form in the Dart layer by lower-casing them on ingestion and ensuring all emitted IDs from streams/callbacks are lower-case, while converting back to the native-required case at platform boundaries (Pigeon channel + Linux BlueZ lookup). This is a breaking change intended for the next major release and is documented in the changelog.

Changes:

  • Canonicalize device IDs to lower-case in UniversalBlePlatform update handlers and simplify stream matching to a single lower-case compare.
  • Upper-case device IDs at the Dart→native boundary for Pigeon platform operations, and normalize Linux device lookup to BlueZ’s expected case.
  • Add tests asserting lower-case emission from update handlers; update changelog and example lockfile.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/device_id_case_insensitivity_test.dart Adds tests asserting lower-case emission from update* handlers.
lib/src/universal_ble_pigeon/universal_ble_pigeon_channel.dart Introduces _nativeId() and applies it to Pigeon native calls; keeps emitted IDs lower-case.
lib/src/universal_ble_linux/universal_ble_linux.dart Normalizes Linux device lookup by upper-casing IDs before BlueZ resolution.
lib/src/interfaces/universal_ble_platform_interface.dart Canonicalizes IDs to lower-case on ingestion and simplifies stream filters accordingly.
example/pubspec.lock Bumps the path-dependency version recorded for the example app.
CHANGELOG.md Documents the breaking change for “next major”.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/src/universal_ble_pigeon/universal_ble_pigeon_channel.dart Outdated
Comment thread lib/src/universal_ble_pigeon/universal_ble_pigeon_channel.dart Outdated
Follow-up to the case-insensitive matching in Navideck#269 (2.1.1): make the emitted
case consistent too. Device ids are now canonicalised to lower-case throughout
the Dart layer — every scan result, callback and stream carries the lower-case
form regardless of the case the platform reports (Android upper-cased MACs,
Windows/WinRT lower-cased them).

Native BLE calls still need the platform's case (Android's getRemoteDevice
REQUIRES upper-case; Apple's peripheral cache, Windows' address parse and
Linux's BlueZ address are upper-case too), so the platform implementations
convert back at their boundary — a single `_nativeId` helper in the pigeon
channel, and one line in the Linux instance's device lookup. No Kotlin / Swift
/ C++ changes.

This also lets the Navideck#269 stream matching collapse from a dual-case compare to a
single lower-case one.

BREAKING: callers that stored/compared an emitted id by exact case must now
lower-case it (or compare case-insensitively). For the next major release.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@postmaxin
postmaxin force-pushed the lowercase-device-ids branch from b7bf158 to 33e19b4 Compare August 28, 2026 14:03
postmaxin and others added 3 commits August 28, 2026 07:08
Copilot's review pointed out that peripheral-mode Pigeon calls were left out of
the id canonicalisation. Peripheral round-trips still worked (emitted ids were
native-case and callers passed them back verbatim), but it broke the promise of
lower-case ids everywhere and Android's getMaximumNotifyLength lookup is
case-sensitive, so a caller normalising ids to the documented lower-case form
would get null there.

- Canonicalise emitted central ids to lower-case in the peripheral base class
  streams (connection state, MTU, characteristic subscription) and in the pigeon
  read/write/descriptor request handlers and getSubscribedClients results.
- Convert back at the pigeon boundary (getMaximumNotifyLength,
  updateCharacteristicValue) via a shared nativeDeviceId helper.
- Correct the boundary comment: Windows formats MACs lower-case and parses them
  case-insensitively; it's Android/Apple/Linux that want upper-case
  (also flagged by Copilot).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011rAEwavZL9tHSe28EVhU2E
Updated breaking changes section to clarify that device IDs are now emitted in lower-case across all platforms, affecting various callbacks and streams. This change follows the case-insensitive matching introduced in version 2.1.1.
Comment thread lib/src/interfaces/universal_ble_platform_interface.dart Outdated
@rohitsangwan01

Copy link
Copy Markdown
Contributor

@postmaxin instead of parsing id's, it would be better to use DeviceId object in interface api

class DeviceId {
  final String _id;
  const DeviceId(this._id);

  String get native => id.toLowerCase();
  
  // Code conversion will remain in this class only

  @override
  String toString() => native;
}

postmaxin and others added 2 commits September 10, 2026 08:56
Web Bluetooth device ids are opaque, case-sensitive browser tokens
(Chromium emits Base64 of a random value), not the case-insensitive
addresses every other platform reports. Lower-casing one corrupted the
id the caller sees and broke the lookup it is passed back to: startScan
stores the device under `BluetoothDevice.id`, so an emitted (folded) id
missed `_bluetoothDeviceList` and connect/read/write reported
deviceNotFound.

Make canonicalisation a platform-overridable operation instead of an
unconditional toLowerCase(): `UniversalBlePlatform.canonicalDeviceId()`
defaults to lower-case, and both emission (the update* handlers) and
matching (the stream filters, `_connectionEventCompleter`) go through
it, so an override cannot desync the two halves. UniversalBleWeb
overrides it to identity. Mirrored on UniversalBlePeripheralPlatform,
with the pigeon sites calling the hook rather than open-coding the
fold; `nativeDeviceId()` (upper-case at the native boundary) is
unchanged.

Reported by @fotiDim in review of Navideck#270.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TkNeiMyH9mAWmK4opt4o8Y
@postmaxin

Copy link
Copy Markdown
Contributor Author

Happy to go that way if you'd like it in this PR — one thing to check first: in the sketch, native => id.toLowerCase(), but the native side needs the upper-case form (Android's getRemoteDevice throws on a lower-case MAC; Apple's peripheral cache is keyed by the upper-case uuidString). So there are two directions to hold: a canonical form for what the Dart layer emits and compares, and a native form at the channel boundary — the PR currently has them as canonicalDeviceId() and nativeDeviceId().

Also worth noting the canonical form can't be a constant toLowerCase(): per @fotiDim's review above, Web Bluetooth ids are opaque case-sensitive browser tokens, so canonicalisation has to be platform-supplied. A DeviceId type would delegate to that rather than replace it.

The cost is scope: every public String deviceId parameter, BleDevice.deviceId, and the string-based Low-Level API change type, on top of an already-breaking PR. Want me to do it here, or land the case fix and do the DeviceId type as a follow-up before 3.0.0?

@rohitsangwan01

Copy link
Copy Markdown
Contributor

@postmaxin public apis should still remain same, except the case difference, DeviceId should be use internally only

Per @rohitsangwan01's review: keep the conversion in a single class
instead of spreading toLowerCase()/toUpperCase() through the layer.
Public APIs still take and return ids as plain Strings — DeviceId is
internal (not exported from the barrel).

DeviceId carries both forms an id needs and is the only place either
conversion happens: `canonical`, what the Dart layer emits, matches and
keys per-device state by, and `native`, what channel calls take (Android's
getRemoteDevice REQUIRES upper case, Apple's peripheral cache is keyed by
the upper-case uuidString, BlueZ addresses are upper-case). `DeviceId.address`
covers case-insensitive addresses; `DeviceId.opaque` carries Web Bluetooth's
opaque, case-sensitive tokens through untouched.

Platforms now declare only which kind they report, via
`UniversalBlePlatform.hasAddressDeviceIds` (Web overrides it to false),
replacing the canonicalDeviceId hook; the interface derives both forms
from it, so emission and matching cannot disagree. nativeDeviceId() and
native_device_id.dart are folded into DeviceId, as are the Linux device
lookup and the service-cache key.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TkNeiMyH9mAWmK4opt4o8Y
@postmaxin

Copy link
Copy Markdown
Contributor Author

Done in a3c7c27 — DeviceId is internal (not exported from the barrel) and the public APIs are unchanged, still taking and returning ids as String.

It carries both forms an id needs and is the only place either conversion happens:

DeviceId.address('AA:BB:CC:DD:EE:FF')  // canonical 'aa:bb:...', native 'AA:BB:...'
DeviceId.opaque('mHZbW+PZqBpUlZlVQrPzOQ==')  // both forms verbatim (Web)

canonical is what the Dart layer emits, matches and keys per-device state by; native is what channel calls take. Platforms now declare only which kind they report — UniversalBlePlatform.hasAddressDeviceIds, which Web overrides to false — and the interface derives both forms from that, so emission and matching can't drift apart. nativeDeviceId() and its file are gone, folded into DeviceId, along with the Linux device lookup and the service-cache key; there is no toLowerCase()/toUpperCase() on a device id left anywhere else in the package.

130 tests pass (flutter analyze clean), including a new test/device_id_test.dart covering both directions, round-trip stability, and that two cases of an address are one device while two cases of an opaque id are not.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Cache keying currently case-folds opaque Web Bluetooth IDs (risking collisions/incorrect per-device cached state) and the example lockfile changes appear unrelated and should be clarified or reverted.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 13/14 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread lib/src/utils/cache_handler.dart Outdated
CacheHandler folded every key to lower case, which is right for an
address but not for Web's opaque, case-sensitive ids: two distinct Web
ids differing only in case would share one cache entry and hand back
another device's services/subscriptions.

Canonicalise where the platform's id kind is known instead —
`UniversalBle.canonicalDeviceId` (@internal), applied at the points a
caller-supplied id enters: isSubscribed, getSubscribedCharacteristics,
updateSubscription and BleDeviceExtension's service-cache reads/writes.
CacheHandler now stores exactly the key it is given, so ids that are not
case-insensitive are never folded together, and an address still lands
on one entry whichever case the caller uses.

Also restore example/pubspec.lock to main's pins — an unrelated
downgrade (matcher, meta, test_api) picked up from an older SDK.

Both reported by Copilot in review of Navideck#270.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TkNeiMyH9mAWmK4opt4o8Y
@fotiDim

fotiDim commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

@postmaxin we are preparing for 3.0 so now it is the time for this to be reviewed & merged. Could you please fix the merge conflicts?

@rohitsangwan01

Copy link
Copy Markdown
Contributor

Close in favor of #337

fotiDim added a commit that referenced this pull request Oct 14, 2026
* Emit device ids in lower-case on every platform (breaking)

Follow-up to the case-insensitive matching in #269 (2.1.1): make the emitted
case consistent too. Device ids are now canonicalised to lower-case throughout
the Dart layer — every scan result, callback and stream carries the lower-case
form regardless of the case the platform reports (Android upper-cased MACs,
Windows/WinRT lower-cased them).

Native BLE calls still need the platform's case (Android's getRemoteDevice
REQUIRES upper-case; Apple's peripheral cache, Windows' address parse and
Linux's BlueZ address are upper-case too), so the platform implementations
convert back at their boundary — a single `_nativeId` helper in the pigeon
channel, and one line in the Linux instance's device lookup. No Kotlin / Swift
/ C++ changes.

This also lets the #269 stream matching collapse from a dual-case compare to a
single lower-case one.

BREAKING: callers that stored/compared an emitted id by exact case must now
lower-case it (or compare case-insensitively). For the next major release.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Extend lower-case device ids to peripheral mode (review follow-up)

Copilot's review pointed out that peripheral-mode Pigeon calls were left out of
the id canonicalisation. Peripheral round-trips still worked (emitted ids were
native-case and callers passed them back verbatim), but it broke the promise of
lower-case ids everywhere and Android's getMaximumNotifyLength lookup is
case-sensitive, so a caller normalising ids to the documented lower-case form
would get null there.

- Canonicalise emitted central ids to lower-case in the peripheral base class
  streams (connection state, MTU, characteristic subscription) and in the pigeon
  read/write/descriptor request handlers and getSubscribedClients results.
- Convert back at the pigeon boundary (getMaximumNotifyLength,
  updateCharacteristicValue) via a shared nativeDeviceId helper.
- Correct the boundary comment: Windows formats MACs lower-case and parses them
  case-insensitively; it's Android/Apple/Linux that want upper-case
  (also flagged by Copilot).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011rAEwavZL9tHSe28EVhU2E

* Clarify breaking changes for device ID casing

Updated breaking changes section to clarify that device IDs are now emitted in lower-case across all platforms, affecting various callbacks and streams. This change follows the case-insensitive matching introduced in version 2.1.1.

* fix: exempt Web Bluetooth ids from lower-case canonicalisation

Web Bluetooth device ids are opaque, case-sensitive browser tokens
(Chromium emits Base64 of a random value), not the case-insensitive
addresses every other platform reports. Lower-casing one corrupted the
id the caller sees and broke the lookup it is passed back to: startScan
stores the device under `BluetoothDevice.id`, so an emitted (folded) id
missed `_bluetoothDeviceList` and connect/read/write reported
deviceNotFound.

Make canonicalisation a platform-overridable operation instead of an
unconditional toLowerCase(): `UniversalBlePlatform.canonicalDeviceId()`
defaults to lower-case, and both emission (the update* handlers) and
matching (the stream filters, `_connectionEventCompleter`) go through
it, so an override cannot desync the two halves. UniversalBleWeb
overrides it to identity. Mirrored on UniversalBlePeripheralPlatform,
with the pigeon sites calling the hook rather than open-coding the
fold; `nativeDeviceId()` (upper-case at the native boundary) is
unchanged.

Reported by @fotiDim in review of #270.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TkNeiMyH9mAWmK4opt4o8Y

* refactor: put device-id case conversion in one internal DeviceId type

Per @rohitsangwan01's review: keep the conversion in a single class
instead of spreading toLowerCase()/toUpperCase() through the layer.
Public APIs still take and return ids as plain Strings — DeviceId is
internal (not exported from the barrel).

DeviceId carries both forms an id needs and is the only place either
conversion happens: `canonical`, what the Dart layer emits, matches and
keys per-device state by, and `native`, what channel calls take (Android's
getRemoteDevice REQUIRES upper case, Apple's peripheral cache is keyed by
the upper-case uuidString, BlueZ addresses are upper-case). `DeviceId.address`
covers case-insensitive addresses; `DeviceId.opaque` carries Web Bluetooth's
opaque, case-sensitive tokens through untouched.

Platforms now declare only which kind they report, via
`UniversalBlePlatform.hasAddressDeviceIds` (Web overrides it to false),
replacing the canonicalDeviceId hook; the interface derives both forms
from it, so emission and matching cannot disagree. nativeDeviceId() and
native_device_id.dart are folded into DeviceId, as are the Linux device
lookup and the service-cache key.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TkNeiMyH9mAWmK4opt4o8Y

* fix: canonicalise cache keys at the API boundary, not inside the cache

CacheHandler folded every key to lower case, which is right for an
address but not for Web's opaque, case-sensitive ids: two distinct Web
ids differing only in case would share one cache entry and hand back
another device's services/subscriptions.

Canonicalise where the platform's id kind is known instead —
`UniversalBle.canonicalDeviceId` (@internal), applied at the points a
caller-supplied id enters: isSubscribed, getSubscribedCharacteristics,
updateSubscription and BleDeviceExtension's service-cache reads/writes.
CacheHandler now stores exactly the key it is given, so ids that are not
case-insensitive are never folded together, and an address still lands
on one entry whichever case the caller uses.

Also restore example/pubspec.lock to main's pins — an unrelated
downgrade (matcher, meta, test_api) picked up from an older SDK.

Both reported by Copilot in review of #270.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TkNeiMyH9mAWmK4opt4o8Y

* Improve usage of id

* Fix Tests

* Resolve Edge cases

* Fix tests

* Update doc and agents.md

* Address PR #337 review comments

- Canonicalise queueId when it represents a device address or matches deviceId
- Gate clearQueue fallback on DeviceId.looksLikeAddress to preserve custom queue names
- Remove unused device_id_util.dart and update AGENTS.md
- Combine Device ID and UUID sections concisely in README.md

* Canonicalise queue ids by the same rule when keying and clearing

clearQueue only canonicalised address-shaped ids while keying canonicalised
any deviceId on an isAddress queue, so a queue could be created that
clearQueue could not remove. Derive both keys from one DeviceId.looksLikeAddress
helper (dropping the redundant queueId == deviceId clause it subsumes), which
keeps custom queue ids verbatim and makes every key clearable by its id.

Docs updated to state the rule; regression test added for a non-address id.

* README: use 'case' consistently in the Device ID section

Drop the informal 'casing' and the ambiguous 'in any case' phrasing.

* README: collapse Device ID notes into two platform-labelled bullets

* README: call out Web as the device-ID case exception in text and example

* README: split Device ID example into native and Web blocks

* README: clarify the library normalises native device IDs for string comparison

* README: keep device ID normalisation description high level

* README: add more any-case API call examples

* README: trim any-case example to three connect calls

* Expose UniversalBle.canonicalDeviceId for callers comparing device IDs (#340)

Device IDs are normalised per platform (lower-case for native addresses,
verbatim for opaque Web tokens), but callers had no public way to obtain that
comparable form, so the Web exception leaked into app code that keys maps or
compares IDs. Make the existing internal helper public, document it, and cover
it with tests.

* Address PR #337 follow-up review comments

- Wire the peripheral command queue to the platform's hasAddressDeviceIds and
  expose it on UniversalBlePeripheralPlatform, so peripheral ids are
  canonicalised by the same lever as the client ones.
- Stop mutating the caller's BleDevice in updateScanResult; add
  BleDevice.copyWith that preserves both the sanitised and raw name.
- Document that the event-stream filters now match the canonical id directly.
- Clarify in AGENTS.md that DeviceId is internal.
- Add tests for the pigeon channel native boundary, copyWith, and the
  non-mutating updateScanResult.

* README: clarify canonicalDeviceId is only for IDs from outside the library

IDs the library returns are already normalised, so plain string equality works
for them; canonicalDeviceId is a helper for IDs obtained elsewhere or used as
map keys. Reword the section so it no longer reads as if the call is required.

---------

Co-authored-by: Tyler MacDonald <tyler@macdonald.name>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Foti Dim <foti@navideck.com>
Co-authored-by: Tyler MacDonald <postmaxin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants