Make cardreader mis-matched a special case to prompt error.

Make cardreader mis-matched a special case to prompt error.

Since only cardreader mis-matched, most of the time, is fogetting to
insert an SD card.

TEST=manual

[Nick Sanders, 2010-08-25 01:11:42]

So, the plan last I heard is that the factory install shim sdcard will be
removed, and that hardware_Components will be run in the runin portion of
the line. If we need interactive sdcard inserting, we should move the
components test in the factory sequence.

On Tue, Aug 24, 2010 at 4:58 PM, <waihong@chromium.org> wrote:

> Reviewers: petkov, Nick Sanders, Hung-Te,
>
> Description:
> Make cardreader mis-matched a special case to prompt error.
>
> Since only cardreader mis-matched, most of the time, is fogetting to
> insert an SD card.
>
> TEST=manual
>
> Please review this at http://codereview.chromium.org/3209001/show
>
> SVN Base: http://src.chromium.org/git/autotest.git
>
> Affected files:
>  M client/site_tests/hardware_Components/hardware_Components.py
>
>
> Index: client/site_tests/hardware_Components/hardware_Components.py
> diff --git a/client/site_tests/hardware_Components/hardware_Components.py
> b/client/site_tests/hardware_Components/hardware_Components.py
> index
>
e6ae5dfde01656077425fe0da1017d428548388f..91d45aa7dc32985ff5b72ea8996a48aedf905f80
> 100644
> --- a/client/site_tests/hardware_Components/hardware_Components.py
> +++ b/client/site_tests/hardware_Components/hardware_Components.py
> @@ -293,7 +293,8 @@ class hardware_Components(test.test):
>
>     def run_once(self, approved_dbs='approved_components',
> ignored_cids=[]):
>         self._ignored = ignored_cids
> -        all_failures = ''
> +        only_cardreader_failed = False
> +        all_failures = 'The following components are not matched.\n'
>         os.chdir(self.bindir)
>
>         # approved_dbs supports shell-like filename expansion.
> @@ -325,7 +326,9 @@ class hardware_Components(test.test):
>             utils.open_write_close(outdb, self.pformat(self._system))
>
>             if self._failures:
> -                all_failures += 'Approved DB: %s' % db
> +                if self._failures.keys() == ['part_id_cardreader']:
> +                    only_cardreader_failed = True
> +                all_failures += 'For DB %s:' % db
>                 all_failures += self.pformat(self._failures)
>             else:
>                 # If one of DBs is matched, record the hwqual_id and exit.
> @@ -333,4 +336,8 @@ class hardware_Components(test.test):
>                     {'hwqual_id': self._approved['part_id_hwqual'][0]})
>                 return
>
> +        if only_cardreader_failed:
> +            all_failures = ('You may forget to insert an SD card.\n' +
> +                            all_failures)
> +
>         raise error.TestFail(all_failures)
>
>
>

[Tom Wai-Hong Tam, 2010-08-25 04:41:15]

The factory install shim sdcard will NOT be removed, right? That's make
sense. But the hardware_Components test will be run twice, one in runin that
match the BOM to know the HWID, and the other one after GBB is written that
confirms it contains the right HWID and RO hash.

The second run should manually insert an SD card. Or to be
more automatic, ignore the cardreader check in the second run. Since it is
checked in the first run.

On Tue, Aug 24, 2010 at 6:11 PM, Nick Sanders <nsanders@chromium.org> wrote:

> So, the plan last I heard is that the factory install shim sdcard will be
> removed, and that hardware_Components will be run in the runin portion of
> the line. If we need interactive sdcard inserting, we should move the
> components test in the factory sequence.
>
>
> On Tue, Aug 24, 2010 at 4:58 PM, <waihong@chromium.org> wrote:
>
>> Reviewers: petkov, Nick Sanders, Hung-Te,
>>
>> Description:
>> Make cardreader mis-matched a special case to prompt error.
>>
>> Since only cardreader mis-matched, most of the time, is fogetting to
>> insert an SD card.
>>
>> TEST=manual
>>
>> Please review this at http://codereview.chromium.org/3209001/show
>>
>> SVN Base: http://src.chromium.org/git/autotest.git
>>
>> Affected files:
>>  M client/site_tests/hardware_Components/hardware_Components.py
>>
>>
>> Index: client/site_tests/hardware_Components/hardware_Components.py
>> diff --git a/client/site_tests/hardware_Components/hardware_Components.py
>> b/client/site_tests/hardware_Components/hardware_Components.py
>> index
>>
e6ae5dfde01656077425fe0da1017d428548388f..91d45aa7dc32985ff5b72ea8996a48aedf905f80
>> 100644
>> --- a/client/site_tests/hardware_Components/hardware_Components.py
>> +++ b/client/site_tests/hardware_Components/hardware_Components.py
>> @@ -293,7 +293,8 @@ class hardware_Components(test.test):
>>
>>     def run_once(self, approved_dbs='approved_components',
>> ignored_cids=[]):
>>         self._ignored = ignored_cids
>> -        all_failures = ''
>> +        only_cardreader_failed = False
>> +        all_failures = 'The following components are not matched.\n'
>>         os.chdir(self.bindir)
>>
>>         # approved_dbs supports shell-like filename expansion.
>> @@ -325,7 +326,9 @@ class hardware_Components(test.test):
>>             utils.open_write_close(outdb, self.pformat(self._system))
>>
>>             if self._failures:
>> -                all_failures += 'Approved DB: %s' % db
>> +                if self._failures.keys() == ['part_id_cardreader']:
>> +                    only_cardreader_failed = True
>> +                all_failures += 'For DB %s:' % db
>>                 all_failures += self.pformat(self._failures)
>>             else:
>>                 # If one of DBs is matched, record the hwqual_id and exit.
>> @@ -333,4 +336,8 @@ class hardware_Components(test.test):
>>                     {'hwqual_id': self._approved['part_id_hwqual'][0]})
>>                 return
>>
>> +        if only_cardreader_failed:
>> +            all_failures = ('You may forget to insert an SD card.\n' +
>> +                            all_failures)
>> +
>>         raise error.TestFail(all_failures)
>>
>>
>>
>

[Nick Sanders, 2010-08-25 05:09:33]

Factory shim sdcard WILL be removed in the current plan. Let's talk about
this tomorrow, as this may need some changes in test ordering or factory
sequence. I do see why the sdcard must be present, for the reader to wake
up.

Thanks,
  -Nick

On Tue, Aug 24, 2010 at 9:40 PM, Tom Wai-Hong Tam (談偉航) <
waihong@chromium.org> wrote:

> The factory install shim sdcard will NOT be removed, right? That's make
> sense. But the hardware_Components test will be run twice, one in runin that
> match the BOM to know the HWID, and the other one after GBB is written that
> confirms it contains the right HWID and RO hash.
>
> The second run should manually insert an SD card. Or to be
> more automatic, ignore the cardreader check in the second run. Since it is
> checked in the first run.
>
> On Tue, Aug 24, 2010 at 6:11 PM, Nick Sanders <nsanders@chromium.org>wrote:
>
>> So, the plan last I heard is that the factory install shim sdcard will be
>> removed, and that hardware_Components will be run in the runin portion of
>> the line. If we need interactive sdcard inserting, we should move the
>> components test in the factory sequence.
>>
>>
>> On Tue, Aug 24, 2010 at 4:58 PM, <waihong@chromium.org> wrote:
>>
>>> Reviewers: petkov, Nick Sanders, Hung-Te,
>>>
>>> Description:
>>> Make cardreader mis-matched a special case to prompt error.
>>>
>>> Since only cardreader mis-matched, most of the time, is fogetting to
>>> insert an SD card.
>>>
>>> TEST=manual
>>>
>>> Please review this at http://codereview.chromium.org/3209001/show
>>>
>>> SVN Base: http://src.chromium.org/git/autotest.git
>>>
>>> Affected files:
>>>  M client/site_tests/hardware_Components/hardware_Components.py
>>>
>>>
>>> Index: client/site_tests/hardware_Components/hardware_Components.py
>>> diff --git a/client/site_tests/hardware_Components/hardware_Components.py
>>> b/client/site_tests/hardware_Components/hardware_Components.py
>>> index
>>>
e6ae5dfde01656077425fe0da1017d428548388f..91d45aa7dc32985ff5b72ea8996a48aedf905f80
>>> 100644
>>> --- a/client/site_tests/hardware_Components/hardware_Components.py
>>> +++ b/client/site_tests/hardware_Components/hardware_Components.py
>>> @@ -293,7 +293,8 @@ class hardware_Components(test.test):
>>>
>>>     def run_once(self, approved_dbs='approved_components',
>>> ignored_cids=[]):
>>>         self._ignored = ignored_cids
>>> -        all_failures = ''
>>> +        only_cardreader_failed = False
>>> +        all_failures = 'The following components are not matched.\n'
>>>         os.chdir(self.bindir)
>>>
>>>         # approved_dbs supports shell-like filename expansion.
>>> @@ -325,7 +326,9 @@ class hardware_Components(test.test):
>>>             utils.open_write_close(outdb, self.pformat(self._system))
>>>
>>>             if self._failures:
>>> -                all_failures += 'Approved DB: %s' % db
>>> +                if self._failures.keys() == ['part_id_cardreader']:
>>> +                    only_cardreader_failed = True
>>> +                all_failures += 'For DB %s:' % db
>>>                 all_failures += self.pformat(self._failures)
>>>             else:
>>>                 # If one of DBs is matched, record the hwqual_id and
>>> exit.
>>> @@ -333,4 +336,8 @@ class hardware_Components(test.test):
>>>                     {'hwqual_id': self._approved['part_id_hwqual'][0]})
>>>                 return
>>>
>>> +        if only_cardreader_failed:
>>> +            all_failures = ('You may forget to insert an SD card.\n' +
>>> +                            all_failures)
>>> +
>>>         raise error.TestFail(all_failures)
>>>
>>>
>>>
>>
>

[Tammo Spalink, 2010-08-25 05:54:50]

So, this discussion is confusing me.  Let me try to summarize my
current understanding ...

The factory install shim, according to our plan (and the current plans
of our ODMs), WILL be removed prior to any of our autotest code being
run.

Assuming that, it sounds like we are having a discussion about the
card reader not being detectable by hardware_Components unless there
is an SD card -- in other words, there needs to be an SD card
re-inserted prior to any run of hardware_Components to allow for
proper scanning of this device?

If this is true, it sounds really annoying, and the ODMs will be
unhappy, especially since we are currently not running
hardware_Components from within any operator-based tests.

Tammo


On Tue, Aug 24, 2010 at 22:09, Nick Sanders <nsanders@chromium.org> wrote:
> Factory shim sdcard WILL be removed in the current plan. Let's talk about
> this tomorrow, as this may need some changes in test ordering or factory
> sequence. I do see why the sdcard must be present, for the reader to wake
> up.
> Thanks,
>   -Nick
>
> On Tue, Aug 24, 2010 at 9:40 PM, Tom Wai-Hong Tam (談偉航)
> <waihong@chromium.org> wrote:
>>
>> The factory install shim sdcard will NOT be removed, right? That's make
>> sense. But the hardware_Components test will be run twice, one in runin that
>> match the BOM to know the HWID, and the other one after GBB is written that
>> confirms it contains the right HWID and RO hash.
>> The second run should manually insert an SD card. Or to be
>> more automatic, ignore the cardreader check in the second run. Since it is
>> checked in the first run.
>> On Tue, Aug 24, 2010 at 6:11 PM, Nick Sanders <nsanders@chromium.org>
>> wrote:
>>>
>>> So, the plan last I heard is that the factory install shim sdcard will be
>>> removed, and that hardware_Components will be run in the runin portion of
>>> the line. If we need interactive sdcard inserting, we should move the
>>> components test in the factory sequence.
>>>
>>> On Tue, Aug 24, 2010 at 4:58 PM, <waihong@chromium.org> wrote:
>>>>
>>>> Reviewers: petkov, Nick Sanders, Hung-Te,
>>>>
>>>> Description:
>>>> Make cardreader mis-matched a special case to prompt error.
>>>>
>>>> Since only cardreader mis-matched, most of the time, is fogetting to
>>>> insert an SD card.
>>>>
>>>> TEST=manual
>>>>
>>>> Please review this at http://codereview.chromium.org/3209001/show
>>>>
>>>> SVN Base: http://src.chromium.org/git/autotest.git
>>>>
>>>> Affected files:
>>>>  M client/site_tests/hardware_Components/hardware_Components.py
>>>>
>>>>
>>>> Index: client/site_tests/hardware_Components/hardware_Components.py
>>>> diff --git
>>>> a/client/site_tests/hardware_Components/hardware_Components.py
>>>> b/client/site_tests/hardware_Components/hardware_Components.py
>>>> index
>>>>
e6ae5dfde01656077425fe0da1017d428548388f..91d45aa7dc32985ff5b72ea8996a48aedf905f80
>>>> 100644
>>>> --- a/client/site_tests/hardware_Components/hardware_Components.py
>>>> +++ b/client/site_tests/hardware_Components/hardware_Components.py
>>>> @@ -293,7 +293,8 @@ class hardware_Components(test.test):
>>>>
>>>>     def run_once(self, approved_dbs='approved_components',
>>>> ignored_cids=[]):
>>>>         self._ignored = ignored_cids
>>>> -        all_failures = ''
>>>> +        only_cardreader_failed = False
>>>> +        all_failures = 'The following components are not matched.\n'
>>>>         os.chdir(self.bindir)
>>>>
>>>>         # approved_dbs supports shell-like filename expansion.
>>>> @@ -325,7 +326,9 @@ class hardware_Components(test.test):
>>>>             utils.open_write_close(outdb, self.pformat(self._system))
>>>>
>>>>             if self._failures:
>>>> -                all_failures += 'Approved DB: %s' % db
>>>> +                if self._failures.keys() == ['part_id_cardreader']:
>>>> +                    only_cardreader_failed = True
>>>> +                all_failures += 'For DB %s:' % db
>>>>                 all_failures += self.pformat(self._failures)
>>>>             else:
>>>>                 # If one of DBs is matched, record the hwqual_id and
>>>> exit.
>>>> @@ -333,4 +336,8 @@ class hardware_Components(test.test):
>>>>                     {'hwqual_id': self._approved['part_id_hwqual'][0]})
>>>>                 return
>>>>
>>>> +        if only_cardreader_failed:
>>>> +            all_failures = ('You may forget to insert an SD card.\n' +
>>>> +                            all_failures)
>>>> +
>>>>         raise error.TestFail(all_failures)
>>>>
>>>>
>>>
>>
>
>

[Tom Wai-Hong Tam, 2010-08-26 20:30:58]

Yes, should insert an SD card before every component tests. Or minimize into
one, either the one for matching HWID or the one for full check.

I created a tracker for that:
http://code.google.com/p/chrome-os-partner/issues/detail?id=877

Let's go back to the CL.

[Nick Sanders, 2010-09-07 23:51:07]

lgtm