From ce8b36e3a8548f4619e79dfb3358a79935be414f Mon Sep 17 00:00:00 2001 From: Thomas Farstrike Date: Mon, 24 Nov 2025 18:03:02 +0100 Subject: [PATCH] OSUpdate: pause when wifi goes away, then redownload --- .../assets/osupdate.py | 72 ++++++++++++++--- .../lib/mpos/net/wifi_service.py | 3 +- tests/network_test_helper.py | 33 +++++--- tests/test_osupdate.py | 79 +++++++++++++++++++ 4 files changed, 168 insertions(+), 19 deletions(-) diff --git a/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py b/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py index ab5c518e..15f2cc52 100644 --- a/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py +++ b/internal_filesystem/builtin/apps/com.micropythonos.osupdate/assets/osupdate.py @@ -308,7 +308,9 @@ class OSUpdate(Activity): while elapsed < max_wait and self.has_foreground(): if self.connectivity_manager.is_online(): - print("OSUpdate: Network reconnected, resuming download") + print("OSUpdate: Network reconnected, waiting for stabilization...") + time.sleep(2) # Let routing table and DNS fully stabilize + print("OSUpdate: Resuming download") self.set_state(UpdateState.DOWNLOADING) break # Exit wait loop and retry download @@ -398,6 +400,33 @@ class UpdateDownloader: print("UpdateDownloader: Partition module not available, will simulate") self.simulate = True + def _is_network_error(self, exception): + """Check if exception is a network connectivity error that should trigger pause. + + Args: + exception: Exception to check + + Returns: + bool: True if this is a recoverable network error + """ + error_str = str(exception).lower() + error_repr = repr(exception).lower() + + # Check for common network error codes and messages + # -113 = ECONNABORTED (connection aborted) + # -104 = ECONNRESET (connection reset by peer) + # -110 = ETIMEDOUT (connection timed out) + # -118 = EHOSTUNREACH (no route to host) + network_indicators = [ + '-113', '-104', '-110', '-118', # Error codes + 'econnaborted', 'econnreset', 'etimedout', 'ehostunreach', # Error names + 'connection reset', 'connection aborted', # Error messages + 'broken pipe', 'network unreachable', 'host unreachable' + ] + + return any(indicator in error_str or indicator in error_repr + for indicator in network_indicators) + def download_and_install(self, url, progress_callback=None, should_continue_callback=None): """Download firmware and install to OTA partition. @@ -467,18 +496,16 @@ class UpdateDownloader: response.close() return result - # Check network connection (if monitoring enabled) + # Check network connection before reading if self.connectivity_manager: is_online = self.connectivity_manager.is_online() elif ConnectivityManager._instance: - # Use global instance if available is_online = ConnectivityManager._instance.is_online() else: - # No connectivity checking available is_online = True if not is_online: - print("UpdateDownloader: Network lost, pausing download") + print("UpdateDownloader: Network lost (pre-check), pausing download") self.is_paused = True self.bytes_written_so_far = bytes_written result['paused'] = True @@ -486,8 +513,26 @@ class UpdateDownloader: response.close() return result - # Read next chunk - chunk = response.raw.read(chunk_size) + # Read next chunk (may raise exception if network drops) + try: + chunk = response.raw.read(chunk_size) + except Exception as read_error: + # Check if this is a network error that should trigger pause + if self._is_network_error(read_error): + print(f"UpdateDownloader: Network error during read ({read_error}), pausing") + self.is_paused = True + self.bytes_written_so_far = bytes_written + result['paused'] = True + result['bytes_written'] = bytes_written + try: + response.close() + except: + pass + return result + else: + # Non-network error, re-raise + raise + if not chunk: break @@ -527,8 +572,17 @@ class UpdateDownloader: print(f"UpdateDownloader: {result['error']}") except Exception as e: - result['error'] = str(e) - print(f"UpdateDownloader: Error during download: {e}") # -113 when wifi disconnected + # Check if this is a network error that should trigger pause + if self._is_network_error(e): + print(f"UpdateDownloader: Network error ({e}), pausing download") + self.is_paused = True + self.bytes_written_so_far = result.get('bytes_written', self.bytes_written_so_far) + result['paused'] = True + result['bytes_written'] = self.bytes_written_so_far + else: + # Non-network error + result['error'] = str(e) + print(f"UpdateDownloader: Error during download: {e}") return result diff --git a/internal_filesystem/lib/mpos/net/wifi_service.py b/internal_filesystem/lib/mpos/net/wifi_service.py index bfa76188..927760e7 100644 --- a/internal_filesystem/lib/mpos/net/wifi_service.py +++ b/internal_filesystem/lib/mpos/net/wifi_service.py @@ -247,7 +247,8 @@ class WifiService: wlan.active(False) print("WifiService: Disconnected and WiFi disabled") except Exception as e: - print(f"WifiService: Error disconnecting: {e}") + #print(f"WifiService: Error disconnecting: {e}") # probably "Wifi Not Started" so harmless + pass @staticmethod def get_saved_networks(): diff --git a/tests/network_test_helper.py b/tests/network_test_helper.py index e3e60b25..c811c1fe 100644 --- a/tests/network_test_helper.py +++ b/tests/network_test_helper.py @@ -122,15 +122,17 @@ class MockRaw: Simulates the 'raw' attribute of requests.Response for chunked reading. """ - def __init__(self, content): + def __init__(self, content, fail_after_bytes=None): """ Initialize mock raw response. Args: content: Binary content to stream + fail_after_bytes: If set, raise OSError(-113) after reading this many bytes """ self.content = content self.position = 0 + self.fail_after_bytes = fail_after_bytes def read(self, size): """ @@ -141,7 +143,14 @@ class MockRaw: Returns: bytes: Chunk of data (may be smaller than size at end of stream) + + Raises: + OSError: If fail_after_bytes is set and reached """ + # Check if we should simulate network failure + if self.fail_after_bytes is not None and self.position >= self.fail_after_bytes: + raise OSError(-113, "ECONNABORTED") + chunk = self.content[self.position:self.position + size] self.position += len(chunk) return chunk @@ -154,7 +163,7 @@ class MockResponse: Simulates requests.Response object with status code, text, headers, etc. """ - def __init__(self, status_code=200, text='', headers=None, content=b''): + def __init__(self, status_code=200, text='', headers=None, content=b'', fail_after_bytes=None): """ Initialize mock response. @@ -163,6 +172,7 @@ class MockResponse: text: Response text content (default: '') headers: Response headers dict (default: {}) content: Binary response content (default: b'') + fail_after_bytes: If set, raise OSError after reading this many bytes """ self.status_code = status_code self.text = text @@ -171,7 +181,7 @@ class MockResponse: self._closed = False # Mock raw attribute for streaming - self.raw = MockRaw(content) + self.raw = MockRaw(content, fail_after_bytes=fail_after_bytes) def close(self): """Close the response.""" @@ -197,6 +207,7 @@ class MockRequests: self.last_headers = None self.last_timeout = None self.last_stream = None + self.last_request = None # Full request info dict self.next_response = None self.raise_exception = None self.call_history = [] @@ -222,14 +233,17 @@ class MockRequests: self.last_timeout = timeout self.last_stream = stream - # Record call in history - self.call_history.append({ + # Store full request info + self.last_request = { 'method': 'GET', 'url': url, 'stream': stream, 'timeout': timeout, - 'headers': headers - }) + 'headers': headers or {} + } + + # Record call in history + self.call_history.append(self.last_request.copy()) if self.raise_exception: exc = self.raise_exception @@ -287,7 +301,7 @@ class MockRequests: return MockResponse() - def set_next_response(self, status_code=200, text='', headers=None, content=b''): + def set_next_response(self, status_code=200, text='', headers=None, content=b'', fail_after_bytes=None): """ Configure the next response to return. @@ -296,11 +310,12 @@ class MockRequests: text: Response text (default: '') headers: Response headers dict (default: {}) content: Binary response content (default: b'') + fail_after_bytes: If set, raise OSError after reading this many bytes Returns: MockResponse: The configured response object """ - self.next_response = MockResponse(status_code, text, headers, content) + self.next_response = MockResponse(status_code, text, headers, content, fail_after_bytes=fail_after_bytes) return self.next_response def set_exception(self, exception): diff --git a/tests/test_osupdate.py b/tests/test_osupdate.py index a087f074..e5a888ba 100644 --- a/tests/test_osupdate.py +++ b/tests/test_osupdate.py @@ -381,4 +381,83 @@ class TestUpdateDownloader(unittest.TestCase): self.assertEqual(result['total_size'], 8192) self.assertEqual(result['bytes_written'], 8192) + def test_network_error_detection_econnaborted(self): + """Test that ECONNABORTED error is detected as network error.""" + error = OSError(-113, "ECONNABORTED") + self.assertTrue(self.downloader._is_network_error(error)) + + def test_network_error_detection_econnreset(self): + """Test that ECONNRESET error is detected as network error.""" + error = OSError(-104, "ECONNRESET") + self.assertTrue(self.downloader._is_network_error(error)) + + def test_network_error_detection_etimedout(self): + """Test that ETIMEDOUT error is detected as network error.""" + error = OSError(-110, "ETIMEDOUT") + self.assertTrue(self.downloader._is_network_error(error)) + + def test_network_error_detection_ehostunreach(self): + """Test that EHOSTUNREACH error is detected as network error.""" + error = OSError(-118, "EHOSTUNREACH") + self.assertTrue(self.downloader._is_network_error(error)) + + def test_network_error_detection_by_message(self): + """Test that network errors are detected by message.""" + self.assertTrue(self.downloader._is_network_error(Exception("Connection reset by peer"))) + self.assertTrue(self.downloader._is_network_error(Exception("Connection aborted"))) + self.assertTrue(self.downloader._is_network_error(Exception("Broken pipe"))) + + def test_non_network_error_not_detected(self): + """Test that non-network errors are not detected as network errors.""" + self.assertFalse(self.downloader._is_network_error(ValueError("Invalid data"))) + self.assertFalse(self.downloader._is_network_error(Exception("File not found"))) + self.assertFalse(self.downloader._is_network_error(KeyError("missing"))) + + def test_download_pauses_on_network_error_during_read(self): + """Test that download pauses when network error occurs during read.""" + # Set up mock to raise network error after first chunk + test_data = b'G' * 16384 # 4 chunks + self.mock_requests.set_next_response( + status_code=200, + headers={'Content-Length': '16384'}, + content=test_data, + fail_after_bytes=4096 # Fail after first chunk + ) + + result = self.downloader.download_and_install( + "http://example.com/update.bin" + ) + + self.assertFalse(result['success']) + self.assertTrue(result['paused']) + self.assertEqual(result['bytes_written'], 4096) # Should have written first chunk + self.assertIsNone(result['error']) # Pause, not error + + def test_download_resumes_from_saved_position(self): + """Test that download resumes from the last written position.""" + # Simulate partial download + test_data = b'H' * 12288 # 3 chunks + self.downloader.bytes_written_so_far = 8192 # Already downloaded 2 chunks + self.downloader.total_size_expected = 12288 + + # Server should receive Range header + remaining_data = b'H' * 4096 # Last chunk + self.mock_requests.set_next_response( + status_code=206, # Partial content + headers={'Content-Length': '4096'}, # Remaining bytes + content=remaining_data + ) + + result = self.downloader.download_and_install( + "http://example.com/update.bin" + ) + + self.assertTrue(result['success']) + self.assertEqual(result['bytes_written'], 12288) + # Check that Range header was set + last_request = self.mock_requests.last_request + self.assertIsNotNone(last_request) + self.assertIn('Range', last_request['headers']) + self.assertEqual(last_request['headers']['Range'], 'bytes=8192-') +