Merge pull request #41 from Josverl/copilot/fix-32

Implement mpremote rm -r :/ fallback for UF2 erase on non-rp2 ports (execute after firmware flash)

Author: Josverl

mpflash/flash/uf2/__init__.py

@@ -4,18 +4,17 @@
 
 import shutil
 import sys
-from pathlib import Path
 import time
+from pathlib import Path
 from typing import Optional
 
 import tenacity
 from loguru import logger as log
-
 from tenacity import stop_after_attempt, wait_fixed
 
+from mpflash.common import PORT_FWTYPES
 from mpflash.mpremoteboard import MPRemoteBoard
 
-from mpflash.common import PORT_FWTYPES
 from .boardid import get_board_id
 from .linux import dismount_uf2_linux, wait_for_UF2_linux
 from .macos import wait_for_UF2_macos
@@ -38,6 +37,10 @@ def flash_uf2(mcu: MPRemoteBoard, fw_file: Path, erase: bool) -> Optional[MPRemo
     if ".uf2" not in PORT_FWTYPES[mcu.port]:
         log.error(f"UF2 not supported on {mcu.board} on {mcu.serialport}")
         return None
+    
+    # For non-rp2 ports, remember if we need to erase filesystem after flashing
+    erase_filesystem_after_flash = erase and mcu.port != "rp2"
+    
     if erase:
         if mcu.port == "rp2":
             rp2_erase =Path(__file__).parent.joinpath("../../vendor/pico-universal-flash-nuke/universal_flash_nuke.uf2").resolve()
@@ -52,8 +55,6 @@ def flash_uf2(mcu: MPRemoteBoard, fw_file: Path, erase: bool) -> Optional[MPRemo
                 dismount_uf2_linux()
             # allow for MCU restart after erase
             time.sleep(0.5)
-        else:
-            log.warning(f"Erase not (yet) supported on .UF2, for port {mcu.port}, skipping erase.")
 
     destination = waitfor_uf2(board_id=mcu.port.upper())
 
@@ -75,6 +76,16 @@ def flash_uf2(mcu: MPRemoteBoard, fw_file: Path, erase: bool) -> Optional[MPRemo
         dismount_uf2_linux()
 
     mcu.wait_for_restart()
+    
+    # For non-rp2 UF2 ports (like SAMD), erase filesystem after flash and restart
+    if erase_filesystem_after_flash:
+        # allow for MCU restart after erase
+        time.sleep(0.5)        
+        log.info(f"Erasing {mcu.port} filesystem using mpremote rm -r :/")
+        try:
+            rc, result = mcu.run_command(["rm", "-r", ":/"], timeout=30, resume=True)
+        except Exception as e:
+            log.warning(f"Failed to erase filesystem on {mcu.port}: {e}")
     return mcu
 
 

mpflash/mpremoteboard/runner.py

@@ -6,6 +6,7 @@
 from dataclasses import dataclass
 from threading import Timer
 from typing import List, Optional, Tuple
+from unittest.mock import DEFAULT
 
 from loguru import logger as log
 
@@ -31,6 +32,13 @@ class LogTags:
     "rst:0x10 (RTCWDT_RTC_RESET)",
 ]
 
+DEFAULT_ERROR_TAGS = ["Traceback ", "Error: ", "Exception: ", "ERROR :", "CRIT  :"]
+DEFAULT_WARNING_TAGS = ["WARN  :", "TRACE :"]
+DEFAULT_SUCCESS_TAGS = ["Done", "File saved", "File removed", "File renamed"]
+DEFAULT_IGNORE_TAGS = [
+    '  File "<stdin>",',
+    "mpremote: rm -r: cannot remove :/ Operation not permitted",
+]
 
 def run(
     cmd: List[str],
@@ -70,13 +78,13 @@ def run(
     if not reset_tags:
         reset_tags = DEFAULT_RESET_TAGS
     if not error_tags:
-        error_tags = ["Traceback ", "Error: ", "Exception: ", "ERROR :", "CRIT  :"]
+        error_tags = DEFAULT_ERROR_TAGS
     if not warning_tags:
-        warning_tags = ["WARN  :", "TRACE :"]
+        warning_tags = DEFAULT_WARNING_TAGS
     if not success_tags:
-        success_tags = []
+        success_tags = DEFAULT_SUCCESS_TAGS
     if not ignore_tags:
-        ignore_tags = ['  File "<stdin>",']
+        ignore_tags = DEFAULT_IGNORE_TAGS
 
     replace_tags = ["\x1b[1A"]
 
@@ -132,6 +140,8 @@ def timed_out():
                         log.info(line)
         if proc.stderr and log_errors:
             for line in proc.stderr:
+                if any(tag in line for tag in ignore_tags):
+                    continue
                 log.warning(line)
     except UnicodeDecodeError as e:
         log.error(f"Failed to decode output: {e}")

pyproject.toml

@@ -256,3 +256,8 @@ line-ending = "auto"
 [tool.ruff.lint]
 exclude = [".*", "__*", "dist", "repos", "mpflash\\vendor"]
 ignore = ["F821"]
+
+[dependency-groups]
+dev = [
+    "pytest>=8.4.1",
+]

tests/flash/test_flash_uf2_A.py

@@ -1,8 +1,10 @@
-import pytest
-from unittest import mock
 from pathlib import Path
-from mpflash.mpremoteboard import MPRemoteBoard
+from unittest import mock
+
+import pytest
+
 from mpflash.flash.uf2 import flash_uf2
+from mpflash.mpremoteboard import MPRemoteBoard
 
 
 @pytest.fixture
@@ -11,6 +13,8 @@ def mock_mcu():
     mcu.port = "rp2"
     mcu.board = "test_board"
     mcu.serialport = "COM3"
+    mcu.run_command = mock.Mock()  # Add run_command method
+    mcu.wait_for_restart = mock.Mock()  # Add wait_for_restart method
     return mcu
 
 
@@ -28,7 +32,10 @@ def mock_erase_file():
 def mock_destination():
     destination = mock.Mock(spec=Path)
     destination.exists.return_value = True
-    # (destination / "INFO_UF2.TXT").exists.return_value = True
+    # Mock the path operation (destination / "INFO_UF2.TXT").exists()
+    info_file = mock.Mock()
+    info_file.exists.return_value = True
+    destination.__truediv__ = mock.Mock(return_value=info_file)
     return destination
 
 
@@ -64,9 +71,64 @@ def test_flash_uf2_board_not_in_bootloader(mock_mcu, mock_fw_file):
 #         assert result == mock_mcu
 
 
+def test_flash_uf2_erase_fallback_samd(mock_mcu, mock_fw_file, mock_destination):
+    """Test that SAMD port uses mpremote rm -r :/ for erase after flashing"""
+    mock_mcu.port = "samd"
+    mock_mcu.run_command.return_value = (0, [""])  # Successful erase
+
+    with (
+        mock.patch("mpflash.flash.uf2.waitfor_uf2", return_value=mock_destination),
+        mock.patch("mpflash.flash.uf2.copy_firmware_to_uf2"),
+        mock.patch("mpflash.flash.uf2.dismount_uf2_linux"),
+        mock.patch("mpflash.flash.uf2.get_board_id", return_value="test_board_id"),
+    ):
+        result = flash_uf2(mock_mcu, mock_fw_file, erase=True)
+
+        # Verify that run_command was called with rm -r :/ after flashing
+        mock_mcu.run_command.assert_called_with(["rm", "-r", ":/"], timeout=30, resume=True)
+        assert result == mock_mcu
+
+
+def test_flash_uf2_erase_fallback_failed(mock_mcu, mock_fw_file, mock_destination):
+    """Test that failed mpremote erase is logged but doesn't stop flashing"""
+    mock_mcu.port = "samd"
+    mock_mcu.run_command.return_value = (1, ["Error message"])  # Failed erase
+
+    with (
+        mock.patch("mpflash.flash.uf2.waitfor_uf2", return_value=mock_destination),
+        mock.patch("mpflash.flash.uf2.copy_firmware_to_uf2"),
+        mock.patch("mpflash.flash.uf2.dismount_uf2_linux"),
+        mock.patch("mpflash.flash.uf2.get_board_id", return_value="test_board_id"),
+    ):
+        result = flash_uf2(mock_mcu, mock_fw_file, erase=True)
+
+        # Verify that run_command was called with rm -r :/ after flashing
+        mock_mcu.run_command.assert_called_with(["rm", "-r", ":/"], timeout=30, resume=True)
+        # Should still complete flashing even if erase failed
+        assert result == mock_mcu
+
+
 def test_flash_uf2_erase_not_supported(mock_mcu, mock_fw_file):
     mock_mcu.port = "unsupported_erase_port"
     with mock.patch("mpflash.flash.uf2.waitfor_uf2", return_value=None):
         with pytest.raises(KeyError):
             result = flash_uf2(mock_mcu, mock_fw_file, erase=True)
             assert result is None
+
+
+def test_flash_uf2_no_erase_command_when_erase_false(mock_mcu, mock_fw_file, mock_destination):
+    """Test that mpremote rm command is not called when erase=False"""
+    mock_mcu.port = "samd"
+    mock_mcu.run_command = mock.Mock()
+
+    with (
+        mock.patch("mpflash.flash.uf2.waitfor_uf2", return_value=mock_destination),
+        mock.patch("mpflash.flash.uf2.copy_firmware_to_uf2"),
+        mock.patch("mpflash.flash.uf2.dismount_uf2_linux"),
+        mock.patch("mpflash.flash.uf2.get_board_id", return_value="test_board_id"),
+    ):
+        result = flash_uf2(mock_mcu, mock_fw_file, erase=False)
+
+        # Verify that run_command was NOT called
+        mock_mcu.run_command.assert_not_called()
+        assert result == mock_mcu