New issue
Advanced search Search tips

Issue 777214 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Implement a 'default' property

Project Member Reported by sjg@chromium.org, Oct 22 2017

Issue description

There has been a request for a means of having one model's settings default to those of another, to reduce duplication.

Note: I am a little skeptical of the benefits (vs. the confusion) of this option, but we can judge the cost / benefit once we have all the config added. In the meantime, adding this allows us to try it out.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 24 2017

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

commit 1d3ef7ce02b96199ad88fef5c45a1e974235b072
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 14:29:41 2017

chromeos-config: Implement a 'default' feature

It is annoying to have to copy the configuration for one model into
another when they are almost identical. The existing sharing mechanisms
after sufficient for whitelabel and firmware, but do not cope well with
the case where two models are virtually the same.

Add a 'default' property, which allows all of the nodes and properties of
one node to default to those of another. This is an experimental feature.

So far the feature only exists in the host library. Further work is needed
to support it in the validator and the run-time library.

NOTE: This is probably taking things a little too far. We could end up
with  a situation where nothing is repeated in the config for a model. The
cost associated with this is that you must refer to other model to see the
actual settings. Once we have a feel for how useful/painful this is, we
can make a decisiion as to whether it stays.

BUG= chromium:777214 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I17243a59c3d6856b360cb6d630b103582bff1602
Reviewed-on: https://chromium-review.googlesource.com/732647
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

[modify] https://crrev.com/1d3ef7ce02b96199ad88fef5c45a1e974235b072/chromeos-config/README.md
[modify] https://crrev.com/1d3ef7ce02b96199ad88fef5c45a1e974235b072/chromeos-config/libcros_config/test.dts
[modify] https://crrev.com/1d3ef7ce02b96199ad88fef5c45a1e974235b072/chromeos-config/libcros_config_host/libcros_config_host.py
[modify] https://crrev.com/1d3ef7ce02b96199ad88fef5c45a1e974235b072/chromeos-config/cros_config_host_unittest.py
[modify] https://crrev.com/1d3ef7ce02b96199ad88fef5c45a1e974235b072/chromeos-config/libcros_config_host_unittest.py

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 24 2017

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

commit 56ab9b23fb10fc294d69e018becb5371cf31418a
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 14:29:41 2017

chromeos-config: Refactor cros_config to use a base offset

At present we use the full path to a node when reading properties from it.
This works correctly, but is inefficient since we repeat searches already
done at init time. It makes sense to conduct the search from the
/chromeos/models/MODEL node since we already have that available.

Refactor the code to handle this. There is no functional change, but this
permits us (in a later CL) to conduct the search relative to a different
base node.

BUG= chromium:777214 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools
Change-Id: Id09a9a6234a5afa94b105a6d5bed9485ffe94f88
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/732646

[modify] https://crrev.com/56ab9b23fb10fc294d69e018becb5371cf31418a/chromeos-config/libcros_config/cros_config.cc
[modify] https://crrev.com/56ab9b23fb10fc294d69e018becb5371cf31418a/chromeos-config/libcros_config/cros_config.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 24 2017

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

commit b76873332bf2541df204d5ffbcc1089d53851a05
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 14:29:41 2017

chromeos-config: Move phandle-following into a function

We will need this code for another situation, so move it into a function
first.

BUG= chromium:777214 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: Ib8054f5c2947f2d0b5f0359fef3fe1d46600c21f
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/732648

[modify] https://crrev.com/b76873332bf2541df204d5ffbcc1089d53851a05/chromeos-config/libcros_config/cros_config.cc
[modify] https://crrev.com/b76873332bf2541df204d5ffbcc1089d53851a05/chromeos-config/libcros_config/cros_config.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 24 2017

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

commit 9cf5303564c55334e5aa810be3e4ae3a7a6e908c
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 24 14:29:42 2017

chromeos-config: Fix validation errors in the test config

At present test.dts has various validation errors. Since it is only used
for unit testing, this is not really a problem. But it might be confusing
to someone who examines it closely. Also, running the validator on the
test file shows up these problems.

Fix these errors for consistency.

BUG= chromium:777214 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I19e22e59b9f8ef7fe58226d7782a75c90ea6106a
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/732649
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/9cf5303564c55334e5aa810be3e4ae3a7a6e908c/chromeos-config/libcros_config/test.dts

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 25 2017

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

commit 10a8f581da40714690a15b5fb8cdeb3b3fee4393
Author: Simon Glass <sjg@chromium.org>
Date: Wed Oct 25 10:58:51 2017

chromeos-config: Implement 'default' in cros_config

Add this feature to libcros_config so that it is available at run-time.
Note that this is an experimental feature which may be taking things too
far. Its impact will be evaluated in December.

BUG= chromium:777214 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I4178e30627ab2950d403dbf081155734eed90816
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/732650

[modify] https://crrev.com/10a8f581da40714690a15b5fb8cdeb3b3fee4393/chromeos-config/libcros_config/cros_config_unittest.cc
[modify] https://crrev.com/10a8f581da40714690a15b5fb8cdeb3b3fee4393/chromeos-config/libcros_config/cros_config.cc
[modify] https://crrev.com/10a8f581da40714690a15b5fb8cdeb3b3fee4393/chromeos-config/libcros_config/cros_config.h

Comment 6 by sjg@chromium.org, Oct 27 2017

Status: Fixed (was: Untriaged)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 31 2017

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

commit 33861882552365ff9dfe9910f083a52e501b97ec
Author: Simon Glass <sjg@chromium.org>
Date: Tue Oct 31 17:43:41 2017

chromeos-config: Add 'default' to the validator

This feature seems to work well enough for trials. Add it to the
validator, making it mutually exclusive with whitelabel, since that really
would create confusion.

BUG= chromium:777214 
TEST=FEATURES=test emerge-reef-uni --nodeps chromeos-config-tools

Change-Id: I43fcc2efe5a570037df1ca84a5595b53f7242938
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/742304
Reviewed-by: Nick Sanders <nsanders@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/33861882552365ff9dfe9910f083a52e501b97ec/chromeos-config/validate_config_unittest.py
[modify] https://crrev.com/33861882552365ff9dfe9910f083a52e501b97ec/chromeos-config/validate_config.py

Sign in to add a comment