Use sequence instead of map for data filters by beaufortfrancois · Pull Request #545 · WebBluetoothCG/web-bluetooth

beaufortfrancois · 2021-04-27T13:26:30Z

The more I'm playing with manufacturer data filters, the more I think we should go with a sequence instead of a map in BluetoothLEScanFilterInit for both manufacturerData and serviceData.

Because users can have different strings to express the same uint16 ('0' and '00' for instance), we'll have to check for the unicity of the key which is what we're doing already with a sequence.
Moreover, having a required companyIdentifier dictionary key seems more readable than a simple object key.

I've added some code samples below to give you an idea of what I'm experiencing.

// It's not easy to understand what we're filtering on at first glance and the
// empty object isn't nice to read as well.
let manufacturerData = { 0x00e0: {} };
navigator.bluetooth.requestDevice({ filters: [{ manufacturerData }] });

// And with the base-10 serialization of a uint16, it's not much better.
// We would have to handle the `0x` case among others or let developers do it.
let manufacturerData = { "224": {} };
navigator.bluetooth.requestDevice({ filters: [{ manufacturerData }] });

And here's what I'm proposing:

// [NEW] Simplest form.
let manufacturerData = [{ companyIdentifier: 0x00e0 }];
navigator.bluetooth.requestDevice({ filters: [{ manufacturerData }] });

// [NEW] Full filtering on manufacturer data.
let manufacturerData = [
  {
    companyIdentifier: 0x00e0,
    dataPrefix: new Uint8Array([0x01, 0x02, 0x03, 0x04]),
    mask: new Uint8Array([0xff, 0xff, 0xff, 0x00]),
  },
];
navigator.bluetooth.requestDevice({ filters: [{ manufacturerData }] });

@reillyeon What do you think?

Preview | Diff

beaufortfrancois · 2021-04-27T13:44:26Z

Note that CI should succeed once #546 is merged.

This CL adds support for new manufacturerData filter so that developers can request Bluetooth LE devices based on manufacturer specific data (company identifier and data). Spec: WebBluetoothCG/web-bluetooth#545 Test: https://manufacturer-data.glitch.me/ Bug: 707635 Change-Id: I63b80812f35c8f0f557ceaf53632d0d6d2d52b9b

reillyeon · 2021-04-27T23:02:27Z

-    1. Let |canonicalizedDataFilter| be the result of <a
-        for="BluetoothDataFilterInit">canonicalizing</a> |dataFilter|,
+    present and 
+    <code>|filter|.{{BluetoothLEScanFilterInit/manufacturerData}}.length === 0</code>,


A nit, as was pointed out to me on the most recent specifications I worked on, the correct syntax for referring to a member of a dictionary is filter["manufacturerData"] rather than filter.manufacturerData. We should do a pass to fix this over the whole specification but might as well avoid introducing new examples of the old style.

reillyeon · 2021-04-27T23:04:57Z

+[
+  {
+    manufacturerData: [
+      { companyIdentifier: 17, dataPrefix: new Uint8Array([1, 2, 3]) },


Put companyIdentifier and dataPrefix on separate lines in this and the next two examples to avoid this table column becoming too wide.

reillyeon · 2021-04-27T23:10:29Z

+1. For each |manufacturerData| in
+    <code>|filter|.{{BluetoothLEScanFilterInit/manufacturerData}}</code>, do the following sub-steps:
+    1. If <code>|manufacturerData|.{{BluetoothManufacturerDataFilterInit/companyIdentifier}}</code>
+        is present in |canonicalizedFilter|.manufacturerData,


reillyeon · 2021-04-27T23:15:19Z

+        for="BluetoothDataFilterInit">canonicalizing</a> |manufacturerData|,
        <a>converted to an ECMAScript value</a>. If this throws an exception,
        propagate that exception and abort these steps.
    1. Call <a abstract-op>CreateDataProperty</a>


Since canonicalizedFilter["manufacturerData"] is now a sequence rather than an object we can append to it.

reillyeon · 2021-04-27T23:15:55Z

@@ -1400,51 +1431,42 @@ returned from the following steps:
        <code><var>filter</var>.namePrefix</code>.
 1. Set <code>|canonicalizedFilter|.manufacturerData</code> to `{}`.


Initialize canonicalizedFilter["manufacturerData"] to an empty array rather than an empty object.

reillyeon · 2021-04-27T23:16:36Z

+        (|canonicalizedFilter|.manufacturerData,
+        <code>|manufacturerData|.{{BluetoothManufacturerDataFilterInit/companyIdentifier}}</code>,
+        |canonicalizedManufacturerDataFilter|).
 1. Set <code>|canonicalizedFilter|.serviceData</code> to `{}`.


reillyeon · 2021-04-27T23:16:54Z

        <a>converted to an ECMAScript value</a>. If this throws an exception,
        propagate that exception and abort these steps.
    1. Call <a
        abstract-op>CreateDataProperty</a>(|canonicalizedFilter|.serviceData,


reillyeon · 2021-04-27T23:20:29Z

-    type:enum-value; for:PermissionName; text:"bluetooth"
 spec: permissions-1
    type: enum-value
+        text: "bluetooth"


In #546 you only removed the conflicting enum value above. Is this change also necessary?

It's not anymore. Thanks for catching!

beaufortfrancois

Thanks for the review!

beaufortfrancois · 2021-04-28T06:11:29Z

+[
+  {
+    manufacturerData: [
+      { companyIdentifier: 17, dataPrefix: new Uint8Array([1, 2, 3]) },


beaufortfrancois · 2021-04-28T06:25:05Z

-    1. Let |canonicalizedDataFilter| be the result of <a
-        for="BluetoothDataFilterInit">canonicalizing</a> |dataFilter|,
+    present and 
+    <code>|filter|.{{BluetoothLEScanFilterInit/manufacturerData}}.length === 0</code>,


beaufortfrancois · 2021-04-28T06:33:43Z

+1. For each |manufacturerData| in
+    <code>|filter|.{{BluetoothLEScanFilterInit/manufacturerData}}</code>, do the following sub-steps:
+    1. If <code>|manufacturerData|.{{BluetoothManufacturerDataFilterInit/companyIdentifier}}</code>
+        is present in |canonicalizedFilter|.manufacturerData,


beaufortfrancois · 2021-04-28T06:34:15Z

-    type:enum-value; for:PermissionName; text:"bluetooth"
 spec: permissions-1
    type: enum-value
+        text: "bluetooth"


It's not anymore. Thanks for catching!

beaufortfrancois · 2021-04-28T06:50:45Z

+        for="BluetoothDataFilterInit">canonicalizing</a> |manufacturerData|,
        <a>converted to an ECMAScript value</a>. If this throws an exception,
        propagate that exception and abort these steps.
    1. Call <a abstract-op>CreateDataProperty</a>


beaufortfrancois · 2021-04-28T06:50:50Z

+        (|canonicalizedFilter|.manufacturerData,
+        <code>|manufacturerData|.{{BluetoothManufacturerDataFilterInit/companyIdentifier}}</code>,
+        |canonicalizedManufacturerDataFilter|).
 1. Set <code>|canonicalizedFilter|.serviceData</code> to `{}`.


beaufortfrancois · 2021-04-28T06:50:53Z

        <a>converted to an ECMAScript value</a>. If this throws an exception,
        propagate that exception and abort these steps.
    1. Call <a
        abstract-op>CreateDataProperty</a>(|canonicalizedFilter|.serviceData,


beaufortfrancois · 2021-04-28T06:51:05Z

@@ -1400,51 +1431,42 @@ returned from the following steps:
        <code><var>filter</var>.namePrefix</code>.
 1. Set <code>|canonicalizedFilter|.manufacturerData</code> to `{}`.


This CL adds support for new manufacturerData filter so that developers can request Bluetooth LE devices based on manufacturer specific data (company identifier and data). Spec: WebBluetoothCG/web-bluetooth#545 Test: https://manufacturer-data.glitch.me/ Bug: 707635 Change-Id: I63b80812f35c8f0f557ceaf53632d0d6d2d52b9b

@beaufortfrancois

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@beaufortfrancois

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@beaufortfrancois

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@beaufortfrancois

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

This CL adds support for new manufacturerData filter so that developers can request Bluetooth LE devices based on manufacturer specific data (company identifier and data). Spec: WebBluetoothCG/web-bluetooth#545 Test: https://manufacturer-data.glitch.me/ Bug: 707635 Change-Id: I63b80812f35c8f0f557ceaf53632d0d6d2d52b9b

This CL adds support for new manufacturerData filter so that developers can request Bluetooth LE devices based on manufacturer specific data (company identifier and data). Spec: WebBluetoothCG/web-bluetooth#545 Test: https://manufacturer-data.glitch.me/ Bug: 707635 Change-Id: I63b80812f35c8f0f557ceaf53632d0d6d2d52b9b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2830933 Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#879284}

…ilter when requesting device, a=testonly Automatic update from web-platform-tests [Web Bluetooth] Add `manufacturerData` filter when requesting device This CL adds support for new manufacturerData filter so that developers can request Bluetooth LE devices based on manufacturer specific data (company identifier and data). Spec: WebBluetoothCG/web-bluetooth#545 Test: https://manufacturer-data.glitch.me/ Bug: 707635 Change-Id: I63b80812f35c8f0f557ceaf53632d0d6d2d52b9b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2830933 Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#879284} -- wpt-commits: d50983bfca33b58be57894725b190b8c1a011ddc wpt-pr: 28718

beaufortfrancois force-pushed the data-filter branch from 02ba744 to 3e350a0 Compare April 27, 2021 13:31

This was referenced Apr 27, 2021

Accept filters on manufacturer and service data. #297

Merged

Adding simple manufacturer id filtering support #544

Closed

beaufortfrancois force-pushed the data-filter branch from 3e350a0 to e3139c1 Compare April 27, 2021 13:48

chromium-wpt-export-bot mentioned this pull request Apr 27, 2021

[Web Bluetooth] Add manufacturerData filter when requesting device web-platform-tests/wpt#28718

Merged

reillyeon reviewed Apr 27, 2021

View reviewed changes

Use sequence instead of map for data filters

509e088

beaufortfrancois force-pushed the data-filter branch from e3139c1 to 509e088 Compare April 28, 2021 06:08

beaufortfrancois commented Apr 28, 2021

View reviewed changes

Address @reillyeon feedback

e7a6e1a

beaufortfrancois force-pushed the data-filter branch from 953f24d to e7a6e1a Compare April 28, 2021 09:39

reillyeon approved these changes Apr 28, 2021

View reviewed changes

beaufortfrancois merged commit d9eeae5 into main Apr 29, 2021

beaufortfrancois deleted the data-filter branch April 29, 2021 07:23

github-actions Bot added a commit that referenced this pull request Apr 29, 2021

Use sequence instead of map for data filters (#545)

9bd70ac

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions Bot added a commit that referenced this pull request Apr 29, 2021

Use sequence instead of map for data filters (#545)

9fcc4b9

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions Bot added a commit that referenced this pull request Apr 29, 2021

Use sequence instead of map for data filters (#545)

ddf681d

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions Bot added a commit that referenced this pull request Apr 29, 2021

Use sequence instead of map for data filters (#545)

7740f52

SHA: d9eeae5 Reason: push, by @beaufortfrancois Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

beaufortfrancois mentioned this pull request Apr 29, 2021

Add Manufacturer Data Filter Web Bluetooth sample GoogleChrome/samples#719

Merged

		@@ -1400,51 +1431,42 @@ returned from the following steps:
		<code><var>filter</var>.namePrefix</code>.
		1. Set <code>\|canonicalizedFilter\|.manufacturerData</code> to `{}`.

Uh oh!

Conversation

beaufortfrancois commented Apr 27, 2021 • edited by pr-preview Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

beaufortfrancois commented Apr 27, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reillyeon Apr 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

beaufortfrancois left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

beaufortfrancois commented Apr 27, 2021 •

edited by pr-preview Bot

Loading

reillyeon Apr 27, 2021 •

edited

Loading