diff --git a/frontend/src/__tests__/issue-268-project-roundtrip.test.ts b/frontend/src/__tests__/issue-268-project-roundtrip.test.ts new file mode 100644 index 00000000..322e5cb8 --- /dev/null +++ b/frontend/src/__tests__/issue-268-project-roundtrip.test.ts @@ -0,0 +1,269 @@ +// @vitest-environment jsdom +/** + * Issue #268 — "import project doesn't work": export a project to .zip, import + * it back, and the board is gone. Two reporters; the second noticed it "only + * works with wokwi components", which is the shape of it — the passives are + * real Wokwi part types and survive, the board was not. + * + * The bug was in the EXPORT. wokwiZip knew four boards and silently wrote + * `wokwi-arduino-uno`/`uno` for anything else, while the connections kept the + * real board's canvas id. The attached repro is exactly that: parts declare an + * `uno`, connections name `esp32`. + * + * These tests round-trip EVERY board kind, because that is the assertion the + * module never had: it was written when Velxio had four boards and nothing + * failed when it grew to thirty. + */ +import { describe, it, expect } from 'vitest'; +import JSZip from 'jszip'; +import { + buildWokwiDiagram, + importFromWokwiZip, + boardKindToWokwiType, + wokwiTypeToBoardKind, + retargetBoardWires, +} from '../utils/wokwiZip'; +import type { VelxioComponent } from '../utils/wokwiZip'; +import type { Wire } from '../types/wire'; +import { BOARD_KIND_LABELS, isKnownBoardKind, type BoardKind } from '../types/board'; + +const ALL_KINDS = Object.keys(BOARD_KIND_LABELS) as BoardKind[]; + +/** A board wired to a resistor and an LED — the reporter's circuit. */ +function project(boardCanvasId: string) { + const components: VelxioComponent[] = [ + { id: 'r-led', metadataId: 'resistor', x: 270, y: 155, properties: { value: '220' } }, + { id: 'led-ext', metadataId: 'led', x: 420, y: 140, properties: { color: 'red' } }, + ]; + const wire = ( + id: string, + a: [string, string], + b: [string, string], + color: string, + ): Wire => ({ + id, + start: { componentId: a[0], pinName: a[1], x: 0, y: 0 }, + end: { componentId: b[0], pinName: b[1], x: 0, y: 0 }, + waypoints: [], + color, + }); + const wires: Wire[] = [ + wire('w1', [boardCanvasId, '4'], ['r-led', '1'], '#e74c3c'), + wire('w2', ['r-led', '2'], ['led-ext', 'A'], '#e74c3c'), + wire('w3', ['led-ext', 'C'], [boardCanvasId, 'GND'], '#2c3e50'), + ]; + return { components, wires }; +} + +async function zipOf(diagram: unknown): Promise { + const zip = new JSZip(); + zip.file('diagram.json', JSON.stringify(diagram)); + zip.file('sketch.ino', 'void setup(){}\nvoid loop(){}\n'); + const blob = await zip.generateAsync({ type: 'blob' }); + return new File([blob], 'sketch.zip', { type: 'application/zip' }); +} + +describe('issue #268 — a project survives export and re-import', () => { + it.each(ALL_KINDS)('%s keeps its board and its wiring', async (kind) => { + const { components, wires } = project(kind); + const diagram = buildWokwiDiagram(components, wires, kind, { x: 50, y: 50 }, kind); + + // The board must never be written as some other board. + const boardPart = diagram.parts[0]; + expect(wokwiTypeToBoardKind(boardPart.type)).toBe(kind); + + // Every connection must name a part the diagram actually defines. This is + // the reporter's zip in one assertion: it declared `uno` and wired `esp32`. + const partIds = new Set(diagram.parts.map((p) => p.id)); + for (const [a, b] of diagram.connections) { + expect(partIds).toContain(a.slice(0, a.indexOf(':'))); + expect(partIds).toContain(b.slice(0, b.indexOf(':'))); + } + + const result = await importFromWokwiZip(await zipOf(diagram)); + expect(result.boardType).toBe(kind); + expect(result.warnings).toEqual([]); + // The board is not left lying around as a component. + expect(result.components.map((c) => c.id).sort()).toEqual(['led-ext', 'r-led']); + // And the wires point back at the board. + const onBoard = result.wires.filter( + (w) => w.start.componentId === kind || w.end.componentId === kind, + ); + expect(onBoard).toHaveLength(2); + }); + + it('lands the wires on the board that is actually on the canvas', async () => { + // Importing an ESP32 project onto a canvas whose board answers to + // 'arduino-uno' (a fresh project, re-kinded in place) must attach the + // wires to THAT id — setBoardType changes a board's kind, never its id. + const { components, wires } = project('esp32'); + const diagram = buildWokwiDiagram(components, wires, 'esp32', { x: 50, y: 50 }, 'esp32'); + const result = await importFromWokwiZip(await zipOf(diagram)); + expect(result.boardType).toBe('esp32'); + const retargeted = retargetBoardWires(result.wires, result.boardType!, 'arduino-uno'); + const ids = retargeted.flatMap((w) => [w.start.componentId, w.end.componentId]); + expect(ids).toContain('arduino-uno'); + expect(ids).not.toContain('esp32'); + // And the components are left alone. + expect(ids.filter((i) => i === 'r-led')).toHaveLength(2); + }); + + it('never answers with a board the file did not name', async () => { + // The old importer returned 'arduino-uno' for any diagram it did not + // recognise, which is how an ESP32 project came back as an Uno. + const diagram = { + version: 1, + author: 'someone', + editor: 'wokwi', + parts: [{ type: 'wokwi-something-unheard-of', id: 'x', top: 0, left: 0, attrs: {} }], + connections: [], + }; + const result = await importFromWokwiZip(await zipOf(diagram)); + expect(result.boardType).toBeNull(); + expect(result.warnings.join(' ')).toMatch(/No board recognised/); + }); + + it('says so when a diagram declares more than one board', async () => { + const diagram = { + version: 1, + author: 'Velxio', + editor: 'wokwi', + parts: [ + { type: boardKindToWokwiType('esp32'), id: 'esp32', top: 0, left: 0, attrs: {} }, + { type: boardKindToWokwiType('arduino-uno'), id: 'uno', top: 0, left: 200, attrs: {} }, + ], + connections: [], + }; + const result = await importFromWokwiZip(await zipOf(diagram)); + expect(result.boardType).toBe('esp32'); + expect(result.warnings.join(' ')).toMatch(/2 boards/); + }); + + it('still writes the Wokwi part type for boards a Wokwi element draws', () => { + // These four render through wokwi-elements, so their pin names are Wokwi's + // and the file genuinely opens over there. Changing them would break that. + expect(boardKindToWokwiType('arduino-uno')).toBe('wokwi-arduino-uno'); + expect(boardKindToWokwiType('arduino-nano')).toBe('wokwi-arduino-nano'); + expect(boardKindToWokwiType('arduino-mega')).toBe('wokwi-arduino-mega'); + expect(boardKindToWokwiType('raspberry-pi-pico')).toBe('wokwi-raspberry-pi-pico'); + }); + + it('round-trips a board this build has never heard of', async () => { + // Overlay boards are runtime-registered strings; the OSS module cannot + // enumerate them, so the type carries the kind instead of guessing. + const kind = 'some-pro-board-2350'; + const diagram = buildWokwiDiagram([], [], kind, { x: 50, y: 50 }, kind); + expect(diagram.parts[0].type).toBe('board-velxio-some-pro-board-2350'); + const result = await importFromWokwiZip(await zipOf(diagram)); + expect(result.boardType).toBe(kind); + }); +}); + +describe('#268 — what else the round trip was quietly dropping', () => { + it('brings a MicroPython project back with its code', async () => { + // CODE_EXTS admitted only the Arduino four, so a .py project imported + // into an empty editor. The export had been writing the files all along. + const zip = new JSZip(); + zip.file('diagram.json', JSON.stringify(buildWokwiDiagram([], [], 'esp32', { x: 50, y: 50 }, 'esp32'))); + zip.file('main.py', 'print("hello")\n'); + zip.file('lib.hpp', '#pragma once\n'); + const blob = await zip.generateAsync({ type: 'blob' }); + const result = await importFromWokwiZip(new File([blob], 's.zip', { type: 'application/zip' })); + expect(result.files.map((f) => f.name).sort()).toEqual(['lib.hpp', 'main.py']); + }); + + it('does not mistake metadata for source', async () => { + const zip = new JSZip(); + zip.file('diagram.json', JSON.stringify(buildWokwiDiagram([], [], 'esp32', { x: 50, y: 50 }, 'esp32'))); + zip.file('wokwi-project.txt', 'Exported from Velxio\n'); + zip.file('libraries.txt', 'Adafruit GFX Library\n'); + const blob = await zip.generateAsync({ type: 'blob' }); + const result = await importFromWokwiZip(new File([blob], 's.zip', { type: 'application/zip' })); + expect(result.files).toEqual([]); + expect(result.libraries).toEqual(['Adafruit GFX Library']); + }); + + it('refuses board kinds that are only there because of the prototype chain', () => { + // The kind comes out of a file someone sent you. `in` would have said yes + // to every one of these. + for (const bogus of ['toString', 'constructor', 'valueOf', '__proto__', 'hasOwnProperty']) { + expect(isKnownBoardKind(bogus)).toBe(false); + // …and the type still round-trips them as data, which is the point of + // validating separately from parsing. + expect(wokwiTypeToBoardKind(`board-velxio-${bogus}`)).toBe(bogus); + } + expect(isKnownBoardKind('esp32')).toBe(true); + expect(isKnownBoardKind('arduino-uno')).toBe(true); + }); +}); + +describe('#268 — the limit this fix does not lift', () => { + const led = (id: string, y: number): VelxioComponent => ({ + id, metadataId: 'led', x: 300, y, properties: {}, + }); + const to = (id: string, boardId: string, pin: string): Wire => ({ + id, + start: { componentId: boardId, pinName: pin, x: 0, y: 0 }, + end: { componentId: id.replace('w', 'led'), pinName: 'A', x: 0, y: 0 }, + waypoints: [], color: '#ff0000', + }); + + it('leaves another board\'s wires out instead of re-attaching them to this one', async () => { + // The failure this replaced was not a loss, it was a corruption: a second + // board of the SAME kind takes the part id this board's kind maps to, so + // led2 came back on led1's chip — on the same pin — and the file was + // internally consistent, so nothing complained. Verified across all 30 + // kinds during review. + const components = [led('led1', 100), led('led2', 200)]; + const wires = [to('w1', 'esp32', '4'), to('w2', 'esp32-2', '4')]; + const diagram = buildWokwiDiagram( + components, wires, 'esp32', { x: 50, y: 50 }, 'esp32-2', ['esp32'], + ); + const refs = diagram.connections.flatMap(([a, b]) => [a, b]); + // Only the exported board's own wire is written. + expect(refs).toContain('esp32:4'); + expect(refs.filter((r) => r.startsWith('esp32:'))).toHaveLength(1); + expect(refs).toContain('led2:A'); + expect(refs).not.toContain('led1:A'); + + const result = await importFromWokwiZip(await zipOf(diagram)); + // led1 is still on the canvas as a part; what it must NOT do is arrive + // wired to a board it was never connected to. + const onBoard = result.wires.filter( + (w) => w.start.componentId === 'esp32' || w.end.componentId === 'esp32', + ); + expect(onBoard).toHaveLength(1); + expect(onBoard[0].end.componentId).toBe('led2'); + }); + + it('announces the wires it cannot honour when a canvas had more than one board', async () => { + // Two boards of DIFFERENT kinds: the other one's wires name a part the + // diagram does not define, and the import says so. + const components = [led('led1', 100), led('led2', 200)]; + const wires = [to('w1', 'esp32', '4'), to('w2', 'arduino-uno', '5')]; + // Nothing declared foreign here — this is a file written before the + // export learned to leave them out. + const diagram = buildWokwiDiagram(components, wires, 'esp32', { x: 50, y: 50 }, 'esp32'); + const result = await importFromWokwiZip(await zipOf(diagram)); + expect(result.boardType).toBe('esp32'); + expect(result.warnings.join(' ')).toMatch(/arduino-uno/); + expect(result.warnings.join(' ')).toMatch(/attach to nothing/); + }); + + it('catches a same-kind stray too, which the first version of this check missed', async () => { + // The check used to allow the board KIND as a defined id, so a wire that + // literally names it — what a second board writes — passed as attached. + const diagram = { + version: 1, + author: 'Velxio', + editor: 'wokwi', + parts: [ + { type: boardKindToWokwiType('esp32'), id: 'esp32', top: 0, left: 0, attrs: {} }, + { type: 'wokwi-led', id: 'led2', top: 100, left: 300, attrs: {} }, + ], + connections: [['esp32-2:4', 'led2:A', 'red', []]], + }; + const result = await importFromWokwiZip(await zipOf(diagram)); + expect(result.warnings.join(' ')).toMatch(/esp32-2/); + }); +}); diff --git a/frontend/src/components/editor/EditorToolbar.tsx b/frontend/src/components/editor/EditorToolbar.tsx index 885f1122..1303bc9e 100644 --- a/frontend/src/components/editor/EditorToolbar.tsx +++ b/frontend/src/components/editor/EditorToolbar.tsx @@ -10,7 +10,7 @@ import { type VerificationResult } from '../../simulation/verify/circuitVerifier import { verifyCircuitFromStore } from '../../simulation/verify/verifyFromStore'; import { CircuitVerificationModal } from '../simulator/CircuitVerificationModal'; import type { BoardKind, LanguageMode } from '../../types/board'; -import { BOARD_KIND_FQBN, BOARD_SUPPORTS_ESPIDF, BOARD_SUPPORTS_MICROPYTHON, isPiBoardKind, boardDisplayName } from '../../types/board'; +import { BOARD_KIND_FQBN, BOARD_SUPPORTS_ESPIDF, BOARD_SUPPORTS_MICROPYTHON, isKnownBoardKind, isPiBoardKind, boardDisplayName } from '../../types/board'; import { compileCode } from '../../services/compilation'; import { compileOptionsForBoard } from '../../utils/boardCompile'; import { @@ -29,7 +29,7 @@ import { InstallLibrariesModal } from '../simulator/InstallLibrariesModal'; import { mergeSuggestedLibraries } from '../../utils/libraryManifest'; import { parseCompileResult, isNoiseBuildLine } from '../../utils/compilationLogger'; import type { CompilationLog, CompileTarget } from '../../utils/compilationLogger'; -import { exportToWokwiZip } from '../../utils/wokwiZip'; +import { exportToWokwiZip, retargetBoardWires } from '../../utils/wokwiZip'; import { importProjectFile, PROJECT_FILE_ACCEPT } from '../../utils/importProject'; import { readFirmwareFile } from '../../utils/firmwareLoader'; import { @@ -1224,15 +1224,47 @@ export const EditorToolbar = ({ const handleExport = async () => { try { - const { - components, - wires, - boardPosition, - boardType: legacyBoardType, - } = useSimulatorStore.getState(); + const { components, wires, boards, activeBoardId, boardPosition, boardType } = + useSimulatorStore.getState(); + // The board itself, not the flat legacy mirror. `boardType` only tracks + // the active board through setActiveBoardId/setBoardType, so a project + // hydrated straight into `boards` leaves it stale — and it reports every + // Raspberry Pi as an 'arduino-uno' on purpose (see setActiveBoardId). + // Exporting from it wrote the wrong board into the file; taking the kind + // and the canvas id off one object means they cannot disagree (#268). + const board = boards.find((b) => b.id === activeBoardId) ?? boards[0]; const projectName = files.find((f) => f.name.endsWith('.ino'))?.name.replace('.ino', '') || 'velxio-project'; - await exportToWokwiZip(files, components, wires, legacyBoardType, projectName, boardPosition); + // The other boards cannot travel: the format stores one. Their wires are + // left out rather than written against this board's part id, which used + // to re-attach their components to this chip on import — same pins, wrong + // board, nothing said (#268 review). + const foreignBoardIds = boards.filter((b) => b.id !== board?.id).map((b) => b.id); + const foreign = new Set(foreignBoardIds); + const strandedWires = wires.filter( + (w) => foreign.has(w.start.componentId) || foreign.has(w.end.componentId), + ).length; + await exportToWokwiZip( + files, + components, + wires, + board?.boardKind ?? boardType, + projectName, + board ? { x: board.x, y: board.y } : boardPosition, + board?.id, + board?.libraries ?? [], + foreignBoardIds, + ); + if (foreignBoardIds.length > 0) { + setMessage({ + type: 'error', + text: + `Exported the ${boardDisplayName(board!)} only — a .zip holds one board, so the ` + + `other ${foreignBoardIds.length === 1 ? 'board' : `${foreignBoardIds.length} boards`}` + + `${strandedWires > 0 ? ` and ${strandedWires} wire${strandedWires === 1 ? '' : 's'}` : ''}` + + ` did not travel. Save as .vlx to keep the whole project.`, + }); + } } catch (err) { setMessage({ type: 'error', text: 'Export failed.' }); } @@ -1398,12 +1430,62 @@ export const EditorToolbar = ({ const { setComponents, setWires, setBoardType, setBoardPosition, stopSimulation } = useSimulatorStore.getState(); stopSimulation(); - if (result.boardType) setBoardType(result.boardType); + // A board kind this build does not know is left alone rather than + // coerced. The importer no longer answers 'arduino-uno' for everything + // it fails to recognise (#268), so say what happened instead of swapping + // the user's board for one the file never mentioned. + const importWarnings = [...result.warnings]; + // Put the board on the canvas. `setBoardType` re-kinds the ACTIVE board, + // so on an empty canvas it changed nothing and the project arrived with + // its circuit and no chip — the reporter's "the board isn't recognized" + // (#268). Adding one when there is none is the other half of the fix. + let boardId: string | null = null; + if (result.boardType && isKnownBoardKind(result.boardType)) { + const sim = useSimulatorStore.getState(); + const current = + sim.boards.find((b) => b.id === sim.activeBoardId) ?? sim.boards[0] ?? null; + if (current) { + setBoardType(result.boardType); + boardId = current.id; + } else { + boardId = sim.addBoard( + result.boardType, + result.boardPosition.x, + result.boardPosition.y, + ); + // addBoard promotes the first board to active but does not sync the + // flat legacy fields; setActiveBoardId is where that happens, and + // whatever still reads `boardType` would otherwise see the board + // this import just replaced. + useSimulatorStore.getState().setActiveBoardId(boardId); + } + } else if (result.boardType) { + // Nothing to swap the board for, so the circuit lands on whatever is + // already there — and its wires have to be told, or the message would + // be describing something that did not happen. + const sim = useSimulatorStore.getState(); + boardId = (sim.boards.find((b) => b.id === sim.activeBoardId) ?? sim.boards[0])?.id ?? null; + importWarnings.push( + boardId + ? `This project is for a "${result.boardType}" board, which this build does not have. The circuit was imported onto the current board.` + : `This project is for a "${result.boardType}" board, which this build does not have, and there is no board on the canvas to put the circuit on.`, + ); + } setBoardPosition(result.boardPosition); setComponents(result.components); - setWires(result.wires); + // The wires name the board by its kind; the board on the canvas may + // answer to something else. + setWires( + boardId && result.boardType + ? retargetBoardWires(result.wires, result.boardType, boardId) + : result.wires, + ); if (result.files.length > 0) loadFiles(result.files); - setMessage({ type: 'success', text: `Imported ${file.name}` }); + setMessage( + importWarnings.length > 0 + ? { type: 'error', text: `Imported ${file.name} — ${importWarnings.join(' ')}` } + : { type: 'success', text: `Imported ${file.name}` }, + ); if (result.libraries.length > 0) { setPendingLibraries(result.libraries); setInstallModalOpen(true); diff --git a/frontend/src/components/editor/FileExplorer.tsx b/frontend/src/components/editor/FileExplorer.tsx index c417b8cb..2dc377f7 100644 --- a/frontend/src/components/editor/FileExplorer.tsx +++ b/frontend/src/components/editor/FileExplorer.tsx @@ -12,8 +12,9 @@ import { DEFAULT_CHIP_PROGRAM_C, } from '../../services/romCompileService'; import type { BoardKind } from '../../types/board'; -import { boardDisplayName, isPiBoardKind } from '../../types/board'; +import { boardDisplayName, isKnownBoardKind, isPiBoardKind } from '../../types/board'; import { importProjectFile, PROJECT_FILE_ACCEPT } from '../../utils/importProject'; +import { retargetBoardWires } from '../../utils/wokwiZip'; import { showMessageDialog, showConfirmDialog } from '../../store/useMessageDialogStore'; import { registerEditorCommand } from '../../lib/editorCommands'; import './FileExplorer.css'; @@ -423,10 +424,44 @@ export const FileExplorer: React.FC = ({ onSaveClick, onNewCl const { setComponents, setWires, setBoardType, setBoardPosition, stopSimulation } = useSimulatorStore.getState(); stopSimulation(); - if (result.boardType) setBoardType(result.boardType); + // Same rule as the toolbar's importer: an unknown board kind is left + // alone, not coerced into an Uno (#268). + // Same as the toolbar's importer: put the board on the canvas (an + // empty one had nothing for setBoardType to re-kind, so the project + // arrived without its chip — #268) and never coerce an unknown kind. + let boardId: string | null = null; + if (result.boardType && isKnownBoardKind(result.boardType)) { + const sim = useSimulatorStore.getState(); + const current = + sim.boards.find((b) => b.id === sim.activeBoardId) ?? sim.boards[0] ?? null; + if (current) { + setBoardType(result.boardType); + boardId = current.id; + } else { + boardId = sim.addBoard( + result.boardType, + result.boardPosition.x, + result.boardPosition.y, + ); + // addBoard promotes the first board to active but does not sync the + // flat legacy fields; setActiveBoardId is where that happens, and + // whatever still reads `boardType` would otherwise see the board + // this import just replaced. + useSimulatorStore.getState().setActiveBoardId(boardId); + } + } else if (result.boardType) { + console.warn( + `[FileExplorer] Project is for a "${result.boardType}" board, which this build does not have — kept the current board.`, + ); + } + for (const w of result.warnings) console.warn(`[FileExplorer] ${w}`); setBoardPosition(result.boardPosition); setComponents(result.components); - setWires(result.wires); + setWires( + boardId && result.boardType + ? retargetBoardWires(result.wires, result.boardType, boardId) + : result.wires, + ); if (result.files.length > 0) loadFiles(result.files); if (result.libraries.length > 0) { console.warn( diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts index ca5881e3..a0049d27 100644 --- a/frontend/src/store/useSimulatorStore.ts +++ b/frontend/src/store/useSimulatorStore.ts @@ -125,21 +125,23 @@ const I2C_SENSOR_MAP: Record< }; // ── Legacy type aliases (keep external consumers working) ────────────────── -export type BoardType = 'arduino-uno' | 'arduino-nano' | 'arduino-mega' | 'raspberry-pi-pico'; +/** + * The active board's kind. + * + * This was a four-literal union left over from when those were the only boards + * Velxio had, while setBoardType below already handled the whole space by + * casting to BoardKind. The stale narrow type is one of the three copies that + * made an ESP32 project unimportable (#268) — the zip importer and the import + * dispatcher each carried their own. It is BoardKind, which is what it always + * was at runtime; the overlay's kinds register into that union too. + */ +export type BoardType = BoardKind; -export const BOARD_FQBN: Record = { - 'arduino-uno': 'arduino:avr:uno', - 'arduino-nano': 'arduino:avr:nano:cpu=atmega328', - 'arduino-mega': 'arduino:avr:mega', - 'raspberry-pi-pico': 'rp2040:rp2040:rpipico', -}; - -export const BOARD_LABELS: Record = { - 'arduino-uno': 'Arduino Uno', - 'arduino-nano': 'Arduino Nano', - 'arduino-mega': 'Arduino Mega 2560', - 'raspberry-pi-pico': 'Raspberry Pi Pico', -}; +// BOARD_FQBN and BOARD_LABELS used to live here: two more four-board records +// from the same era, exported and imported by nothing. The names that are +// actually used are BOARD_KIND_LABELS (types/board.ts, exhaustive over +// BoardKind) and the fqbn each board declares. Removed rather than widened — +// a fifth stale copy of "Velxio has four boards" is not worth keeping. export const DEFAULT_BOARD_POSITION = { x: 50, y: 50 }; export const ARDUINO_POSITION = DEFAULT_BOARD_POSITION; diff --git a/frontend/src/types/board.ts b/frontend/src/types/board.ts index 37e1ff0a..77d38fa4 100644 --- a/frontend/src/types/board.ts +++ b/frontend/src/types/board.ts @@ -1,3 +1,5 @@ +import { getProBoard } from '../lib/proBoardRegistry'; + export type BoardKind = | 'arduino-uno' | 'arduino-nano' @@ -171,6 +173,27 @@ export interface BoardInstance { libraries?: string[]; } +/** + * Is this string a board kind Velxio can actually put on a canvas? + * + * BOARD_KIND_LABELS is exhaustive over the OSS union by construction (the + * compiler enforces the Record), and the overlay's kinds live in the pro + * registry, so the two together are the whole space. Callers that receive a + * kind from OUTSIDE the app — an imported project file, a URL, a saved + * document — need this: importing a diagram used to answer 'arduino-uno' for + * anything it did not recognise, and the user got an Uno with no explanation + * (issue #268). + */ +export function isKnownBoardKind(kind: string): kind is BoardKind { + // hasOwnProperty, not `in`: `in` walks the prototype chain, so 'toString', + // 'constructor' and '__proto__' would all pass — and the strings reaching + // here come from files people send each other. + return ( + Object.prototype.hasOwnProperty.call(BOARD_KIND_LABELS, kind) || + getProBoard(kind) !== undefined + ); +} + export const BOARD_KIND_LABELS: Record = { 'arduino-uno': 'Arduino Uno', 'arduino-nano': 'Arduino Nano', diff --git a/frontend/src/utils/importProject.ts b/frontend/src/utils/importProject.ts index e8c9862e..907799e4 100644 --- a/frontend/src/utils/importProject.ts +++ b/frontend/src/utils/importProject.ts @@ -19,9 +19,9 @@ * place to extend if a third format ever shows up. */ -import type { Wire } from '../types/wire'; import { importVlxFile, VlxParseError } from './vlxFile'; -import { importFromWokwiZip, type VelxioComponent } from './wokwiZip'; +import { importFromWokwiZip } from './wokwiZip'; +import type { ImportResult } from './wokwiZip'; /** * Common result shape: `kind` tells callers which path ran, so they can @@ -30,15 +30,11 @@ import { importFromWokwiZip, type VelxioComponent } from './wokwiZip'; */ export type ProjectImportResult = | { kind: 'vlx' } - | { - kind: 'zip'; - boardType: 'arduino-uno' | 'arduino-nano' | 'arduino-mega' | 'raspberry-pi-pico'; - boardPosition: { x: number; y: number }; - components: VelxioComponent[]; - wires: Wire[]; - files: Array<{ name: string; content: string }>; - libraries: string[]; - }; + // Derived from ImportResult rather than restated. The zip branch used to + // hand-copy a four-literal board union that the importer also declared, so + // the two could disagree — and widening one without the other is a compile + // error nobody would connect to a lost board (#268). + | ({ kind: 'zip' } & ImportResult); /** * Detect format from the filename + MIME type and dispatch to the right diff --git a/frontend/src/utils/wokwiZip.ts b/frontend/src/utils/wokwiZip.ts index eefbcbb8..7be0769c 100644 --- a/frontend/src/utils/wokwiZip.ts +++ b/frontend/src/utils/wokwiZip.ts @@ -43,43 +43,94 @@ export interface VelxioComponent { } export interface ImportResult { - boardType: 'arduino-uno' | 'arduino-nano' | 'arduino-mega' | 'raspberry-pi-pico'; + /** + * The board kind the diagram declares, or null when it declares none. + * + * A plain string, not the BoardKind union: the overlay registers its board + * kinds at runtime (proBoardRegistry), so a closed union here would make + * every pro board unimportable by construction. Null rather than a default: + * answering 'arduino-uno' for anything unrecognised is what made an ESP32 + * project come back as an Uno (#268). + */ + boardType: string | null; boardPosition: { x: number; y: number }; components: VelxioComponent[]; wires: Wire[]; files: Array<{ name: string; content: string }>; /** Library names parsed from libraries.txt. Includes both standard Arduino Library Manager names and Wokwi-hosted entries in the form "LibName@wokwi:hash". */ libraries: string[]; + /** + * What the import could not do faithfully, in words a user can act on. + * Everything here used to happen silently. + */ + warnings: string[]; } // ── Board mappings ──────────────────────────────────────────────────────────── -// Wokwi board type → Velxio boardType -const WOKWI_TYPE_TO_BOARD: Record< - string, - 'arduino-uno' | 'arduino-nano' | 'arduino-mega' | 'raspberry-pi-pico' -> = { - 'wokwi-arduino-uno': 'arduino-uno', - 'wokwi-arduino-nano': 'arduino-nano', - 'wokwi-arduino-mega': 'arduino-mega', - 'wokwi-raspberry-pi-pico': 'raspberry-pi-pico', +/** + * Boards a real Wokwi element draws, and the Wokwi part type written for them. + * + * The membership rule is not taste, it is pin names. A diagram's connections + * are `partId:pinName`, and the pin names Velxio wires carry are whatever the + * element on the canvas calls its pins. These four render through + * wokwi-elements, so their names ARE Wokwi's — an Uno wire says `GND.2`, `A4`, + * `5V` — and a file naming them opens correctly over there. + * + * Every other board renders through a `velxio-*` element with our own names: + * our ESP32 calls a pin `34` where Wokwi's esp32-devkit-v1 calls it `D34`. + * Claiming a Wokwi board type for one of those would produce a file that loads + * in Wokwi and silently attaches every wire to a pin that does not exist — + * the same class of quiet wrongness this table caused in issue #268. So they + * get a Velxio type instead, and the file is honest about what it is. + * + * The split is visible elsewhere: BOARD_TAG in ComponentPickerModal.tsx names + * exactly these four with a `wokwi-` tag and everything else with `velxio-`. + */ +const WOKWI_NATIVE_BOARDS: Record = { + 'arduino-uno': { type: 'wokwi-arduino-uno', id: 'uno' }, + 'arduino-nano': { type: 'wokwi-arduino-nano', id: 'nano' }, + 'arduino-mega': { type: 'wokwi-arduino-mega', id: 'mega' }, + // Velxio draws its Pico with wokwi-nano-rp2040-connect, but the type written + // here has always been wokwi-raspberry-pi-pico. Kept verbatim so zips + // exported before this change still import. + 'raspberry-pi-pico': { type: 'wokwi-raspberry-pi-pico', id: 'pico' }, }; -// Velxio boardType → Wokwi type -const BOARD_TO_WOKWI_TYPE: Record = { - 'arduino-uno': 'wokwi-arduino-uno', - 'arduino-nano': 'wokwi-arduino-nano', - 'arduino-mega': 'wokwi-arduino-mega', - 'raspberry-pi-pico': 'wokwi-raspberry-pi-pico', -}; +/** + * Part type for every other board — the ESP32 family, the STM32s, the Pi + * boards, and the overlay's boards, which are runtime-registered strings the + * OSS build has never heard of. `board-` is Wokwi's own prefix for a custom + * board; the `velxio-` namespace says whose it is, and the kind rides along so + * the round trip is exact for a board nothing in this file knows about. + */ +const VELXIO_BOARD_TYPE_PREFIX = 'board-velxio-'; -// Velxio boardType → default Wokwi part id -const BOARD_TO_WOKWI_ID: Record = { - 'arduino-uno': 'uno', - 'arduino-nano': 'nano', - 'arduino-mega': 'mega', - 'raspberry-pi-pico': 'pico', -}; +/** The Wokwi part type to write for a Velxio board kind. Never guesses. */ +export function boardKindToWokwiType(kind: string): string { + return WOKWI_NATIVE_BOARDS[kind]?.type ?? `${VELXIO_BOARD_TYPE_PREFIX}${kind}`; +} + +/** The part id to write for a Velxio board kind (Wokwi's short names). */ +export function boardKindToWokwiPartId(kind: string): string { + return WOKWI_NATIVE_BOARDS[kind]?.id ?? kind; +} + +/** + * The Velxio board kind a Wokwi part type denotes, or null when the part is + * not a board. Null is the point: the old code answered 'arduino-uno' for + * everything it did not recognise, so importing an ESP32 project produced an + * Uno and the real board vanished (#268). + */ +export function wokwiTypeToBoardKind(type: string): string | null { + if (type.startsWith(VELXIO_BOARD_TYPE_PREFIX)) { + return type.slice(VELXIO_BOARD_TYPE_PREFIX.length) || null; + } + for (const [kind, entry] of Object.entries(WOKWI_NATIVE_BOARDS)) { + if (entry.type === type) return kind; + } + return null; +} // ── Pin name aliases ───────────────────────────────────────────────────────── @@ -183,18 +234,43 @@ export function parseLibrariesTxt(content: string): string[] { // ── Export ──────────────────────────────────────────────────────────────────── -export async function exportToWokwiZip( - files: Array<{ name: string; content: string }>, +/** + * The diagram a project becomes, as a value. + * + * Split out of exportToWokwiZip so the conversion can be tested at all: the + * export function returns void and downloads through the DOM, so for its whole + * life nothing could assert on what it produced — which is why a board silently + * exporting as the wrong Arduino went unnoticed (#268). + */ +export function buildWokwiDiagram( components: VelxioComponent[], wires: Wire[], boardType: string, - projectName: string, boardPosition: { x: number; y: number } = { x: 50, y: 50 }, -): Promise { - const zip = new JSZip(); - - const boardWokwiType = BOARD_TO_WOKWI_TYPE[boardType] ?? 'wokwi-arduino-uno'; - const boardId = BOARD_TO_WOKWI_ID[boardType] ?? 'uno'; + /** + * The board's id ON THE CANVAS, which is what its wires name. addBoard gives + * the first board of a kind the kind itself and later ones `${kind}-2` + * (useSimulatorStore.ts:1508), so the kind is the right default but not a + * safe assumption — the caller knows. The old code compared against three + * hardcoded ids, one of them ('nano-rp2040') no longer issued to anything + * and one board ('arduino-mega') missing outright, so those boards' wires + * were written pointing at a part id the diagram never defines. + */ + boardCanvasId: string = boardType, + /** + * The canvas ids of the OTHER boards in this project. + * + * The format stores one board, so a wire touching a different board cannot + * be represented — and writing it anyway was worse than dropping it. The + * board being exported takes the part id its KIND maps to, which for every + * non-Arduino family IS the first-of-kind canvas id, so a second ESP32's + * wires collided with it and both boards' components came back attached to + * the same chip. Silently, because the file was internally consistent. + */ + foreignBoardIds: readonly string[] = [], +): WokwiDiagram { + const boardWokwiType = boardKindToWokwiType(boardType); + const boardId = boardKindToWokwiPartId(boardType); // Build parts — board first, then user components // Subtract boardPosition so coords are relative to the board @@ -216,17 +292,14 @@ export async function exportToWokwiZip( ]; // Build connections - const connections: [string, string, string, string[]][] = wires.map((w) => { - const isBoardStart = - w.start.componentId === 'arduino-uno' || - w.start.componentId === 'arduino-nano' || - w.start.componentId === 'nano-rp2040'; - const isBoardEnd = - w.end.componentId === 'arduino-uno' || - w.end.componentId === 'arduino-nano' || - w.end.componentId === 'nano-rp2040'; - const startId = isBoardStart ? boardId : w.start.componentId; - const endId = isBoardEnd ? boardId : w.end.componentId; + const foreign = new Set(foreignBoardIds.filter((id) => id !== boardCanvasId)); + const connections: [string, string, string, string[]][] = wires + .filter((w) => !foreign.has(w.start.componentId) && !foreign.has(w.end.componentId)) + .map((w) => { + // An endpoint on the board is rewritten to the board's part id; everything + // else keeps its component id, which is the same on both sides. + const startId = w.start.componentId === boardCanvasId ? boardId : w.start.componentId; + const endId = w.end.componentId === boardCanvasId ? boardId : w.end.componentId; // Breadboard seating wires round-trip as Wokwi's hidden `["$bb"]` entries. if (w.bb) { return [`${startId}:${w.start.pinName}`, `${endId}:${w.end.pinName}`, '', ['$bb']]; @@ -239,19 +312,53 @@ export async function exportToWokwiZip( ]; }); - const diagram: WokwiDiagram = { + return { version: 1, author: 'Velxio', editor: 'wokwi', parts, connections, }; +} + +export async function exportToWokwiZip( + files: Array<{ name: string; content: string }>, + components: VelxioComponent[], + wires: Wire[], + boardType: string, + projectName: string, + boardPosition: { x: number; y: number } = { x: 50, y: 50 }, + boardCanvasId: string = boardType, + /** + * Library names to declare. The importer has always read libraries.txt and + * offered to install what it finds; the exporter never wrote one, so a + * project's libraries did not survive its own round trip. + */ + libraries: string[] = [], + /** The other boards on the canvas — see buildWokwiDiagram. */ + foreignBoardIds: readonly string[] = [], +): Promise { + const zip = new JSZip(); + const diagram = buildWokwiDiagram( + components, + wires, + boardType, + boardPosition, + boardCanvasId, + foreignBoardIds, + ); zip.file('diagram.json', JSON.stringify(diagram, null, 2)); zip.file( 'wokwi-project.txt', `Exported from Velxio\n\nSimulate this project on https://velxio.dev\n`, ); + if (libraries.length > 0) { + zip.file( + 'libraries.txt', + `# Wokwi Library List\n# See https://docs.wokwi.com/guides/libraries\n\n${libraries.join('\n')}\n`, + ); + } for (const f of files) { zip.file(f.name, f.content); @@ -268,6 +375,24 @@ export async function exportToWokwiZip( setTimeout(() => URL.revokeObjectURL(url), 1000); } +/** + * Point the board's wires at the id the board actually got. + * + * importFromWokwiZip names them by the board's KIND; the canvas may have + * called it something else — a second board of that kind is `${kind}-2`, and a + * project imported onto an existing board keeps that board's id whatever its + * new kind. Wires naming an id nothing answers to is the failure this whole + * issue is made of. + */ +export function retargetBoardWires(wires: Wire[], fromId: string, toId: string): Wire[] { + if (!fromId || fromId === toId) return wires; + return wires.map((w) => ({ + ...w, + start: w.start.componentId === fromId ? { ...w.start, componentId: toId } : w.start, + end: w.end.componentId === fromId ? { ...w.end, componentId: toId } : w.end, + })); +} + // ── Import ──────────────────────────────────────────────────────────────────── export async function importFromWokwiZip(file: File): Promise { @@ -280,25 +405,54 @@ export async function importFromWokwiZip(file: File): Promise { const diagramText = await diagramEntry.async('string'); const diagram: WokwiDiagram = JSON.parse(diagramText); - // Detect board - const boardPart = diagram.parts.find((p) => WOKWI_TYPE_TO_BOARD[p.type]); - const boardType = boardPart ? WOKWI_TYPE_TO_BOARD[boardPart.type] : 'arduino-uno'; - const boardId = boardPart?.id ?? 'uno'; + const warnings: string[] = []; - // Velxio internal component ID for the board element (must match DOM element id) - const VELXIO_BOARD_ID: Record = { - 'arduino-uno': 'arduino-uno', - 'arduino-nano': 'arduino-nano', - 'arduino-mega': 'arduino-mega', - 'raspberry-pi-pico': 'nano-rp2040', - }; - const velxioBoardId = VELXIO_BOARD_ID[boardType] ?? 'arduino-uno'; + // Detect the board. Parts that name a board come first; anything else is a + // component. + const boardParts = diagram.parts.filter((p) => wokwiTypeToBoardKind(p.type) !== null); + const boardPart = boardParts[0] ?? null; + const boardType = boardPart ? wokwiTypeToBoardKind(boardPart.type) : null; + + if (!boardPart) { + // Do NOT invent an Uno. A diagram whose board type we do not know is a + // diagram whose board we would silently replace, and the user would see a + // circuit wired to the wrong chip with no hint why. + const unknown = [...new Set(diagram.parts.map((p) => p.type))].join(', '); + warnings.push( + diagram.parts.length === 0 + ? 'The diagram has no parts, so no board could be identified.' + : `No board recognised in the diagram (parts: ${unknown}). The circuit was imported; add the board yourself and its wires will attach.`, + ); + } + if (boardParts.length > 1) { + // Velxio canvases hold several boards, but this format writes one, and the + // extras used to be dropped without a word — not even as components. + warnings.push( + `The diagram declares ${boardParts.length} boards; only the first (${boardType}) was imported.`, + ); + } + + const boardId = boardPart?.id ?? ''; + + // Wires that touched the board are named by its KIND, which is the id + // addBoard gives the first board of a kind (useSimulatorStore.ts:1508). + // Whoever applies this result knows the id the board really ends up with and + // finishes the job with retargetBoardWires — the importer cannot, because + // the kind is not known until the diagram is parsed. The old code used a + // hardcoded map that answered 'nano-rp2040' for a Pico, an id nothing has + // been issued since boards became instances, so every imported Pico wire + // attached to a component that does not exist. + const velxioBoardId = boardType ?? ''; // Board position from diagram. Apply a minimum offset so the board is never // crammed against the canvas top-left corner (Wokwi diagrams often use 0,0). + // With no board part there is no origin to measure from, so the components + // keep their own coordinates and boardPosition follows them. Defaulting the + // raw origin to MIN_OFFSET while leaving the offsets at 0 (what this did + // before) put the board 50px from the corner and the circuit somewhere else. const MIN_OFFSET = 50; - const rawBoardX = boardPart?.left ?? MIN_OFFSET; - const rawBoardY = boardPart?.top ?? MIN_OFFSET; + const rawBoardX = boardPart?.left ?? 0; + const rawBoardY = boardPart?.top ?? 0; const offsetX = rawBoardX < MIN_OFFSET ? MIN_OFFSET - rawBoardX : 0; const offsetY = rawBoardY < MIN_OFFSET ? MIN_OFFSET - rawBoardY : 0; const boardPosition = { @@ -309,7 +463,7 @@ export async function importFromWokwiZip(file: File): Promise { // Convert non-board parts to Velxio components. // Apply the same offset so components keep their relative position to the board. const components: VelxioComponent[] = diagram.parts - .filter((p) => !WOKWI_TYPE_TO_BOARD[p.type]) + .filter((p) => wokwiTypeToBoardKind(p.type) === null) .map((p) => ({ id: p.id, metadataId: wokwiTypeToMetadataId(p.type), @@ -360,8 +514,38 @@ export async function importFromWokwiZip(file: File): Promise { }; }); - // Read code files (.ino, .h, .cpp, .c) - const CODE_EXTS = new Set(['.ino', '.h', '.cpp', '.c']); + // A connection naming a part the diagram never defines attaches to nothing. + // Diagrams written by Velxio before #268 are exactly this: the board part + // says `uno` while the connections say `esp32`, because the export wrote the + // board's type from one table and the wires' ids from another. Importing one + // is not recoverable — the real board is not in the file — but arriving with + // half a circuit and no explanation is worse than being told. + // + // Checked against the RAW diagram, not the wires built above. Those have + // already had the board's id rewritten to the board KIND, and seeding the + // allowed set with that kind (what this did first) let a wire that literally + // names the kind pass as defined — which is exactly the wire a second board + // on the canvas writes. The check went quiet on the one case worth catching. + const definedIds = new Set(diagram.parts.map((p) => p.id)); + const dangling = new Set(); + for (const [a, b] of diagram.connections) { + for (const ref of [a, b]) { + const id = ref.includes(':') ? ref.slice(0, ref.indexOf(':')) : ref; + if (id && !definedIds.has(id)) dangling.add(id); + } + } + if (dangling.size > 0) { + warnings.push( + `Some wires refer to parts the diagram does not contain (${[...dangling].join(', ')}); they were imported but attach to nothing.`, + ); + } + + // Read code files. The export writes every file in the project verbatim, so + // this list is the one that decides what comes back — and it used to admit + // only the Arduino four, which meant a MicroPython project round-tripped + // with an empty editor. Deliberately not '.txt': libraries.txt and + // wokwi-project.txt are metadata, read separately. + const CODE_EXTS = new Set(['.ino', '.h', '.hpp', '.cpp', '.cc', '.cxx', '.c', '.py', '.s']); const files: Array<{ name: string; content: string }> = []; for (const [filename, entry] of Object.entries(zip.files)) { @@ -390,5 +574,5 @@ export async function importFromWokwiZip(file: File): Promise { libraries.push(...parseLibrariesTxt(await libEntry.async('string'))); } - return { boardType, boardPosition, components, wires, files, libraries }; + return { boardType, boardPosition, components, wires, files, libraries, warnings }; }