From a841562c6c3c15b2df41e858909e5b28ecf75b8d Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 16 Feb 2023 11:50:05 -0600 Subject: [PATCH] firmware/installPybricksDialog: remove hard-coded hub name size check The UI has been rearranged so we now have firmware metadata available when we validate the hub name. --- .../InstallPybricksDialog.tsx | 9 +++- .../installPybricksDialog/index.test.ts | 52 ++++++++++++++++++- src/firmware/installPybricksDialog/index.ts | 25 ++++++--- 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/firmware/installPybricksDialog/InstallPybricksDialog.tsx b/src/firmware/installPybricksDialog/InstallPybricksDialog.tsx index 904def74..ba49eadd 100644 --- a/src/firmware/installPybricksDialog/InstallPybricksDialog.tsx +++ b/src/firmware/installPybricksDialog/InstallPybricksDialog.tsx @@ -376,15 +376,17 @@ const AcceptLicensePanel: React.VoidFunctionComponent = type SelectOptionsPanelProps = { hubName: string; + metadata: FirmwareMetadata | undefined; onChangeHubName(hubName: string): void; }; const ConfigureOptionsPanel: React.VoidFunctionComponent = ({ hubName, + metadata, onChangeHubName, }) => { const i18n = useI18n(); - const isHubNameValid = validateHubName(hubName); + const isHubNameValid = metadata ? validateHubName(hubName, metadata) : true; return (
@@ -512,6 +514,11 @@ export const InstallPybricksDialog: React.VoidFunctionComponent = () => { panel={ } diff --git a/src/firmware/installPybricksDialog/index.test.ts b/src/firmware/installPybricksDialog/index.test.ts index b4cbd01d..2cdb6967 100644 --- a/src/firmware/installPybricksDialog/index.test.ts +++ b/src/firmware/installPybricksDialog/index.test.ts @@ -1,7 +1,57 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2023 The Pybricks Authors -import { validateMetadata } from '.'; +import { validateHubName, validateMetadata } from '.'; + +describe('validateHubName', () => { + it('should accept short name', () => { + expect( + validateHubName('good', { + 'metadata-version': '1.1.0', + 'device-id': 0x40, + 'firmware-version': '3.0.0', + 'checksum-type': 'sum', + 'max-firmware-size': 100000, + 'mpy-abi-version': 6, + 'mpy-cross-options': ['-mno-unicode'], + 'user-mpy-offset': 90000, + 'hub-name-offset': 80000, + 'max-hub-name-size': 16, + }), + ).toBeTruthy(); + }); + + it('should accept empty name', () => { + expect( + validateHubName('', { + 'metadata-version': '2.0.0', + 'device-id': 0x40, + 'firmware-version': '3.0.0', + 'checksum-type': 'sum', + 'checksum-size': 100000, + 'hub-name-offset': 100000 - 16, + 'hub-name-size': 16, + }), + ).toBeTruthy(); + }); + + it('should reject long name', () => { + expect( + validateHubName('this name is too long', { + 'metadata-version': '1.1.0', + 'device-id': 0x40, + 'firmware-version': '3.0.0', + 'checksum-type': 'sum', + 'max-firmware-size': 100000, + 'mpy-abi-version': 6, + 'mpy-cross-options': ['-mno-unicode'], + 'user-mpy-offset': 90000, + 'hub-name-offset': 80000, + 'max-hub-name-size': 16, + }), + ).toBeFalsy(); + }); +}); describe('validateMetadata', () => { it('should accept 1.x firmware metadata', () => { diff --git a/src/firmware/installPybricksDialog/index.ts b/src/firmware/installPybricksDialog/index.ts index c8e9acd4..129c51fe 100644 --- a/src/firmware/installPybricksDialog/index.ts +++ b/src/firmware/installPybricksDialog/index.ts @@ -1,7 +1,12 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022-2023 The Pybricks Authors -import { FirmwareMetadata, HubType } from '@pybricks/firmware'; +import { + FirmwareMetadata, + FirmwareMetadataV110, + FirmwareMetadataV200, + HubType, +} from '@pybricks/firmware'; import * as semver from 'semver'; const encoder = new TextEncoder(); @@ -9,17 +14,21 @@ const encoder = new TextEncoder(); /** * Validates the hub name. * @param hubName The hub name. + * @param metadata The firmware metadata. * @returns True if the name if valid, otherwise false. */ -export function validateHubName(hubName: string): boolean { +export function validateHubName(hubName: string, metadata: FirmwareMetadata): boolean { const encoded = encoder.encode(hubName); - // Technically, the max hub name size is determined by each individual - // firmware file, so we can't check until the firmware has been selected. - // However all firmware currently have 16 bytes allocated (including zero- - // termination), so we can hard code the check here to allow notifying the - // user earlier for better UX. - return encoded.length < 16; + if (semver.satisfies(metadata['metadata-version'], '^1.1')) { + return encoded.length < (metadata as FirmwareMetadataV110)['max-hub-name-size']; + } + + if (semver.satisfies(metadata['metadata-version'], '^2.0')) { + return encoded.length < (metadata as FirmwareMetadataV200)['hub-name-size']; + } + + return false; } const supportHubs: readonly HubType[] = [