Cleaning up

[kikkiakki]

Trying to move things out of flash.py and compartmentalize code by functionality a bit more.

Major changes:

• Move live_countdown to utility.py as it's a CLI display util
• Improve the readability of flash_chip_stage1 by pulling out the FW version compatibility check and the FlashWrite building functionality.
• Created wormhole.py to put all the tag handlers used to build the flash data. Although the wormhole.py tag handlers are not necessarily Wormhole-specific and work for Grayskull as well, Wormhole will be the only non-deprecated chip that uses them.
• Formatted all the files with black.

[Copilot]

Pull Request Overview

This PR refactors the codebase to improve organization and readability by compartmentalizing code by functionality. The main changes include moving utility functions to appropriate modules, extracting complex logic into separate functions, and creating a new module for tag handlers.

• Extracted live_countdown CLI utility function to utility.py
• Refactored flash_chip_stage1 by extracting firmware version compatibility checking logic into a separate function
• Created wormhole.py module to consolidate tag handlers and flash write building functionality
• Applied black formatting across all modified files

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 16 comments.

File	Description
tt_flash/wormhole.py	New module containing tag handlers and build_flash_writes_wh function for building flash write data structures
tt_flash/utility.py	Added live_countdown function moved from flash.py for CLI display functionality
tt_flash/flash.py	Refactored to extract firmware version checking logic, updated imports, and applied formatting
tt_flash/blackhole.py	Added build_flash_writes_bh function to encapsulate flash write building logic for Blackhole chips

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Incorrect usage of __len__() method. Use the built-in len() function instead: for _ in range(len(version_parts), 4):. The __len__() method is a dunder method that should not be called directly.

Suggested change

	for _ in range(version_parts.__len__(), 4):
	for _ in range(len(version_parts), 4):

[Copilot]

The FlashWrite dataclass is duplicated across multiple files (defined in both tt_flash/wormhole.py and tt_flash/blackhole.py). This creates maintenance issues and potential inconsistencies. Consider defining FlashWrite once in a shared module (e.g., a common types module) and importing it in both files.

Suggested change

	@dataclass
	class FlashWrite:
	offset: int
	write: bytearray
	def rmw_param(
	chip: TTChip, data: bytearray, spi_addr: int, data_addr: int, len: int
	) -> bytearray:
	# Read the existing data
	existing_data = chip.spi_read(spi_addr, len)
	from tt_flash.types import FlashWrite

[Copilot]

Inconsistent use of constants. The inline comment states "M3 takes 20 seconds to boot" which duplicates the comment for M3_DELAY_NORMAL at line 36. Instead of the magic number 20, use the constant: m3_delay = M3_DELAY_NORMAL.

Suggested change

	m3_delay = 20 # M3 takes 20 seconds to boot and be ready after a reset
	m3_delay = M3_DELAY_NORMAL # M3 takes 20 seconds to boot and be ready after a reset

[Copilot]

The return type annotation is incorrect. This function should return FlashStageResult, not tuple[bool, str]. The function body returns FlashStageResult objects (lines 247-249), which is inconsistent with the type annotation.

Suggested change

	) -> tuple[bool, str]:
	) -> FlashStageResult:

[Copilot]

The parameter type annotation is incorrect. The image parameter is typed as BufferedReader but is actually used as bytes (see line 98 where image.decode("utf-8") is called). The parameter should be typed as bytes instead.

Suggested change

	chip: BhChip, image: BufferedReader, mask: dict, boardname_to_display: str
	chip: BhChip, image: bytes, mask: dict, boardname_to_display: str

[Copilot]

Typo in function name: 'compatability' should be 'compatibility'. This should match the corrected function name at line 90.

Suggested change

	should_flash, reason_message = check_fw_version_compatability(
	should_flash, reason_message = check_fw_version_compatibility(

[Copilot]

The function modifies the global list __SEMANTIC_BUNDLE_VERSION in place by appending elements (line 104). This can lead to unexpected behavior if the function is called multiple times, as the list will continue to grow beyond 4 elements. Consider creating a copy of the list or resetting it before modification: version_parts = __SEMANTIC_BUNDLE_VERSION.copy() followed by extending it to length 4.

Suggested change

	for _ in range(__SEMANTIC_BUNDLE_VERSION.__len__(), 4):
	__SEMANTIC_BUNDLE_VERSION.append(0)
	version_parts = __SEMANTIC_BUNDLE_VERSION[:4]
	version_parts = __SEMANTIC_BUNDLE_VERSION.copy()
	for _ in range(len(version_parts), 4):
	version_parts.insert(0, 0)
	version_parts = version_parts[:4]

[Copilot]

Incorrect usage of __len__() method. Use the built-in len() function instead: for _ in range(len(__SEMANTIC_BUNDLE_VERSION), 4):. The __len__() method is a dunder method that should not be called directly.

Suggested change

	for _ in range(__SEMANTIC_BUNDLE_VERSION.__len__(), 4):
	for _ in range(len(__SEMANTIC_BUNDLE_VERSION), 4):

[Copilot]

Typo in function name: 'compatability' should be 'compatibility'. The correct spelling is 'compatibility' throughout the codebase.

Suggested change

	def check_fw_version_compatability(
	chip: TTChip, manifest: Manifest, force: bool, allow_major_downgrades: bool
	) -> tuple[bool, str]:
	"""
	This function is used in Flash Stage 1 to perform the following:
	1. Read the fw_bundle_version from the chip. If any errors are encountered, only proceed if we detect that exceptions are allowed (e.g. in the case of very old GS or WH firmware)
	2. Check major version compatability between a chip's currently running FW and the FW we want to update to. Typically disallow major downgrades and only allow upgrades across one major version boundary.
	def check_fw_version_compatibility(
	chip: TTChip, manifest: Manifest, force: bool, allow_major_downgrades: bool
	) -> tuple[bool, str]:
	"""
	This function is used in Flash Stage 1 to perform the following:
	1. Read the fw_bundle_version from the chip. If any errors are encountered, only proceed if we detect that exceptions are allowed (e.g. in the case of very old GS or WH firmware)
	2. Check major version compatibility between a chip's currently running FW and the FW we want to update to. Typically disallow major downgrades and only allow upgrades across one major version boundary.

[Copilot]

This statement is unreachable.