<div dir="ltr"><div class="gmail_default" style="font-family:verdana,sans-serif">Thanks Colin. I also found it can be improved a little be in addition to the continue. I will send the patch v2 shortly.<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 7, 2014 at 3:14 PM, Colin Ian King <span dir="ltr"><<a href="mailto:colin.king@canonical.com" target="_blank">colin.king@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 07/10/14 07:43, Alex Hung wrote:<br>
> Signed-off-by: Alex Hung <<a href="mailto:alex.hung@canonical.com">alex.hung@canonical.com</a>><br>
> ---<br>
>  src/acpi/method/method.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++--<br>
>  1 file changed, 75 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c<br>
> index cb873cb..879e072 100644<br>
> --- a/src/acpi/method/method.c<br>
> +++ b/src/acpi/method/method.c<br>
> @@ -38,7 +38,7 @@<br>
>   * _ALC  9.2.5               Y<br>
>   * _ALI  9.2.2               Y<br>
>   * _ALP  9.2.6               Y<br>
> - * _ALR  9.2.5               N<br>
> + * _ALR  9.2.5               Y<br>
>   * _ALT  9.2.3               Y<br>
>   * _ALx  11.4.2              N<br>
>   * _ART  11.4.3              Y<br>
> @@ -3555,7 +3555,79 @@ method_test_integer(_ALC, METHOD_OPTIONAL)<br>
>  method_test_integer(_ALI, METHOD_OPTIONAL)<br>
>  method_test_integer(_ALT, METHOD_OPTIONAL)<br>
><br>
> -/* TODO _ALR */<br>
> +static void method_test_ALR_return(<br>
> +     fwts_framework *fw,<br>
> +     char *name,<br>
> +     ACPI_BUFFER *buf,<br>
> +     ACPI_OBJECT *obj,<br>
> +     void *private)<br>
> +{<br>
> +     uint32_t i;<br>
> +     bool failed = false;<br>
> +<br>
> +     FWTS_UNUSED(private);<br>
> +<br>
> +     if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)<br>
> +             return;<br>
> +<br>
> +     /* Could be one or more sub-packages */<br>
> +     for (i = 0; i < obj->Package.Count; i++) {<br>
> +             ACPI_OBJECT *pkg;<br>
> +             uint32_t adjustment = 0, illuminance = 0;<br>
> +             bool elements_ok = true;<br>
> +<br>
> +             pkg = &obj->Package.Elements[i];<br>
> +             if (pkg->Package.Count != 2) {<br>
> +                     fwts_failed(fw, LOG_LEVEL_MEDIUM,<br>
> +                             "Method_ALRBadSubPackageElementCount",<br>
> +                             "%s sub-package %" PRIu32 " was expected to "<br>
> +                             "have 2 elements, got %" PRIu32 " elements instead.",<br>
> +                             name, i, pkg->Package.Count);<br>
> +                     failed = true;<br>
> +                     continue;<br>
> +             } else {<br>
> +                     /* elements should be listed in monotonically increasing order */<br>
> +                     if (pkg->Package.Elements[0].Type != ACPI_TYPE_INTEGER ||<br>
> +                         adjustment > pkg->Package.Elements[0].Integer.Value) {<br>
> +                             fwts_failed(fw, LOG_LEVEL_MEDIUM,<br>
> +                                     "Method_ALRBadSubPackageReturnType",<br>
> +                                     "%s sub-package %" PRIu32<br>
> +                                     " element 0 is an invalid integer.",<br>
> +                                     name, i);<br>
> +                             elements_ok = false;<br>
> +                     }<br>
> +<br>
> +                     if (pkg->Package.Elements[1].Type != ACPI_TYPE_INTEGER ||<br>
> +                         illuminance > pkg->Package.Elements[1].Integer.Value) {<br>
> +                             fwts_failed(fw, LOG_LEVEL_MEDIUM,<br>
> +                                     "Method_ALRBadSubPackageReturnType",<br>
> +                                     "%s sub-package %" PRIu32<br>
> +                                     " element 1 is an invalid integer.",<br>
> +                                     name, i);<br>
> +                             elements_ok = false;<br>
> +                     }<br>
> +                     adjustment = pkg->Package.Elements[0].Integer.Value;<br>
> +                     illuminance = pkg->Package.Elements[1].Integer.Value;<br>
> +<br>
> +             }<br>
> +<br>
> +             if (!elements_ok) {<br>
> +                     failed = true;<br>
> +                     continue;<br>
<br>
</div></div>minor point, we don't really need the continue;  I think it is redundant<br>
<span class=""><br>
> +             }<br>
> +     }<br>
> +<br>
> +     if (!failed)<br>
> +             method_passed_sane(fw, name, "package");<br>
> +<br>
> +     method_passed_sane(fw, name, "package");<br>
> +}<br>
> +<br>
> +static int method_test_ALR(fwts_framework *fw)<br>
> +{<br>
> +     return method_evaluate_method(fw, METHOD_OPTIONAL,<br>
> +             "_ALR", NULL, 0, method_test_ALR_return, NULL);<br>
> +}<br>
><br>
>  static int method_test_ALP(fwts_framework *fw)<br>
>  {<br>
> @@ -5825,7 +5897,7 @@ static fwts_framework_minor_test method_tests[] = {<br>
>       { method_test_ALI, "Test _ALI (Ambient Light Illuminance)." },<br>
>       { method_test_ALT, "Test _ALT (Ambient Light Temperature)." },<br>
>       { method_test_ALP, "Test _ALP (Ambient Light Polling). "},<br>
> -     /* { method_test_ALR, "Test _ALR (Ambient Light Response). "}, */<br>
> +     { method_test_ALR, "Test _ALR (Ambient Light Response). "},<br>
><br>
>       /* Section 9.3 Battery Device */<br>
><br>
><br>
<br>
<br>
--<br>
</span>fwts-devel mailing list<br>
<a href="mailto:fwts-devel@lists.ubuntu.com">fwts-devel@lists.ubuntu.com</a><br>
Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/fwts-devel" target="_blank">https://lists.ubuntu.com/mailman/listinfo/fwts-devel</a><br>
</blockquote></div><br><br clear="all"><br>-- <br><div dir="ltr">Cheers,<br>Alex Hung<br></div>
</div>