From 28cf87c3565a2c0ff1be4b6ee8fed4403c3626f2 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Wed, 19 Oct 2022 17:33:04 -0500 Subject: [PATCH] alerts: allow overriding key This adds an optional key parameter to alertsShowAlert() to allow overriding the key. This allows us to call flashProgress using that action instead of having to use the toaster directly. --- src/alerts/actions.ts | 10 +- src/alerts/sagas.ts | 4 +- src/firmware/alerts/index.ts | 2 + src/firmware/sagas.test.ts | 210 +++++++++++++++++++++++++++++++++++ src/firmware/sagas.ts | 136 +++++++++++++++-------- 5 files changed, 312 insertions(+), 50 deletions(-) diff --git a/src/alerts/actions.ts b/src/alerts/actions.ts index ce483d1b..81e837b9 100644 --- a/src/alerts/actions.ts +++ b/src/alerts/actions.ts @@ -10,20 +10,22 @@ import { AlertActions, AlertDomain, AlertProps, AlertSpecific } from '../alerts' * @param domain The alert domain (app subsystem). * @param specific The specific alert for the domain. * @param props Any additional properties required by this specific alert. + * @param key Optional key to use as unique identifier for toast instead `..`. */ export const alertsShowAlert = createAction( >( domain: D, specific: S, - ...props: AlertProps extends never + ...args: AlertProps extends never ? [props?: never] - : [props: AlertProps] + : [props: AlertProps, key?: string] ) => ({ type: 'alerts.action.showAlert', domain, specific, - // HACK: using varargs to allow props to be optional, but it is only one arg - props: props.at(0), + // HACK: using varargs to allow props and key to be optional + props: args.at(0), + key: args.at(1), }), ); diff --git a/src/alerts/sagas.ts b/src/alerts/sagas.ts index 4bda446c..b821a84c 100644 --- a/src/alerts/sagas.ts +++ b/src/alerts/sagas.ts @@ -15,7 +15,9 @@ function* handleShowAlert(action: ReturnType): Generator const toaster = (yield* getContext('toasterRef')).current; defined(toaster); - const key = `${action.domain}.${action.specific}.${JSON.stringify(action.props)}`; + const key = + action.key ?? + `${action.domain}.${action.specific}.${JSON.stringify(action.props)}`; const existing = toaster.getToasts().filter((t) => t.key === key); diff --git a/src/firmware/alerts/index.ts b/src/firmware/alerts/index.ts index 7b99c303..2ae9b719 100644 --- a/src/firmware/alerts/index.ts +++ b/src/firmware/alerts/index.ts @@ -3,6 +3,7 @@ import { dfuError } from './DfuError'; import { firmwareMismatch } from './FirmwareMismatch'; +import { flashProgress } from './FlashProgress'; import { noDfuHub } from './NoDfuHub'; import { noDfuInterface } from './NoDfuInterface'; import { noWebUsb } from './NoWebUsb'; @@ -11,6 +12,7 @@ import { releaseButton } from './ReleaseButton'; export default { dfuError, firmwareMismatch, + flashProgress, noDfuHub, noDfuInterface, noWebUsb, diff --git a/src/firmware/sagas.test.ts b/src/firmware/sagas.test.ts index ff230720..21630eaf 100644 --- a/src/firmware/sagas.test.ts +++ b/src/firmware/sagas.test.ts @@ -136,6 +136,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -170,6 +180,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: offset / totalFirmwareSize, + }, + 'firmware.ble.progress', + ), + ); + // Have to be careful that a checksum request is not sent after // last payload is sent, otherwise the hub gets confused. @@ -194,6 +217,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(1)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: 1, + }, + 'firmware.ble.progress', + ), + ); + // and finally reboot the hub action = await saga.take(); @@ -264,6 +300,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -298,6 +344,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: offset / totalFirmwareSize, + }, + 'firmware.ble.progress', + ), + ); + // Have to be careful that a checksum request is not sent after // last payload is sent, otherwise the hub gets confused. @@ -322,6 +381,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(1)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: 1, + }, + 'firmware.ble.progress', + ), + ); + // and finally reboot the hub action = await saga.take(); @@ -936,6 +1008,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -1035,6 +1117,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -1143,6 +1235,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -1177,6 +1279,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: offset / totalFirmwareSize, + }, + 'firmware.ble.progress', + ), + ); + // Have to be careful that a checksum request is not sent after // last payload is sent, otherwise the hub gets confused. @@ -1291,6 +1406,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -1325,6 +1450,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: offset / totalFirmwareSize, + }, + 'firmware.ble.progress', + ), + ); + // Have to be careful that a checksum request is not sent after // last payload is sent, otherwise the hub gets confused. @@ -1444,6 +1582,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -1479,6 +1627,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: offset / totalFirmwareSize, + }, + 'firmware.ble.progress', + ), + ); + // Have to be careful that a checksum request is not sent after // last payload is sent, otherwise the hub gets confused. @@ -1503,6 +1664,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(1)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: 1, + }, + 'firmware.ble.progress', + ), + ); + // and finally reboot the hub action = await saga.take(); @@ -1956,6 +2130,16 @@ describe('flashFirmware', () => { // erase first + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { action: 'erase', progress: undefined }, + 'firmware.ble.progress', + ), + ); + action = await saga.take(); expect(action).toEqual(alertsShowAlert('firmware', 'releaseButton')); @@ -1989,6 +2173,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(offset / totalFirmwareSize)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: offset / totalFirmwareSize, + }, + 'firmware.ble.progress', + ), + ); + // Have to be careful that a checksum request is not sent after // last payload is sent, otherwise the hub gets confused. @@ -2012,6 +2209,19 @@ describe('flashFirmware', () => { action = await saga.take(); expect(action).toEqual(didProgress(1)); + action = await saga.take(); + expect(action).toEqual( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: 1, + }, + 'firmware.ble.progress', + ), + ); + // and finally reboot the hub action = await saga.take(); diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 5b78916d..e65a2b5a 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -74,7 +74,6 @@ import { firmwareInstallPybricks, flashFirmware, } from './actions'; -import { flashProgress } from './alerts/FlashProgress'; import { firmwareInstallPybricksDialogAccept, firmwareInstallPybricksDialogCancel, @@ -87,6 +86,8 @@ const firmwareZipMap = new Map([ [HubType.MoveHub, moveHubZip], ]); +const firmwareBleProgressToastId = 'firmware.ble.progress'; + /** * Disconnects the BLE if we are connected and cancels the task (including the * parent task). @@ -95,7 +96,7 @@ function* disconnectAndCancel(): SagaGenerator { const toaster = (yield* getContext('toasterRef')).current; defined(toaster); - toaster.dismiss('firmware.ble.progress'); + toaster.dismiss(firmwareBleProgressToastId); const connection = yield* select((s: RootState) => s.bootloader.connection); @@ -376,9 +377,6 @@ function* loadFirmware( * @param action The action that triggered this saga. */ function* handleFlashFirmware(action: ReturnType): Generator { - const toaster = (yield* getContext('toasterRef')).current; - defined(toaster); - try { let firmware: Uint8Array | undefined = undefined; let deviceId: HubType | undefined = undefined; @@ -436,12 +434,16 @@ function* handleFlashFirmware(action: ReturnType): Generat yield* put(didStart()); - toaster.show( - flashProgress(() => undefined, { - action: 'erase', - progress: undefined, - }), - 'firmware.ble.progress', + yield* put( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'erase', + progress: undefined, + }, + firmwareBleProgressToastId, + ), ); yield* put(alertsShowAlert('firmware', 'releaseButton')); @@ -497,12 +499,16 @@ function* handleFlashFirmware(action: ReturnType): Generat yield* put(didProgress(offset / firmware.length)); - toaster.show( - flashProgress(() => undefined, { - action: 'flash', - progress: offset / firmware.length, - }), - 'firmware.ble.progress', + yield* put( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: offset / firmware.length, + }, + firmwareBleProgressToastId, + ), ); // we don't want to request checksum if this is the last packet since @@ -578,12 +584,16 @@ function* handleFlashFirmware(action: ReturnType): Generat yield* put(didProgress(1)); - toaster.show( - flashProgress(() => undefined, { - action: 'flash', - progress: 1, - }), - 'firmware.ble.progress', + yield* put( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: 1, + }, + firmwareBleProgressToastId, + ), ); // this will cause the remote device to disconnect and reboot @@ -609,6 +619,40 @@ const dfuFirmwareStartAddress = 0x08008000; const firmwareDfuProgressToastId = 'firmware.dfu.progress'; +function* handleDfuEraseProcess(event: { + bytesSent: number; + expectedSize: number; +}): Generator { + yield* put( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'erase', + progress: event.bytesSent / event.expectedSize, + }, + firmwareDfuProgressToastId, + ), + ); +} + +function* handleDfuWriteProcess(event: { + bytesSent: number; + expectedSize: number; +}): Generator { + yield* put( + alertsShowAlert( + 'firmware', + 'flashProgress', + { + action: 'flash', + progress: event.bytesSent / event.expectedSize, + }, + firmwareDfuProgressToastId, + ), + ); +} + function* handleFlashUsbDfu(action: ReturnType): Generator { const defer = new Array<() => void>(); @@ -708,29 +752,31 @@ function* handleFlashUsbDfu(action: ReturnType): Gen const toaster = (yield* getContext('toasterRef')).current; defined(toaster); - defer.push( - writeProc.events.on('erase/process', (sent, total) => { - toaster.show( - flashProgress(() => undefined, { - action: 'erase', - progress: sent / total, - }), - firmwareDfuProgressToastId, - ); - }), - ); + const eraseProcessChan = eventChannel<{ + bytesSent: number; + expectedSize: number; + }>((emit) => { + return writeProc.events.on('erase/process', (bytesSent, expectedSize) => + emit({ bytesSent, expectedSize }), + ); + }); - defer.push( - writeProc.events.on('write/process', (sent, total) => { - toaster.show( - flashProgress(() => undefined, { - action: 'flash', - progress: sent / total, - }), - firmwareDfuProgressToastId, - ); - }), - ); + defer.push(() => eraseProcessChan.close()); + + yield* takeEvery(eraseProcessChan, handleDfuEraseProcess); + + const writeProcessChan = eventChannel<{ + bytesSent: number; + expectedSize: number; + }>((emit) => { + return writeProc.events.on('write/process', (bytesSent, expectedSize) => + emit({ bytesSent, expectedSize }), + ); + }); + + defer.push(() => writeProcessChan.close()); + + yield* takeEvery(writeProcessChan, handleDfuWriteProcess); const endChan = eventChannel((emit) => { // can't emit null or undefined, so have to emit something