From 325c61e4f6955b275b3c952a02c1844cd4eab7d9 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 4 Jan 2026 23:01:53 +0000 Subject: [PATCH 1/4] firmware/actions: reuse isError from utils Remove duplicate isError function. --- src/firmware/actions.ts | 11 +---------- src/utils/index.ts | 4 ++-- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/firmware/actions.ts b/src/firmware/actions.ts index 8a4bc484e..5dff5c99a 100644 --- a/src/firmware/actions.ts +++ b/src/firmware/actions.ts @@ -4,6 +4,7 @@ import { FirmwareReaderError, HubType } from '@pybricks/firmware'; import { createAction } from '../actions'; import { Hub } from '../components/hubPicker'; +import { isError } from '../utils'; export enum MetadataProblem { Missing = 'metadata.missing', @@ -144,16 +145,6 @@ export const didFinish = createAction(() => ({ type: 'flashFirmware.action.didFinish', })); -function isError(err: unknown): err is Error { - const maybeError = err as Error; - - return ( - maybeError !== undefined && - typeof maybeError.name === 'string' && - typeof maybeError.message === 'string' - ); -} - // FIXME: get rid of this monstrosity const didFailToFinishType = 'flashFirmware.action.didFailToFinish'; diff --git a/src/utils/index.ts b/src/utils/index.ts index 1b154e1ae..309a34cf3 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -// Copyright (c) 2020-2022 The Pybricks Authors +// Copyright (c) 2020-2026 The Pybricks Authors /** * Asserts that an assumption is true. This is used to detect programmer errors @@ -43,7 +43,7 @@ export function hex(n: number, pad: number): string { return `0x${n.toString(16).padStart(pad, '0')}`; } -function isError(err: unknown): err is Error { +export function isError(err: unknown): err is Error { const maybeError = err as Error; return ( From a30b38232e14c773a6431c4b25cfdbdd52e2d50d Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 4 Jan 2026 23:12:54 +0000 Subject: [PATCH 2/4] firmware/sagas: fix duplicate error alerts for ev3 flashing Remove alertsShowAlert() calls. didFailToFinish() already triggers an error toast. --- src/firmware/sagas.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index f6f938f33..69a624771 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -1230,11 +1230,6 @@ function* handleFlashEV3(action: ReturnType): Generator ); if (eraseError) { - yield* put( - alertsShowAlert('alerts', 'unexpectedError', { - error: eraseError, - }), - ); yield* put(alertsHideAlert(firmwareEv3ProgressToastId)); // FIXME: should have a better error reason yield* put(didFailToFinish(FailToFinishReasonType.Unknown, eraseError)); @@ -1248,11 +1243,6 @@ function* handleFlashEV3(action: ReturnType): Generator const [, sendError] = yield* sendCommand(0xf2, new Uint8Array(payload)); if (sendError) { - yield* put( - alertsShowAlert('alerts', 'unexpectedError', { - error: sendError, - }), - ); yield* put(alertsHideAlert(firmwareEv3ProgressToastId)); // FIXME: should have a better error reason yield* put(didFailToFinish(FailToFinishReasonType.Unknown, sendError)); From 423c910eb119b4f5974c9fa9b21b43cd8bd2ec8c Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 4 Jan 2026 23:16:59 +0000 Subject: [PATCH 3/4] firmware/reducers: remove 'flashing' state The 'flashing' state variable in the firmware reducer is not used anymore so can be removed. --- src/firmware/reducers.test.ts | 21 +-------------------- src/firmware/reducers.ts | 16 ---------------- 2 files changed, 1 insertion(+), 36 deletions(-) diff --git a/src/firmware/reducers.test.ts b/src/firmware/reducers.test.ts index 1bbd527ad..3a37cb2d2 100644 --- a/src/firmware/reducers.test.ts +++ b/src/firmware/reducers.test.ts @@ -2,15 +2,8 @@ // Copyright (c) 2021-2026 The Pybricks Authors import { AnyAction } from 'redux'; -import { - FailToFinishReasonType, - didFailToFinish, - didFinish, - didStart, -} from './actions'; -import reducers from './reducers'; -type State = ReturnType; +import reducers from './reducers'; test('initial state', () => { expect(reducers(undefined, {} as AnyAction)).toMatchInlineSnapshot(` @@ -18,7 +11,6 @@ test('initial state', () => { "dfuWindowsDriverInstallDialog": { "isOpen": false, }, - "flashing": false, "installPybricksDialog": { "isOpen": false, }, @@ -31,14 +23,3 @@ test('initial state', () => { } `); }); - -test('flashing', () => { - expect(reducers({ flashing: false } as State, didStart()).flashing).toBe(true); - expect(reducers({ flashing: true } as State, didFinish()).flashing).toBe(false); - expect( - reducers( - { flashing: true } as State, - didFailToFinish(FailToFinishReasonType.TimedOut), - ).flashing, - ).toBe(false); -}); diff --git a/src/firmware/reducers.ts b/src/firmware/reducers.ts index acd3e6227..e6306219f 100644 --- a/src/firmware/reducers.ts +++ b/src/firmware/reducers.ts @@ -3,9 +3,6 @@ import { Reducer, combineReducers } from 'redux'; import { - didFailToFinish, - didFinish, - didStart, firmwareDidFailToFlashUsbDfu, firmwareDidFailToRestoreOfficialDfu, firmwareDidFailToRestoreOfficialEV3, @@ -20,18 +17,6 @@ import dfuWindowsDriverInstallDialog from './dfuWindowsDriverInstallDialog/reduc import installPybricksDialog from './installPybricksDialog/reducers'; import restoreOfficialDialog from './restoreOfficialDialog/reducers'; -const flashing: Reducer = (state = false, action) => { - if (didStart.matches(action)) { - return true; - } - - if (didFinish.matches(action) || didFailToFinish.matches(action)) { - return false; - } - - return state; -}; - const isFirmwareFlashUsbDfuInProgress: Reducer = (state = false, action) => { if (firmwareFlashUsbDfu.matches(action)) { return true; @@ -86,7 +71,6 @@ export default combineReducers({ dfuWindowsDriverInstallDialog, installPybricksDialog, restoreOfficialDialog, - flashing, isFirmwareFlashUsbDfuInProgress, isFirmwareRestoreOfficialDfuInProgress, isFirmwareFlashEV3InProgress, From a63b3e0917169046cb7c5fb2711f99cefef4630b Mon Sep 17 00:00:00 2001 From: David Lechner Date: Sun, 4 Jan 2026 23:21:14 +0000 Subject: [PATCH 4/4] firmware/actions: remove didStart and didFinish actions Remove didStart and didFinish actions from firmware flashing sagas as they are not used for anything. didFailToFinish is not so easily removed as it is used to trigger error toasts. --- src/firmware/actions.ts | 13 -------- src/firmware/sagas.test.ts | 62 -------------------------------------- src/firmware/sagas.ts | 11 ------- 3 files changed, 86 deletions(-) diff --git a/src/firmware/actions.ts b/src/firmware/actions.ts index 5dff5c99a..77d1929d0 100644 --- a/src/firmware/actions.ts +++ b/src/firmware/actions.ts @@ -132,19 +132,6 @@ export const flashFirmware = createAction( }), ); -/** - * Action that indicates flashing firmware started. - * @param total The total number of bytes to be flashed. - */ -export const didStart = createAction(() => ({ - type: 'flashFirmware.action.didStart', -})); - -/** Action that indicates that flashing firmware completed successfully. */ -export const didFinish = createAction(() => ({ - type: 'flashFirmware.action.didFinish', -})); - // FIXME: get rid of this monstrosity const didFailToFinishType = 'flashFirmware.action.didFailToFinish'; diff --git a/src/firmware/sagas.test.ts b/src/firmware/sagas.test.ts index 97d446a40..5af6da28f 100644 --- a/src/firmware/sagas.test.ts +++ b/src/firmware/sagas.test.ts @@ -42,8 +42,6 @@ import { HubError, MetadataProblem, didFailToFinish, - didFinish, - didStart, flashFirmware as flashFirmwareAction, } from './actions'; import flashFirmware from './sagas'; @@ -124,12 +122,6 @@ describe('flashFirmware', () => { const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -232,9 +224,6 @@ describe('flashFirmware', () => { // then we are done - action = await saga.take(); - expect(action).toEqual(didFinish()); - await saga.end(); }); @@ -284,12 +273,6 @@ describe('flashFirmware', () => { saga.put(didRequest(0)); saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -392,9 +375,6 @@ describe('flashFirmware', () => { // then we are done - action = await saga.take(); - expect(action).toEqual(didFinish()); - await saga.end(); }); @@ -1008,12 +988,6 @@ describe('flashFirmware', () => { const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -1121,12 +1095,6 @@ describe('flashFirmware', () => { const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -1243,12 +1211,6 @@ describe('flashFirmware', () => { const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -1416,12 +1378,6 @@ describe('flashFirmware', () => { const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -1594,12 +1550,6 @@ describe('flashFirmware', () => { saga.put(didRequest(0)); saga.put(infoResponse(0x01000000, 0x08005000, 0x081f800, HubType.MoveHub)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -1703,9 +1653,6 @@ describe('flashFirmware', () => { // then we are done - action = await saga.take(); - expect(action).toEqual(didFinish()); - await saga.end(); }); @@ -2156,12 +2103,6 @@ describe('flashFirmware', () => { const mpyBinaryData = new Uint8Array(mpySize); saga.put(didCompile(mpyBinaryData)); - // then start flashing the firmware - - // should get didStart action just before starting to erase - action = await saga.take(); - expect(action).toEqual(didStart()); - // erase first action = await saga.take(); @@ -2262,9 +2203,6 @@ describe('flashFirmware', () => { // then we are done - action = await saga.take(); - expect(action).toEqual(didFinish()); - await saga.end(); }); }); diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 69a624771..946b14478 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -74,8 +74,6 @@ import { HubError, MetadataProblem, didFailToFinish, - didFinish, - didStart, firmwareDidFailToFlashEV3, firmwareDidFailToFlashUsbDfu, firmwareDidFailToRestoreOfficialDfu, @@ -465,8 +463,6 @@ function* handleFlashFirmware(action: ReturnType): Generat } } - yield* put(didStart()); - yield* put( alertsShowAlert( 'firmware', @@ -630,8 +626,6 @@ function* handleFlashFirmware(action: ReturnType): Generat // this will cause the remote device to disconnect and reboot const rebootAction = yield* put(rebootRequest(nextMessageId())); yield* waitForDidRequest(rebootAction.id); - - yield* put(didFinish()); } catch (err) { yield* put(didFailToFinish(FailToFinishReasonType.Unknown, ensureError(err))); yield* disconnectAndCancel(); @@ -1211,9 +1205,6 @@ function* handleFlashEV3(action: ReturnType): Generator return [new DataView(reply.payload), undefined]; } - // 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 @@ -1288,8 +1279,6 @@ function* handleFlashEV3(action: ReturnType): Generator return; } - yield* put(didFinish()); - yield* cleanup(); yield* put(firmwareDidFlashEV3());