From 0fe8deb37b82fde485e586283d8688c8b4c74c18 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 12 May 2026 06:32:38 +0000 Subject: [PATCH 1/2] Initial plan From 52a2d052c42eb6cf41ff741a43202fb40f144e30 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 12 May 2026 06:37:52 +0000 Subject: [PATCH 2/2] Apply fixes: device ID validation, COE file injection refactor, type annotations, spelling corrections, and test mock fix Agent-Logs-Url: https://github.com/VoltCyclone/PCILeechFWGenerator/sessions/4af478b0-41d8-4479-959d-0a8f53d36fe0 Co-authored-by: ramseymcgrath <2006510+ramseymcgrath@users.noreply.github.com> --- src/device_clone/device_config.py | 2 +- src/file_management/file_manager.py | 126 +++++++++++---------- src/vivado_handling/build_state_cleaner.py | 2 +- src/vivado_handling/vivado_utils.py | 13 ++- tests/test_file_manager.py | 5 +- 5 files changed, 82 insertions(+), 66 deletions(-) diff --git a/src/device_clone/device_config.py b/src/device_clone/device_config.py index fbb66ddd..9d2bca06 100644 --- a/src/device_clone/device_config.py +++ b/src/device_clone/device_config.py @@ -155,7 +155,7 @@ def validate(self) -> None: """Validate device identification values.""" if not (0x0001 <= self.vendor_id <= 0xFFFE): raise ValueError(f"Invalid vendor ID: 0x{self.vendor_id:04X}") - if not (0x0001 <= self.device_id <= 0xFFFF): + if not (0x0000 <= self.device_id <= 0xFFFF): raise ValueError(f"Invalid device ID: 0x{self.device_id:04X}") if not (0x000000 <= self.class_code <= 0xFFFFFF): raise ValueError(f"Invalid class code: 0x{self.class_code:06X}") diff --git a/src/file_management/file_manager.py b/src/file_management/file_manager.py index eacda311..a27f7a02 100644 --- a/src/file_management/file_manager.py +++ b/src/file_management/file_manager.py @@ -816,6 +816,73 @@ def copy_vivado_tcl_scripts(self, board: str) -> List[Path]: return copied_scripts + def _inject_generated_coe_files(self, ip_dir: Path, copied_files: List[Path]) -> None: + """Overwrite template .coe files with generated ones from output/src.""" + src_dir = self.output_dir / "src" + if src_dir.exists(): + generated_coe_files = list(src_dir.glob("*.coe")) + if generated_coe_files: + log_info_safe( + logger, + "Injecting device IDs into IP configuration files", + prefix="FILEMGR", + ) + + # Extract and display device IDs from config space file + for coe_file in generated_coe_files: + if "cfgspace" in coe_file.name and "writemask" not in coe_file.name: + try: + content = coe_file.read_text() + # .coe config-space files are expected to contain hex words; for cfgspace files + # the first relevant word is the PCI config dword at offset 0x00. + # That dword is encoded as 8 hex chars in the form DDDDVVVV (0xDDDDVVVV): + # - chars [0:4] => Device ID (DDDD) + # - chars [4:8] => Vendor ID (VVVV) + # We therefore capture the first 8-hex token at line start and split accordingly. + match = re.search( + r'^\s*([0-9a-fA-F]{8})', + content, + re.MULTILINE + ) + if match: + hex_value = match.group(1) + vendor_id = hex_value[4:8].upper() + device_id = hex_value[0:4].upper() + log_info_safe( + logger, + safe_format( + "Device: 0x{device} Vendor: 0x{vendor}", + device=device_id, + vendor=vendor_id + ), + prefix="FILEMGR", + ) + except Exception as exc: + log_error_safe( + logger, + safe_format( + "Failed to parse device IDs from COE file {file}: {error}", + file=coe_file, + error=exc, + ), + prefix="FILEMGR", + ) + + # Copy generated files over templates + for coe_file in generated_coe_files: + dest_file = ip_dir / coe_file.name + shutil.copy2(coe_file, dest_file) + + log_info_safe( + logger, + safe_format("✓ {ip_name}", ip_name=coe_file.name), + prefix="FILEMGR", + ) + + # Add to copied files if not already there + if dest_file not in copied_files: + copied_files.append(dest_file) + def copy_ip_files(self, board: str) -> List[Path]: """Copy IP files (.coe, .xci) from voltcyclone-fpga submodule to output. @@ -872,64 +939,7 @@ def copy_ip_files(self, board: str) -> List[Path]: # CRITICAL FIX: Overwrite template .coe files with generated ones # The generator creates device-specific .coe files in output/src/ # These must replace the template .coe files to inject device IDs - src_dir = self.output_dir / "src" - if src_dir.exists(): - generated_coe_files = list(src_dir.glob("*.coe")) - if generated_coe_files: - log_info_safe( - logger, - "Injecting device IDs into IP configuration files", - prefix="FILEMGR", - ) - - # Extract and display device IDs from config space file - for coe_file in generated_coe_files: - if "cfgspace" in coe_file.name and "writemask" not in coe_file.name: - try: - content = coe_file.read_text() - match = re.search( - r'^\s*([0-9a-fA-F]{8})', - content, - re.MULTILINE - ) - if match: - hex_value = match.group(1) - vendor_id = hex_value[4:8].upper() - device_id = hex_value[0:4].upper() - log_info_safe( - logger, - safe_format( - "Device: 0x{device} Vendor: 0x{vendor}", - device=device_id, - vendor=vendor_id - ), - prefix="FILEMGR", - ) - except Exception as exc: - log_error_safe( - logger, - safe_format( - "Failed to parse device IDs from COE file {file}: {error}", - file=coe_file, - error=exc, - ), - prefix="FILEMGR", - ) - - # Copy generated files over templates - for coe_file in generated_coe_files: - dest_file = ip_dir / coe_file.name - shutil.copy2(coe_file, dest_file) - - log_info_safe( - logger, - safe_format("✓ {ip_name}", ip_name=coe_file.name), - prefix="FILEMGR", - ) - - # Add to copied files if not already there - if dest_file not in copied_files: - copied_files.append(dest_file) + self._inject_generated_coe_files(ip_dir, copied_files) log_info_safe( logger, diff --git a/src/vivado_handling/build_state_cleaner.py b/src/vivado_handling/build_state_cleaner.py index 1f677d80..b06ad23b 100644 --- a/src/vivado_handling/build_state_cleaner.py +++ b/src/vivado_handling/build_state_cleaner.py @@ -51,7 +51,7 @@ def _remove(path: Path, *, is_dir: bool, prefix: str, logger) -> int: def _iter_xil_dirs(root: Path) -> Iterable[Path]: """Yield every ``.Xil`` directory under *root*, de-duped via resolved path so a symlink loop can't cause repeat visits.""" - seen: set = set() + seen: set[Path] = set() for candidate in root.rglob(".Xil"): if not candidate.is_dir(): continue diff --git a/src/vivado_handling/vivado_utils.py b/src/vivado_handling/vivado_utils.py index db504b71..1e853d34 100644 --- a/src/vivado_handling/vivado_utils.py +++ b/src/vivado_handling/vivado_utils.py @@ -77,7 +77,7 @@ def _vivado_executable(dir_: Path) -> Optional[Path]: return exe if exe.is_file() else None -def _VIVADOct_version(dir_: Path) -> str: +def _vivado_detect_version(dir_: Path) -> str: """Infer version string from directory name (fallback to runtime query).""" for part in dir_.parts: if part[0].isdigit() and "." in part: @@ -85,6 +85,11 @@ def _VIVADOct_version(dir_: Path) -> str: return "unknown" +def _VIVADOct_version(dir_: Path) -> str: + """Backward-compatible alias for the typoed historic helper name.""" + return _vivado_detect_version(dir_) + + # ───────────────────────── Public API ────────────────────────── @@ -136,7 +141,7 @@ def find_vivado_installation( prefix="VIVADO", ) - # Fallback to automatic VIVADOction + # Fallback to automatic detection for root in _iter_candidate_dirs(): exe = _vivado_executable(root) if not exe: @@ -310,8 +315,8 @@ def get_vivado_executable() -> Optional[str]: def debug_vivado_search() -> None: - """Pretty print search logic and VIVADOction results (stdout‑only).""" - print("# Vivado VIVADOction report ({}):".format(time.strftime("%F %T"))) + """Pretty print search logic and detection results (stdout‑only).""" + print("# Vivado detection report ({}):".format(time.strftime("%F %T"))) print("Search order:") for p in get_vivado_search_paths(): print(" •", p) diff --git a/tests/test_file_manager.py b/tests/test_file_manager.py index 964130af..f6f0cd36 100644 --- a/tests/test_file_manager.py +++ b/tests/test_file_manager.py @@ -794,8 +794,9 @@ def test_bug528_coe_files_overwrite_template_with_device_ids( file_manager = FileManager(output_dir=output_dir) # Mock RepoManager to return our mock repository - with mock.patch("pcileechfwgenerator.file_management.repo_manager.RepoManager") as mock_repo_manager: - mock_repo_manager.get_board_path.return_value = mock_voltcyclone_repo["board_path"] + with mock.patch("pcileechfwgenerator.file_management.repo_manager.RepoManager") as mock_repo_manager_class: + repo_manager_instance = mock_repo_manager_class.return_value + repo_manager_instance.get_board_path.return_value = mock_voltcyclone_repo["board_path"] # Execute: Copy IP files (this should copy templates then overwrite with generated) copied_files = file_manager.copy_ip_files(board="CaptainDMA_75t")