From 86c0e933f3ba37204209d105abf436367b8b3840 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 19 Nov 2021 11:59:36 -0600 Subject: [PATCH 1/6] settings: add new hubName setting This setting will be used to customize the hub name when flashing firmware. This commit just implements the setting. Some changes needed to be made to the existing settings infrastructure since previously it only allowed boolean settings. --- src/app/App.tsx | 4 +- src/editor/Editor.tsx | 4 +- src/index.scss | 4 ++ src/settings/SettingsDrawer.tsx | 62 ++++++++++++++++++-- src/settings/actions.ts | 73 ++++++++++++++++++----- src/settings/defaults.ts | 26 +++++++-- src/settings/i18n.en.json | 6 ++ src/settings/i18n.ts | 2 + src/settings/reducers.test.ts | 100 ++++++++++++++++++++++++++++++++ src/settings/reducers.ts | 63 +++++++++++++++++--- src/settings/sagas.test.ts | 56 ++++++++++++------ src/settings/sagas.ts | 79 +++++++++++++++++++++---- src/settings/settings.scss | 10 ++++ 13 files changed, 423 insertions(+), 66 deletions(-) create mode 100644 src/settings/reducers.test.ts create mode 100644 src/settings/settings.scss diff --git a/src/app/App.tsx b/src/app/App.tsx index 04211183..660492e5 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -8,7 +8,7 @@ import SplitterLayout from 'react-splitter-layout'; import Editor from '../editor/Editor'; import { RootState } from '../reducers'; import { toggleBoolean } from '../settings/actions'; -import { SettingId } from '../settings/defaults'; +import { BooleanSettingId } from '../settings/defaults'; import StatusBar from '../status-bar/StatusBar'; import Terminal from '../terminal/Terminal'; import Toolbar from '../toolbar/Toolbar'; @@ -142,7 +142,7 @@ function App(): JSX.Element { e.key == 'd' ) { e.preventDefault(); - dispatch(toggleBoolean(SettingId.ShowDocs)); + dispatch(toggleBoolean(BooleanSettingId.ShowDocs)); } }); diff --git a/src/editor/Editor.tsx b/src/editor/Editor.tsx index 4a32ff01..c76be844 100644 --- a/src/editor/Editor.tsx +++ b/src/editor/Editor.tsx @@ -12,7 +12,7 @@ import { IDisposable } from 'xterm'; import { compile } from '../mpy/actions'; import { RootState } from '../reducers'; import { toggleBoolean } from '../settings/actions'; -import { SettingId } from '../settings/defaults'; +import { BooleanSettingId } from '../settings/defaults'; import { IContextMenuTarget, handleContextMenu } from '../utils/IContextMenuTarget'; import { isMacOS } from '../utils/os'; import { setEditSession, storageChanged } from './actions'; @@ -254,7 +254,7 @@ const mapDispatchToProps: DispatchProps = { // REVISIT: the options here might need to be changed - hopefully there is // one setting that works for all hub types for cases where we aren't connected. onCheck: (script) => compile(script, []), - onToggleDocs: () => toggleBoolean(SettingId.ShowDocs), + onToggleDocs: () => toggleBoolean(BooleanSettingId.ShowDocs), }; export default connect( diff --git a/src/index.scss b/src/index.scss index 0d3d22f9..24590bed 100644 --- a/src/index.scss +++ b/src/index.scss @@ -93,3 +93,7 @@ a.#{$ns}-button { ::-webkit-scrollbar-thumb:hover { background: $pt-icon-color-hover; } + +.#{$ns}-input-group .#{$ns}-icon { + margin: 7px; +} diff --git a/src/settings/SettingsDrawer.tsx b/src/settings/SettingsDrawer.tsx index b668d03f..d46254b6 100644 --- a/src/settings/SettingsDrawer.tsx +++ b/src/settings/SettingsDrawer.tsx @@ -6,11 +6,16 @@ import { Button, ButtonGroup, Classes, + ControlGroup, Drawer, FormGroup, Hotkey, Hotkeys, HotkeysTarget, + Icon, + InputGroup, + Intent, + Label, Position, Switch, Tooltip, @@ -32,10 +37,11 @@ import { RootState } from '../reducers'; import ExternalLinkIcon from '../utils/ExternalLinkIcon'; import { BeforeInstallPromptEvent } from '../utils/dom'; import { isMacOS } from '../utils/os'; -import { setBoolean, toggleBoolean } from './actions'; -import { SettingId } from './defaults'; +import { setBoolean, setString, toggleBoolean } from './actions'; +import { BooleanSettingId, StringSettingId } from './defaults'; import { SettingsStringId } from './i18n'; import en from './i18n.en.json'; +import './settings.scss'; type StateProps = { showDocs: boolean; @@ -47,6 +53,8 @@ type StateProps = { beforeInstallPrompt: BeforeInstallPromptEvent | null; promptingInstall: boolean; readyForOfflineUse: boolean; + hubName: string; + isHubNameValid: boolean; }; type DispatchProps = { @@ -57,6 +65,7 @@ type DispatchProps = { onCheckForUpdate: (registration: ServiceWorkerRegistration) => void; onReload: (registration: ServiceWorkerRegistration) => void; onInstallPrompt: (event: BeforeInstallPromptEvent) => void; + onHubNameChange: React.FormEventHandler; }; type OwnProps = { @@ -92,6 +101,9 @@ class SettingsDrawer extends React.PureComponent { isOpen, onClose, i18n, + hubName, + isHubNameValid, + onHubNameChange, } = this.props; return ( { } /> + + + ({ beforeInstallPrompt: state.app.beforeInstallPrompt, promptingInstall: state.app.promptingInstall, readyForOfflineUse: state.app.readyForOfflineUse, + hubName: state.settings.hubName, + isHubNameValid: state.settings.isHubNameValid, }); const mapDispatchToProps: DispatchProps = { - onShowDocsChanged: (checked) => setBoolean(SettingId.ShowDocs, checked), - onDarkModeChanged: (checked) => setBoolean(SettingId.DarkMode, checked), + onShowDocsChanged: (checked) => setBoolean(BooleanSettingId.ShowDocs, checked), + onDarkModeChanged: (checked) => setBoolean(BooleanSettingId.DarkMode, checked), onFlashCurrentProgramChanged: (checked) => - setBoolean(SettingId.FlashCurrentProgram, checked), - onToggleDocs: () => toggleBoolean(SettingId.ShowDocs), + setBoolean(BooleanSettingId.FlashCurrentProgram, checked), + onToggleDocs: () => toggleBoolean(BooleanSettingId.ShowDocs), onCheckForUpdate: checkForUpdate, onReload: reload, onInstallPrompt: installPrompt, + onHubNameChange: (event) => + setString(StringSettingId.HubName, event.currentTarget.value), }; export default connect( diff --git a/src/settings/actions.ts b/src/settings/actions.ts index 47959b37..2e1dd651 100644 --- a/src/settings/actions.ts +++ b/src/settings/actions.ts @@ -2,7 +2,7 @@ // Copyright (c) 2021 The Pybricks Authors import { Action } from 'redux'; -import { SettingId } from './defaults'; +import { BooleanSettingId, StringSettingId } from './defaults'; /** Actions related to settings. */ export enum SettingsActionType { @@ -10,44 +10,50 @@ export enum SettingsActionType { ToggleBoolean = 'settings.action.toggleBoolean', DidFailToSetBoolean = 'settings.action.didFailToSetBoolean', DidBooleanChange = 'settings.action.didBooleanChange', + SetString = 'settings.action.setString', + DidFailToSetString = 'settings.action.didFailToSetString', + DidStringChange = 'settings.action.didStringChange', } -type SettingInfo = { +type SettingInfo = { /** The ID of the setting. */ - id: SettingId; + id: TId; /** The new state for the setting. */ - newState: T; + newState: TState; }; /** Action to set/store a setting. */ export type SettingsSetBooleanAction = Action & - SettingInfo; + SettingInfo; /** Creates an action to set/store a setting. */ -export function setBoolean(id: SettingId, newState: boolean): SettingsSetBooleanAction { +export function setBoolean( + id: BooleanSettingId, + newState: boolean, +): SettingsSetBooleanAction { return { type: SettingsActionType.SetBoolean, id, newState }; } /** Action to toggle a setting. */ export type SettingsToggleBooleanAction = Action & { - id: SettingId; + id: BooleanSettingId; }; /** Creates an action to toggle a setting. */ -export function toggleBoolean(id: SettingId): SettingsToggleBooleanAction { +export function toggleBoolean(id: BooleanSettingId): SettingsToggleBooleanAction { return { type: SettingsActionType.ToggleBoolean, id }; } /** Action that indicates setting/storing a setting failed. */ export type SettingsDidFailToSetBooleanAction = Action & { - id: SettingId; + id: BooleanSettingId; err: Error; }; /** Creates an action indicating that setting/storing a setting failed. */ export function didFailToSetBoolean( - id: SettingId, + id: BooleanSettingId, err: Error, ): SettingsDidFailToSetBooleanAction { return { type: SettingsActionType.DidFailToSetBoolean, id, err }; @@ -55,19 +61,60 @@ export function didFailToSetBoolean( /** Action that indicates a stored boolean setting value changed. */ export type SettingsDidBooleanChangeAction = - Action & SettingInfo; + Action & + SettingInfo; /** Creates an action that indicates a stored boolean setting value changed. */ export function didBooleanChange( - id: SettingId, + id: BooleanSettingId, newState: boolean, ): SettingsDidBooleanChangeAction { return { type: SettingsActionType.DidBooleanChange, id, newState }; } +/** Action to set/store a setting. */ +export type SettingsSetStringAction = Action & + SettingInfo; + +/** Creates an action to set/store a setting. */ +export function setString( + id: StringSettingId, + newState: string, +): SettingsSetStringAction { + return { type: SettingsActionType.SetString, id, newState }; +} + +/** Action that indicates setting/storing a setting failed. */ +export type SettingsDidFailToSetStringAction = + Action & { + id: StringSettingId; + err: Error; + }; + +/** Creates an action indicating that setting/storing a setting failed. */ +export function didFailToSetString( + id: StringSettingId, + err: Error, +): SettingsDidFailToSetStringAction { + return { type: SettingsActionType.DidFailToSetString, id, err }; +} + +export type SettingsDidStringChangeAction = Action & + SettingInfo; + +export function didStringChange( + id: StringSettingId, + newState: string, +): SettingsDidStringChangeAction { + return { type: SettingsActionType.DidStringChange, id, newState }; +} + /** Common type for all settings actions. */ export type SettingsAction = | SettingsSetBooleanAction | SettingsToggleBooleanAction | SettingsDidFailToSetBooleanAction - | SettingsDidBooleanChangeAction; + | SettingsDidBooleanChangeAction + | SettingsSetStringAction + | SettingsDidFailToSetStringAction + | SettingsDidStringChangeAction; diff --git a/src/settings/defaults.ts b/src/settings/defaults.ts index 0066ddff..5b2a6fb3 100644 --- a/src/settings/defaults.ts +++ b/src/settings/defaults.ts @@ -5,22 +5,36 @@ import { prefersDarkMode } from '../utils/os'; -export enum SettingId { +export enum BooleanSettingId { ShowDocs = 'showDocs', DarkMode = 'darkMode', FlashCurrentProgram = 'flashCurrentProgram', } -export function getDefaultBooleanValue(id: SettingId): boolean { +export function getDefaultBooleanValue(id: BooleanSettingId): boolean { switch (id) { - case SettingId.ShowDocs: + case BooleanSettingId.ShowDocs: return window.innerWidth >= 1024; - case SettingId.DarkMode: + case BooleanSettingId.DarkMode: return prefersDarkMode(); - case SettingId.FlashCurrentProgram: + case BooleanSettingId.FlashCurrentProgram: return false; // istanbul ignore next: it is a programmer error if we hit this default: - throw Error(`Bad setting id: ${id}`); + throw Error(`Bad BooleanSettingId: ${id}`); + } +} + +export enum StringSettingId { + HubName = 'hubName', +} + +export function getDefaultStringValue(id: StringSettingId): string { + switch (id) { + case StringSettingId.HubName: + return ''; // empty string will result in 'Pybricks Hub' + // istanbul ignore next: it is a programmer error if we hit this + default: + throw Error(`Bad StringSettingId: ${id}`); } } diff --git a/src/settings/i18n.en.json b/src/settings/i18n.en.json index cebe80b7..d70bda6f 100644 --- a/src/settings/i18n.en.json +++ b/src/settings/i18n.en.json @@ -20,6 +20,12 @@ "flash-current-program": { "label": "Include current program", "tooltip": "Select to include your program when installing the firmware." + }, + "hub-name": { + "label": "Hub name", + "error": { + "tooltip": "The name is too long." + } } }, "help": { diff --git a/src/settings/i18n.ts b/src/settings/i18n.ts index 9f8f0b84..3644b2e3 100644 --- a/src/settings/i18n.ts +++ b/src/settings/i18n.ts @@ -14,6 +14,8 @@ export enum SettingsStringId { FirmwareTitle = 'settings.firmware.title', FirmwareCurrentProgramLabel = 'settings.firmware.flash-current-program.label', FirmwareCurrentProgramTooltip = 'settings.firmware.flash-current-program.tooltip', + FirmwareHubNameLabel = 'settings.firmware.hub-name.label', + FirmwareHubNameErrorTooltip = 'settings.firmware.hub-name.error.tooltip', HelpTitle = 'settings.help.title', HelpProjectsLabel = 'settings.help.projects.label', HelpSupportLabel = 'settings.help.support.label', diff --git a/src/settings/reducers.test.ts b/src/settings/reducers.test.ts new file mode 100644 index 00000000..9c237c9e --- /dev/null +++ b/src/settings/reducers.test.ts @@ -0,0 +1,100 @@ +import { Action } from '../actions'; +import { didBooleanChange, didStringChange } from './actions'; +import { BooleanSettingId, StringSettingId } from './defaults'; +import reducers from './reducers'; + +type State = ReturnType; + +test('initial state', () => { + expect(reducers(undefined, {} as Action)).toMatchInlineSnapshot(` + Object { + "darkMode": false, + "flashCurrentProgram": false, + "hubName": "", + "isHubNameValid": true, + "showDocs": true, + } + `); +}); + +describe('darkMode', () => { + test('setting changed', () => { + expect( + reducers( + { darkMode: false } as State, + didBooleanChange(BooleanSettingId.DarkMode, true), + ).darkMode, + ).toBe(true); + }); +}); + +describe('showDocs', () => { + test('setting changed', () => { + expect( + reducers( + { showDocs: false } as State, + didBooleanChange(BooleanSettingId.ShowDocs, true), + ).showDocs, + ).toBe(true); + }); +}); + +describe('flashCurrentProgram', () => { + test('setting changed', () => { + expect( + reducers( + { flashCurrentProgram: false } as State, + didBooleanChange(BooleanSettingId.FlashCurrentProgram, true), + ).flashCurrentProgram, + ).toBe(true); + }); +}); + +describe('hubName', () => { + const testName = 'test name'; + test('setting changed', () => { + expect( + reducers( + { hubName: '' } as State, + didStringChange(StringSettingId.HubName, testName), + ).hubName, + ).toBe(testName); + }); +}); + +describe('isHubNameValid', () => { + test('default name is ok', () => { + expect( + reducers( + undefined, + didStringChange(StringSettingId.HubName, 'Pybricks Hub'), + ).isHubNameValid, + ).toBe(true); + }); + + test('empty name is ok', () => { + expect( + reducers(undefined, didStringChange(StringSettingId.HubName, '')) + .isHubNameValid, + ).toBe(true); + }); + + test('too long name fails', () => { + expect( + reducers( + undefined, + didStringChange(StringSettingId.HubName, 'this name is way too long'), + ).isHubNameValid, + ).toBe(false); + }); + + test('the number of bytes matter, not the number of characters', () => { + expect( + reducers( + undefined, + // Chinese characters are 3 bytes each. + didStringChange(StringSettingId.HubName, 'Pybricks 枢纽!'), + ).isHubNameValid, + ).toBe(false); + }); +}); diff --git a/src/settings/reducers.ts b/src/settings/reducers.ts index 2fb1ba05..29e9d5c6 100644 --- a/src/settings/reducers.ts +++ b/src/settings/reducers.ts @@ -4,15 +4,22 @@ import { Reducer, combineReducers } from 'redux'; import { Action } from '../actions'; import { SettingsActionType } from './actions'; -import { SettingId, getDefaultBooleanValue } from './defaults'; +import { + BooleanSettingId, + StringSettingId, + getDefaultBooleanValue, + getDefaultStringValue, +} from './defaults'; + +const encoder = new TextEncoder(); const darkMode: Reducer = ( - state = getDefaultBooleanValue(SettingId.DarkMode), + state = getDefaultBooleanValue(BooleanSettingId.DarkMode), action, ) => { switch (action.type) { case SettingsActionType.DidBooleanChange: - if (action.id === SettingId.DarkMode) { + if (action.id === BooleanSettingId.DarkMode) { return action.newState; } return state; @@ -22,12 +29,12 @@ const darkMode: Reducer = ( }; const showDocs: Reducer = ( - state = getDefaultBooleanValue(SettingId.ShowDocs), + state = getDefaultBooleanValue(BooleanSettingId.ShowDocs), action, ) => { switch (action.type) { case SettingsActionType.DidBooleanChange: - if (action.id === SettingId.ShowDocs) { + if (action.id === BooleanSettingId.ShowDocs) { return action.newState; } return state; @@ -37,12 +44,12 @@ const showDocs: Reducer = ( }; const flashCurrentProgram: Reducer = ( - state = getDefaultBooleanValue(SettingId.FlashCurrentProgram), + state = getDefaultBooleanValue(BooleanSettingId.FlashCurrentProgram), action, ) => { switch (action.type) { case SettingsActionType.DidBooleanChange: - if (action.id === SettingId.FlashCurrentProgram) { + if (action.id === BooleanSettingId.FlashCurrentProgram) { return action.newState; } return state; @@ -51,4 +58,44 @@ const flashCurrentProgram: Reducer = ( } }; -export default combineReducers({ darkMode, showDocs, flashCurrentProgram }); +const hubName: Reducer = ( + state = getDefaultStringValue(StringSettingId.HubName), + action, +) => { + switch (action.type) { + case SettingsActionType.DidStringChange: + if (action.id === StringSettingId.HubName) { + return action.newState; + } + return state; + default: + return state; + } +}; + +const isHubNameValid: Reducer = (state = true, action) => { + switch (action.type) { + case SettingsActionType.DidStringChange: + if (action.id === StringSettingId.HubName) { + const encoded = encoder.encode(action.newState); + + // 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 has 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; + } + return state; + default: + return state; + } +}; + +export default combineReducers({ + darkMode, + showDocs, + flashCurrentProgram, + hubName, + isHubNameValid, +}); diff --git a/src/settings/sagas.test.ts b/src/settings/sagas.test.ts index 4dae2884..e67fdac0 100644 --- a/src/settings/sagas.test.ts +++ b/src/settings/sagas.test.ts @@ -8,10 +8,13 @@ import { didStart } from '../app/actions'; import { didBooleanChange, didFailToSetBoolean, + didFailToSetString, + didStringChange, setBoolean, + setString, toggleBoolean, } from './actions'; -import { SettingId } from './defaults'; +import { BooleanSettingId, StringSettingId } from './defaults'; import settings from './sagas'; afterAll(() => { @@ -95,7 +98,7 @@ describe('startup', () => { // requests documentation to be shown const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, true)); + expect(action).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true)); await saga.end(); }); @@ -120,7 +123,7 @@ describe('startup', () => { // requests documentation to be hidden const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, false)); + expect(action).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, false)); await saga.end(); }); @@ -184,7 +187,7 @@ describe('startup', () => { // requests to enable dark mode const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.DarkMode, true)); + expect(action).toEqual(didBooleanChange(BooleanSettingId.DarkMode, true)); await saga.end(); }); @@ -215,7 +218,9 @@ describe('startup', () => { describe('store settings to local storage', () => { test('failed storage', async () => { - const saga = new AsyncSaga(settings, { settings: { showDocs: false } }); + const saga = new AsyncSaga(settings, { + settings: { showDocs: false, hubName: '' }, + }); const testError = new Error('local storage is disabled'); @@ -225,16 +230,31 @@ describe('store settings to local storage', () => { throw testError; }); - saga.put(setBoolean(SettingId.ShowDocs, true)); + saga.put(setBoolean(BooleanSettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); // raises error that storing setting didn't work const action1 = await saga.take(); - expect(action1).toEqual(didFailToSetBoolean(SettingId.ShowDocs, testError)); + expect(action1).toEqual( + didFailToSetBoolean(BooleanSettingId.ShowDocs, testError), + ); // but the setting is still applied anyway const action2 = await saga.take(); - expect(action2).toEqual(didBooleanChange(SettingId.ShowDocs, true)); + expect(action2).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true)); + + mockSetItem.mockClear(); + + saga.put(setString(StringSettingId.HubName, 'test name')); + expect(mockSetItem).toHaveBeenCalled(); + + // raises error that storing setting didn't work + const action3 = await saga.take(); + expect(action3).toEqual(didFailToSetString(StringSettingId.HubName, testError)); + + // but the setting is still applied anyway + const action4 = await saga.take(); + expect(action4).toEqual(didStringChange(StringSettingId.HubName, 'test name')); await saga.end(); }); @@ -249,11 +269,11 @@ describe('store settings to local storage', () => { expect(value).toBe('true'); }); - saga.put(setBoolean(SettingId.ShowDocs, true)); + saga.put(setBoolean(BooleanSettingId.ShowDocs, true)); expect(mockSetItem).toHaveBeenCalled(); const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, true)); + expect(action).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true)); await saga.end(); }); @@ -268,11 +288,11 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.put(setBoolean(SettingId.DarkMode, false)); + saga.put(setBoolean(BooleanSettingId.DarkMode, false)); expect(mockSetItem).toHaveBeenCalled(); const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.DarkMode, false)); + expect(action).toEqual(didBooleanChange(BooleanSettingId.DarkMode, false)); await saga.end(); }); @@ -289,11 +309,13 @@ describe('store settings to local storage', () => { expect(value).toBe('false'); }); - saga.put(setBoolean(SettingId.FlashCurrentProgram, false)); + saga.put(setBoolean(BooleanSettingId.FlashCurrentProgram, false)); expect(mockSetItem).toHaveBeenCalled(); const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.FlashCurrentProgram, false)); + expect(action).toEqual( + didBooleanChange(BooleanSettingId.FlashCurrentProgram, false), + ); await saga.end(); }); @@ -328,7 +350,7 @@ describe('storage monitor', () => { ); const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, true)); + expect(action).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true)); await saga.end(); }); @@ -362,11 +384,11 @@ describe('toggle', () => { expect(value).toBe('true'); }); - saga.put(toggleBoolean(SettingId.ShowDocs)); + saga.put(toggleBoolean(BooleanSettingId.ShowDocs)); expect(mockSetItem).toHaveBeenCalled(); const action = await saga.take(); - expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, true)); + expect(action).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true)); await saga.end(); }); diff --git a/src/settings/sagas.ts b/src/settings/sagas.ts index 8ed598fd..a0821317 100644 --- a/src/settings/sagas.ts +++ b/src/settings/sagas.ts @@ -13,12 +13,20 @@ import { ensureError } from '../utils'; import { SettingsActionType, SettingsSetBooleanAction, + SettingsSetStringAction, SettingsToggleBooleanAction, didBooleanChange, didFailToSetBoolean, + didFailToSetString, + didStringChange, setBoolean, } from './actions'; -import { SettingId, getDefaultBooleanValue } from './defaults'; +import { + BooleanSettingId, + StringSettingId, + getDefaultBooleanValue, + getDefaultStringValue, +} from './defaults'; function stringToBoolean(value: string): boolean { return value.toLowerCase().match(/(true|yes|1)/) !== null; @@ -49,20 +57,30 @@ function* monitorLocalStorage(): Generator { continue; } - const id = event.key.replace(/^setting\./, '') as SettingId; + const id = event.key.replace(/^setting\./, ''); - // istanbul ignore if: should not happen normally - if (!Object.values(SettingId).includes(id)) { - console.error(`Bad setting id: ${id}`); + if (Object.values(BooleanSettingId).includes(id as BooleanSettingId)) { + yield* put( + didBooleanChange( + id as BooleanSettingId, + stringToBoolean(event.newValue || 'false'), + ), + ); continue; } - yield* put(didBooleanChange(id, stringToBoolean(event.newValue || 'false'))); + if (Object.values(StringSettingId).includes(id as StringSettingId)) { + yield* put(didStringChange(id as StringSettingId, event.newValue || '')); + continue; + } + + // istanbul ignore next: should not happen normally + console.error(`Bad setting id: ${id}`); } } function* loadSettings(): Generator { - for (const id of Object.values(SettingId)) { + for (const id of Object.values(BooleanSettingId)) { const storageValue = localStorage.getItem(`setting.${id}`); const defaultValue = getDefaultBooleanValue(id); const value = @@ -72,9 +90,19 @@ function* loadSettings(): Generator { yield* put(didBooleanChange(id, value)); } } + + for (const id of Object.values(StringSettingId)) { + const storageValue = localStorage.getItem(`setting.${id}`); + const defaultValue = getDefaultStringValue(id); + const value = storageValue === null ? defaultValue : storageValue; + + if (value !== defaultValue) { + yield* put(didStringChange(id, value)); + } + } } -function* storeSetting(action: SettingsSetBooleanAction): Generator { +function* storeBooleanSetting(action: SettingsSetBooleanAction): Generator { const key = `setting.${action.id}`; const newValue = String(action.newState); @@ -100,14 +128,41 @@ function* storeSetting(action: SettingsSetBooleanAction): Generator { } } -function* toggleSetting(action: SettingsToggleBooleanAction): Generator { +function* toggleBooleanSetting(action: SettingsToggleBooleanAction): Generator { const oldValue = yield* select((s: RootState) => s.settings[action.id]); - yield* storeSetting(setBoolean(action.id, !oldValue)); + yield* storeBooleanSetting(setBoolean(action.id, !oldValue)); +} + +function* storeStringSetting(action: SettingsSetStringAction): Generator { + const key = `setting.${action.id}`; + const newValue = action.newState; + + try { + localStorage.setItem(key, newValue); + } catch (err) { + yield* put(didFailToSetString(action.id, ensureError(err))); + } + + // storage event is only raised when a value is changed externally, so we + // mimic the event when we call setItem(), whether it actually succeeded + // or not. + const oldValue = yield* select((s: RootState) => s.settings[action.id]); + if (action.newState !== oldValue) { + window.dispatchEvent( + new StorageEvent('storage', { + key, + newValue, + oldValue, + storageArea: localStorage, + }), + ); + } } export default function* (): Generator { yield* fork(monitorLocalStorage); yield* takeEvery(AppActionType.DidStart, loadSettings); - yield* takeEvery(SettingsActionType.SetBoolean, storeSetting); - yield* takeEvery(SettingsActionType.ToggleBoolean, toggleSetting); + yield* takeEvery(SettingsActionType.SetBoolean, storeBooleanSetting); + yield* takeEvery(SettingsActionType.ToggleBoolean, toggleBooleanSetting); + yield* takeEvery(SettingsActionType.SetString, storeStringSetting); } diff --git a/src/settings/settings.scss b/src/settings/settings.scss new file mode 100644 index 00000000..25b015e3 --- /dev/null +++ b/src/settings/settings.scss @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2021 The Pybricks Authors + +// Custom styling for the settings controls. + +@import '../variables.scss'; + +.pb-hub-name-input .#{$ns}-input { + width: 200px; +} From ae40c2f5401077d55445ea5e2ca80969759804ee Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 19 Nov 2021 15:54:10 -0600 Subject: [PATCH 2/6] firmware: flash custom hub name This adds support for customizing the hub name when flashing firmware. Fixes: https://github.com/pybricks/support/issues/52 --- CHANGELOG.md | 5 +++++ src/firmware/sagas.test.ts | 6 ++++-- src/firmware/sagas.ts | 17 ++++++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0080ee9e..fd257682 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ## [Unreleased] +### Added +- Hub name setting for selecting hub name when flashing firmware ([support#52]). + +[support#52]: https://github.com/pybricks/support/issues/52 + ## [1.1.0-beta.6] - 2021-09-21 ### Added diff --git a/src/firmware/sagas.test.ts b/src/firmware/sagas.test.ts index 5ca74b2b..23046fd0 100644 --- a/src/firmware/sagas.test.ts +++ b/src/firmware/sagas.test.ts @@ -63,6 +63,8 @@ describe('flashFirmware', () => { 'mpy-abi-version': 5, 'mpy-cross-options': ['-mno-unicode'], 'user-mpy-offset': 100, + 'hub-name-offset': 90, + 'max-hub-name-size': 10, }; const zip = new JSZip(); @@ -79,7 +81,7 @@ describe('flashFirmware', () => { flashFirmware, { bootloader: { connection: BootloaderConnectionState.Disconnected }, - settings: { flashCurrentProgram: false }, + settings: { flashCurrentProgram: false, hubName: 'test name' }, }, { nextMessageId: createCountFunc(), @@ -185,7 +187,7 @@ describe('flashFirmware', () => { // hub indicates success - saga.put(programResponse(0x62, totalFirmwareSize)); + saga.put(programResponse(0x33, totalFirmwareSize)); action = await saga.take(); expect(action).toEqual(didProgress(1)); diff --git a/src/firmware/sagas.ts b/src/firmware/sagas.ts index 680350e1..c96d8e3d 100644 --- a/src/firmware/sagas.ts +++ b/src/firmware/sagas.ts @@ -1,7 +1,12 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2020-2021 The Pybricks Authors -import { FirmwareReader, FirmwareReaderError, HubType } from '@pybricks/firmware'; +import { + FirmwareReader, + FirmwareReaderError, + HubType, + encodeHubName, +} from '@pybricks/firmware'; import cityHubZip from '@pybricks/firmware/build/cityhub.zip'; import moveHubZip from '@pybricks/firmware/build/movehub.zip'; import technicHubZip from '@pybricks/firmware/build/technichub.zip'; @@ -241,6 +246,16 @@ function* loadFirmware( firmwareView.setUint32(metadata['user-mpy-offset'], mpy.data.length, true); firmware.set(mpy.data, metadata['user-mpy-offset'] + 4); + // if the firmware supports it, we can set a custom hub name + if (metadata['max-hub-name-size']) { + const hubName = yield* select((s: RootState) => s.settings.hubName); + + // empty string means use default name (don't write over firmware) + if (hubName) { + firmware.set(encodeHubName(hubName, metadata), metadata['hub-name-offset']); + } + } + if (metadata['checksum-type'] !== 'sum') { yield* put( didFailToFinish( From d8179413ec7be7d3c65273ec83d69aab0b0a600b Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 19 Nov 2021 15:58:16 -0600 Subject: [PATCH 3/6] CHANGELOG: copy from Pybricks MicroPython v3.1.0c1 --- CHANGELOG.md | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd257682..072af641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,38 @@ ### Added - Hub name setting for selecting hub name when flashing firmware ([support#52]). +### Changed +- Updated to Pybricks firmware v3.1.0c1: + + ### Added + - Added `DriveBase.curve()` method to drive an arc segment. + - Added `then` and `wait` arguments to `DriveBase` methods ([support#57]). + + ### Changed + - Dropped `integral_range` argument from `Control.pid()`. This setting was + ineffective and never used. When set incorrectly, the motor could get stuck + for certain combinations of `kp` and `ki`. + - Improved motor behavior for cases with low-speed, low-load, but high + inertia ([support#366]). + - Changed how the duty cycle limit is set for `Motor` and `DCMotor`. It is now + set as a voltage limit via a dedicated method, instead of `Motor.control`. + + ### Fixed + - Fixed `then=Stop.COAST` being ignored in most motor commands. + - Fixed `brake()`/`light.off()` not working on Move hub I/O port C ([support#501]). + - Fixed `Remote()` failing to connect when hub is connected to 2019 or newer + MacBooks ([support#397]). + - Fixed intermittent improper detection of hot-plugged I/O devices ([support#500]). + - A program now stops when a `Motor` is unplugged while it is running, instead + of getting in a bad state. + + [support#57]: https://github.com/pybricks/support/issues/57 + [support#366]: https://github.com/pybricks/support/issues/366 + [support#397]: https://github.com/pybricks/support/issues/397 + [support#500]: https://github.com/pybricks/support/issues/500 + [support#501]: https://github.com/pybricks/support/issues/501 + + [support#52]: https://github.com/pybricks/support/issues/52 ## [1.1.0-beta.6] - 2021-09-21 @@ -134,7 +166,7 @@ - Updated projects URL. - Updated hub firmware to [v3.1.0a1]. - Updated docs. -- +- [v3.1.0a1]: https://github.com/pybricks/pybricks-micropython/blob/master/CHANGELOG.md#310a1---2021-06-23 ## [1.0.0] - 2021-06-08 From 97c5a3bfb8c4420f920e94b1e389c5d44b4771e1 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 19 Nov 2021 16:30:17 -0600 Subject: [PATCH 4/6] settings: add hub name tooltip --- src/settings/SettingsDrawer.tsx | 77 ++++++++++++++++++++------------- src/settings/i18n.en.json | 1 + src/settings/i18n.ts | 1 + src/settings/settings.scss | 1 + 4 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/settings/SettingsDrawer.tsx b/src/settings/SettingsDrawer.tsx index d46254b6..a2452299 100644 --- a/src/settings/SettingsDrawer.tsx +++ b/src/settings/SettingsDrawer.tsx @@ -193,38 +193,53 @@ class SettingsDrawer extends React.PureComponent { /> - + boundary="window" + position={Position.LEFT} + targetTagName="div" + hoverOpenDelay={tooltipDelay} + > + + + e.preventDefault()} + className="pb-hub-name-input" + intent={ + isHubNameValid ? Intent.NONE : Intent.DANGER + } + placeholder="Pybricks Hub" + rightElement={ + isHubNameValid ? undefined : ( + + + + ) + } + /> diff --git a/src/settings/i18n.en.json b/src/settings/i18n.en.json index d70bda6f..66f0b25b 100644 --- a/src/settings/i18n.en.json +++ b/src/settings/i18n.en.json @@ -23,6 +23,7 @@ }, "hub-name": { "label": "Hub name", + "tooltip": "Hub name to use when flashing the firmware.", "error": { "tooltip": "The name is too long." } diff --git a/src/settings/i18n.ts b/src/settings/i18n.ts index 3644b2e3..69902f61 100644 --- a/src/settings/i18n.ts +++ b/src/settings/i18n.ts @@ -15,6 +15,7 @@ export enum SettingsStringId { FirmwareCurrentProgramLabel = 'settings.firmware.flash-current-program.label', FirmwareCurrentProgramTooltip = 'settings.firmware.flash-current-program.tooltip', FirmwareHubNameLabel = 'settings.firmware.hub-name.label', + FirmwareHubNameTooltip = 'settings.firmware.hub-name.tooltip', FirmwareHubNameErrorTooltip = 'settings.firmware.hub-name.error.tooltip', HelpTitle = 'settings.help.title', HelpProjectsLabel = 'settings.help.projects.label', diff --git a/src/settings/settings.scss b/src/settings/settings.scss index 25b015e3..fa4d7dba 100644 --- a/src/settings/settings.scss +++ b/src/settings/settings.scss @@ -7,4 +7,5 @@ .pb-hub-name-input .#{$ns}-input { width: 200px; + margin-left: 8px; } From b945a0d0d8739bd4099db28a23f955ce1e78742b Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 19 Nov 2021 20:07:55 -0600 Subject: [PATCH 5/6] CHANGELOG: copy changelog from docs --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 072af641..752e1530 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,11 @@ [support#500]: https://github.com/pybricks/support/issues/500 [support#501]: https://github.com/pybricks/support/issues/501 +- Updated docs: + + ### Added + - Added `ColorLightMatrix` class. + - Added `LWP3Device` class. [support#52]: https://github.com/pybricks/support/issues/52 From a1e3ffb814947bf0d8d4632aabc2dce61c222ab0 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 19 Nov 2021 20:53:16 -0600 Subject: [PATCH 6/6] v1.1.0-rc.1 --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 752e1530..d507db7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ## [Unreleased] +## [1.1.0-rc.1] - 2021-11-19 + ### Added - Hub name setting for selecting hub name when flashing firmware ([support#52]). @@ -192,7 +194,8 @@ Prerelease changes are documented at [support#48]. -[Unreleased]: https://github.com/pybricks/pybricks-code/compare/v1.1.0-beta.6...HEAD +[Unreleased]: https://github.com/pybricks/pybricks-code/compare/v1.1.0-rc.1...HEAD +[1.1.0-rc.1]: https://github.com/pybricks/pybricks-code/compare/v1.1.0-beta.6...v1.1.0-rc.1 [1.1.0-beta.6]: https://github.com/pybricks/pybricks-code/compare/v1.1.0-beta.5...v1.1.0-beta.6 [1.1.0-beta.5]: https://github.com/pybricks/pybricks-code/compare/v1.1.0-beta.4...v1.1.0-beta.5 [1.1.0-beta.4]: https://github.com/pybricks/pybricks-code/compare/v1.1.0-beta.3...v1.1.0-beta.4 diff --git a/package.json b/package.json index 6d199f90..7354c917 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pybricks/pybricks-code", - "version": "1.1.0-beta.6", + "version": "1.1.0-rc.1", "license": "MIT", "author": "The Pybricks Authors", "repository": {