-
Notifications
You must be signed in to change notification settings - Fork 60
fix otg serial and networking tests (Bugfix) #1828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
fix otg serial and networking tests
contrib/checkbox-ce-oem/checkbox-provider-ce-oem/bin/multiple_otg.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my inline suggestion
cp_modules = copy.deepcopy(modules) | ||
i = 0 | ||
while i <= 3 and cp_modules: | ||
for module in modules: | ||
if module not in cp_modules: | ||
continue | ||
cmd = "modprobe -r {}".format(module) | ||
logging.info("Removing %s module", module) | ||
logging.info("$ %s", cmd) | ||
ret = subprocess.run(cmd, shell=True) | ||
if ret.returncode == 0: | ||
cp_modules.remove(module) | ||
i += 1 | ||
|
||
if cp_modules: | ||
raise RuntimeError( | ||
"failed to remove modules: {}".format(cp_modules) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume modules is just a straightforward list of module names—is that right? If so, I refactored the code below to make it easier to maintain and more intuitive.
cp_modules = copy.deepcopy(modules) | |
i = 0 | |
while i <= 3 and cp_modules: | |
for module in modules: | |
if module not in cp_modules: | |
continue | |
cmd = "modprobe -r {}".format(module) | |
logging.info("Removing %s module", module) | |
logging.info("$ %s", cmd) | |
ret = subprocess.run(cmd, shell=True) | |
if ret.returncode == 0: | |
cp_modules.remove(module) | |
i += 1 | |
if cp_modules: | |
raise RuntimeError( | |
"failed to remove modules: {}".format(cp_modules) | |
if not modules: | |
logging.info("No need to remove any OTG module since no candidate be provided") | |
return | |
retry_failed_set = set() | |
# Go through each module | |
for module in modules: | |
max_retry_count = 3 | |
while max_retry_count: | |
cmd = "modprobe -r {}".format(module) | |
logging.info("Removing %s module", module) | |
logging.info("$ %s", cmd) | |
try: | |
ret = subprocess.run(cmd, shell=True) | |
if ret.returncode == 0: | |
break | |
max_retry_count -= 1 | |
if max_retry_count == 0: | |
retry_failed_set.add(module) | |
except subprocess.CalledProcessError as e: | |
logging.warning("Command failed:", e) | |
if retry_failed_set: | |
raise RuntimeError( | |
"Failed to remove modules: {}".format(retry_failed_set) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's just a list of module name. But there're some dependencies between them, and you have to remove them in specific order. Otherwise, you won't be able to remove all of them.
let's said the list is usb_f_rndis, g_ether, usb_f_ecm
, so the first module we would like to remove is usb_f_rndis
, but we will get an error while we trying to remove it.
ubuntu@ubuntu:~$ lsmod | grep libcom
libcomposite 77824 3 usb_f_rndis,g_ether,usb_f_ecm
ubuntu@ubuntu:~$ sudo modprobe -r usb_f_rndis
modprobe: FATAL: Module usb_f_rndis is in use.
So I use another mechansim to remove modules, I trying to remove all of modules at first round, and then do the same actions for rest of modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. We have no idea of the modules' dependency and it causes this issue.
But it might be a risk if there are more than 3
modules and the have a dependency in a chain. For instance, four modules A, B, C, D and A depends on B, B depends C depends on D (A -> B -> C -> D). D
will be removed in the first round, C
will be removed in the second round, then B
in third round, but will never remove A
since we hardcode the iteration count as 3. I suggest we can use the count of modules we passed in as the retry count.
Description
Fix OTG test scripts for following issues
Resolved issues
Documentation
Tests
https://certification.canonical.com/hardware/202309-32027/submission/420194/