Merge ~alexhung/plainbox-provider-checkbox:dell-hotkeys-v3 into plainbox-provider-checkbox:master
- Git
- lp:~alexhung/plainbox-provider-checkbox
- dell-hotkeys-v3
- Merge into master
Status: | Work in progress |
---|---|
Proposed branch: | ~alexhung/plainbox-provider-checkbox:dell-hotkeys-v3 |
Merge into: | plainbox-provider-checkbox:master |
Diff against target: |
432 lines (+406/-0) 4 files modified
bin/dell_hotkeys.py (+320/-0) units/keys/dell.pxu (+65/-0) units/keys/resource.pxu (+11/-0) units/keys/test-plan.pxu (+10/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Maciej Kisielewski | Needs Fixing | ||
Review via email: mp+402546@code.launchpad.net |
Commit message
BIOS interface tests for OEM hotkey features
Description of the change
BIOS interface tests trigger or stimulate BIOS events that are used for hotkeys. This allows remote testing without human intervention to press keys (except display switch that requires connect to an external monitor).
V3 - replace bash by python as commented in V2 (https:/
Maciej Kisielewski (kissiel) wrote : | # |
I'm setting this MR to a "in progress" status so it doesn't pop up in our feed.
Also consider amending your branch instead of proposing a new one. It helps with continuity of reviews.
OEM Taipei Bot (oem-taipei-bot) wrote : | # |
[BOT]
$ cat plainbox-
bootstrap-
https:/
OEM Taipei Bot (oem-taipei-bot) wrote : | # |
Execute `curl -X POST http://
[autopkgtest]
$ cat plainbox-
bootstrap-
https:/
Unmerged commits
- b958253... by Alex Hung
-
add BIOS hotkey tests for dell laptops
Signed-off-by: Alex Hung <email address hidden>
Preview Diff
1 | diff --git a/bin/dell_hotkeys.py b/bin/dell_hotkeys.py |
2 | new file mode 100755 |
3 | index 0000000..22703f3 |
4 | --- /dev/null |
5 | +++ b/bin/dell_hotkeys.py |
6 | @@ -0,0 +1,320 @@ |
7 | +#!/usr/bin/env python3 |
8 | +''' |
9 | +script to test Dell hotkey functionality |
10 | + |
11 | +Copyright (C) 2021 Canonical Ltd. |
12 | + |
13 | +Authors |
14 | + Alex Hung <alex.hung@canonical.com> |
15 | + |
16 | +This program is free software: you can redistribute it and/or modify |
17 | +it under the terms of the GNU General Public License version 3, |
18 | +as published by the Free Software Foundation. |
19 | + |
20 | +This program is distributed in the hope that it will be useful, |
21 | +but WITHOUT ANY WARRANTY; without even the implied warranty of |
22 | +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
23 | +GNU General Public License for more details. |
24 | + |
25 | +You should have received a copy of the GNU General Public License |
26 | +along with this program. If not, see <http://www.gnu.org/licenses/>. |
27 | + |
28 | +The purpose of this script is to simply interact with an onboard |
29 | +accelerometer, and check to be sure that the x, y, z axis respond |
30 | +to physical movement of hardware. |
31 | +''' |
32 | +import sys, os, argparse, time, evdev, subprocess, re |
33 | +from argparse import ArgumentParser |
34 | +from evdev import UInput, ecodes as e |
35 | +from subprocess import STDOUT, check_call, CalledProcessError |
36 | + |
37 | +class Acpidbg(object): |
38 | + def __init__(self, method): |
39 | + self.method = method |
40 | + # install acpidbg if it is not yet installed |
41 | + kernel_ver = os.uname().release |
42 | + dir = "/usr/lib/linux-tools/" + kernel_ver +"/acpidbg" |
43 | + if not os.path.exists(dir): |
44 | + try: |
45 | + check_call(['apt', 'install', '-y','linux-tools-generic', |
46 | + 'linux-tools-' + kernel_ver], stdout=open(os.devnull, 'wb')) |
47 | + except CalledProcessError as e: |
48 | + print(e.output) |
49 | + |
50 | + def run_command(self, command = ''): |
51 | + command = 'e ' + self.method + ' ' + command |
52 | + try: |
53 | + check_call(['acpidbg', '-b', command], stdout=open(os.devnull, 'wb')) |
54 | + except CalledProcessError as e: |
55 | + print(e.output) |
56 | + time.sleep(1) |
57 | + |
58 | +class Wireless(object): |
59 | + def __init__(self, name): |
60 | + self.name = name |
61 | + |
62 | + def get_state(self): |
63 | + rfkill_cmd = 'rfkill list ' + self.name |
64 | + out = subprocess.Popen(rfkill_cmd, shell=True, stdout=subprocess.PIPE) |
65 | + (stdout, stderr) = out.communicate() |
66 | + |
67 | + if stdout.decode().strip().find('Soft blocked: no') > 0: |
68 | + return 1 |
69 | + else: |
70 | + return 0 |
71 | + |
72 | + def was_toggled(self, state): |
73 | + if self.get_state() == state: |
74 | + return False |
75 | + else: |
76 | + return True |
77 | + |
78 | +class Display(object): |
79 | + def __init__(self): |
80 | + self.original_active_monitors = self.get_active_monitor() |
81 | + |
82 | + def get_active_monitor(self): |
83 | + xrandr_cmd = "xrandr | awk '/ connected/ && /[[:digit:]]x[[:digit:]].*+/{print $1}'" |
84 | + out = subprocess.Popen(xrandr_cmd, shell=True, stdout=subprocess.PIPE) |
85 | + (stdout, stderr) = out.communicate() |
86 | + return stdout |
87 | + |
88 | + def trigger_display_switch (self): |
89 | + ''' Trigger super+p twice ''' |
90 | + ui = UInput() |
91 | + ui.write(e.EV_KEY, e.KEY_LEFTMETA, 1) |
92 | + ui.syn() |
93 | + time.sleep(1) |
94 | + ui.write(e.EV_KEY, e.KEY_P, 1) |
95 | + ui.write(e.EV_KEY, e.KEY_P, 0) |
96 | + ui.write(e.EV_KEY, e.KEY_P, 1) |
97 | + ui.write(e.EV_KEY, e.KEY_P, 0) |
98 | + ui.write(e.EV_KEY, e.KEY_LEFTMETA, 0) |
99 | + ui.syn() |
100 | + ui.close() |
101 | + |
102 | +class Brightness(object): |
103 | + def __init__(self, path): |
104 | + self.sysfs_path = path |
105 | + self.original_level = self.get_brightness() |
106 | + |
107 | + def read_value(self, path): |
108 | + bsys = open(path, 'r') |
109 | + lines = bsys.readlines() |
110 | + bsys.close() |
111 | + return int(''.join(lines).strip()) |
112 | + |
113 | + def set_brightness(self, value): |
114 | + value = '%d' % value |
115 | + path = open(os.path.join(self.sysfs_path, 'brightness'), 'w') |
116 | + path.write(value) |
117 | + path.close() |
118 | + |
119 | + def get_max_brightness(self): |
120 | + full_path = os.path.join(self.sysfs_path, 'max_brightness') |
121 | + return self.read_value(full_path) |
122 | + |
123 | + def get_brightness(self): |
124 | + full_path = os.path.join(self.sysfs_path, 'brightness') |
125 | + return self.read_value(full_path) |
126 | + |
127 | + def restore_brightness(self): |
128 | + self.set_brightness(self.original_level) |
129 | + |
130 | + def was_brightness_applied(self, bl): |
131 | + # See if the selected brightness was applied |
132 | + # Note: this doesn't guarantee that brightness changed. |
133 | + if self.get_brightness() != bl: |
134 | + return False |
135 | + return True |
136 | + |
137 | + def was_brightness_up(self, bl): |
138 | + if self.get_brightness() > bl: |
139 | + return True |
140 | + return False |
141 | + |
142 | + def was_brightness_down(self, bl): |
143 | + if self.get_brightness() < bl: |
144 | + return True |
145 | + return False |
146 | + |
147 | +class Suspend(object): |
148 | + |
149 | + def prepare_to_sleep(self): |
150 | + # write a stamp for sleep time |
151 | + kmsg = open('/dev/kmsg', 'w') |
152 | + kmsg.write(os.path.basename(__file__) + ': prepare to sleep') |
153 | + kmsg.close() |
154 | + return 0 |
155 | + |
156 | + def go_to_sleep(self): |
157 | + self.prepare_to_sleep() |
158 | + # Trigger an ACPI sleep button event |
159 | + acpidbg = Acpidbg('_SB.BTNV') |
160 | + acpidbg.run_command('2 0') |
161 | + # wait until OS handles events and transit to sleep |
162 | + time.sleep(10) |
163 | + return 0 |
164 | + |
165 | + def get_sleep_time(self): |
166 | + cmd = "dmesg | grep 'prepare to sleep' | tail -1 | awk -F'[].[]' ' { print $2 }'" |
167 | + out = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE) |
168 | + (stdout, stderr) = out.communicate() |
169 | + return int(stdout) |
170 | + |
171 | + def get_wakeup_time(self): |
172 | + cmd = "dmesg | grep 'PM: suspend exit' | tail -1 | awk -F'[].[]' ' { print $2 }'" |
173 | + out = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE) |
174 | + (stdout, stderr) = out.communicate() |
175 | + return int(stdout) |
176 | + |
177 | + def set_wakeup_time(self, sec): |
178 | + subprocess.Popen('rtcwake --seconds %d' % sec, shell=True) |
179 | + return 0 |
180 | + |
181 | +def airplane_mode_test(): |
182 | + ''' |
183 | + This function triggers ACPI rfkill events to the Intel ACPI HID device and |
184 | + verifies whether WLAN and bluetooth's status is changed in rfkill accordingly. |
185 | + ''' |
186 | + |
187 | + acpidbg = Acpidbg('_SB.HIDD.NRBT') |
188 | + wlan = Wireless("wlan") |
189 | + bluetooth = Wireless("bluetooth") |
190 | + |
191 | + # Toggle airplane modes by ACPI events |
192 | + wlan_state = wlan.get_state() |
193 | + bt_state = bluetooth.get_state() |
194 | + acpidbg.run_command() |
195 | + if not wlan.was_toggled(wlan_state) or not bluetooth.was_toggled(bt_state): |
196 | + return 1 |
197 | + |
198 | + wlan_state = wlan.get_state() |
199 | + bt_state = bluetooth.get_state() |
200 | + acpidbg.run_command() |
201 | + if not wlan.was_toggled(wlan_state) or not bluetooth.was_toggled(bt_state): |
202 | + return 1 |
203 | + |
204 | + return 0 |
205 | + |
206 | +def brightness_test(): |
207 | + ''' |
208 | + This function triggers ACPI brightness up and down events and |
209 | + verifies whether brightness levels are changed in sysfs accordingly. |
210 | + ''' |
211 | + |
212 | + # Find intel or AMD backlight interface (they are mutually exclusive) |
213 | + find_bl = 'find /sys/class/backlight/ -name "[ai]*"' |
214 | + out = subprocess.Popen(find_bl, shell=True, stdout=subprocess.PIPE) |
215 | + (stdout, stderr) = out.communicate() |
216 | + |
217 | + # Prepare brightness interface |
218 | + brightness = Brightness(stdout.decode().strip()) |
219 | + brightness.set_brightness(brightness.get_max_brightness() / 2) |
220 | + |
221 | + # Inialize for acpidbg |
222 | + method = '_SB.PCI0.GFX0.BRT6' |
223 | + acpi_dir = os.listdir('/sys/firmware/acpi/tables/') |
224 | + for file in acpi_dir: |
225 | + full_path = '/sys/firmware/acpi/tables/' + file |
226 | + if os.path.isdir(full_path): |
227 | + continue |
228 | + f = open(full_path, 'rb') |
229 | + s = f.read() |
230 | + if s.find(b'PC00GFX') > 0: |
231 | + method = '_SB.PC00.GFX0.BRT6' |
232 | + f.close() |
233 | + |
234 | + acpidbg = Acpidbg(method) |
235 | + |
236 | + # Test brightness up & down by ACPI control methods |
237 | + cur_bl = brightness.get_brightness() |
238 | + acpidbg.run_command('1 0') |
239 | + if not brightness.was_brightness_up(cur_bl): |
240 | + return 1 |
241 | + |
242 | + cur_bl = brightness.get_brightness() |
243 | + acpidbg.run_command('2 0') |
244 | + if not brightness.was_brightness_down(cur_bl): |
245 | + return 1 |
246 | + |
247 | + brightness.restore_brightness() |
248 | + return 0 |
249 | + |
250 | +def display_switch_test(): |
251 | + ''' |
252 | + This function generates 2 Super+P keycodes twice and |
253 | + verifies displays are changed in xrandr. |
254 | + |
255 | + Note: it requires an external monitor to be attached. |
256 | + ''' |
257 | + |
258 | + display = Display() |
259 | + display.trigger_display_switch() |
260 | + time.sleep(5) |
261 | + if display.get_active_monitor() == display.original_active_monitors: |
262 | + return 1 |
263 | + |
264 | + display.trigger_display_switch() |
265 | + time.sleep(5) |
266 | + if display.get_active_monitor() != display.original_active_monitors: |
267 | + return 1 |
268 | + |
269 | + return 0 |
270 | + |
271 | +def keyboard_backlight_test(): |
272 | + ''' |
273 | + This function changes keyboard backlights in kbd_backlight |
274 | + sysfs and verified backlights are updated correctly. |
275 | + ''' |
276 | + |
277 | + backlight = Brightness('/sys/class/leds/dell::kbd_backlight') |
278 | + backlight.set_brightness(0) |
279 | + |
280 | + for bl in range (0, backlight.get_max_brightness() + 1): |
281 | + backlight.set_brightness(bl) |
282 | + time.sleep(0.5) |
283 | + |
284 | + if not backlight.was_brightness_applied(bl): |
285 | + backlight.restore_brightness() |
286 | + return 1 |
287 | + |
288 | + backlight.restore_brightness() |
289 | + return 0 |
290 | + |
291 | +def suspend_button_test(): |
292 | + ''' |
293 | + This function sets a wakeup time and triggers an ACPI sleep button events to |
294 | + verifies a system goes into sleep and then wakes up. |
295 | + ''' |
296 | + |
297 | + suspend = Suspend() |
298 | + |
299 | + suspend.set_wakeup_time(60) |
300 | + suspend.go_to_sleep() |
301 | + if suspend.get_sleep_time() > suspend.get_wakeup_time(): |
302 | + return 1 |
303 | + |
304 | + return 0 |
305 | + |
306 | +def main(): |
307 | + |
308 | + parser = argparse.ArgumentParser(description='Check Dell hotkey functionality.') |
309 | + parser.add_argument("-f", "--function", help="Function name", required=True) |
310 | + args = parser.parse_args() |
311 | + |
312 | + if args.function == "airplane_mode": |
313 | + exit_status = airplane_mode_test() |
314 | + elif args.function == "brightness": |
315 | + exit_status = brightness_test() |
316 | + elif args.function == "display_switch": |
317 | + exit_status = display_switch_test() |
318 | + elif args.function == "keyboard_backlight": |
319 | + exit_status = keyboard_backlight_test() |
320 | + elif args.function == "suspend_button": |
321 | + exit_status = suspend_button_test() |
322 | + |
323 | + exit(exit_status) |
324 | + |
325 | +if __name__ == '__main__': |
326 | + main() |
327 | diff --git a/units/keys/dell.pxu b/units/keys/dell.pxu |
328 | new file mode 100644 |
329 | index 0000000..f6e0e80 |
330 | --- /dev/null |
331 | +++ b/units/keys/dell.pxu |
332 | @@ -0,0 +1,65 @@ |
333 | +plugin: shell |
334 | +category_id: com.canonical.plainbox::keys |
335 | +id: dell/dell-airplane_mode |
336 | +_summary: test airplane mode hotkey |
337 | +_description: |
338 | + This tests airplane mode hotkey for Dell laptops |
339 | +user: root |
340 | +estimated_duration: 6 |
341 | +requires: |
342 | + dmi.vendor in ['Dell Inc.'] |
343 | + dmi.product in ['Notebook','Laptop'] |
344 | +command: dell_hotkeys.py -f airplane_mode |
345 | + |
346 | +plugin: shell |
347 | +category_id: com.canonical.plainbox::keys |
348 | +id: dell/dell-brightness |
349 | +_summary: test brightness hotkeys |
350 | +_description: |
351 | + This tests brightness up & down hotkeys for Dell laptops |
352 | +user: root |
353 | +estimated_duration: 2 |
354 | +requires: |
355 | + dmi.vendor in ['Dell Inc.'] |
356 | + dmi.product in ['Notebook','Laptop'] |
357 | +command: dell_hotkeys.py -f brightness |
358 | + |
359 | +plugin: user-interact |
360 | +category_id: com.canonical.plainbox::keys |
361 | +id: dell/dell-display_switch |
362 | +_summary: test display switch hotkey |
363 | +_purpose: |
364 | + This tests display switch hotkeys for Dell laptops |
365 | +_steps: |
366 | + 1. Make sure an external monitor is connected |
367 | + 2. Perform the test |
368 | +user: root |
369 | +estimated_duration: 12 |
370 | +command: dell_hotkeys.py -f display_switch |
371 | + |
372 | +plugin: shell |
373 | +category_id: com.canonical.plainbox::keys |
374 | +id: dell/dell-keyboard_backlight |
375 | +_summary: test keyboard backlight |
376 | +_description: |
377 | + This tests backlight for Dell laptops |
378 | +user: root |
379 | +estimated_duration: 1 |
380 | +requires: |
381 | + dmi.vendor in ['Dell Inc.'] |
382 | + dmi.product in ['Notebook','Laptop'] |
383 | + kbd_backlight.state == 'supported' |
384 | +command: dell_hotkeys.py -f keyboard_backlight |
385 | + |
386 | +plugin: shell |
387 | +category_id: com.canonical.plainbox::keys |
388 | +id: dell/dell-suspend_button |
389 | +_summary: test suspend button |
390 | +_description: |
391 | + This tests suspend button for Dell laptops |
392 | +user: root |
393 | +estimated_duration: 1 |
394 | +requires: |
395 | + dmi.vendor in ['Dell Inc.'] |
396 | + dmi.product in ['Notebook','Laptop'] |
397 | +command: dell_hotkeys.py -f suspend_button |
398 | diff --git a/units/keys/resource.pxu b/units/keys/resource.pxu |
399 | new file mode 100644 |
400 | index 0000000..5b1e363 |
401 | --- /dev/null |
402 | +++ b/units/keys/resource.pxu |
403 | @@ -0,0 +1,11 @@ |
404 | +id: kbd_backlight |
405 | +plugin: resource |
406 | +_description: Check whether keyboard backlight interfaces exist |
407 | +category_id: com.canonical.plainbox::keys |
408 | +estimated_duration: 0.02 |
409 | +command: |
410 | + if udevadm info --export-db | grep -q "kbd_backlight" ; then |
411 | + echo "state: supported" |
412 | + else |
413 | + echo "state: Unsupported" |
414 | + fi |
415 | diff --git a/units/keys/test-plan.pxu b/units/keys/test-plan.pxu |
416 | index ffc111c..39380ab 100644 |
417 | --- a/units/keys/test-plan.pxu |
418 | +++ b/units/keys/test-plan.pxu |
419 | @@ -92,3 +92,13 @@ include: |
420 | after-suspend-manual-keys/keyboard-backlight certification-status=blocker |
421 | after-suspend-manual-keys/microphone-mute certification-status=blocker |
422 | after-suspend-manual-keys/power-button certification-status=blocker |
423 | + |
424 | +id: dell_laptop_hotkeys |
425 | +unit: test plan |
426 | +_name: Dell laptop hotkeys |
427 | +include: |
428 | + dell/dell-airplane_mode |
429 | + dell/dell-brightness |
430 | + dell/dell-display_switch |
431 | + dell/dell-keyboard_backlight |
432 | + dell/dell-suspend_button |
Huge work, thank you!
Both: flake8 and pylint report multiple issues with the program you wrote, so I'd recommend checking them out.
Throughout the code you use C-like error codes that in python are just unnecessary (and more difficult to read).
I understand that you have multiple functions that may be used for testing, and you wanted to have one place in "main" to report the result from.
So in python there's a SystemExit exception, that when raised with a string as its arg, will halt the program printing that string (with 1 return value, and clean - without backtrace).
So what is often done is this:
def airplane_ mode_test( ): went_wrong: "Airplane mode did not work!")
(...)
if something_
raise SystemExit(
(...)
And you don't need a return value. If it did not raise, it means everything was ok. So your main could be something like that:
def main():
test_fun1()
test_fun2()
# if we're here, it means the functions did not raise exceptions
print("The test(s) passed!")
And that's it. No need for low-level error handling.
I also left some more comments inline below