Adds initial support for the x3 device#25
Adds initial support for the x3 device#25itsthisjustin wants to merge 4 commits intocrosspoint-reader:masterfrom
Conversation
Note: The flash crosspoint/english/chinese firmware buttons have not been updated to support x3 yet
📝 WalkthroughWalkthroughAdds selectable device model state to the UI and hook; EspController and firmware fetching now support per-model partition layouts and model-aware firmware retrieval. Partition-table validation, reset/disconnect behavior, firmware caching, and restart instructions vary by the selected device model. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Home (UI)
participant Hook as useEspOperations
participant Controller as EspController
participant Device as ESP Device
participant Remote as Firmware Remote
UI->>Hook: setDeviceModel('x3' or 'x4')
Hook->>Hook: update deviceModel state
UI->>Hook: start identification/start flow
Hook->>Controller: fromRequestedDevice(partitionLayout for deviceModel)
Controller->>Device: connect & read partition table
Device-->>Controller: partition table data
Controller-->>Hook: partition data
Hook->>Hook: matchesPartitionTable(data, x3/x4)
alt matches x3
Hook->>Controller: setPartitionLayout(X3_PARTITION_LAYOUT)
else matches x4
Hook->>Controller: layout remains X4
end
Hook->>Remote: getOfficialFirmwareVersions(deviceModel)
Remote-->>Hook: firmware versions (model-specific)
Hook->>Controller: disconnect(skipReset: deviceModel === 'x3')
Controller->>Device: reset or skip reset
Hook-->>UI: update stepData / isRunning
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/esp/useEspOperations.ts (3)
208-237: Duplicated validation logic — consider extracting to a helper.This partition table validation logic is identical to lines 98-127. Extracting it to a shared helper function would reduce duplication and ease maintenance.
♻️ Example helper extraction
async function validateAndSetPartitionLayout( espController: EspController, deviceModel: DeviceModel, ) { const partitionTable = await espController.readPartitionTable(); const validTables = deviceModel === 'x3' ? [x3PartitionTable, x4PartitionTable] : [x4PartitionTable]; const matched = validTables.find((t) => matchesPartitionTable(partitionTable, t), ); if (!matched) { throw new Error( `Unexpected partition configuration for ${deviceModel.toUpperCase()}. Make sure you've selected the correct device model.\nGot ${JSON.stringify( partitionTable, null, 2, )}`, ); } espController.setPartitionLayout( matched === x3PartitionTable ? X3_PARTITION_LAYOUT : X4_PARTITION_LAYOUT, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/esp/useEspOperations.ts` around lines 208 - 237, Extract the duplicated partition-table validation block into a helper (e.g., validateAndSetPartitionLayout) that accepts (espController, deviceModel), calls espController.readPartitionTable(), builds validTables using x3PartitionTable/x4PartitionTable, finds a match via matchesPartitionTable, throws the same Error if no match, and calls espController.setPartitionLayout(...) with X3_PARTITION_LAYOUT or X4_PARTITION_LAYOUT based on the matched table; then replace both inline blocks with calls to this helper.
107-126: Consider using the already-matched table to avoid redundant comparison.The code finds
matchedbut then re-runsmatchesPartitionTableto determine which layout to use. You could compare against the already-foundmatchedreference instead.♻️ Suggested simplification
const matched = validTables.find((t) => matchesPartitionTable(partitionTable, t), ); if (!matched) { throw new Error( `Unexpected partition configuration for ${deviceModel.toUpperCase()}. Make sure you've selected the correct device model.\nGot ${JSON.stringify( partitionTable, null, 2, )}`, ); } // Update controller to use the detected layout espController.setPartitionLayout( - matchesPartitionTable(partitionTable, x3PartitionTable) - ? X3_PARTITION_LAYOUT - : X4_PARTITION_LAYOUT, + matched === x3PartitionTable ? X3_PARTITION_LAYOUT : X4_PARTITION_LAYOUT, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/esp/useEspOperations.ts` around lines 107 - 126, You found a redundant call to matchesPartitionTable — use the already-found matched entry to pick the layout: after finding matched via validTables.find, check whether matched === x3PartitionTable (or another identifying property on matched) and call espController.setPartitionLayout with X3_PARTITION_LAYOUT if true, otherwise X4_PARTITION_LAYOUT; keep the existing thrown error for no match and remove the extra matchesPartitionTable(...) invocation to avoid duplicate comparisons.
91-93: Fix indentation and consider extracting repeated initialization.Static analysis flagged indentation issues. The same
EspController.fromRequestedDevice(...)pattern with the device model ternary is repeated ~8 times. Consider extracting to a small helper.♻️ Example helper
const getPartitionLayout = () => deviceModel === 'x3' ? X3_PARTITION_LAYOUT : X4_PARTITION_LAYOUT;Then use:
const c = await EspController.fromRequestedDevice(getPartitionLayout());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/esp/useEspOperations.ts` around lines 91 - 93, Indentation on the EspController.fromRequestedDevice(...) calls is inconsistent and the ternary deviceModel === 'x3' ? X3_PARTITION_LAYOUT : X4_PARTITION_LAYOUT is repeated multiple times; extract a small helper (e.g., getPartitionLayout referencing deviceModel, X3_PARTITION_LAYOUT, X4_PARTITION_LAYOUT) and replace each EspController.fromRequestedDevice(...) call with EspController.fromRequestedDevice(getPartitionLayout()), and fix the surrounding indentation to align with the surrounding async/await usage (ensure calls like const c = await EspController.fromRequestedDevice(getPartitionLayout()) are consistently indented).src/app/page.tsx (1)
14-14: Remove unusedTextimport.
Textis imported but not used in this file.🧹 Fix
HStack, - Text, } from '@chakra-ui/react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/page.tsx` at line 14, The import named Text is unused in this module; remove the Text identifier from the import statement (or delete the entire import if it only imports Text) so there are no unused imports in src/app/page.tsx; search for the import that includes Text and remove that symbol to satisfy the linter and avoid dead code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/page.tsx`:
- Around line 231-232: The JSX text node in src/app/page.tsx containing the
phrase “Settings" icon, then click “OK / Confirm" ... has unescaped double-quote
characters causing ESLint errors; update that JSX string to escape the quotes
(e.g., replace each " with " or use JSX string concatenation with {'"'}
where appropriate) so the text node uses valid JSX/HTML entities and the linter
warning is resolved.
---
Nitpick comments:
In `@src/app/page.tsx`:
- Line 14: The import named Text is unused in this module; remove the Text
identifier from the import statement (or delete the entire import if it only
imports Text) so there are no unused imports in src/app/page.tsx; search for the
import that includes Text and remove that symbol to satisfy the linter and avoid
dead code.
In `@src/esp/useEspOperations.ts`:
- Around line 208-237: Extract the duplicated partition-table validation block
into a helper (e.g., validateAndSetPartitionLayout) that accepts (espController,
deviceModel), calls espController.readPartitionTable(), builds validTables using
x3PartitionTable/x4PartitionTable, finds a match via matchesPartitionTable,
throws the same Error if no match, and calls
espController.setPartitionLayout(...) with X3_PARTITION_LAYOUT or
X4_PARTITION_LAYOUT based on the matched table; then replace both inline blocks
with calls to this helper.
- Around line 107-126: You found a redundant call to matchesPartitionTable — use
the already-found matched entry to pick the layout: after finding matched via
validTables.find, check whether matched === x3PartitionTable (or another
identifying property on matched) and call espController.setPartitionLayout with
X3_PARTITION_LAYOUT if true, otherwise X4_PARTITION_LAYOUT; keep the existing
thrown error for no match and remove the extra matchesPartitionTable(...)
invocation to avoid duplicate comparisons.
- Around line 91-93: Indentation on the EspController.fromRequestedDevice(...)
calls is inconsistent and the ternary deviceModel === 'x3' ? X3_PARTITION_LAYOUT
: X4_PARTITION_LAYOUT is repeated multiple times; extract a small helper (e.g.,
getPartitionLayout referencing deviceModel, X3_PARTITION_LAYOUT,
X4_PARTITION_LAYOUT) and replace each EspController.fromRequestedDevice(...)
call with EspController.fromRequestedDevice(getPartitionLayout()), and fix the
surrounding indentation to align with the surrounding async/await usage (ensure
calls like const c = await
EspController.fromRequestedDevice(getPartitionLayout()) are consistently
indented).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5774f0f7-71a5-4fa5-923f-da7d991eb529
📒 Files selected for processing (3)
src/app/page.tsxsrc/esp/EspController.tssrc/esp/useEspOperations.ts
📜 Review details
🧰 Additional context used
🪛 ESLint
src/app/page.tsx
[error] 15-15: Unable to resolve path to module '@chakra-ui/react'.
(import-x/no-unresolved)
[error] 231-231: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 232-232: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 232-232: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 245-245: Delete ·and
(prettier/prettier)
[error] 246-246: Insert ·and
(prettier/prettier)
[error] 261-261: Insert ⏎
(prettier/prettier)
src/esp/useEspOperations.ts
[error] 40-40: Use an interface instead of a type.
(@typescript-eslint/consistent-type-definitions)
[error] 92-92: Delete ····
(prettier/prettier)
[error] 93-93: Delete ····
(prettier/prettier)
[error] 202-202: Delete ····
(prettier/prettier)
[error] 203-203: Delete ····
(prettier/prettier)
[error] 291-291: Delete ····
(prettier/prettier)
[error] 292-292: Delete ····
(prettier/prettier)
[error] 331-331: Delete ····
(prettier/prettier)
[error] 332-332: Delete ····
(prettier/prettier)
[error] 357-357: Delete ····
(prettier/prettier)
[error] 358-358: Delete ····
(prettier/prettier)
[error] 387-387: Delete ····
(prettier/prettier)
[error] 388-388: Delete ····
(prettier/prettier)
[error] 418-418: Delete ····
(prettier/prettier)
[error] 419-419: Delete ····
(prettier/prettier)
[error] 530-530: Delete ····
(prettier/prettier)
[error] 531-531: Delete ····
(prettier/prettier)
src/esp/EspController.ts
[error] 83-83: Expected blank line between class members.
(@stylistic/lines-between-class-members)
[error] 85-85: Replace device:·SerialPort,·partitionLayout:·PartitionLayout·=·X4_PARTITION_LAYOUT with ⏎····device:·SerialPort,⏎····partitionLayout:·PartitionLayout·=·X4_PARTITION_LAYOUT,⏎··
(prettier/prettier)
[error] 213-213: Replace offset,·this.layout.appSize,·onPacketReceived with ⏎······offset,⏎······this.layout.appSize,⏎······onPacketReceived,⏎····
(prettier/prettier)
🔇 Additional comments (10)
src/esp/EspController.ts (5)
44-60: LGTM! Well-structured partition layout configuration.The
PartitionLayoutinterface and the two layout constants cleanly encapsulate device-specific partition configurations. The offsets and sizes appear consistent with the partition tables defined inuseEspOperations.ts.
85-98: LGTM! Flexible layout configuration approach.The design correctly supports both initial layout configuration and post-construction updates via
setPartitionLayout(). This is essential since the actual partition layout is determined after reading the device's partition table.Note: Static analysis flagged formatting issues on line 85. Consider running prettier to fix.
209-213: LGTM!Partition offset and size now correctly reference
this.layout, enabling proper read operations for both x4 and x3 devices.
259-276: LGTM!The size validation and offset selection correctly use the dynamic layout configuration, and the error message helpfully displays the maximum allowed size in hex.
104-107: No action required. The'no_reset_stub'option is a valid parameter forESPLoader.after()in esptool-js v0.5.7, added in PR#146. The code is correct.> Likely an incorrect or invalid review comment.src/esp/useEspOperations.ts (3)
22-55: LGTM! Partition table definitions and matching logic are well-structured.The partition table values are consistent with the layout constants in
EspController.ts, and thematchesPartitionTablefunction correctly validates entries by type, offset, and size with proper length checking.
169-171: LGTM!The conditional
skipResetbased on device model aligns with the x3's requirement for manual USB disconnect/reconnect restart.
611-631: LGTM!Clean exposure of
deviceModelandsetDeviceModelfor UI integration.src/app/page.tsx (2)
48-63: LGTM! Clean device model selection UI.The button group with variant toggling provides clear visual feedback, and disabling during operations prevents state changes mid-flash.
243-255: LGTM! Clear device-specific restart instructions.The conditional rendering correctly displays the appropriate restart procedure for each device model. Note: Prettier flagged a minor line-break issue on line 245-246.
Extends firmware fetching to support both X4 and X3 device models. X3 uses different API endpoints and has distinct fallback versions for Chinese and English firmware. The cache key is now scoped per device model to prevent conflicts.
Use proper HTML entities (“ and ”) for quotation marks in the settings instruction text to improve typography.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/remote/firmwareFetcher.ts (1)
169-173:⚠️ Potential issue | 🔴 CriticalAdd HTTP response status validation before processing downloaded firmware.
The current code at lines 169-173 does not check
response.okbefore returning the buffer. If the download endpoint returns a 403, 404, or other error status with an error page body, those bytes will be passed directly to the flasher, potentially corrupting the device. The same issue exists ingetCommunityFirmwareat lines 181-182.🔧 Suggested fix
export async function getOfficialFirmware( region: 'en' | 'ch', deviceModel: DeviceModel, ) { const url = await getOfficialFirmwareRemoteData(deviceModel).then( (data) => data[region].download_url, ); const response = await fetch(url); + if (!response.ok) { + throw new Error( + `Failed to download ${deviceModel.toUpperCase()} ${region} firmware: ${response.status} ${response.statusText}`, + ); + } return new Uint8Array(await response.arrayBuffer()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/remote/firmwareFetcher.ts` around lines 169 - 173, The fetch result is used without validating HTTP status, so update the firmware download paths (the code around getOfficialFirmwareRemoteData usage that assigns url and the getCommunityFirmware block) to check response.ok after awaiting fetch; if not ok, throw or return a clear error including response.status and response.statusText (and optionally the URL) instead of returning the response body bytes, otherwise proceed to return new Uint8Array(await response.arrayBuffer()). Ensure both the official firmware fetch (the function handling deviceModel/region download_url) and getCommunityFirmware perform this response.ok validation to prevent passing error pages to the flasher.
♻️ Duplicate comments (1)
src/app/page.tsx (1)
232-233:⚠️ Potential issue | 🟡 MinorThese quote characters are still tripping
react/no-unescaped-entities.
“Settings"and“OK / Confirm"still include raw"characters, so the lint error from the earlier review remains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/page.tsx` around lines 232 - 233, The JSX text contains raw double-quote characters ("Settings" and "OK / Confirm") that trigger react/no-unescaped-entities; update the string literals in src/app/page.tsx by replacing the raw quotes with HTML entities (e.g., "Settings" and "OK / Confirm") or by using escaped JS string syntax (e.g., {'"Settings"'}), so the displayed text remains identical but the linter no longer reports unescaped quote characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/page.tsx`:
- Around line 53-60: The buttons are only visually indicating selection; make
the active model programmatically exposed by adding an aria-pressed attribute
based on the selected state (deviceModel === model) to the Button elements (the
ones using setDeviceModel, deviceModel and isRunning) so screen readers can
announce the pressed/selected state; ensure aria-pressed is a boolean and leave
the existing disabled and onClick behavior intact.
- Around line 38-45: The current effect calling
getOfficialFirmwareVersions(deviceModel) can set state after a stale request
resolves; change the useEffect that depends on deviceModel (the one calling
getOfficialFirmwareVersions and setOfficialFirmwareVersions) to track the
current request (e.g., capture the deviceModel in a local variable or use an
AbortController/cleanup flag) and only call
setOfficialFirmwareVersions(versions) if the model still matches or the request
wasn't aborted; also move getCommunityFirmwareRemoteData() into its own
mount-only useEffect (no deviceModel dep) so community data is fetched once on
mount instead of refiring on model changes.
In `@src/esp/useEspOperations.ts`:
- Around line 98-126: The partition-layout detection done inside the 'Validate
partition table' runStep updates espController via
espController.setPartitionLayout but is only used for flash path construction;
move this detection into a shared helper (e.g., detectAndSetPartitionLayout)
that encapsulates reading espController.readPartitionTable, using
matchesPartitionTable against x3PartitionTable/x4PartitionTable, and calling
espController.setPartitionLayout appropriately (using X3_PARTITION_LAYOUT or
X4_PARTITION_LAYOUT). Call that helper before any code that reads app
partitions—specifically before readAppPartition and before
readAndIdentifyAllFirmware—to ensure those functions use the detected layout
rather than the optimistic deviceModel-based layout.
In `@src/remote/firmwareFetcher.ts`:
- Around line 62-63: The constant x3ChineseFirmwareCheckUrl contains a
hard-coded lang=en query param that conflicts with its Chinese metadata purpose;
update x3ChineseFirmwareCheckUrl to either use lang=zh or remove the lang
parameter entirely, and make the same change to the cached copy of this URL used
by the firmware fetcher so both the primary constant and its cached reference
are consistent (modify the constant x3ChineseFirmwareCheckUrl and the cache
entry that stores this URL).
---
Outside diff comments:
In `@src/remote/firmwareFetcher.ts`:
- Around line 169-173: The fetch result is used without validating HTTP status,
so update the firmware download paths (the code around
getOfficialFirmwareRemoteData usage that assigns url and the
getCommunityFirmware block) to check response.ok after awaiting fetch; if not
ok, throw or return a clear error including response.status and
response.statusText (and optionally the URL) instead of returning the response
body bytes, otherwise proceed to return new Uint8Array(await
response.arrayBuffer()). Ensure both the official firmware fetch (the function
handling deviceModel/region download_url) and getCommunityFirmware perform this
response.ok validation to prevent passing error pages to the flasher.
---
Duplicate comments:
In `@src/app/page.tsx`:
- Around line 232-233: The JSX text contains raw double-quote characters
("Settings" and "OK / Confirm") that trigger react/no-unescaped-entities; update
the string literals in src/app/page.tsx by replacing the raw quotes with HTML
entities (e.g., "Settings" and "OK / Confirm") or by using
escaped JS string syntax (e.g., {'"Settings"'}), so the displayed text remains
identical but the linter no longer reports unescaped quote characters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0195507-145b-4eec-a161-86a81753c06c
📒 Files selected for processing (3)
src/app/page.tsxsrc/esp/useEspOperations.tssrc/remote/firmwareFetcher.ts
📜 Review details
🧰 Additional context used
🪛 ESLint
src/app/page.tsx
[error] 15-15: Unable to resolve path to module '@chakra-ui/react'.
(import-x/no-unresolved)
[error] 232-232: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 233-233: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 233-233: " can be escaped with ", “, ", ”.
(react/no-unescaped-entities)
[error] 246-246: Delete ·and
(prettier/prettier)
[error] 247-247: Insert ·and
(prettier/prettier)
[error] 262-262: Insert ⏎
(prettier/prettier)
src/esp/useEspOperations.ts
[error] 40-40: Use an interface instead of a type.
(@typescript-eslint/consistent-type-definitions)
[error] 92-92: Delete ····
(prettier/prettier)
[error] 93-93: Delete ····
(prettier/prettier)
[error] 202-202: Delete ····
(prettier/prettier)
[error] 203-203: Delete ····
(prettier/prettier)
[error] 291-291: Delete ····
(prettier/prettier)
[error] 292-292: Delete ····
(prettier/prettier)
[error] 331-331: Delete ····
(prettier/prettier)
[error] 332-332: Delete ····
(prettier/prettier)
[error] 357-357: Delete ····
(prettier/prettier)
[error] 358-358: Delete ····
(prettier/prettier)
[error] 387-387: Delete ····
(prettier/prettier)
[error] 388-388: Delete ····
(prettier/prettier)
[error] 418-418: Delete ····
(prettier/prettier)
[error] 419-419: Delete ····
(prettier/prettier)
[error] 530-530: Delete ····
(prettier/prettier)
[error] 531-531: Delete ····
(prettier/prettier)
src/remote/firmwareFetcher.ts
Outdated
| const x3ChineseFirmwareCheckUrl = | ||
| 'https://api-prod.xteink.cn/api/v1/check-update?current_version=V5.1.3&device_type=ESP32C3_X3&device_id=1052463&choose=1&lang=en'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
url='https://api-prod.xteink.cn/api/v1/check-update?current_version=V5.1.3&device_type=ESP32C3_X3&device_id=1052463&choose=1&lang=en'
curl -fsSL "$url" | jq '.data | {version, download_url, change_log}'Repository: crosspoint-reader/xteink-flasher
Length of output: 353
Remove or correct hard-coded lang=en parameter in Chinese firmware URL.
The X3 Chinese metadata request at lines 62-63 includes lang=en in the query string, which is inconsistent with its intended purpose. While API testing shows the endpoint returns Chinese metadata (filename contains "X3-CH-PROD" and changelog is in Chinese) regardless of this parameter, the mismatch should be corrected—either change to the appropriate language code (e.g., lang=zh) or remove the parameter if it's not required. Also applies to where this URL is cached at lines 80-87.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/remote/firmwareFetcher.ts` around lines 62 - 63, The constant
x3ChineseFirmwareCheckUrl contains a hard-coded lang=en query param that
conflicts with its Chinese metadata purpose; update x3ChineseFirmwareCheckUrl to
either use lang=zh or remove the lang parameter entirely, and make the same
change to the cached copy of this URL used by the firmware fetcher so both the
primary constant and its cached reference are consistent (modify the constant
x3ChineseFirmwareCheckUrl and the cache entry that stores this URL).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/page.tsx (3)
13-14: Unused import:Textis imported but not used.The
Textcomponent is imported from@chakra-ui/reactbut does not appear to be used anywhere in this file.🧹 Proposed fix
HStack, - Text, } from '@chakra-ui/react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/page.tsx` around lines 13 - 14, Remove the unused Text import from the Chakra import list in page.tsx: locate the import that currently lists HStack and Text (symbol: Text) and delete Text so only used components (e.g., HStack) are imported; run a quick lint/typecheck to ensure no other references to Text remain and adjust imports if necessary.
262-262: Add trailing newline at end of file.Prettier expects a newline at the end of the file. Running the formatter should resolve this.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/page.tsx` at line 262, The file src/app/page.tsx is missing a trailing newline at EOF; open the file and add a single newline character after the final closing brace (the end of the Page component / export) so the file ends with a newline, then re-run Prettier/formatting to ensure the newline is preserved.
232-233: Formatting: Prettier flags spacing inconsistency.The quote escaping is correct. However, static analysis indicates a Prettier formatting issue on these lines. Running the formatter should resolve it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/page.tsx` around lines 232 - 233, Prettier reports a spacing/formatting inconsistency in the JSX text block in src/app/page.tsx around the string containing “Settings” and “OK / Confirm” — run the project's Prettier (or your editor's format command) against page.tsx to reflow the JSX text nodes and remove any inconsistent spaces or trailing whitespace; if manual edit is needed, normalize spacing around the quoted phrases and line breaks in that JSX fragment so the formatter no longer flags it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/page.tsx`:
- Around line 244-256: In the JSX conditional that renders the device restart
instructions (the ternary using deviceModel === 'x3' in page.tsx), escape the
double quotes around "Reset" by replacing them with the HTML entity
("Reset") to satisfy react/no-unescaped-entities, and reflow the
paragraph text surrounding the reset/power button instructions to match project
Prettier formatting (adjust line breaks and spacing in that <p> block so it
formats cleanly).
---
Nitpick comments:
In `@src/app/page.tsx`:
- Around line 13-14: Remove the unused Text import from the Chakra import list
in page.tsx: locate the import that currently lists HStack and Text (symbol:
Text) and delete Text so only used components (e.g., HStack) are imported; run a
quick lint/typecheck to ensure no other references to Text remain and adjust
imports if necessary.
- Line 262: The file src/app/page.tsx is missing a trailing newline at EOF; open
the file and add a single newline character after the final closing brace (the
end of the Page component / export) so the file ends with a newline, then re-run
Prettier/formatting to ensure the newline is preserved.
- Around line 232-233: Prettier reports a spacing/formatting inconsistency in
the JSX text block in src/app/page.tsx around the string containing “Settings”
and “OK / Confirm” — run the project's Prettier (or your editor's format
command) against page.tsx to reflow the JSX text nodes and remove any
inconsistent spaces or trailing whitespace; if manual edit is needed, normalize
spacing around the quoted phrases and line breaks in that JSX fragment so the
formatter no longer flags it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0cc9d974-1c91-469a-bca8-ce0c17b3a8fb
📒 Files selected for processing (1)
src/app/page.tsx
📜 Review details
🧰 Additional context used
🪛 ESLint
src/app/page.tsx
[error] 15-15: Unable to resolve path to module '@chakra-ui/react'.
(import-x/no-unresolved)
[error] 232-232: Delete ·then·click
(prettier/prettier)
[error] 246-246: Delete ·and
(prettier/prettier)
[error] 247-247: Insert ·and
(prettier/prettier)
[error] 262-262: Insert ⏎
(prettier/prettier)
🔇 Additional comments (2)
src/app/page.tsx (2)
38-45: Race condition: stale fetch responses can overwrite state after rapid model toggles.Quick
deviceModelchanges may cause an earliergetOfficialFirmwareVersions()call to resolve after a later one, overwriting the correct firmware versions with stale data. Add a cleanup function with a cancellation flag.Additionally,
getCommunityFirmwareRemoteData()is model-independent and should be moved to its own mount-only effect.🔧 Suggested fix
useEffect(() => { + let cancelled = false; setOfficialFirmwareVersions(null); - getOfficialFirmwareVersions(deviceModel).then((versions) => - setOfficialFirmwareVersions(versions), - ); - - getCommunityFirmwareRemoteData().then(setCommunityFirmwareVersions); + void getOfficialFirmwareVersions(deviceModel).then((versions) => { + if (!cancelled) { + setOfficialFirmwareVersions(versions); + } + }); + + return () => { + cancelled = true; + }; }, [deviceModel]); + +useEffect(() => { + void getCommunityFirmwareRemoteData().then(setCommunityFirmwareVersions); +}, []);
52-61: Accessibility: expose selected state to screen readers witharia-pressed.The active model is only visually indicated via the
variantprop. Screen readers won't announce which model is selected.🔧 Minimal fix
<Button key={model} variant={deviceModel === model ? 'solid' : 'outline'} + aria-pressed={deviceModel === model} onClick={() => setDeviceModel(model)} disabled={isRunning} >
Add cleanup function to cancel state updates when component unmounts, preventing setState calls on unmounted components. Also split community firmware fetching into separate effect that only runs once on mount.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/page.tsx`:
- Around line 58-73: The X3 selector currently allows switching to 'x3' which
can trigger model-agnostic CrossPoint flashing; update the Device model buttons
in the component that renders deviceModel (the map over (['x4','x3'] as const)
that calls setDeviceModel) to disable the X3 option until community firmware
resolution is model-aware: modify the Button disabled prop to include model ===
'x3' (e.g., disabled={isRunning || model === 'x3'}) and also ensure
aria-pressed/aria-disabled reflect the disabled state so X3 cannot be selected
while firmwareFetcher (src/remote/firmwareFetcher.ts) lacks model scoping.
In `@src/esp/useEspOperations.ts`:
- Around line 75-82: The partition-table check in useEspOperations (validTables
/ matchesPartitionTable) can’t distinguish a true X4 device from an X3 running
the X4 layout, so keep using deviceModel to pick firmware and skipReset leads to
mis-flashes; add an independent hardware discriminator or an explicit gate for
“X3 running X4 layout” before proceeding. Concretely: in useEspOperations,
introduce a new check (e.g., detectHardwareVariant or validateHardwareSignature)
that queries a hardware-unique property (chip ID, efuse, or exposed bootrom
signature) and returns a definitive device type; only if that discriminator
agrees with deviceModel allow selecting x4 firmware URLs and skipReset behavior,
otherwise branch into an explicit “X3-on-X4-layout” flow that either refuses
flashing or uses X3 firmware and appropriate reset flags. Ensure to update logic
that currently uses deviceModel, validTables, x3PartitionTable,
x4PartitionTable, matchesPartitionTable, firmware URL selection, and skipReset
to consult the new discriminator/gate before choosing images or reset behavior.
In `@src/remote/firmwareFetcher.ts`:
- Around line 80-96: The code that fetches X3 firmware metadata (when
deviceModel === 'x3' using x3ChineseFirmwareCheckUrl) naively trusts res.json()
and writes an OfficialFirmwareVersions object to cache via cache.set, which can
store invalid/undefined en/ch values; update the fetch flow in that block (and
the analogous block later) to first check response.ok, validate the parsed
payload has a well-formed payload.data (and expected fields) before constructing
the OfficialFirmwareVersions object, and only call cache.set when validation
succeeds; on any validation failure return the fallback object (or propagate an
error) instead of caching undefined fields.
- Around line 43-49: The fallback X3 firmware entry x3FirmwareVersionFallback
currently contains an insecure http:// download_url and the fetch-to-flash path
(the code that converts HTTP responses into flash bytes) will accept any body;
update x3FirmwareVersionFallback to use a trusted HTTPS URL or remove the public
fallback, and modify the fetch/convert logic to fail closed: reject non-2xx
responses, validate the payload against a pinned checksum or cryptographic
signature, and only pass verified bytes to the flashing routine (refer to the
code that consumes OfficialFirmwareVersions and the fetch-to-flash conversion
code invoked around the bytes conversion). Ensure verification errors abort
flashing and return clear errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 705576a1-62ee-4cd3-a543-6132c11fad15
📒 Files selected for processing (3)
src/app/page.tsxsrc/esp/useEspOperations.tssrc/remote/firmwareFetcher.ts
📜 Review details
🧰 Additional context used
🪛 ESLint
src/esp/useEspOperations.ts
[error] 40-40: Use an interface instead of a type.
(@typescript-eslint/consistent-type-definitions)
[error] 123-123: Delete ····
(prettier/prettier)
[error] 124-124: Delete ····
(prettier/prettier)
[error] 206-206: Delete ····
(prettier/prettier)
[error] 207-207: Delete ····
(prettier/prettier)
[error] 268-268: Delete ····
(prettier/prettier)
[error] 269-269: Delete ····
(prettier/prettier)
[error] 308-308: Delete ····
(prettier/prettier)
[error] 309-309: Delete ····
(prettier/prettier)
[error] 334-334: Delete ····
(prettier/prettier)
[error] 335-335: Delete ····
(prettier/prettier)
[error] 365-365: Delete ····
(prettier/prettier)
[error] 366-366: Delete ····
(prettier/prettier)
[error] 400-400: Delete ····
(prettier/prettier)
[error] 401-401: Delete ····
(prettier/prettier)
[error] 513-513: Delete ····
(prettier/prettier)
[error] 514-514: Delete ····
(prettier/prettier)
src/app/page.tsx
[error] 15-15: Unable to resolve path to module '@chakra-ui/react'.
(import-x/no-unresolved)
[error] 242-242: Delete ·then·click
(prettier/prettier)
[error] 256-256: Delete ·and
(prettier/prettier)
[error] 257-257: Insert ·and
(prettier/prettier)
[error] 262-264: Replace ·“Reset”·button⏎················near·the·bottom·right,·followed·quickly·by·pressing·and·holding⏎················of·the·main·power·button·for with ⏎················“Reset”·button·near·the·bottom·right,·followed⏎················quickly·by·pressing·and·holding·of·the·main·power·button·for⏎···············
(prettier/prettier)
[error] 272-272: Insert ⏎
(prettier/prettier)
🔇 Additional comments (1)
src/app/page.tsx (1)
38-54: Nice stale-request cleanup.The cancellation guard on the model-specific fetch and the mount-only community fetch close the race without reissuing community requests on every model toggle.
| <Stack gap={3} as="section"> | ||
| <Heading size="xl">Device model</Heading> | ||
| <HStack gap={3}> | ||
| {(['x4', 'x3'] as const).map((model) => ( | ||
| <Button | ||
| key={model} | ||
| variant={deviceModel === model ? 'solid' : 'outline'} | ||
| aria-pressed={deviceModel === model} | ||
| onClick={() => setDeviceModel(model)} | ||
| disabled={isRunning} | ||
| > | ||
| Xteink {model.toUpperCase()} | ||
| </Button> | ||
| ))} | ||
| </HStack> | ||
| </Stack> |
There was a problem hiding this comment.
Don't expose the CrossPoint flash path in X3 mode yet.
This selector makes X3 look fully supported, but the community firmware path is still model-agnostic in src/remote/firmwareFetcher.ts. Users can now switch to X3 and still trigger a CrossPoint flash with no model scoping. Please disable that button for X3 until community firmware resolution is model-aware.
🧭 Minimal UI guard
<Button
variant="subtle"
onClick={actions.flashCrossPointFirmware}
- disabled={isRunning || !communityFirmwareVersions}
+ disabled={
+ isRunning || !communityFirmwareVersions || deviceModel === 'x3'
+ }
loading={!communityFirmwareVersions}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/page.tsx` around lines 58 - 73, The X3 selector currently allows
switching to 'x3' which can trigger model-agnostic CrossPoint flashing; update
the Device model buttons in the component that renders deviceModel (the map over
(['x4','x3'] as const) that calls setDeviceModel) to disable the X3 option until
community firmware resolution is model-aware: modify the Button disabled prop to
include model === 'x3' (e.g., disabled={isRunning || model === 'x3'}) and also
ensure aria-pressed/aria-disabled reflect the disabled state so X3 cannot be
selected while firmwareFetcher (src/remote/firmwareFetcher.ts) lacks model
scoping.
| const validTables = | ||
| deviceModel === 'x3' | ||
| ? [x3PartitionTable, x4PartitionTable] | ||
| : [x4PartitionTable]; | ||
|
|
||
| const matched = validTables.find((t) => | ||
| matchesPartitionTable(partitionTable, t), | ||
| ); |
There was a problem hiding this comment.
Partition-table validation no longer proves the selected model.
Once X3 mode accepts x4PartitionTable, this step cannot tell a real X4 apart from an X3 still running the CrossPoint/X4 layout. The rest of the hook still uses deviceModel to choose firmware URLs and skipReset, so a wrong selection can pass validation and flash the wrong image. This needs an independent hardware discriminator or a separately gated “X3 on X4 layout” path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/esp/useEspOperations.ts` around lines 75 - 82, The partition-table check
in useEspOperations (validTables / matchesPartitionTable) can’t distinguish a
true X4 device from an X3 running the X4 layout, so keep using deviceModel to
pick firmware and skipReset leads to mis-flashes; add an independent hardware
discriminator or an explicit gate for “X3 running X4 layout” before proceeding.
Concretely: in useEspOperations, introduce a new check (e.g.,
detectHardwareVariant or validateHardwareSignature) that queries a
hardware-unique property (chip ID, efuse, or exposed bootrom signature) and
returns a definitive device type; only if that discriminator agrees with
deviceModel allow selecting x4 firmware URLs and skipReset behavior, otherwise
branch into an explicit “X3-on-X4-layout” flow that either refuses flashing or
uses X3 firmware and appropriate reset flags. Ensure to update logic that
currently uses deviceModel, validTables, x3PartitionTable, x4PartitionTable,
matchesPartitionTable, firmware URL selection, and skipReset to consult the new
discriminator/gate before choosing images or reset behavior.
| const x3FirmwareVersionFallback: OfficialFirmwareVersions = { | ||
| en: { | ||
| change_log: '', | ||
| download_url: | ||
| 'http://8.216.34.42:5001/api/v1/download/ESP32C3_X3/V5.1.6/V5.1.6-X3-EN-PROD-0304_.bin?choose=1&lang=en', | ||
| version: 'V5.1.6', | ||
| }, |
There was a problem hiding this comment.
Don't flash the X3 English image from an unauthenticated source.
Line 47 introduces a plain http:// firmware URL, and Lines 169-173 turn whatever comes back into flash bytes. That means an on-path tamper or even an error body can become firmware. Please fail closed on non-OK responses and keep this path behind a trusted source (https or a pinned checksum/signature) before release.
🔒 Minimum hardening
+async function downloadFirmware(url: string) {
+ const response = await fetch(url);
+ if (!response.ok) {
+ throw new Error(`Firmware download failed: ${response.status}`);
+ }
+
+ return new Uint8Array(await response.arrayBuffer());
+}
+
export async function getOfficialFirmware(
region: 'en' | 'ch',
deviceModel: DeviceModel,
) {
const url = await getOfficialFirmwareRemoteData(deviceModel).then(
(data) => data[region].download_url,
);
- const response = await fetch(url);
- return new Uint8Array(await response.arrayBuffer());
+ return downloadFirmware(url);
}Also applies to: 165-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/remote/firmwareFetcher.ts` around lines 43 - 49, The fallback X3 firmware
entry x3FirmwareVersionFallback currently contains an insecure http://
download_url and the fetch-to-flash path (the code that converts HTTP responses
into flash bytes) will accept any body; update x3FirmwareVersionFallback to use
a trusted HTTPS URL or remove the public fallback, and modify the fetch/convert
logic to fail closed: reject non-2xx responses, validate the payload against a
pinned checksum or cryptographic signature, and only pass verified bytes to the
flashing routine (refer to the code that consumes OfficialFirmwareVersions and
the fetch-to-flash conversion code invoked around the bytes conversion). Ensure
verification errors abort flashing and return clear errors.
| if (deviceModel === 'x3') { | ||
| // X3: Chinese has a check-update API, English only has a static download URL | ||
| return fetch(x3ChineseFirmwareCheckUrl) | ||
| .then((res) => res.json()) | ||
| .then(async (chData) => { | ||
| const data: OfficialFirmwareVersions = { | ||
| en: fallback.en, | ||
| ch: chData.data, | ||
| }; | ||
|
|
||
| await cache.set(cacheKey, data, { | ||
| ttl: 60 * 60 * 24, // 24 hours | ||
| }); | ||
|
|
||
| return data; | ||
| }) | ||
| .catch(() => fallback); |
There was a problem hiding this comment.
Validate official metadata before caching it.
Lines 82-95 and 103-114 assume payload.data exists whenever res.json() succeeds. A JSON error payload or partial response will cache en/ch as undefined for 24 hours, and the next version lookup or download path will blow up on missing fields. Check response.ok and the shape of payload.data before writing to cache.
🧪 One way to harden the metadata path
+function isOfficialFirmwareData(value: unknown): value is OfficialFirmwareData {
+ return (
+ typeof value === 'object' &&
+ value !== null &&
+ typeof (value as OfficialFirmwareData).change_log === 'string' &&
+ typeof (value as OfficialFirmwareData).download_url === 'string' &&
+ typeof (value as OfficialFirmwareData).version === 'string'
+ );
+}
+
+async function parseOfficialFirmwareResponse(response: Response) {
+ if (!response.ok) {
+ throw new Error(`Firmware metadata request failed: ${response.status}`);
+ }
+
+ const payload = await response.json();
+ if (!isOfficialFirmwareData(payload?.data)) {
+ throw new Error('Malformed firmware metadata response');
+ }
+
+ return payload.data;
+}
+
if (deviceModel === 'x3') {
// X3: Chinese has a check-update API, English only has a static download URL
return fetch(x3ChineseFirmwareCheckUrl)
- .then((res) => res.json())
- .then(async (chData) => {
+ .then(parseOfficialFirmwareResponse)
+ .then(async (chData) => {
const data: OfficialFirmwareVersions = {
en: fallback.en,
- ch: chData.data,
+ ch: chData,
};
await cache.set(cacheKey, data, {
ttl: 60 * 60 * 24, // 24 hours
});
@@
return Promise.all([
fetch(x4ChineseFirmwareCheckUrl),
fetch(x4EnglishFirmwareCheckUrl),
])
- .then(([chRes, enRes]) => Promise.all([chRes.json(), enRes.json()]))
+ .then(([chRes, enRes]) =>
+ Promise.all([
+ parseOfficialFirmwareResponse(chRes),
+ parseOfficialFirmwareResponse(enRes),
+ ]),
+ )
.then(async ([chData, enData]) => {
const data: OfficialFirmwareVersions = {
- en: enData.data,
- ch: chData.data,
+ en: enData,
+ ch: chData,
};Also applies to: 99-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/remote/firmwareFetcher.ts` around lines 80 - 96, The code that fetches X3
firmware metadata (when deviceModel === 'x3' using x3ChineseFirmwareCheckUrl)
naively trusts res.json() and writes an OfficialFirmwareVersions object to cache
via cache.set, which can store invalid/undefined en/ch values; update the fetch
flow in that block (and the analogous block later) to first check response.ok,
validate the parsed payload has a well-formed payload.data (and expected fields)
before constructing the OfficialFirmwareVersions object, and only call cache.set
when validation succeeds; on any validation failure return the fallback object
(or propagate an error) instead of caching undefined fields.
1 participant
Note: I do not have an API url for the english stock firmware, just a raw dowload url