From 3fb56f8b9b3add0a4e526387d1b332971b0495cb Mon Sep 17 00:00:00 2001 From: James Aguilar Date: Tue, 23 Dec 2025 23:27:13 +0000 Subject: [PATCH 1/4] firmware: Don't add checksums when not needed. This fixes an issue where the EV3 firmware had checksums appended to it that made its size not align to the sector size, causing flashing to fail. --- src/firmware/sagas.ts | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index a25e5b26..7ac4fea7 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -351,32 +351,20 @@ function* loadFirmware( 'Expected metadata to be v2.x', ); - const firmware = new Uint8Array(firmwareBase.length + 4); - const firmwareView = new DataView(firmware.buffer); - - firmware.set(firmwareBase); - - // empty string means use default name (don't write over firmware) - if (hubName) { - firmware.set(encodeHubName(hubName, metadata), metadata['hub-name-offset']); - } - - const checksum = (function () { + const [checksumFunc, checksumExtraLength] = (() => { switch (metadata['checksum-type']) { case 'sum': - return sumComplement32( - firmwareIterator(firmwareView, metadata['checksum-size']), - ); + return [sumComplement32, 4]; case 'crc32': - return crc32(firmwareIterator(firmwareView, metadata['checksum-size'])); + return [crc32, 4]; case 'none': - return null; + return [null, 0]; default: - return undefined; + return [undefined, 0]; } })(); - if (checksum === undefined) { + if (checksumFunc === undefined) { // FIXME: we should return error/throw instead yield* put( didFailToFinish( @@ -391,8 +379,22 @@ function* loadFirmware( throw new Error('unreachable'); } - if (checksum !== null) { - firmwareView.setUint32(firmwareBase.length, checksum, true); + const firmware = new Uint8Array(firmwareBase.length + checksumExtraLength); + const firmwareView = new DataView(firmware.buffer); + + firmware.set(firmwareBase); + + // empty string means use default name (don't write over firmware) + if (hubName) { + firmware.set(encodeHubName(hubName, metadata), metadata['hub-name-offset']); + } + + if (checksumFunc !== null) { + firmwareView.setUint32( + firmwareBase.length, + checksumFunc(firmwareIterator(firmwareView, metadata['checksum-size'])), + true, + ); } return { firmware, deviceId: metadata['device-id'] }; From 44237e010d0f191332d38f188cb18c61b97aa1ed Mon Sep 17 00:00:00 2001 From: James Aguilar Date: Tue, 23 Dec 2025 23:28:32 +0000 Subject: [PATCH 2/4] firmware: Ignore EV3 error on GetVersion. We don't use this datum for anything, and the rest of the flashing process apparently proceeds fine even if this fails. This could use further investigation, though. --- src/firmware/sagas.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 7ac4fea7..1efe4063 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -1221,12 +1221,19 @@ function* handleFlashEV3(action: ReturnType): Generator defined(version); - console.debug( - `EV3 bootloader version: ${version.getUint32( - 0, - true, - )}, HW version: ${version.getUint32(4, true)}`, - ); + // For reasons that we do not currently understand, some EV3s do not return + // anything to our GetVersion command. We don't actually use the version + // for anything so we will just ignore this error. + try { + console.debug( + `EV3 bootloader version: ${version.getUint32( + 0, + true, + )}, HW version: ${version.getUint32(4, true)}`, + ); + } catch (err) { + console.error(`Failed to parse ev3 version response: ${ensureError(err)}`); + } // FIXME: should be called much earlier. yield* put(didStart()); From c36c1aeff2c7d52da2e95975eff9199e6cfd817f Mon Sep 17 00:00:00 2001 From: James Aguilar Date: Wed, 24 Dec 2025 17:31:18 +0000 Subject: [PATCH 3/4] firmware: Flash the whole EV3 at once. Research in https://github.com/pybricks/support/issues/2375#issuecomment-3677356872 and below shows that erasing the firmware and writing it sector by sector causes hangs during the flashing process. Instead, we should erase the whole firmware at once, and flash it at once. This successfully loads new EV3 firmware without hangs. --- src/firmware/sagas.ts | 84 +++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 35 deletions(-) diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 1efe4063..09ccd4c6 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -1238,55 +1238,69 @@ function* handleFlashEV3(action: ReturnType): Generator // FIXME: should be called much earlier. yield* put(didStart()); - const sectorSize = 64 * 1024; // flash memory sector size const maxPayloadSize = 1018; // maximum payload size for EV3 commands - for (let i = 0; i < action.firmware.byteLength; i += sectorSize) { - const sectorData = action.firmware.slice(i, i + sectorSize); - assert(sectorData.byteLength <= sectorSize, 'sector data too large'); + const erasePayload = new DataView(new ArrayBuffer(8)); + erasePayload.setUint32(0, 0, true); // start address + erasePayload.setUint32(4, action.firmware.byteLength, true); // size + console.debug(`Erasing bytes [0x0, ${hex(action.firmware.byteLength, 0)})`); - const erasePayload = new DataView(new ArrayBuffer(8)); - erasePayload.setUint32(0, i, true); - erasePayload.setUint32(4, sectorData.byteLength, true); - const [, eraseError] = yield* sendCommand( - 0xf0, - new Uint8Array(erasePayload.buffer), + yield* put( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'erase', + progress: undefined, + }, + firmwareBleProgressToastId, + true, + ), + ); + + const [, eraseError] = yield* sendCommand( + 0xf0, + new Uint8Array(erasePayload.buffer), + ); + + if (eraseError) { + yield* put( + alertsShowAlert('alerts', 'unexpectedError', { + error: eraseError, + }), + ); + // FIXME: should have a better error reason + yield* put(didFailToFinish(FailToFinishReasonType.Unknown, eraseError)); + yield* put(firmwareDidFailToFlashEV3()); + yield* cleanup(); + return; + } + + // If we don't write an exact multiple of the sector size, the flash process + // will hang on the last write we send. + const firmware = action.firmware; + for (let i = 0; i < firmware.byteLength; i += maxPayloadSize) { + const payload = firmware.slice(i, i + maxPayloadSize); + console.debug( + `Programming bytes [${hex(i, 0)}, ${hex(i + maxPayloadSize, 0)})`, ); - if (eraseError) { + const [, sendError] = yield* sendCommand(0xf2, new Uint8Array(payload)); + if (sendError) { yield* put( alertsShowAlert('alerts', 'unexpectedError', { - error: eraseError, + error: sendError, }), ); // FIXME: should have a better error reason - yield* put(didFailToFinish(FailToFinishReasonType.Unknown, eraseError)); + yield* put(didFailToFinish(FailToFinishReasonType.Unknown, sendError)); yield* put(firmwareDidFailToFlashEV3()); yield* cleanup(); return; } - for (let j = 0; j < sectorData.byteLength; j += maxPayloadSize) { - const payload = sectorData.slice(j, j + maxPayloadSize); - - const [, sendError] = yield* sendCommand(0xf2, new Uint8Array(payload)); - if (sendError) { - yield* put( - alertsShowAlert('alerts', 'unexpectedError', { - error: sendError, - }), - ); - // FIXME: should have a better error reason - yield* put(didFailToFinish(FailToFinishReasonType.Unknown, sendError)); - yield* put(firmwareDidFailToFlashEV3()); - yield* cleanup(); - return; - } - } - - yield* put( - didProgress((i + sectorData.byteLength) / action.firmware.byteLength), - ); + const progress = (i + payload.byteLength) / firmware.byteLength; + yield* put(didProgress(progress)); yield* put( alertsShowAlert( @@ -1294,7 +1308,7 @@ function* handleFlashEV3(action: ReturnType): Generator 'flashProgress', { action: 'flash', - progress: (i + sectorData.byteLength) / action.firmware.byteLength, + progress: progress, }, firmwareBleProgressToastId, true, From 1528acf455f47e7d6cfa4739d0bc29b3e8a3e912 Mon Sep 17 00:00:00 2001 From: James Aguilar Date: Wed, 24 Dec 2025 17:34:02 +0000 Subject: [PATCH 4/4] firmware: Firmware must be sector size multiple. If the EV3 firmware size is not a multiple of the sector size, the flashing process will hang. Raise an error if this happens. --- src/firmware/actions.ts | 6 ++++++ src/firmware/sagas.ts | 16 ++++++++++++++++ src/notifications/i18n.ts | 3 ++- src/notifications/sagas.test.ts | 3 ++- src/notifications/sagas.ts | 8 +++++++- src/notifications/translations/en.json | 3 ++- 6 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/firmware/actions.ts b/src/firmware/actions.ts index af64e80f..855c9f34 100644 --- a/src/firmware/actions.ts +++ b/src/firmware/actions.ts @@ -47,6 +47,8 @@ export enum FailToFinishReasonType { FailedToCompile = 'flashFirmware.failToFinish.reason.failedToCompile', /** The combined firmware-base.bin and main.mpy are too big. */ FirmwareSize = 'flashFirmware.failToFinish.reason.firmwareSize', + /** The firmware has the wrong format (e.g. is not a multiple of sector size). */ + FirmwareSizeNotMultipleOfSector = 'flashFirmware.failToFinish.reason.firmwareSizeNotMultipleOfSector', /** An unexpected error occurred. */ Unknown = 'flashFirmware.failToFinish.reason.unknown', } @@ -94,6 +96,9 @@ export type FailToFinishReasonBadMetadata = export type FailToFinishReasonFirmwareSize = Reason; +export type FailToFinishReasonFirmwareFormat = + Reason; + export type FailToFinishReasonFailedToCompile = Reason; @@ -114,6 +119,7 @@ export type FailToFinishReason = | FailToFinishReasonBadMetadata | FailToFinishReasonFirmwareSize | FailToFinishReasonFailedToCompile + | FailToFinishReasonFirmwareFormat | FailToFinishReasonUnknown; // High-level bootloader actions. diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 09ccd4c6..1a3dc769 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -1238,6 +1238,22 @@ function* handleFlashEV3(action: ReturnType): Generator // FIXME: should be called much earlier. yield* put(didStart()); + console.debug(`Firmware size: ${action.firmware.byteLength} bytes`); + + // Apparently, erasing a span of the flash creates some sort of record in + // the EV3, and we can only write within a given erase span. Writes that + // cross the boundary will hang. To avoid this, we erase the whole firmware + // range at once. + const sectorSize = 64 * 1024; // flash memory sector size + if (action.firmware.byteLength % sectorSize !== 0) { + yield* put( + didFailToFinish(FailToFinishReasonType.FirmwareSizeNotMultipleOfSector), + ); + yield* put(firmwareDidFailToFlashEV3()); + yield* cleanup(); + return; + } + const maxPayloadSize = 1018; // maximum payload size for EV3 commands const erasePayload = new DataView(new ArrayBuffer(8)); diff --git a/src/notifications/i18n.ts b/src/notifications/i18n.ts index eceeb4b2..91c0250c 100644 --- a/src/notifications/i18n.ts +++ b/src/notifications/i18n.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020-2022 The Pybricks Authors +// Copyright (c) 2020-2025 The Pybricks Authors // // Notification translation keys. @@ -35,6 +35,7 @@ export enum I18nId { FlashFirmwareBadMetadata = 'flashFirmware.badMetadata', FlashFirmwareCompileError = 'flashFirmware.compileError', FlashFirmwareSizeTooBig = 'flashFirmware.sizeTooBig', + FlashFirmwareSizeNotMultipleOfSector = 'flashFirmware.sizeNotMultipleOfSector', FlashFirmwareUnexpectedError = 'flashFirmware.unexpectedError', ServiceWorkerUpdateMessage = 'serviceWorker.update.message', ServiceWorkerUpdateAction = 'serviceWorker.update.action', diff --git a/src/notifications/sagas.test.ts b/src/notifications/sagas.test.ts index 4a9ebecc..1406f158 100644 --- a/src/notifications/sagas.test.ts +++ b/src/notifications/sagas.test.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2021-2023 The Pybricks Authors +// Copyright (c) 2021-2025 The Pybricks Authors import type { ToastOptions, Toaster } from '@blueprintjs/core'; import { FirmwareReaderError, FirmwareReaderErrorCode } from '@pybricks/firmware'; @@ -95,6 +95,7 @@ test.each([ ), didFailToFinish(FailToFinishReasonType.FailedToCompile), didFailToFinish(FailToFinishReasonType.FirmwareSize), + didFailToFinish(FailToFinishReasonType.FirmwareSizeNotMultipleOfSector), didFailToFinish(FailToFinishReasonType.Unknown, new Error('test error')), appDidCheckForUpdate(false), fileStorageDidFailToInitialize(new Error('test error')), diff --git a/src/notifications/sagas.ts b/src/notifications/sagas.ts index dca25276..1bef79ca 100644 --- a/src/notifications/sagas.ts +++ b/src/notifications/sagas.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020-2023 The Pybricks Authors +// Copyright (c) 2020-2025 The Pybricks Authors // Saga for managing notifications (toasts) @@ -225,6 +225,12 @@ function* showFlashFirmwareError( case FailToFinishReasonType.FailedToCompile: yield* showSingleton(Level.Error, I18nId.FlashFirmwareCompileError); break; + case FailToFinishReasonType.FirmwareSizeNotMultipleOfSector: + yield* showSingleton( + Level.Error, + I18nId.FlashFirmwareSizeNotMultipleOfSector, + ); + break; case FailToFinishReasonType.FirmwareSize: yield* showSingleton(Level.Error, I18nId.FlashFirmwareSizeTooBig); break; diff --git a/src/notifications/translations/en.json b/src/notifications/translations/en.json index ad6f2e47..3862e513 100644 --- a/src/notifications/translations/en.json +++ b/src/notifications/translations/en.json @@ -33,7 +33,8 @@ "badMetadata": "The firmware.metadata.json file contains missing or invalid entries. Fix it then try again.", "compileError": "The included main.py file could not be compiled. Fix it then try again.", "sizeTooBig": "The combined firmware and main.py are too big to fit in the flash memory.", - "unexpectedError": "Unexpected error while trying to install firmware: {errorMessage}" + "unexpectedError": "Unexpected error while trying to install firmware: {errorMessage}", + "sizeNotMultipleOfSector": "The firmware size is not a multiple of the flash memory sector size." }, "mpy": { "error": "{errorMessage}"