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

Issue 659919 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

HWID: do not match "unsupported" component item

Project Member Reported by akahuang@chromium.org, Oct 27 2016

Issue description

Sometimes the hardware component might be probed to different value, and we don't notice at first. We would create more than one component item for the same component.
For example, the driver might add ".[1-9].auto" randomly in the probed result, and we might set the HWID database like this:

components:
  bar:
    foo_EVT:
      value:
        compact_str: "foo.1.auto"
    foo_DVT:
      value:
        compact_str: "foo.2.auto"

To solve this, we add a new status called "invisible". The invisible component item would not be matched. Then we can add the correct item and then set the wrong items to invisible. For example:

components:
  bar:
    foo_equivalent_EVT:
      status: invisible
      value:
        compact_str: "foo.1.auto"
    foo_equivalent_DVT:
      status: invisible
      value:
        compact_str: "foo.2.auto"
    foo:
      value:
        compact_str: !re "foo.[1-9].auto"

When we match the "foo.1.auto", it will ignore "foo_1" and only match "foo". To clarify foo_1, foo_2 are actually the same component as foo", we can only change the name to "foo_equivalent_xxx" and keep the value.
 

Comment 1 by hungte@chromium.org, Oct 27 2016

Cc: kitching@chromium.org
Not sure if 'invisible' is the best name since it makes me feel "this is still valid to match, just don't show it".

What about
 abandoned
 inactive
 ignored
 uneffective
 void

+kitching, native speaker?

Comment 2 by kitching@google.com, Oct 27 2016

Hidden? Masked?

Comment 3 by hungte@chromium.org, Oct 27 2016

Re#2: some correction - "this is still valid to match, just don't show it" <- this is NOT what we want.

What we want is
 "This value is here for historical reasons and for reference, but we do not want to use it at all. It should not be used for matching, nor displayed or whatever"

Comment 4 by hungte@chromium.org, Oct 27 2016

or defective
How about "unmatched"? It's pretty straightforward :P

BTW, currently we have these 4 statuses:
supported
  Default status. The component is currently used to build new units.

unqualified
  The component is now allowed to be used after PVT phase.

deprecated
  The component is no longer being used to build new units, but is supported in RMA process.

unsupported
  The component is not allowed to be used to build new units, and is not supported in RMA process.

Comment 6 by hungte@chromium.org, Oct 27 2016

or should we just use unsupported?

Comment 7 by hungte@chromium.org, Oct 27 2016

I mean, if unsupported = a component not allowed in both new and RMA units, isn't it supposed to be "not used to match anything"?

Although the semantic is slightly different, but consider the real cases,

 camera1:
  status: unsupported
    compact_str: !re ".*"
 camera2:
  values:
    compact_str: "something"

Does this make sense? If yes, then it's very close to what we'd like to use.
Summary: HWID: do not match "unsupported" component item (was: HWID: add "invisible" status)
Great idea!! Let's change the logic of "unsupported" status directly. Also change the title of the issue.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 8 2016

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

commit 9b5155ca68a82cc0a111e5c57f65331d50224730
Author: Chih-Yu Huang <akahuang@google.com>
Date: Wed Nov 02 10:40:12 2016

hwid: do not match "unsupported" component item.

BUG= chromium:659919 
TEST=unittest in py/hwid/v3

Change-Id: I1603e78d57f2befdb484b93409e80e3693e2df6e
Reviewed-on: https://chromium-review.googlesource.com/406890
Commit-Ready: Chih-Yu Huang <akahuang@chromium.org>
Tested-by: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Wei-Han Chen <stimim@chromium.org>

[modify] https://crrev.com/9b5155ca68a82cc0a111e5c57f65331d50224730/py/hwid/v3/database.py

Status: Fixed (was: Assigned)
Status: Assigned (was: Fixed)
Hi chih-yu, please merge this to gru factory branch. Thanks!
I cherry-pick to gru factory branch in this CL:

https://chromium-review.googlesource.com/#/c/411687/

Wait for merge.
The CL in #9 would let decoding old HWID string failed if the HWID contains unsupported component. Revert it first and then find another way to implement it.
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 17 2016

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

commit d8932584b1c1a7a8909ff3a68cace159689c901d
Author: Chih-Yu Huang <akahuang@google.com>
Date: Thu Nov 17 04:20:53 2016

Revert "hwid: do not match "unsupported" component item."

The CL causes the decoding HWID fails if the HWID string contains unsupported
components. Revert it first and find another way to implement the feature.

BUG= chromium:659919 
TEST=none

This reverts commit 9b5155ca68a82cc0a111e5c57f65331d50224730.

Change-Id: I719440ee299580d8b9e1f9712e603d6624e308a9
Reviewed-on: https://chromium-review.googlesource.com/411559
Commit-Ready: Chih-Yu Huang <akahuang@chromium.org>
Tested-by: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/d8932584b1c1a7a8909ff3a68cace159689c901d/py/hwid/v3/database.py

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 17 2016

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

commit d8932584b1c1a7a8909ff3a68cace159689c901d
Author: Chih-Yu Huang <akahuang@google.com>
Date: Thu Nov 17 04:20:53 2016

Revert "hwid: do not match "unsupported" component item."

The CL causes the decoding HWID fails if the HWID string contains unsupported
components. Revert it first and find another way to implement the feature.

BUG= chromium:659919 
TEST=none

This reverts commit 9b5155ca68a82cc0a111e5c57f65331d50224730.

Change-Id: I719440ee299580d8b9e1f9712e603d6624e308a9
Reviewed-on: https://chromium-review.googlesource.com/411559
Commit-Ready: Chih-Yu Huang <akahuang@chromium.org>
Tested-by: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/d8932584b1c1a7a8909ff3a68cace159689c901d/py/hwid/v3/database.py

Cc: petershih@chromium.org
Labels: OS-Chrome
Chih-yu, do we have anything left for this issue?
Cc: yhong@chromium.org
This issue is trying to handling the wrong component item which is covered by new item. Originally we try to ignore "unsupported" item but it makes decoding failed. Now this issue hasn't been fixed.


I think there are two solutions: one is add a label "ignored" or "shadowed". The meaning is: 
- Ignore the item when we encode the HWID, because it is covered by other item.
- Treat the item as valid when we decode HWID, because it is valid when it was encoded.

The hard thing is: it's hard to integrate with database builder. Currently the database builder cannot output the item with regex rule. We must detect this by human.


Alternative solution is: ask people to "extend" the component item directly. For example, the original database is:

components:
  bar:
    foo_EVT:
      value:
        compact_str: "foo.1.auto"

Now we found the same component item has different probe result: "foo.2.auto". Then we just modify the item to:

components:
  bar:
    foo_EVT:
      value:
        compact_str: !re "foo.[1-9].auto"

It also require human to detect it. But the effort should be smaller than the first solution.


BTW, the root cause of this issue is issue:659921. We fixed it by removing ".*.auto" suffix in probe result. Maybe the third solution is: prevent this kind of situation occurs again...
I hate regex in hwid config since that'll add extra entry barrier for partner and SIE to work on HWID....


Cc: -petershih@chromium.org -yhong@chromium.org -kitching@chromium.org -hungte@chromium.org chromeos-factory-eng@google.com
Owner: yhong@chromium.org
Move owner to yhong.
Project Member

Comment 20 by bugdroid1@chromium.org, Apr 17 2018

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

commit 5d998f67103a49ae4f7aee75c2c2b70b82f9b98a
Author: Yong Hong <yhong@chromium.org>
Date: Tue Apr 17 07:47:07 2018

hwid: Do not match "unsupported" component item.

This CL makes HWID framework ignores the unsupported component
while it is converting the probed results into the BOM object.

BUG= chromium:659919 
TEST=make test

Change-Id: Ifbf82c91f1ee09130b58ab18db6791ba9fd71a49
Reviewed-on: https://chromium-review.googlesource.com/1011483
Commit-Ready: Marco Chen <marcochen@chromium.org>
Tested-by: Marco Chen <marcochen@chromium.org>
Reviewed-by: Yilun Lin <yllin@chromium.org>
Reviewed-by: Marco Chen <marcochen@chromium.org>

[modify] https://crrev.com/5d998f67103a49ae4f7aee75c2c2b70b82f9b98a/py/hwid/v3/probe_unittest.py
[modify] https://crrev.com/5d998f67103a49ae4f7aee75c2c2b70b82f9b98a/py/hwid/v3/database.py
[modify] https://crrev.com/5d998f67103a49ae4f7aee75c2c2b70b82f9b98a/py/hwid/v3/builder.py
[modify] https://crrev.com/5d998f67103a49ae4f7aee75c2c2b70b82f9b98a/py/hwid/v3/testdata/test_probe_db.yaml
[modify] https://crrev.com/5d998f67103a49ae4f7aee75c2c2b70b82f9b98a/py/hwid/v3/probe.py

Status: Fixed (was: Assigned)

Sign in to add a comment