Implement LOG015 and G ruff ruleset recommendations in scripts/ (project-chip#42062)

* Implement LOG015 and G ruff ruleset recommendations in scripts/

- created per-file logger as LOG015 recommends
- updated log messages to not use lazy formatting instead of f-strings, .format() and %
- %s -> '%s' where applicable in log messages to better indicate empty strings
- log.error -> log.exception in exception handlers
- used logging.getLevelNamesMapping() instead of magic dicts

Signed-off-by: Maciej Grela <[email protected]>

* Restyled by isort

* Implement copilot review changes

Signed-off-by: Maciej Grela <[email protected]>

* Fix two missed LOGGER instances

Signed-off-by: Maciej Grela <[email protected]>

---------

Signed-off-by: Maciej Grela <[email protected]>
Co-authored-by: Restyled.io <[email protected]>

Author: enkiusz

scripts/checkout_submodules.py

@@ -22,6 +22,8 @@
 import subprocess
 from collections import namedtuple
 
+log = logging.getLogger(__name__)
+
 CHIP_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))
 
 ALL_PLATFORMS = {
@@ -102,15 +104,14 @@ def make_chip_root_safe_directory() -> None:
         config.check_returncode()
         existing = config.stdout.split('\0')
     if CHIP_ROOT not in existing:
-        logging.info(
-            "Adding CHIP_ROOT to global git safe.directory configuration")
+        log.info("Adding CHIP_ROOT to global git safe.directory configuration")
         subprocess.check_call(
             ['git', 'config', '--global', '--add', 'safe.directory', CHIP_ROOT])
 
 
 def checkout_modules(modules: list, shallow: bool, force: bool, recursive: bool, jobs: int) -> None:
     names = ', '.join([module.name for module in modules])
-    logging.info(f'Checking out: {names}')
+    log.info("Checking out '%s'", names)
 
     cmd = ['git', '-c', 'core.symlinks=true', '-C', CHIP_ROOT]
     cmd += ['submodule', '--quiet', 'update', '--init']
@@ -137,7 +138,7 @@ def checkout_modules(modules: list, shallow: bool, force: bool, recursive: bool,
 
 def deinit_modules(modules: list, force: bool) -> None:
     names = ', '.join([module.name for module in modules])
-    logging.info(f'Deinitializing: {names}')
+    log.info("Deinitializing: '%s'", names)
 
     cmd = ['git', '-C', CHIP_ROOT, 'submodule', '--quiet', 'deinit']
     cmd += ['--force'] if force else []

scripts/codegen.py

@@ -41,6 +41,8 @@
 except ImportError:
     _has_coloredlogs = False
 
+log = logging.getLogger(__name__)
+
 
 class ListGeneratedFilesStorage(GeneratorStorage):
     """
@@ -59,12 +61,7 @@ def write_new_data(self, relative_path: str, content: str):
 
 # Supported log levels, mapping string values required for argument
 # parsing into logging constants
-__LOG_LEVELS__ = {
-    'debug': logging.DEBUG,
-    'info': logging.INFO,
-    'warn': logging.WARNING,
-    'fatal': logging.FATAL,
-}
+__LOG_LEVELS__ = logging.getLevelNamesMapping()
 
 
 @click.command()
@@ -111,9 +108,9 @@ def main(log_level, generator, option, output_dir, dry_run, name_only, expected_
 
     def formatKotlinFiles(paths):
         try:
-            logging.info("Prettifying %d kotlin files:", len(paths))
+            log.info("Prettifying %d kotlin files:", len(paths))
             for name in paths:
-                logging.info("    %s" % name)
+                log.info("    '%s'", name)
 
             VERSION = "0.58"
             JAR_NAME = f"ktfmt-{VERSION}-with-dependencies.jar"
@@ -148,31 +145,31 @@ def formatKotlinFiles(paths):
     else:
         storage = FileSystemGeneratorStorage(output_dir)
 
-    logging.info("Parsing idl from %s" % idl_path)
+    log.info("Parsing idl from '%s'", idl_path)
     idl_tree = CreateParser().parse(open(idl_path, "rt").read(), file_name=idl_path)
 
     plugin_module = None
     if generator.startswith('custom:'):
         # check that the plugin path is provided
         custom_params = generator.split(':')
         if len(custom_params) != 3:
-            logging.fatal("Custom generator format not valid. Please use --generator custom:<path>:<module>")
+            log.fatal("Custom generator format not valid. Please use --generator custom:<path>:<module>")
             sys.exit(1)
         (generator, plugin_path, plugin_module) = custom_params
 
-        logging.info("Using CustomGenerator at plugin path %s.%s" % (plugin_path, plugin_module))
+        log.info("Using CustomGenerator at plugin path '%s.%s'", plugin_path, plugin_module)
         sys.path.append(plugin_path)
         generator = 'CUSTOM'
 
     extra_args = {}
     for o in option:
         if ':' not in o:
-            logging.fatal("Please specify options as '<key>:<value>'. %r is not valid. " % o)
+            log.fatal("Please specify options as '<key>:<value>'. %r is not valid. ", o)
             sys.exit(1)
         key, value = o.split(':')
         extra_args[key] = value
 
-    logging.info("Running code generator %s" % generator)
+    log.info("Running code generator '%s'", generator)
     generator = CodeGenerator.FromString(generator).Create(storage, idl=idl_tree, plugin_module=plugin_module, **extra_args)
     generator.render(dry_run)
 
@@ -186,7 +183,7 @@ def formatKotlinFiles(paths):
 
     if name_dict.get('.kt', []):
         try:
-            logging.debug("Formatting kt_files: %s" % name_dict['.kt'])
+            log.debug("Formatting kt_files: '%s'", name_dict['.kt'])
         except Exception:
             traceback.print_exc()
         formatKotlinFiles(name_dict['.kt'])
@@ -196,7 +193,7 @@ def formatKotlinFiles(paths):
         cpp_files.extend(name_dict.get(ext, []))
     if cpp_files:
         try:
-            logging.debug("Formatting cpp_files: %s", cpp_files)
+            log.debug("Formatting cpp_files: '%s'", cpp_files)
             subprocess.check_call([getClangFormatBinary(), "-i"] + cpp_files)
         except Exception:
             traceback.print_exc()
@@ -210,20 +207,20 @@ def formatKotlinFiles(paths):
                     expected.add(expanded_path)
 
             if expected != storage.generated_paths:
-                logging.fatal("expected and generated files do not match.")
+                log.fatal("expected and generated files do not match.")
 
                 extra = storage.generated_paths - expected
                 missing = expected - storage.generated_paths
 
                 for name in extra:
-                    logging.fatal("   '%s' was generated but not expected" % name)
+                    log.fatal("   '%s' was generated but not expected", name)
 
                 for name in missing:
-                    logging.fatal("   '%s' was expected but not generated" % name)
+                    log.fatal("   '%s' was expected but not generated", name)
 
                 sys.exit(1)
 
-    logging.info("Done")
+    log.info("Done")
 
 
 if __name__ == '__main__':

scripts/codepregen.py

@@ -37,14 +37,11 @@
 except ImportError:
     _has_coloredlogs = False
 
+log = logging.getLogger(__name__)
+
 # Supported log levels, mapping string values required for argument
 # parsing into logging constants
-__LOG_LEVELS__ = {
-    'debug': logging.DEBUG,
-    'info': logging.INFO,
-    'warn': logging.WARNING,
-    'fatal': logging.FATAL,
-}
+__LOG_LEVELS__ = logging.getLevelNamesMapping()
 
 
 def _ParallelGenerateOne(arg):
@@ -111,7 +108,7 @@ def main(log_level, parallel, dry_run, generator, input_glob, sdk_root, external
 
     output_dir = os.path.abspath(output_dir)
 
-    logging.info(f"Pre-generating {sdk_root} data into {output_dir}")
+    log.info("Pre-generating '%s' data into '%s'", sdk_root, output_dir)
 
     if not dry_run:
         runner = ShellRunner()
@@ -137,7 +134,7 @@ def main(log_level, parallel, dry_run, generator, input_glob, sdk_root, external
         for target in targets:
             target.Generate(output_dir)
 
-    logging.info("Done")
+    log.info("Done")
 
 
 if __name__ == '__main__':

scripts/copyfiles.py

@@ -26,14 +26,11 @@
 except ImportError:
     _has_coloredlogs = False
 
+log = logging.getLogger(__name__)
+
 # Supported log levels, mapping string values required for argument
 # parsing into logging constants
-__LOG_LEVELS__ = {
-    'debug': logging.DEBUG,
-    'info': logging.INFO,
-    'warn': logging.WARNING,
-    'fatal': logging.FATAL,
-}
+__LOG_LEVELS__ = logging.getLevelNamesMapping()
 
 
 @click.command()
@@ -72,7 +69,7 @@ def main(log_level, source_dir: str, target_dir: str, filenames: list[str]):
 
     # We are copying to target ...
     if not os.path.exists(target_dir):
-        logging.info("Creating output directory %s", target_dir)
+        log.info("Creating output directory '%s'", target_dir)
         os.makedirs(target_dir)
 
     for filename in filenames:
@@ -88,7 +85,7 @@ def main(log_level, source_dir: str, target_dir: str, filenames: list[str]):
         filename = os.path.abspath(filename)
         relative_path = os.path.relpath(filename, source_dir)
         if not filename.endswith(relative_path):
-            logging.error("%s does not seem to be a child of %s: relative path is %s", filename, source_dir, relative_path)
+            log.error("'%s' does not seem to be a child of '%s': relative path is '%s'", filename, source_dir, relative_path)
             sys.exit(1)
 
         # Prepend the destination directory to relative path. So that if we have:
@@ -105,10 +102,10 @@ def main(log_level, source_dir: str, target_dir: str, filenames: list[str]):
 
         destination_dir = os.path.dirname(destination)
         if not os.path.exists(destination_dir):
-            logging.info("Creating output directory %s", destination_dir)
+            log.info("Creating output directory '%s'", destination_dir)
             os.makedirs(destination_dir)
 
-        logging.info("Copying %s into %s", filename, destination)
+        log.info("Copying '%s' into '%s'", filename, destination)
         shutil.copyfile(filename, destination)
 
 

scripts/examples/tests/test.py

@@ -23,6 +23,8 @@
 
 import coloredlogs
 
+log = logging.getLogger(__name__)
+
 SCRIPT_ROOT = os.path.dirname(__file__)
 
 
@@ -60,9 +62,9 @@ def main():
     diffs = list(difflib.unified_diff(expected, actual))
 
     if diffs:
-        logging.error("DIFFERENCE between expected and generated output")
+        log.error("DIFFERENCE between expected and generated output")
         for line in diffs:
-            logging.warning("  " + line.strip())
+            log.warning("  %s", line.strip())
         sys.exit(1)
 
 

scripts/flashing/bouffalolab_firmware_utils.py

@@ -20,6 +20,7 @@
 import pathlib
 import platform
 import re
+import shlex
 import shutil
 import subprocess
 import sys
@@ -31,6 +32,8 @@
 
 coloredlogs.install(level='DEBUG')
 
+log = logging.getLogger(__name__)
+
 # Additional options that can be use to configure an `Flasher`
 # object (as dictionary keys) and/or passed as command line options.
 
@@ -359,14 +362,14 @@ def get_tools():
                 raise Exception("Do NOT support {} operating system to program firmware.".format(sys.platform))
 
             if not os.path.exists(flashtool_exe):
-                logging.fatal('*' * 80)
-                logging.error('Flashtool is not installed, or environment variable BOUFFALOLAB_SDK_ROOT is not exported.')
-                logging.fatal('\tPlease make sure Bouffalo Lab SDK installs as below:')
-                logging.fatal('\t\t./third_party/bouffalolab/env-setup.sh')
-
-                logging.fatal('\tPlease make sure BOUFFALOLAB_SDK_ROOT exports before building as below:')
-                logging.fatal('\t\texport BOUFFALOLAB_SDK_ROOT="your install path"')
-                logging.fatal('*' * 80)
+                log.fatal("*" * 80)
+                log.error("Flashtool is not installed, or environment variable BOUFFALOLAB_SDK_ROOT is not exported.")
+                log.fatal("\tPlease make sure Bouffalo Lab SDK installs as below:")
+                log.fatal("\t\t./third_party/bouffalolab/env-setup.sh")
+
+                log.fatal("\tPlease make sure BOUFFALOLAB_SDK_ROOT exports before building as below:")
+                log.fatal("\t\texport BOUFFALOLAB_SDK_ROOT='your install path'")
+                log.fatal("*" * 80)
                 raise Exception("Flash tool is not installed.")
 
             return flashtool_path, flashtool_exe
@@ -428,12 +431,12 @@ def exe_gen_ota_image_cmd(flashtool_exe):
             if self.args["sk"]:
                 gen_ota_img_cmd += ["--sk", self.args["sk"]]
 
-            logging.info("ota image generating: {}".format(" ".join(gen_ota_img_cmd)))
+            log.info("ota image generating: %s", shlex.join(gen_ota_img_cmd))
             process = subprocess.Popen(gen_ota_img_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             while process.poll() is None:
                 line = process.stdout.readline().decode('utf-8').rstrip()
                 if line:
-                    logging.info(line)
+                    log.info(line)
 
             fw_ota_images = self.find_file(self.work_dir, r'^FW_OTA.+\.hash$')
             if not fw_ota_images or 0 == len(fw_ota_images):
@@ -542,7 +545,7 @@ def exe_prog_cmd(flashtool_exe, mfd_addr, flashtool_path):
 
                 if mfd_addr and self.args["mfd_str"]:
                     if self.args["key"] and not self.args["iv"]:
-                        logging.warning("mfd file has no iv, do NOT program mfd key.")
+                        log.warning("mfd file has no iv, do NOT program mfd key.")
                     else:
                         prog_cmd += ["--dac_key", self.args["key"]]
                         prog_cmd += ["--dac_iv", self.args["iv"]]
@@ -552,12 +555,12 @@ def exe_prog_cmd(flashtool_exe, mfd_addr, flashtool_path):
                 if self.option.erase:
                     prog_cmd += ["--erase"]
 
-            logging.info("firmware programming: {}".format(" ".join(prog_cmd)))
+            log.info("firmware programming: %s", shlex.join(prog_cmd))
             process = subprocess.Popen(prog_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             while process.poll() is None:
                 line = process.stdout.readline().decode('utf-8').rstrip()
                 if line:
-                    logging.info(line)
+                    log.info(line)
 
         flashtool_path, flashtool_exe = get_tools()
         self.args["pt"] = os.path.join(os.getcwd(), str(self.args["pt"]))
@@ -595,10 +598,10 @@ def get_tools():
                 raise Exception("Do NOT support {} operating system to program firmware.".format(sys.platform))
 
             if not os.path.exists(flashtool_exe) or not os.path.exists(fw_proc_exe):
-                logging.fatal('*' * 80)
-                logging.error("Expecting tools as below:")
-                logging.error(fw_proc_exe)
-                logging.error(flashtool_exe)
+                log.fatal("*" * 80)
+                log.error("Expecting tools as below:")
+                log.error(fw_proc_exe)
+                log.error(flashtool_exe)
                 raise Exception("Flashtool or fw tool doesn't contain in SDK")
 
             return fw_proc_exe, flashtool_exe
@@ -670,12 +673,12 @@ def exe_proc_cmd(fw_proc_exe):
                     "--edata", "0x80,{};0x7c,{};0xfc,{}".format(self.args["key"], lock0, lock1)
                 ]
 
-            logging.info("firmware process command: {}".format(" ".join(fw_proc_cmd)))
+            log.info("firmware process command: %s", shlex.join(fw_proc_cmd))
             process = subprocess.Popen(fw_proc_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             while process.poll() is None:
                 line = process.stdout.readline().decode('utf-8').rstrip()
                 if line:
-                    logging.info(line)
+                    log.info(line)
 
             os.system("mkdir -p {}/ota_images".format(self.work_dir))
             os.system("mv {}/*.ota {}/ota_images/".format(self.work_dir, self.work_dir))
@@ -701,12 +704,12 @@ def exe_prog_cmd(flashtool_exe):
                     "--port", self.args["port"],
                 ]
 
-                logging.info("firwmare programming: {}".format(" ".join(prog_cmd)))
+                log.info("firwmare programming: %s", shlex.join(prog_cmd))
                 process = subprocess.Popen(prog_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
                 while process.poll() is None:
                     line = process.stdout.readline().decode('utf-8').rstrip()
                     if line:
-                        logging.info(line)
+                        log.info(line)
 
         fw_proc_exe, flashtool_exe = get_tools()
         os.chdir(self.work_dir)