New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 624257 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

region, hwid: Allow specifying approved regions in !region_field.

Project Member Reported by hungte@chromium.org, Jun 29 2016

Issue description

Background:

Previously we want to unify the mapping from HWID config to region names by using a fixed "numeric_id". With that, we can do an unified function "!region_field" that populates all regions so people don't need to write explicit region list that the device has to launch. However, this introduced few problems:

 - Sometimes it's not easy to make sure the numeric id is consistent on different branches.
 - Most projects have reserved only 8 bits for region_field and we've already declared more than 256 region codes
 - We find that in a real flow, peng team still has to restrict "approved countries" because of keyboard review flow (even if a country is approved, different OEM / ODM may have different keyboard layout design that has to be reviewed).

Currently most HWID files do:

-------------------------------------------------
region_field: !region_field

region_component: !region_component

rules:
 - name: verify.regions
  evaluate: >
      Assert(GetVPDValue('ro', 'region') in [
          'us', 'gb',
          ])

-------------------------------------------------

Since writing rules is more complicated and the 'unified region encoding' is not making benefits, we'd like to propose changing the region_field to only populate given list, for example the above snippet should be revised as:

-------------------------------------------------
region_field: !region_field [us, gb]

region_component: !region_component
-------------------------------------------------

Cleaner and easier, with only the drawback of region fields not unified across different projects.

For backward compatibility, region_field will still populate all legacy (<256) regions if no list is given.

 

Comment 1 Deleted

Comment 2 Deleted

Comment 3 Deleted

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/8a0cb73e3332ab461bcca6eb0173487350f58e24

commit 8a0cb73e3332ab461bcca6eb0173487350f58e24
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Jun 29 04:53:29 2016

hwid: Allow specifying approved regions in !region_field.

To allow a more simplified way of controlling region approval, this
change has extended the !region_field to support a list as argument.

Previous:
-------------------------------------------------------------------------
region_field: !region_field

rules:
 - name: verify.regions
  evaluate: >
      Assert(GetVPDValue('ro', 'region') in ['us', 'gb',])
-------------------------------------------------------------------------

Now you only need to write:
-------------------------------------------------------------------------
region_field: !region_field [us, gb]
-------------------------------------------------------------------------

This also fixed the problem that region numeric_id is going to exceed the
default reserved size for region_field (8 bit).

BUG= chromium:624257 
TEST=make test

Change-Id: Ia12f8c66039a3dc73184d86342441e53d2f07819
Reviewed-on: https://chromium-review.googlesource.com/356873
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Ricky Liang <jcliang@chromium.org>

[modify] https://crrev.com/8a0cb73e3332ab461bcca6eb0173487350f58e24/py/hwid/v3/yaml_tags.py
[modify] https://crrev.com/8a0cb73e3332ab461bcca6eb0173487350f58e24/py/test/l10n/regions.py

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 2 2016

Labels: merge-merged-factory-glados-7828.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/876a0efbd6303d7939d0ed012e2b6acf53657496

commit 876a0efbd6303d7939d0ed012e2b6acf53657496
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Jun 29 04:53:29 2016

hwid: Allow specifying approved regions in !region_field.

To allow a more simplified way of controlling region approval, this
change has extended the !region_field to support a list as argument.

Previous:
-------------------------------------------------------------------------
region_field: !region_field

rules:
 - name: verify.regions
  evaluate: >
      Assert(GetVPDValue('ro', 'region') in ['us', 'gb',])
-------------------------------------------------------------------------

Now you only need to write:
-------------------------------------------------------------------------
region_field: !region_field [us, gb]
-------------------------------------------------------------------------

This also fixed the problem that region numeric_id is going to exceed the
default reserved size for region_field (8 bit).

BUG= chromium:624257 
TEST=make test

Original-Change-Id: Ia12f8c66039a3dc73184d86342441e53d2f07819
Original-Reviewed-on: https://chromium-review.googlesource.com/356873
Original-Reviewed-by: Ricky Liang <jcliang@chromium.org>
(cherry picked from commit 8a0cb73e3332ab461bcca6eb0173487350f58e24)

Change-Id: I6b004101f35843aac7255da84552d797f80f5f6e
Reviewed-on: https://chromium-review.googlesource.com/358180
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/876a0efbd6303d7939d0ed012e2b6acf53657496/py/hwid/v3/yaml_tags.py
[modify] https://crrev.com/876a0efbd6303d7939d0ed012e2b6acf53657496/py/test/l10n/regions.py

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/876a0efbd6303d7939d0ed012e2b6acf53657496

commit 876a0efbd6303d7939d0ed012e2b6acf53657496
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Jun 29 04:53:29 2016

hwid: Allow specifying approved regions in !region_field.

To allow a more simplified way of controlling region approval, this
change has extended the !region_field to support a list as argument.

Previous:
-------------------------------------------------------------------------
region_field: !region_field

rules:
 - name: verify.regions
  evaluate: >
      Assert(GetVPDValue('ro', 'region') in ['us', 'gb',])
-------------------------------------------------------------------------

Now you only need to write:
-------------------------------------------------------------------------
region_field: !region_field [us, gb]
-------------------------------------------------------------------------

This also fixed the problem that region numeric_id is going to exceed the
default reserved size for region_field (8 bit).

BUG= chromium:624257 
TEST=make test

Original-Change-Id: Ia12f8c66039a3dc73184d86342441e53d2f07819
Original-Reviewed-on: https://chromium-review.googlesource.com/356873
Original-Reviewed-by: Ricky Liang <jcliang@chromium.org>
(cherry picked from commit 8a0cb73e3332ab461bcca6eb0173487350f58e24)

Change-Id: I6b004101f35843aac7255da84552d797f80f5f6e
Reviewed-on: https://chromium-review.googlesource.com/358180
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/876a0efbd6303d7939d0ed012e2b6acf53657496/py/hwid/v3/yaml_tags.py
[modify] https://crrev.com/876a0efbd6303d7939d0ed012e2b6acf53657496/py/test/l10n/regions.py

Comment 7 by hungte@chromium.org, Aug 11 2016

Owner: hungte@chromium.org
Status: Verified (was: Untriaged)
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/32e8f8fbfb224fbdf86667aac3e3a94cb81661b8

commit 32e8f8fbfb224fbdf86667aac3e3a94cb81661b8
Author: Hung-Te Lin <hungte@chromium.org>
Date: Fri Aug 12 04:29:43 2016

hwid: Change !region to use it own numeric mapping.

Per offline discussion we believe there is no need to maintain original
mapping in selective region tag because
 - That was not really requested before.
 - Maintaining values means HWID config must increase reserved region
   field length, which also makes it incompatible.

BUG= chromium:624257 
TEST=make test

Change-Id: Ie5f581f31cc8b6392465f84941ec73937805e5be
Reviewed-on: https://chromium-review.googlesource.com/368501
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Ricky Liang <jcliang@chromium.org>

[modify] https://crrev.com/32e8f8fbfb224fbdf86667aac3e3a94cb81661b8/py/hwid/v3/yaml_tags.py

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 12 2016

Labels: merge-merged-factory-gru-8557.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/385b9077ca452ba511ee93ba1c8ac5e591da7ac2

commit 385b9077ca452ba511ee93ba1c8ac5e591da7ac2
Author: Hung-Te Lin <hungte@chromium.org>
Date: Fri Aug 12 04:29:43 2016

hwid: Change !region to use it own numeric mapping.

Per offline discussion we believe there is no need to maintain original
mapping in selective region tag because
 - That was not really requested before.
 - Maintaining values means HWID config must increase reserved region
   field length, which also makes it incompatible.

BUG= chromium:624257 
TEST=make test

Change-Id: Ia2d1f2ebb7382a869513d845e66ef1af4804b57e
Original-Reviewed-On: https://chromium-review.googlesource.com/368501
Original-Change-Id: Ie5f581f31cc8b6392465f84941ec73937805e5be
Reviewed-on: https://chromium-review.googlesource.com/368563
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/385b9077ca452ba511ee93ba1c8ac5e591da7ac2/py/hwid/v3/yaml_tags.py

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/fc812b05549f58c8798e46f56a888918189e1d82

commit fc812b05549f58c8798e46f56a888918189e1d82
Author: Hung-Te Lin <hungte@chromium.org>
Date: Fri Aug 12 04:29:43 2016

hwid: Change !region to use it own numeric mapping.

Per offline discussion we believe there is no need to maintain original
mapping in selective region tag because
 - That was not really requested before.
 - Maintaining values means HWID config must increase reserved region
   field length, which also makes it incompatible.

BUG= chromium:624257 
TEST=make test

Change-Id: Iea04c5b946bea3849c3f19e4df47027052bf92e7
Original-Reviewed-On: https://chromium-review.googlesource.com/368501
Original-Change-Id: Ie5f581f31cc8b6392465f84941ec73937805e5be
Reviewed-on: https://chromium-review.googlesource.com/368562
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/fc812b05549f58c8798e46f56a888918189e1d82/py/hwid/v3/yaml_tags.py

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 17 2016

Labels: merge-merged-factory-gru-8652.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/d6411f2426e938073f1188e32ba449d4e078b3d4

commit d6411f2426e938073f1188e32ba449d4e078b3d4
Author: Hung-Te Lin <hungte@chromium.org>
Date: Fri Aug 12 04:29:43 2016

hwid: Change !region to use it own numeric mapping.

Per offline discussion we believe there is no need to maintain original
mapping in selective region tag because
 - That was not really requested before.
 - Maintaining values means HWID config must increase reserved region
   field length, which also makes it incompatible.

BUG= chromium:624257 
TEST=make test

Change-Id: Iea34a4ee99c64073ab2cf7fbef5a32acd2028f06
Original-Reviewed-On: https://chromium-review.googlesource.com/368501
Original-Change-Id: Ie5f581f31cc8b6392465f84941ec73937805e5be
Reviewed-on: https://chromium-review.googlesource.com/368564
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Commit-Queue: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/d6411f2426e938073f1188e32ba449d4e078b3d4/py/hwid/v3/yaml_tags.py

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 22

Labels: merge-merged-factory-oak-8182.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/17f4335e42bff4f51090f07b078ad6708ec9d7a5

commit 17f4335e42bff4f51090f07b078ad6708ec9d7a5
Author: Hung-Te Lin <hungte@chromium.org>
Date: Thu Nov 22 07:49:10 2018

hwid: Allow specifying approved regions in !region_field.

To allow a more simplified way of controlling region approval, this
change has extended the !region_field to support a list as argument.

Previous:
-------------------------------------------------------------------------
region_field: !region_field

rules:
 - name: verify.regions
  evaluate: >
      Assert(GetVPDValue('ro', 'region') in ['us', 'gb',])
-------------------------------------------------------------------------

Now you only need to write:
-------------------------------------------------------------------------
region_field: !region_field [us, gb]
-------------------------------------------------------------------------

This also fixed the problem that region numeric_id is going to exceed the
default reserved size for region_field (8 bit).

BUG= chromium:624257 
TEST=make test

Change-Id: Ia12f8c66039a3dc73184d86342441e53d2f07819
Reviewed-on: https://chromium-review.googlesource.com/356873
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Ricky Liang <jcliang@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/1347307
Reviewed-by: Wei-Han Chen <stimim@chromium.org>
Commit-Queue: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>
Trybot-Ready: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/17f4335e42bff4f51090f07b078ad6708ec9d7a5/py/hwid/v3/yaml_tags.py
[modify] https://crrev.com/17f4335e42bff4f51090f07b078ad6708ec9d7a5/py/test/l10n/regions.py

Sign in to add a comment