Skip to content

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env python3

import argparse
import copy
import glob
import logging
import os
Expand Down Expand Up @@ -102,7 +103,8 @@ def _get_child_modules(self):
text=True,
universal_newlines=True,
)
return output.strip("\n").split(",") if output.strip("\n") else []
output = output.strip("\n")
return output.split(",") if output else []

def enable_otg_module(self, modules):
for module in modules:
Expand All @@ -111,9 +113,23 @@ def enable_otg_module(self, modules):
)

def disable_otg_related_modules(self, modules):
for module in modules:
subprocess.run(
"modprobe -r {}".format(module), shell=True, check=True
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)
Comment on lines +116 to +132
Copy link
Contributor

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.

Suggested change
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)

Copy link
Collaborator Author

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.

Copy link
Contributor

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.

)

def otg_setup(self):
Expand Down Expand Up @@ -340,6 +356,8 @@ def function_check_with_rpyc(self, rpyc_ip):
self._target_net_dev,
"169.254.0.10/24",
)
# wait few seconds to activate networking
time.sleep(2)
logging.info("Ping from DUT to Target")
_module = SourceFileLoader(
"_",
Expand All @@ -348,7 +366,9 @@ def function_check_with_rpyc(self, rpyc_ip):
test_func = getattr(_module, "perform_ping_test")
ret = test_func([self._net_dev], "169.254.0.10")
if ret != 0:
raise RuntimeError("Failed to ping DUT from RPYC server")
raise RuntimeError(
"Failed to ping RPYC server from DUT through OTG network"
)

def otg_test_process(self, rpyc_ip):
self.enable_otg()
Expand Down Expand Up @@ -379,15 +399,15 @@ def self_check(self):
logging.info("Validate a new serial interface been generated")
cur_ser_intfs = self._collect_serial_intfs()
if len(cur_ser_intfs) == len(self._ser_intfs):
raise RuntimeError("OTG network interface not available")
raise RuntimeError("OTG serial interface not available")

otg_ser_intf = [x for x in cur_ser_intfs if x not in self._ser_intfs]
if len(otg_ser_intf) != 1:
logging.error(
"Found more than one new interface. %s", otg_ser_intf
)
else:
logging.info("Found new network interface '%s'", otg_ser_intf[0])
logging.info("Found new serial interface '%s'", otg_ser_intf[0])
self._serial_iface = otg_ser_intf[0]

def detection_check_on_rpyc(self, rpyc_ip):
Expand All @@ -399,18 +419,11 @@ def detection_check_on_rpyc(self, rpyc_ip):
self._target_serial_dev = list(ret)[0]

def function_check_with_rpyc(self, rpyc_ip):
logging.info("perform serial client test on DUT")
func = getattr(import_module("serial_test"), "client_mode")
func(
"/dev/{}".format(self._serial_iface),
"USB",
[],
115200,
8,
"N",
1,
3,
1024,
logging.info("perform serial client test on Rpyc server")
rpyc_client(
rpyc_ip,
"serial_client_test",
"/dev/serial/by-id/{}".format(self._target_serial_dev),
)

def otg_test_process(self, rpyc_ip):
Expand All @@ -419,28 +432,16 @@ def otg_test_process(self, rpyc_ip):
self.detection_check_on_rpyc(rpyc_ip)

try:
logging.info("start serial server on rpyc server")
logging.info(
"start serial server on %s interface", self._serial_iface
)
func = getattr(import_module("serial_test"), "server_mode")
t_thread = Process(
target=rpyc_client,
args=(
rpyc_ip,
"enable_serial_server",
"/dev/serial/by-id/{}".format(self._target_serial_dev),
"USB",
[],
115200,
8,
"N",
1,
3,
1024,
),
target=func, args=("/dev/{}".format(self._serial_iface),)
)
t_thread.start()
time.sleep(3)
self.function_check_with_rpyc(rpyc_ip)
except SystemExit as err:
logging.debug(err)
finally:
t_thread.kill()
self.disable_otg()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
"source": os.path.join(CHECKBOX_PROVIDER_CEOEM_PATH, "serial_test.py"),
"function": "server_mode",
},
"serial_client_test": {
"source": os.path.join(CHECKBOX_PROVIDER_CEOEM_PATH, "serial_test.py"),
"function": "client_mode",
},
"serial_check": {
"source": os.path.join(
CHECKBOX_PROVIDER_CEOEM_PATH, "rpyc_test_methods.py"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def configure_local_network(interface, net_info):
shell=True,
text=True,
)
logging.info("Turn down the link of %s interface", interface)
logging.info("Configure the %s interface to %s", interface, net_info)
subprocess.check_output(
"ip addr add {} dev {}".format(net_info, interface),
shell=True,
Expand Down
Loading