Huge UI facelift and upgrades to libs (including xterm migration) #297
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request implements a comprehensive UI facelift and migrates from the legacy xterm package to the modern @xterm/xterm package. The changes include:
Summary: Major UI/UX redesign with modernized styling, xterm v5 to v6 migration, dependency updates, and enhanced visual feedback throughout the application.
Key Changes:
- Migration from
xtermv5 to@xterm/xtermv6 with FitAddon for terminal functionality - Complete UI redesign with glass-morphism, gradient buttons, improved animations, and modern card layouts
- SerialMonitor reimplemented using xterm terminal directly instead of custom buffer rendering
- Addition of 23 new locale translation keys for "waiting_for_data" across all supported languages
Reviewed changes
Copilot reviewed 42 out of 46 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated 15+ dependencies including xterm migration, Nuxt 3.19→3.20, Vue 3.5.19→3.5.26, and various build tools |
| utils/terminal.ts | Updated import path from 'xterm' to '@xterm/xterm' |
| types/manifest.ts | New TypeScript interfaces for firmware manifest structure |
| tailwind.config.js | Added meshtastic color palette, custom animations (fade-in, glow, float), and extended shadows |
| stores/serialMonitorStore.ts | Added rawBuffer field to store unprocessed terminal data for xterm |
| stores/firmwareStore.ts | Added DTR reset line before RTS toggle for improved ESP32 reset behavior |
| public/img/devices/unknown-new.svg | New SVG device illustration with modern styling |
| i18n/locales/*.json | Added "waiting_for_data" translation key across 23 locale files; updated header_title in several locales |
| components/SerialMonitor.vue | Complete rewrite using xterm Terminal with FitAddon, removed log level filtering UI, modernized toolbar |
| components/targets/*.vue | UI refresh with modern card styling, improved spacing, and gradient buttons (Uf2, Esp32, EraseUf2, FlashHeader, ReleaseNotes) |
| components/LogoHeader.vue | Redesigned with gradient text effects, glow animations, and responsive sizing |
| components/Flash.vue | Converted to Teleport-based modals with backdrop blur and modern styling |
| components/Firmware.vue | Custom dropdown implementation replacing Flowbite with manual positioning and Teleport |
| components/Device.vue | Modal redesign with improved vendor tags, device cards, and modern layout |
| components/DeviceHeader.vue | Added icon, improved close button styling, accent line |
| assets/css/main.css | 240+ lines of new utility classes for modern UI components (btn-primary, card-modern, modals, etc.) |
| app.vue | Layout improvements with flex layout, animated step indicators, gradient footer, enhanced connection status |
| .gitignore | Added .playwright-mcp to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Manfest based refactor
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Suggestion:
Missing await calls on async firmware flashing functions causes race conditions and allows code to continue before flash operations complete
View Details
📝 Patch Details
diff --git a/components/targets/Esp32.vue b/components/targets/Esp32.vue
index 3ec6cc4..18ee0ff 100644
--- a/components/targets/Esp32.vue
+++ b/components/targets/Esp32.vue
@@ -346,7 +346,7 @@ const cleanInstallEsp32 = async () => {
// Use manifest-driven flashing if manifest is available
if (firmwareStore.$state.manifest) {
console.log('Using manifest-driven clean install')
- firmwareStore.cleanInstallEspFlash(selectedTarget)
+ await firmwareStore.cleanInstallEspFlash(selectedTarget)
}
else {
// Legacy: use convention-based file naming
@@ -391,7 +391,7 @@ const updateEsp32 = async () => {
// Use manifest-driven flashing if manifest is available
if (firmwareStore.$state.manifest) {
console.log('Using manifest-driven update flash')
- firmwareStore.updateEspFlash(selectedTarget)
+ await firmwareStore.updateEspFlash(selectedTarget)
}
else {
// Legacy: use convention-based file naming
Analysis
Bug Analysis
Why it happens:
The functions cleanInstallEsp32 and updateEsp32 in components/targets/Esp32.vue are marked as async, but they do not await calls to firmwareStore.cleanInstallEspFlash() and firmwareStore.updateEspFlash() respectively. These store methods are themselves async functions that:
- Request and open serial ports via
navigator.serial.requestPort() - Perform asynchronous flashing operations via
espLoader.writeFlash() - Manage state flags like
isFlashingandflashPercentDone
When it manifests:
When using manifest-driven flashing (the new code path), if users initiate a clean install or update:
- The async function starts the flashing operation but doesn't wait for it
- Code continues executing immediately
- State is set incorrectly (isFlashing may be reset)
- UI updates happen at the wrong time
- Multiple flash operations could be triggered simultaneously (race condition)
Impact:
- Flash operations may not complete properly
- Race conditions could corrupt device firmware
- UI progress tracking becomes unreliable
- User might proceed to other actions before flash is complete
Fix Applied
Added await keyword to both calls:
- Line 362:
await firmwareStore.cleanInstallEspFlash(selectedTarget) - Line 391:
await firmwareStore.updateEspFlash(selectedTarget)
This ensures the async functions properly wait for the entire flashing operation (serial communication, writing, progress callbacks) to complete before returning control. Since both cleanInstallEsp32 and updateEsp32 are already declared as async functions, they can properly use await.
| * Find app0 (firmware) file by convention name pattern | ||
| * @returns The FirmwareManifestFile or undefined if not found | ||
| */ | ||
| findAppFileByConvention(): FirmwareManifestFile | undefined { |
There was a problem hiding this comment.
Missing error handling and validation in manifest-driven firmware flash methods allows incomplete file fetches to proceed to flash, potentially causing device failures
View Details
📝 Patch Details
diff --git a/stores/firmwareStore.ts b/stores/firmwareStore.ts
index 82b3d0a..133bf40 100644
--- a/stores/firmwareStore.ts
+++ b/stores/firmwareStore.ts
@@ -429,6 +429,8 @@ export const useFirmwareStore = defineStore('firmware', {
try {
const filesToFlash: Array<{ data: string, address: number }> = []
const fileDescriptions: string[] = []
+ let hasApp0 = false
+ let hasApp1 = false
// Find the app0 file (main firmware binary)
let appFile = this.findFileByPartName(PARTITION_NAMES.APP0)
@@ -437,10 +439,18 @@ export const useFirmwareStore = defineStore('firmware', {
}
const appOffset = this.getPartitionOffset(PARTITION_NAMES.APP0)
if (appFile && appOffset !== undefined) {
- const appContent = await this.fetchBinaryContent(appFile.name)
- filesToFlash.push({ data: appContent, address: appOffset })
- fileDescriptions.push('Flashing app')
- console.log(`App0: ${appFile.name} at offset 0x${appOffset.toString(16)}`)
+ try {
+ const appContent = await this.fetchBinaryContent(appFile.name)
+ if (appContent) {
+ filesToFlash.push({ data: appContent, address: appOffset })
+ fileDescriptions.push('Flashing app')
+ hasApp0 = true
+ console.log(`App0: ${appFile.name} at offset 0x${appOffset.toString(16)}`)
+ }
+ }
+ catch (error) {
+ console.error(`Failed to fetch app0 file '${appFile.name}': ${error}`)
+ }
}
else {
console.error(`Could not find app0 file or partition offset in manifest`)
@@ -453,10 +463,18 @@ export const useFirmwareStore = defineStore('firmware', {
}
const otaOffset = this.getPartitionOffset(PARTITION_NAMES.APP1)
if (otaFile && otaOffset !== undefined) {
- const otaContent = await this.fetchBinaryContent(otaFile.name)
- filesToFlash.push({ data: otaContent, address: otaOffset })
- fileDescriptions.push('Flashing OTA')
- console.log(`App1 (OTA): ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
+ try {
+ const otaContent = await this.fetchBinaryContent(otaFile.name)
+ if (otaContent) {
+ filesToFlash.push({ data: otaContent, address: otaOffset })
+ fileDescriptions.push('Flashing OTA')
+ hasApp1 = true
+ console.log(`App1 (OTA): ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
+ }
+ }
+ catch (error) {
+ console.error(`Failed to fetch OTA file '${otaFile.name}': ${error}`)
+ }
}
else {
console.error(`Could not find app1 (OTA) file or partition offset in manifest`)
@@ -464,6 +482,11 @@ export const useFirmwareStore = defineStore('firmware', {
this.flashingFileDescriptions = fileDescriptions
+ // For update flash, we need both app0 and app1 files
+ if (!hasApp0 || !hasApp1) {
+ throw new Error(`Update flash requires both app0 and app1 files. Found: app0=${hasApp0}, app1=${hasApp1}`)
+ }
+
if (filesToFlash.length === 0) {
throw new Error('No files found to flash')
}
@@ -520,14 +543,23 @@ export const useFirmwareStore = defineStore('firmware', {
try {
const filesToFlash: Array<{ data: string, address: number }> = []
const fileDescriptions: string[] = []
+ let hasFactory = false
// Find the factory binary (combined binary for clean install)
const factoryFile = this.findFactoryFile()
if (factoryFile) {
- const appContent = await this.fetchBinaryContent(factoryFile.name)
- filesToFlash.push({ data: appContent, address: 0x00 })
- fileDescriptions.push('Flashing factory app')
- console.log(`Factory: ${factoryFile.name} at offset 0x00`)
+ try {
+ const appContent = await this.fetchBinaryContent(factoryFile.name)
+ if (appContent) {
+ filesToFlash.push({ data: appContent, address: 0x00 })
+ fileDescriptions.push('Flashing factory app')
+ hasFactory = true
+ console.log(`Factory: ${factoryFile.name} at offset 0x00`)
+ }
+ }
+ catch (error) {
+ console.error(`Failed to fetch factory file '${factoryFile.name}': ${error}`)
+ }
}
else {
console.error('Could not find factory binary (.factory.bin) in manifest')
@@ -540,10 +572,17 @@ export const useFirmwareStore = defineStore('firmware', {
}
const otaOffset = this.getPartitionOffset(PARTITION_NAMES.APP1)
if (otaFile && otaOffset !== undefined) {
- const otaContent = await this.fetchBinaryContent(otaFile.name)
- filesToFlash.push({ data: otaContent, address: otaOffset })
- fileDescriptions.push('Flashing OTA app')
- console.log(`OTA: ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
+ try {
+ const otaContent = await this.fetchBinaryContent(otaFile.name)
+ if (otaContent) {
+ filesToFlash.push({ data: otaContent, address: otaOffset })
+ fileDescriptions.push('Flashing OTA app')
+ console.log(`OTA: ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
+ }
+ }
+ catch (error) {
+ console.error(`Failed to fetch OTA file '${otaFile.name}': ${error}`)
+ }
}
else {
console.error(`Could not find OTA file or partition offset for '${PARTITION_NAMES.APP1}' in manifest`)
@@ -556,10 +595,17 @@ export const useFirmwareStore = defineStore('firmware', {
}
const spiffsOffset = this.getPartitionOffset(PARTITION_NAMES.SPIFFS)
if (spiffsFile && spiffsOffset !== undefined) {
- const spiffsContent = await this.fetchBinaryContent(spiffsFile.name)
- filesToFlash.push({ data: spiffsContent, address: spiffsOffset })
- fileDescriptions.push('Flashing filesystem')
- console.log(`SPIFFS: ${spiffsFile.name} at offset 0x${spiffsOffset.toString(16)}`)
+ try {
+ const spiffsContent = await this.fetchBinaryContent(spiffsFile.name)
+ if (spiffsContent) {
+ filesToFlash.push({ data: spiffsContent, address: spiffsOffset })
+ fileDescriptions.push('Flashing filesystem')
+ console.log(`SPIFFS: ${spiffsFile.name} at offset 0x${spiffsOffset.toString(16)}`)
+ }
+ }
+ catch (error) {
+ console.error(`Failed to fetch SPIFFS file '${spiffsFile.name}': ${error}`)
+ }
}
else {
console.error(`Could not find SPIFFS file or partition offset for '${PARTITION_NAMES.SPIFFS}' in manifest`)
@@ -567,6 +613,11 @@ export const useFirmwareStore = defineStore('firmware', {
this.flashingFileDescriptions = fileDescriptions
+ // For clean install, we need at least the factory file
+ if (!hasFactory) {
+ throw new Error('Clean install requires a factory binary (.factory.bin) file')
+ }
+
if (filesToFlash.length === 0) {
throw new Error('No files found to flash')
}
Analysis
Bug Analysis
Why it happens:
The updateEspFlash and cleanInstallEspFlash methods in stores/firmwareStore.ts checked whether files and partition offsets existed in the manifest, but did not wrap the async fetchBinaryContent calls in error handling. If a binary fetch failed (network error, file not found, etc.), the exception would propagate without properly validating which files were successfully fetched.
When it manifests:
- User selects a firmware and initiates manifest-driven flash
- App0 file is found and offset exists → code attempts to fetch binary
- Network error occurs or file doesn't exist at expected URL
- Exception thrown but caught by outer try-catch block
- BUT: The critical issue is the validation logic only checked
filesToFlash.length === 0 - If app0 fetch failed but app1 succeeded (or vice versa), partial data would be in
filesToFlash - Flash would proceed with incomplete binary, potentially corrupting device firmware
Impact:
- Device could be left in a bricked/unstable state with incomplete firmware
- Silent failures where fetch errors are caught but validation still allows partial flash
- No distinction between different types of failures
Fix Explanation
The fix implements comprehensive error handling and validation:
-
Added tracking flags for each required file (hasApp0, hasApp1, hasFactory) to explicitly track which files were successfully fetched AND had valid content
-
Wrapped fetchBinaryContent calls in try-catch blocks with specific error logging instead of letting exceptions propagate uncaught
-
Added content validation checking
if (appContent)to ensure fetched data is non-empty -
Added mandatory requirement validation:
- updateEspFlash now requires BOTH app0 AND app1 before attempting flash
- cleanInstallEspFlash now requires at least factory file before attempting flash
- Throws clear error messages indicating which files are missing
-
Improved error messages showing exactly which file failed and why, instead of generic error propagation
This ensures that:
- All required files are present before attempting any flash operation
- Fetch errors are logged with context about which file failed
- Empty/invalid content is rejected
- Users get clear feedback about what went wrong
| lastRawLength.value = newLength | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
Vue watcher can attempt to write to a disposed xterm terminal after component unmount, causing runtime errors
View Details
📝 Patch Details
diff --git a/components/SerialMonitor.vue b/components/SerialMonitor.vue
index ba4f3cb..0ad4aae 100644
--- a/components/SerialMonitor.vue
+++ b/components/SerialMonitor.vue
@@ -148,7 +148,7 @@ onUnmounted(() => {
watch(
() => serialMonitorStore.rawBuffer,
(newRawBuffer) => {
- if (!terminal.value) {
+ if (!terminal.value || terminal.value.isDisposed()) {
initTerminal()
return
}
@@ -185,7 +185,7 @@ const clearTerminal = () => {
serialMonitorStore.terminalBuffer = []
serialMonitorStore.rawBuffer = ''
lastRawLength.value = 0
- if (terminal.value) {
+ if (terminal.value && !terminal.value.isDisposed()) {
terminal.value.clear()
}
}
Analysis
Bug Explanation
Why it happens:
The component uses Vue's watch() to monitor serialMonitorStore.rawBuffer and write new data to the xterm terminal. When the component unmounts, onUnmounted() calls terminal.value.dispose() to clean up the terminal instance. However, there's a race condition where the watcher can fire AFTER the terminal has been disposed but before all references are cleaned up.
When it manifests:
This occurs in rare timing scenarios where:
- The component is being unmounted
- Simultaneously, new data arrives in
rawBuffer - The watcher callback fires before it's unregistered
- It attempts to call
write(),clear(), or other methods on an already-disposed terminal - xterm throws an error: "Terminal has been disposed"
Impact:
- Runtime errors that can crash the serial monitor component
- Potential console errors during normal disconnect/reconnect cycles
- Component state becomes unreliable
Fix Explanation
What changed:
Added explicit disposal state checks using xterm's isDisposed() method before attempting any operations on the terminal:
-
In the watch callback (line 151): Changed the condition from
if (!terminal.value)toif (!terminal.value || terminal.value.isDisposed()). This ensures the watcher doesn't attempt to write to a disposed terminal and instead re-initializes if needed. -
In clearTerminal() function (line 188): Added check to condition from
if (terminal.value)toif (terminal.value && !terminal.value.isDisposed()). This prevents errors if the clear button is clicked after unmount during a race condition.
Why it solves the issue:
- The
isDisposed()method is xterm's official API to check if a terminal instance has been disposed - By checking this state before any write/clear operations, we prevent attempting to use a disposed terminal
- If a disposed terminal is detected, we attempt re-initialization (which may fail gracefully if the component is already unmounting)
- This covers both the main watcher and the manual clear function, providing comprehensive protection
| }, | ||
| /** | ||
| * Load the target-specific manifest for a given target board | ||
| * This should be called before flashing when variant options (MUI/InkHUD) are selected |
There was a problem hiding this comment.
Partial firmware flash can proceed with incomplete data when some required files are missing, with only console errors and no user-facing error UI feedback.
View Details
📝 Patch Details
diff --git a/stores/firmwareStore.ts b/stores/firmwareStore.ts
index 82b3d0a..fb8d5b9 100644
--- a/stores/firmwareStore.ts
+++ b/stores/firmwareStore.ts
@@ -436,38 +436,32 @@ export const useFirmwareStore = defineStore('firmware', {
appFile = this.findAppFileByConvention()
}
const appOffset = this.getPartitionOffset(PARTITION_NAMES.APP0)
- if (appFile && appOffset !== undefined) {
- const appContent = await this.fetchBinaryContent(appFile.name)
- filesToFlash.push({ data: appContent, address: appOffset })
- fileDescriptions.push('Flashing app')
- console.log(`App0: ${appFile.name} at offset 0x${appOffset.toString(16)}`)
- }
- else {
- console.error(`Could not find app0 file or partition offset in manifest`)
+ if (!appFile || appOffset === undefined) {
+ throw new Error(`Could not find app0 file or partition offset in manifest`)
}
+ const appContent = await this.fetchBinaryContent(appFile.name)
+ filesToFlash.push({ data: appContent, address: appOffset })
+ fileDescriptions.push('Flashing app')
+ console.log(`App0: ${appFile.name} at offset 0x${appOffset.toString(16)}`)
+
// Find the OTA file (app1 partition)
let otaFile = this.findFileByPartName(PARTITION_NAMES.APP1)
if (!otaFile) {
otaFile = this.findOtaFileByConvention()
}
const otaOffset = this.getPartitionOffset(PARTITION_NAMES.APP1)
- if (otaFile && otaOffset !== undefined) {
- const otaContent = await this.fetchBinaryContent(otaFile.name)
- filesToFlash.push({ data: otaContent, address: otaOffset })
- fileDescriptions.push('Flashing OTA')
- console.log(`App1 (OTA): ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
- }
- else {
- console.error(`Could not find app1 (OTA) file or partition offset in manifest`)
+ if (!otaFile || otaOffset === undefined) {
+ throw new Error(`Could not find app1 (OTA) file or partition offset in manifest`)
}
+
+ const otaContent = await this.fetchBinaryContent(otaFile.name)
+ filesToFlash.push({ data: otaContent, address: otaOffset })
+ fileDescriptions.push('Flashing OTA')
+ console.log(`App1 (OTA): ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
this.flashingFileDescriptions = fileDescriptions
- if (filesToFlash.length === 0) {
- throw new Error('No files found to flash')
- }
-
this.port = await navigator.serial.requestPort({})
this.isConnected = true
this.port.ondisconnect = () => {
@@ -523,54 +517,47 @@ export const useFirmwareStore = defineStore('firmware', {
// Find the factory binary (combined binary for clean install)
const factoryFile = this.findFactoryFile()
- if (factoryFile) {
- const appContent = await this.fetchBinaryContent(factoryFile.name)
- filesToFlash.push({ data: appContent, address: 0x00 })
- fileDescriptions.push('Flashing factory app')
- console.log(`Factory: ${factoryFile.name} at offset 0x00`)
- }
- else {
- console.error('Could not find factory binary (.factory.bin) in manifest')
+ if (!factoryFile) {
+ throw new Error('Could not find factory binary (.factory.bin) in manifest')
}
+ const appContent = await this.fetchBinaryContent(factoryFile.name)
+ filesToFlash.push({ data: appContent, address: 0x00 })
+ fileDescriptions.push('Flashing factory app')
+ console.log(`Factory: ${factoryFile.name} at offset 0x00`)
+
// Find the OTA binary (app1 partition)
let otaFile = this.findFileByPartName(PARTITION_NAMES.APP1)
if (!otaFile) {
otaFile = this.findOtaFileByConvention()
}
const otaOffset = this.getPartitionOffset(PARTITION_NAMES.APP1)
- if (otaFile && otaOffset !== undefined) {
- const otaContent = await this.fetchBinaryContent(otaFile.name)
- filesToFlash.push({ data: otaContent, address: otaOffset })
- fileDescriptions.push('Flashing OTA app')
- console.log(`OTA: ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
- }
- else {
- console.error(`Could not find OTA file or partition offset for '${PARTITION_NAMES.APP1}' in manifest`)
+ if (!otaFile || otaOffset === undefined) {
+ throw new Error(`Could not find OTA file or partition offset for '${PARTITION_NAMES.APP1}' in manifest`)
}
+ const otaContent = await this.fetchBinaryContent(otaFile.name)
+ filesToFlash.push({ data: otaContent, address: otaOffset })
+ fileDescriptions.push('Flashing OTA app')
+ console.log(`OTA: ${otaFile.name} at offset 0x${otaOffset.toString(16)}`)
+
// Find the LittleFS/SPIFFS binary
let spiffsFile = this.findFileByPartName(PARTITION_NAMES.SPIFFS)
if (!spiffsFile) {
spiffsFile = this.findSpiffsFileByConvention()
}
const spiffsOffset = this.getPartitionOffset(PARTITION_NAMES.SPIFFS)
- if (spiffsFile && spiffsOffset !== undefined) {
- const spiffsContent = await this.fetchBinaryContent(spiffsFile.name)
- filesToFlash.push({ data: spiffsContent, address: spiffsOffset })
- fileDescriptions.push('Flashing filesystem')
- console.log(`SPIFFS: ${spiffsFile.name} at offset 0x${spiffsOffset.toString(16)}`)
- }
- else {
- console.error(`Could not find SPIFFS file or partition offset for '${PARTITION_NAMES.SPIFFS}' in manifest`)
+ if (!spiffsFile || spiffsOffset === undefined) {
+ throw new Error(`Could not find SPIFFS file or partition offset for '${PARTITION_NAMES.SPIFFS}' in manifest`)
}
+
+ const spiffsContent = await this.fetchBinaryContent(spiffsFile.name)
+ filesToFlash.push({ data: spiffsContent, address: spiffsOffset })
+ fileDescriptions.push('Flashing filesystem')
+ console.log(`SPIFFS: ${spiffsFile.name} at offset 0x${spiffsOffset.toString(16)}`)
this.flashingFileDescriptions = fileDescriptions
- if (filesToFlash.length === 0) {
- throw new Error('No files found to flash')
- }
-
this.port = await navigator.serial.requestPort({})
this.isConnected = true
this.port.ondisconnect = () => {
Analysis
Bug Analysis
Root Cause:
In both updateEspFlash() and cleanInstallEspFlash() functions, missing files are handled with console.error() but the function continues execution. The only validation is checking if filesToFlash.length === 0, which only throws an error if NO files are found at all.
When It Manifests:
- User attempts to flash firmware with a manifest that has missing files
- One required file is found (e.g., app0) but another is missing (e.g., app1)
- The missing file's error is logged to browser console only
- Function continues and performs partial flash (only app0)
- User sees no error in the UI, only sees flash complete
Impact:
- For
updateEspFlash(): Device gets incomplete OTA setup (app0 without app1 fallback partition) - For
cleanInstallEspFlash(): Device gets incomplete installation (factory without app1 and/or SPIFFS) - Users are not aware the flash was incomplete
- Device may be left in a non-functional state
- Critical files required for proper operation are silently skipped
Specific Issues:
updateEspFlash()requires both app0 AND app1 - if either file or offset is missing, flash should failcleanInstallEspFlash()requires factory binary, app1 partition, and SPIFFS - if any is missing, flash should fail- Errors are only logged to console, not displayed in terminal UI to user
- No validation prevents the flash from proceeding with incomplete data
Fix Applied
Changed error handling from logging and continuing to throwing errors that prevent partial flashing:
In updateEspFlash():
- Changed app0 check from
if (appFile && appOffset !== undefined) { add } else { console.error() }toif (!appFile || appOffset === undefined) { throw error } - Changed app1 check from
if (otaFile && otaOffset !== undefined) { add } else { console.error() }toif (!otaFile || otaOffset === undefined) { throw error } - Removed the final
if (filesToFlash.length === 0)check since it's now redundant - Now requires BOTH files before proceeding
In cleanInstallEspFlash():
- Changed factory file check from
if (factoryFile) { add } else { console.error() }toif (!factoryFile) { throw error } - Changed app1 check similarly to
updateEspFlash() - Changed SPIFFS check similarly to
updateEspFlash() - Now requires ALL THREE files before proceeding
Benefits:
- Prevents partial flashing with incomplete data
- Errors are thrown and caught by
handleError(), which displays them in the terminal UI - User gets clear error messages about what's missing
- Flash operation fails completely rather than partially, preventing device corruption
- Consistent error handling pattern across both functions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.