Change subject: client/site_tests: add power_BacklightControl test
......................................................................

Patch Set 3: (8 inline comments)

....................................................
File client/site_tests/power_BacklightControl/control
Line 15:   - There are too many steps (> 16) between min and max brightness.
Aren't there supposed to be exactly 16 steps?  If not please details the nuances.  We should also check too few steps in that case as well.

Based on Chrome OS requirements

  Backlight Driver Behavior The backlight for the display must be controlled by the operating system and not by the EC. A sysfs interface is required, the driver must support a node under /sys/class/backlight/ providing readable actual_brightness and max_brightness files as well as a read/write brightness file. If multiple potential devices are found the one with the largest range is selected - this can happen if there is a direct driver as well as a legacy ACPI driver.

Should this test verify that we're using the 'largest range' backlight in sysfs to satisfy the req?

....................................................
File client/site_tests/power_BacklightControl/power_BacklightControl.py
Line 94:         if utils.system_output('status powerd').find('start/running') !=-1:
Won't file tagger/inotify see these file changes and re-read?  Do you need to manipulate powerd at all?

Line 106:
Nit,

  logging.debug('num_outputs_on = %d', ...)

Line 112:         # Set brightness to max.
shouldn't you set initial brightness to minimum

Line 117:             time.sleep(adjustment_delay_s)
Perhaps dbus-monitor can be used inside self._*_brightness to determine arrival of the message.

Line 120:         if num_steps_taken > max_num_steps:
Don't we expect exactly 16 brightness steps?  If not then we must also want to check a min_num_steps as well?

Nit: instead of raising exception just log error so that test can continue w/ other subtests then raise final exception w/ summary of errors

Line 205:         cmd = 'backlight-tool --get_max_brightness'
Agreed.  Comparison at least out to be drawn to

   preferred_max = int(utils.system_output("cat /sys/class/backlight/*/max_brightness | sort -nr | head -n 1").strip())

Line 207:         if max_brightness < 4:
could 5 be min_num_steps?  In any case lets make it a constant

  if max_brightness < min_num_steps - 1:
  ...

--
To view, visit https://gerrit.chromium.org/gerrit/37388
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id399956ee55443d711429c191839bb227885da16
Gerrit-PatchSet: 3
Gerrit-Project: chromiumos/third_party/autotest
Gerrit-Branch: master
Gerrit-Owner: Simon Que <s...@chromium.org>
Gerrit-Reviewer: Daniel Erat <de...@chromium.org>
Gerrit-Reviewer: Gerrit <chrome-...@google.com>
Gerrit-Reviewer: Sameer Nanda <sna...@chromium.org>
Gerrit-Reviewer: Simon Que <s...@chromium.org>
Gerrit-Reviewer: Stéphane Marchesin <marc...@chromium.org>
Gerrit-Reviewer: Todd Broch <tbr...@chromium.org>