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

Issue 889250 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task



Sign in to add a comment

Improve process for biod to include ec_commands.h

Project Member Reported by norvez@chromium.org, Sep 25

Issue description

cros_fp_biometrics_manager needs to know about EC commands. Right now we're manually copying ec_commands.h from plaftorm/ec/include to platform2/biod/ec. That:

- is cumbersome
- triggers presubmit failures in biod since ec_commands.h does not conform the platform2/s coding style


Label R-V-G since the feature hasn't been officially released yet.
 
Is it possible to move this to src/platform/system_api ? Sounds like this would be appropriate for that location?
Agree with comment 1. system_api/biod currently list the names shared between biod and Chrome. 
Cc: derat@chromium.org
norvez@ does this sound OK? +derat@ for guidance regarding whether platform/system_api can house headers shared b/w platform2 and platform/ec.
Cc: vapier@chromium.org hidehiko@chromium.org
Components: OS>Systems
system_api is in platform2 now:  https://crbug.com/874735 
Components: UI>Shell>Fingerprint
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/9ddc84f9c6c7e7ced001549f491b111505e0f438

commit 9ddc84f9c6c7e7ced001549f491b111505e0f438
Author: Nicolas Norvez <norvez@chromium.org>
Date: Thu Nov 15 16:11:41 2018

biod: disable linter for EC headers

EC headers come from the EC codebase which has a different coding style,
so linter errors are expected (and extremely numerous...). Exclude them
from the linter in the presubmit tests.

BUG= chromium:889250 
TEST=no "cros lint" errors for CLs that include a change to
EC headers

Change-Id: I146c08b93cdd43d8c34d4ab30066f3d15c850d09
Reviewed-on: https://chromium-review.googlesource.com/1322029
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Prashant Malani <pmalani@google.com>

[add] https://crrev.com/9ddc84f9c6c7e7ced001549f491b111505e0f438/biod/ec/CPPLINT.cfg

Labels: -Restrict-View-Google
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/3d4652613c6d2beea8e0dd90f12612fad8eec4d8

commit 3d4652613c6d2beea8e0dd90f12612fad8eec4d8
Author: Nicolas Norvez <norvez@chromium.org>
Date: Sat Nov 17 03:50:45 2018

headers: make EC commands headers C++-friendly

- wrap headers in 'extern "C"'
- use relative path to #include

BRANCH=None
BUG= chromium:889250 
TEST=make buildall -j
TEST=emerge-nocturne ec-utils

Change-Id: I67d8ba88edf77f72bd54500eff169537ffb6257f
Signed-off-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1338599
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/3d4652613c6d2beea8e0dd90f12612fad8eec4d8/include/ec_commands.h
[modify] https://crrev.com/3d4652613c6d2beea8e0dd90f12612fad8eec4d8/util/cros_ec_dev.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/dc8e34ae6575ef3032f86c21c233dcdc78ea613c

commit dc8e34ae6575ef3032f86c21c233dcdc78ea613c
Author: Nicolas Norvez <norvez@chromium.org>
Date: Sat Nov 17 06:33:34 2018

cros-ec-headers: add EC commands

Install the headers defining the EC command interface so userspace (e.g.
biod) can use them.

BUG= chromium:889250 
TEST=emerge-nocturne biod
CQ-DEPEND=CL:1338599

Change-Id: Ief1f188d1c3bc6c0a0335dedea27399da9f9ddff
Reviewed-on: https://chromium-review.googlesource.com/1338497
Commit-Ready: Nicolas Norvez <norvez@chromium.org>
Tested-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/dc8e34ae6575ef3032f86c21c233dcdc78ea613c/chromeos-base/chromeos-ec-headers/chromeos-ec-headers-9999.ebuild
[modify] https://crrev.com/dc8e34ae6575ef3032f86c21c233dcdc78ea613c/chromeos-base/biod/biod-9999.ebuild

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/51bbb1151fccf54a71a8aa9123fb8d25e634af7e

commit 51bbb1151fccf54a71a8aa9123fb8d25e634af7e
Author: Nicolas Norvez <norvez@chromium.org>
Date: Sun Nov 18 00:41:44 2018

biod: use C++-friendly EC headers

- EC headers are now installed by chromeos-ec-headers, we no longer need
to keep duplicate copies around
- No need for 'extern "C"' anymore.

BUG= chromium:889250 
TEST=emerge-nocturne biod
CQ-DEPEND=CL:1338497

Change-Id: I879de8881ef6906fb573590b196a5040738849e0
Reviewed-on: https://chromium-review.googlesource.com/1337707
Commit-Ready: Nicolas Norvez <norvez@chromium.org>
Tested-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/51bbb1151fccf54a71a8aa9123fb8d25e634af7e/biod/cros_fp_biometrics_manager.h
[delete] https://crrev.com/bc11e699c37073c11d044939a7a15ef093b6e664/biod/ec/CPPLINT.cfg
[delete] https://crrev.com/bc11e699c37073c11d044939a7a15ef093b6e664/biod/ec/compile_time_macros.h
[delete] https://crrev.com/bc11e699c37073c11d044939a7a15ef093b6e664/biod/ec/ec_commands.h
[modify] https://crrev.com/51bbb1151fccf54a71a8aa9123fb8d25e634af7e/biod/bio_crypto_init.cc
[delete] https://crrev.com/bc11e699c37073c11d044939a7a15ef093b6e664/biod/ec/cros_ec_dev.h

Status: Fixed (was: Assigned)
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5

Labels: merge-merged-factory-nami-10715.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/1676271a53f83b42e4df758a97aeaf4474010371

commit 1676271a53f83b42e4df758a97aeaf4474010371
Author: Nicolas Norvez <norvez@chromium.org>
Date: Wed Dec 05 23:34:12 2018

headers: make EC commands headers C++-friendly

- wrap headers in 'extern "C"'
- use relative path to #include

BRANCH=None
BUG= chromium:889250 
TEST=make buildall -j
TEST=emerge-nocturne ec-utils

Change-Id: I67d8ba88edf77f72bd54500eff169537ffb6257f
Signed-off-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1338599
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 3d4652613c6d2beea8e0dd90f12612fad8eec4d8)
Reviewed-on: https://chromium-review.googlesource.com/c/1361818
Reviewed-by: Marco Chen <marcochen@chromium.org>
Reviewed-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/1676271a53f83b42e4df758a97aeaf4474010371/include/ec_commands.h
[modify] https://crrev.com/1676271a53f83b42e4df758a97aeaf4474010371/util/cros_ec_dev.h

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/78a4d1ed780312c1e285b12c17c2e1a723e87110

commit 78a4d1ed780312c1e285b12c17c2e1a723e87110
Author: Nicolas Norvez <norvez@chromium.org>
Date: Sat Dec 08 02:09:13 2018

biod: disable linter for EC headers

EC headers come from the EC codebase which has a different coding style,
so linter errors are expected (and extremely numerous...). Exclude them
from the linter in the presubmit tests.

BUG= chromium:889250 
TEST=no "cros lint" errors for CLs that include a change to
EC headers

Change-Id: I146c08b93cdd43d8c34d4ab30066f3d15c850d09
Reviewed-on: https://chromium-review.googlesource.com/1322029
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Prashant Malani <pmalani@google.com>
(cherry picked from commit 9ddc84f9c6c7e7ced001549f491b111505e0f438)
Reviewed-on: https://chromium-review.googlesource.com/c/1362359
Reviewed-by: YH Lin <yueherngl@chromium.org>
Commit-Queue: YH Lin <yueherngl@chromium.org>
Tested-by: YH Lin <yueherngl@chromium.org>

[add] https://crrev.com/78a4d1ed780312c1e285b12c17c2e1a723e87110/biod/ec/CPPLINT.cfg

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2f3006431dee699ceaf0e855ac1dadd0af67f9f3

commit 2f3006431dee699ceaf0e855ac1dadd0af67f9f3
Author: Nicolas Norvez <norvez@chromium.org>
Date: Sat Dec 08 02:09:16 2018

biod: use C++-friendly EC headers

- EC headers are now installed by chromeos-ec-headers, we no longer need
to keep duplicate copies around
- No need for 'extern "C"' anymore.

BUG= chromium:889250 
TEST=emerge-nocturne biod
CQ-DEPEND=CL:1338497

Change-Id: I879de8881ef6906fb573590b196a5040738849e0
Reviewed-on: https://chromium-review.googlesource.com/1337707
Commit-Ready: Nicolas Norvez <norvez@chromium.org>
Tested-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 51bbb1151fccf54a71a8aa9123fb8d25e634af7e)
Reviewed-on: https://chromium-review.googlesource.com/c/1362362
Reviewed-by: YH Lin <yueherngl@chromium.org>
Commit-Queue: YH Lin <yueherngl@chromium.org>
Tested-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/2f3006431dee699ceaf0e855ac1dadd0af67f9f3/biod/cros_fp_biometrics_manager.h
[delete] https://crrev.com/28f6d509e108d330d4269c2ef4e3807a24d8a629/biod/ec/CPPLINT.cfg
[delete] https://crrev.com/28f6d509e108d330d4269c2ef4e3807a24d8a629/biod/ec/compile_time_macros.h
[delete] https://crrev.com/28f6d509e108d330d4269c2ef4e3807a24d8a629/biod/ec/ec_commands.h
[modify] https://crrev.com/2f3006431dee699ceaf0e855ac1dadd0af67f9f3/biod/bio_crypto_init.cc
[delete] https://crrev.com/28f6d509e108d330d4269c2ef4e3807a24d8a629/biod/ec/cros_ec_dev.h

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/878d4d11b976ad366fa0f6431bfb6ce127fd66a9

commit 878d4d11b976ad366fa0f6431bfb6ce127fd66a9
Author: Nicolas Norvez <norvez@chromium.org>
Date: Sat Dec 08 02:14:30 2018

cros-ec-headers: add EC commands

Install the headers defining the EC command interface so userspace (e.g.
biod) can use them.

Manually merged to resolve conflict due to the missing of

  doins include/u2f.h
  doins board/cr50/tpm2/virtual_nvmem.h

in chromeos-ec-headers ebuild.

BUG= chromium:889250 
TEST=emerge-nocturne biod
CQ-DEPEND=CL:1338599

Change-Id: Ief1f188d1c3bc6c0a0335dedea27399da9f9ddff
Reviewed-on: https://chromium-review.googlesource.com/1338497
Commit-Ready: Nicolas Norvez <norvez@chromium.org>
Tested-by: Nicolas Norvez <norvez@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit dc8e34ae6575ef3032f86c21c233dcdc78ea613c)
Reviewed-on: https://chromium-review.googlesource.com/c/1363721
Reviewed-by: YH Lin <yueherngl@chromium.org>
Commit-Queue: YH Lin <yueherngl@chromium.org>
Tested-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/878d4d11b976ad366fa0f6431bfb6ce127fd66a9/chromeos-base/chromeos-ec-headers/chromeos-ec-headers-9999.ebuild
[modify] https://crrev.com/878d4d11b976ad366fa0f6431bfb6ce127fd66a9/chromeos-base/biod/biod-9999.ebuild

Sign in to add a comment