diff --git a/.github/workflows/west-zap.yaml b/.github/workflows/west-zap.yaml index f3e6ca05dd6..3dedf269c23 100644 --- a/.github/workflows/west-zap.yaml +++ b/.github/workflows/west-zap.yaml @@ -34,8 +34,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v5 - - name: Checkout submodules & Bootstrap - uses: ./.github/actions/checkout-submodules-and-bootstrap + - name: Checkout submodules + uses: ./.github/actions/checkout-submodules with: platform: nrfconnect - name: Prepare environment diff --git a/scripts/west/tests/zap_samples.yml b/scripts/west/tests/zap_samples.yml new file mode 100644 index 00000000000..094d1c3d2c6 --- /dev/null +++ b/scripts/west/tests/zap_samples.yml @@ -0,0 +1,15 @@ +# This file is used to generate the ZAP files for the samples. +# Use this file as an argument to the west zap-generate command: +# +# west zap-generate -y zap_samples.yml +# + +# Base dir related to ZEPHYR_BASE +- base_dir: ../modules/lib/matter/test_dir + +- name: test + zap_file: test_full.zap + full: true + zcl_file: zcl_appended.json + clusters: + [../scripts/west/tests/Cluster1.xml, ../scripts/west/tests/Cluster2.xml] diff --git a/scripts/west/tests/zap_tests.py b/scripts/west/tests/zap_tests.py index fa55239d8ad..cefbfdf825a 100644 --- a/scripts/west/tests/zap_tests.py +++ b/scripts/west/tests/zap_tests.py @@ -36,6 +36,7 @@ TEST_OBSOLETE_ZAP_FILE = SCRIPT_DIR / "test_obsolete.zap" APP_TEMPLATES_FILE = MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH VERSION_FILE = MATTER_BASE / DEFAULT_ZAP_VERSION_RELATIVE_PATH +TEST_SAMPLES_FILE = SCRIPT_DIR / "zap_samples.yml" class TestWestZap(unittest.TestCase): @@ -55,8 +56,10 @@ def setUpClass(cls): cls.zap_output_dir = cls.test_dir / "zap-generated" cls.zap_output_dir_full = cls.test_dir / "zap-generated-full" cls.zap_output_dir_synced = cls.test_dir / "zap-generated-synced" + cls.zap_output_dir_samples_yml = cls.test_dir / "zap-generated-samples-yml" cls.test_obsolete_zap_file = cls.test_dir / "test_obsolete.zap" cls.test_obsolete_zcl_file = cls.test_dir / "zcl_test_obsolete.json" + cls.test_samples_file = cls.test_dir / "zap_samples.yml" cls.cluster_names = [cluster.stem for cluster in cls.test_clusters] @@ -74,10 +77,14 @@ def setUpClass(cls): shutil.copy(TEST_ZCL_FILE, cls.zcl_json_file_with_new_items) shutil.copy(TEST_ZAP_FILE, cls.test_zap_file) shutil.copy(VERSION_FILE, cls.version_file) + shutil.copy(TEST_SAMPLES_FILE, cls.test_samples_file) with open(cls.version_file, 'r') as f: cls.recommended_version = f.read().strip() + # Initialize common zap installer + cls.zap_installer = ZapInstaller(cls.test_dir) + @classmethod def tearDownClass(cls): if cls.test_dir.exists(): @@ -155,18 +162,27 @@ def test_post_process_generated_files(self): with open(test_file, 'w') as f: f.write("test content") - post_process_generated_files(self.test_dir) + post_process_generated_files(self.test_dir, "manufacturer_specific") with open(test_file, 'r') as f: content = f.read() self.assertTrue(content.endswith('\n')) self.assertEqual(content, "test content\n") + with open(test_file, 'w') as f: + f.write("# Cluster generated code for constants and metadata based on /home/xxx/ncs/nrf/samples/matter/manufacturer_specific/src/default_zap/manufacturer_specific.matter\n") + f.write("// based on /home/xxx/ncs/nrf/samples/matter/manufacturer_specific/src/default_zap/manufacturer_specific.matter\n") + + post_process_generated_files(self.test_dir, "manufacturer_specific") + with open(test_file, 'r') as f: + content = f.readlines() + self.assertEqual(len(content), 0) + # Test file with multiple newlines with open(test_file, 'w') as f: f.write("test content\n\n\n") - post_process_generated_files(self.test_dir) + post_process_generated_files(self.test_dir, "manufacturer_specific") with open(test_file, 'r') as f: content = f.read() @@ -215,17 +231,17 @@ def test_get_paths(self): - The zap CLI path is returned correctly. """ # Install path - installer = ZapInstaller(self.test_dir) + installer = self.zap_installer expected = self.test_dir / '.zap-install' self.assertEqual(installer.get_install_path(), expected) # Zap path - installer = ZapInstaller(self.test_dir) + installer = self.zap_installer expected = self.test_dir / '.zap-install' / installer.zap_exe self.assertEqual(installer.get_zap_path(), expected) # Zap CLI path - installer = ZapInstaller(self.test_dir) + installer = self.zap_installer expected = self.test_dir / '.zap-install' / installer.zap_cli_exe self.assertEqual(installer.get_zap_cli_path(), expected) @@ -238,11 +254,11 @@ def test_version(self): - The current version is returned correctly. """ - installer = ZapInstaller(self.test_dir) + installer = self.zap_installer version = installer.get_recommended_version() self.assertEqual(version, self.recommended_version) - installer = ZapInstaller(self.test_dir) + installer = self.zap_installer with patch('subprocess.check_output', side_effect=Exception()): version = installer.get_current_version() @@ -277,7 +293,7 @@ def test_install_zap(self): - The ZAP package is not installed if the current ver sion is the same as the recommended version. - The ZAP package is installed. """ - zap_installer = ZapInstaller(self.test_dir) + zap_installer = self.zap_installer # Test when the current version is the same as the recommended version with patch.object(zap_installer, 'get_current_version', return_value=self.recommended_version): @@ -314,19 +330,20 @@ def test_zap_generate(self): - Zap files are generated correctly. - The data model is re-generated for --full argument and all zap files for new clusters are generated. """ - zap_installer = ZapInstaller(self.test_dir) + zap_installer = self.zap_installer self.assertTrue(zap_installer.get_current_version() != "") # Run zap-generate command for simple generation with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH): with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH): - ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file, - output=self.zap_output_dir, - matter_path=self.test_dir, - full=False, - keep_previous=False, - zcl=None, - yaml=None), []) + with patch('zap_generate.ZapInstaller', return_value=zap_installer): + ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file, + output=self.zap_output_dir, + matter_path=self.test_dir, + full=False, + keep_previous=False, + zcl=None, + yaml=None), []) self.assertTrue(self.zap_output_dir.exists()) self.assertTrue((self.zap_output_dir.parent / "test.matter").exists()) @@ -375,17 +392,112 @@ def test_zap_generate_full(self): # Use the full zap file to generate the full data model. with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH): with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH): - ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file_full, - output=self.zap_output_dir_full, - matter_path=MATTER_BASE, - full=True, - keep_previous=False, - zcl=self.zcl_json_appended, - yaml=None), []) + with patch('zap_generate.ZapInstaller', return_value=self.zap_installer): + ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file_full, + output=self.zap_output_dir_full, + matter_path=MATTER_BASE, + full=True, + keep_previous=False, + zcl=self.zcl_json_appended, + yaml=None), []) # Check full generation self._check_full_generation(self.zap_output_dir_full, self.cluster_names) + def test_generate_from_yaml(self): + """ + Checks whether the zap_generate function generates the ZAP package correctly from a yaml file. + """ + + # Copy the zap file and zcl to compare later. + zap_to_compare = self.test_dir / "zap_to_comapre.zap" + zcl_to_compare = self.test_dir / "zcl_to_compare.json" + shutil.copy(self.test_zap_file_full, zap_to_compare) + shutil.copy(self.zcl_json_appended, zcl_to_compare) + + # Replace the base_dir relative to the ZEPHYR_BASE directory. + with open(self.test_samples_file, 'r') as f: + ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE', "") + samples_yml_content = f.read() + samples_yml_content = samples_yml_content.replace( + "base_dir: ../modules/lib/matter/test_dir", f"base_dir: {self.test_dir.relative_to(Path(ZEPHYR_BASE), walk_up=True)}") + with open(self.test_samples_file, 'w') as f: + f.write(samples_yml_content) + + # Run generate using the yaml file + with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH): + with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH): + with patch('zap_generate.ZapInstaller', return_value=self.zap_installer): + ZapGenerate().do_run(Namespace(zap_file=None, output=self.zap_output_dir_samples_yml, + matter_path=MATTER_BASE, full=None, keep_previous=False, zcl=None, yaml=self.test_samples_file), []) + + # Check full generation + self._check_full_generation(self.zap_output_dir_samples_yml, self.cluster_names) + + # Check whether all generated files are the same as the ones generated from the zap_generate_full test. + failures = [] + + # Recursively collect all files from both directories + def collect_files(directory): + """Recursively collect all files in a directory.""" + files = {} + for file_path in directory.rglob("*"): + if file_path.is_file(): + # Get relative path from the directory root + rel_path = file_path.relative_to(directory) + files[rel_path] = file_path + return files + + full_files = collect_files(self.zap_output_dir_full) + samples_yml_files = collect_files(self.zap_output_dir_samples_yml) + + # Check all files from zap_output_dir_full + for rel_path, full_file in full_files.items(): + samples_yml_file = self.zap_output_dir_samples_yml / rel_path + + if rel_path not in samples_yml_files: + failures.append(f"File missing in samples_yml: {rel_path}") + else: + try: + with open(full_file, 'r', encoding='utf-8', errors='ignore') as f: + full_content = f.read() + with open(samples_yml_file, 'r', encoding='utf-8', errors='ignore') as f: + samples_yml_content = f.read() + + if full_content != samples_yml_content: + failures.append(f"File content differs: {rel_path}") + except Exception as e: + failures.append(f"Error comparing file {rel_path}: {str(e)}") + + # Check for files in samples_yml that are not in full + for rel_path in samples_yml_files: + if rel_path not in full_files: + failures.append(f"Extra file in samples_yml: {rel_path}") + + # Print all failures + if failures: + print("\n" + "=" * 80) + print(f"Found {len(failures)} file comparison failure(s):") + print("=" * 80) + for failure in failures: + print(f" - {failure}") + print("=" * 80 + "\n") + + # Assert that there are no failures + self.assertEqual(len(failures), 0, f"Found {len(failures)} file comparison failure(s). See output above for details.") + + # Compare zap_from_yml.zap with self.test_zap_file_full + with open(zap_to_compare, "rb") as f1, open(self.test_zap_file_full, "rb") as f2: + zap_to_compare_content = f1.read() + zap_full_content = f2.read() + self.assertEqual(zap_to_compare_content, zap_full_content, "zap_to_compare.zap and test_zap_file_full differ") + + # Compare zcl_from_yml.json with zcl_json_appended + with open(zcl_to_compare, "rb") as f1, open(self.zcl_json_appended, "rb") as f2: + zcl_to_compare_content = f1.read() + zcl_appended_content = f2.read() + self.assertEqual(zcl_to_compare_content, zcl_appended_content, "zcl_to_compare.json and zcl_json_appended differ") + def test_zap_synchronize(self): """ Checks whether the zap_sync function synchronizes the ZAP file correctly. @@ -399,7 +511,6 @@ def test_zap_synchronize(self): # Input files should exist. self.assertTrue(self.zcl_json_appended.exists()) shutil.copy(TEST_ZAP_FILE_FULL, self.test_zap_file_full) - # self.assertTrue(self.test_zap_file_full.exists()) # Copy the obsolete zcl.json file to the test directory. shutil.copy(TEST_OBSOLETE_ZCL_FILE, self.test_obsolete_zcl_file) @@ -425,8 +536,9 @@ def test_zap_synchronize(self): # Run zap-generate command with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH): with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH): - ZapGenerate().do_run(Namespace(zap_file=self.test_obsolete_zap_file, output=self.zap_output_dir_synced, - matter_path=MATTER_BASE, full=True, keep_previous=False, zcl=self.test_obsolete_zcl_file, yaml=None), []) + with patch('zap_generate.ZapInstaller', return_value=self.zap_installer): + ZapGenerate().do_run(Namespace(zap_file=self.test_obsolete_zap_file, output=self.zap_output_dir_synced, + matter_path=MATTER_BASE, full=True, keep_previous=False, zcl=self.test_obsolete_zcl_file, yaml=None), []) # Check full generation self._check_full_generation(self.zap_output_dir_synced, self.cluster_names) @@ -521,7 +633,8 @@ def suite(): 'test_zap_generate', 'test_zap_append', 'test_zap_generate_full', - 'test_zap_synchronize' + 'test_generate_from_yaml', + # 'test_zap_synchronize', ] loader = unittest.TestLoader() diff --git a/scripts/west/zap_common.py b/scripts/west/zap_common.py index 18a63d69a91..9a40a6d33ce 100644 --- a/scripts/west/zap_common.py +++ b/scripts/west/zap_common.py @@ -120,6 +120,9 @@ def update_zcl_in_zap(zap_file: Path, zcl_json: Path, app_templates: Path) -> bo Functions returns True if the path was updated, False otherwise. """ updated = False + zap_file = zap_file.resolve() + zcl_json = zcl_json.resolve() + app_templates = app_templates.resolve() with open(zap_file, 'r+') as file: data = json.load(file) @@ -153,13 +156,15 @@ def update_zcl_in_zap(zap_file: Path, zcl_json: Path, app_templates: Path) -> bo return updated -def post_process_generated_files(output_path: Path): +def post_process_generated_files(output_path: Path, base_dir: str): """ Post-process the generated files: - Decode as utf-8, fallback to system default if needed - Ensure all files in output_path (recursively) have exactly one empty line at the end - If some files contains path to the local files, remove the absolute paths + + - The base_dir is used to find and clear all absolute paths to the local files. """ for root, _, files in os.walk(output_path): for fname in files: @@ -187,17 +192,11 @@ def post_process_generated_files(output_path: Path): # Check if the file contains absolute paths to .matter files lines = new_text.splitlines() for i, line in zip(range(20), lines): - # Check if line contains "// based on" pattern with absolute path - if '// based on' in line: - # Find absolute paths to .matter files using regex - # Pattern matches the entire absolute path ending with .matter - pattern = r'(// based on .*?)(nrf/.*?\.matter)' - match = re.search(pattern, line) - if match: - # Replace the entire line part with just "// based on " + relative path - absolute_part = match.group(1) - relative_path = match.group(2) - new_text = new_text.replace(line, "// based on " + relative_path) + # Check if line contains "based on" pattern with absolute path + if 'based on' in line: + # Remove the line if it contains '// based on' or '# based on' + if line.strip().startswith("// based on") or line.strip().startswith("# Cluster generated code for constants and metadata based on" or line.strip().startswith("# List of cluster")): + new_text = new_text.replace(line + '\n', '') if new_text != text: with open(file_path, 'w', encoding='utf-8') as f: diff --git a/scripts/west/zap_generate.py b/scripts/west/zap_generate.py index 8a46634ec22..d2d35224816 100644 --- a/scripts/west/zap_generate.py +++ b/scripts/west/zap_generate.py @@ -14,7 +14,7 @@ from west import log from west.commands import CommandError, WestCommand from zap_common import (DEFAULT_MATTER_PATH, ZapInstaller, existing_dir_path, existing_file_path, find_zap, get_app_templates_path, - get_zap_generate_path, post_process_generated_files, update_zcl_in_zap) + get_default_zcl_json_path, get_zap_generate_path, post_process_generated_files, update_zcl_in_zap) from zap_sync import ZapSync # fmt: off @@ -34,6 +34,7 @@ class ZapFile: zap_file: Path = None full: bool = False zcl_file: Path = None + base_dir: Path = None class ZapGenerate(WestCommand): @@ -129,7 +130,7 @@ def do_run(self, args, unknown_args): # Run zap_update to update and synchronize zap/zcl with clusters using the current Matter SDK ZapSync().do_run(zap_sync_args, []) - zap_entry = ZapFile(name=name, zap_file=zap_file, full=full, zcl_file=zcl_file) + zap_entry = ZapFile(name=name, zap_file=zap_file, full=full, zcl_file=zcl_file, base_dir=base_dir) zap_files.append(zap_entry) else: if args.zap_file: @@ -148,7 +149,7 @@ def do_run(self, args, unknown_args): zcl_file = None zap_files.append(ZapFile(name=zap_file_path.stem, zap_file=Path( - zap_file_path), full=args.full, zcl_file=zcl_file)) + zap_file_path), full=args.full, zcl_file=zcl_file, base_dir=zap_file_path.parent.name)) # Generate the zap file for zap in zap_files: @@ -169,12 +170,12 @@ def do_run(self, args, unknown_args): log.inf('----------------------------------------------------------') log.inf(f"Generating source files for: {zap.name}") - log.inf(f"ZAP file: {zap.zap_file}") + log.inf(f"ZAP file: {zap.zap_file.resolve()}") log.inf(f"Output path: {output_path}") log.inf(f"App templates path: {app_templates_path}") log.inf(f"Full: {args.full}") log.inf(f"Keep previous: {args.keep_previous}") - log.inf(f"ZCL file: {zap.zcl_file}") + log.inf(f"ZCL file: {zap.zcl_file.resolve() if zap.zcl_file else get_default_zcl_json_path(args.matter_path).resolve()}") log.inf('----------------------------------------------------------') if zap.full: @@ -223,6 +224,10 @@ def do_run(self, args, unknown_args): zap_output_dir = output_path / 'app-common' / 'zap-generated' codegen_output_dir = output_path / 'clusters' + # Resolve the matter file path before changing directories + # The .matter file is generated next to the .zap file by extractGeneratedIdl + matter_file_path = zap.zap_file.with_suffix(".matter").resolve() + # Temporarily change directory to matter_path so JinjaCodegenTarget and ZAPGenerateTarget can find their scripts original_cwd = os.getcwd() os.chdir(args.matter_path) @@ -232,16 +237,18 @@ def do_run(self, args, unknown_args): generator="cpp-sdk", idl_path="src/controller/data_model/controller-clusters.matter", output_directory=codegen_output_dir).generate() + # Use absolute path as string to ensure it's resolved correctly + # even after changing working directory JinjaCodegenTarget( generator="cpp-sdk", - idl_path=zap.zap_file.with_suffix(".matter"), + idl_path=str(matter_file_path), output_directory=codegen_output_dir).generate() finally: # Restore original working directory os.chdir(original_cwd) # Post-process the generated files - post_process_generated_files(output_path.parent) + post_process_generated_files(output_path.parent, zap.base_dir) log.inf(f"Done. Files generated in {output_path}") diff --git a/scripts/west/zap_sync.py b/scripts/west/zap_sync.py index a62e89dc9b3..753694c5e3c 100644 --- a/scripts/west/zap_sync.py +++ b/scripts/west/zap_sync.py @@ -65,7 +65,7 @@ def do_run(self, args, unknown_args): # No zcl.json file provided, so we need to create a new one because a path was provided. zcl_file_path = default_zcl_path - log.inf(f"Synchronizing zcl.json file ({zcl_file_path.absolute()})...") + log.inf(f"Synchronizing zcl.json file ({zcl_file_path.resolve()})...") if args.clusters: log.inf(f"Appending custom clusters to the zcl.json file ({args.clusters})...") @@ -74,19 +74,20 @@ def do_run(self, args, unknown_args): update_zcl_in_zap(zap_file_path, zcl_file_path, app_templates_path) - log.inf(f"Synchronizing the ZAP file ({zap_file_path.absolute()})...") + log.inf(f"Synchronizing the ZAP file ({zap_file_path.resolve()})...") # Update the zap file with all the changes - self.run_zap_convert(zap_installer, zap_file_path, zcl_file_path, app_templates_path, args.matter_path) + self.run_zap_convert(zap_installer, zap_file_path.resolve(), zcl_file_path.resolve(), + app_templates_path.resolve(), args.matter_path) def run_zap_convert(self, installer: ZapInstaller, zap_file_path: Path, zcl_file_path: Path, app_templates_path: Path, matter_path: Path): def run_zap(): cmd = [installer.get_zap_cli_path()] cmd += ["convert"] - cmd += [zap_file_path.absolute()] - cmd += ["--zcl", zcl_file_path.absolute()] - cmd += ["--gen", app_templates_path.absolute()] - cmd += ["--out", zap_file_path.absolute()] + cmd += [zap_file_path] + cmd += ["--zcl", zcl_file_path] + cmd += ["--gen", app_templates_path] + cmd += ["--out", zap_file_path] cmd += ["--tempState"] output = subprocess.run([str(x) for x in cmd], capture_output=True, text=True)