Add generic file management with upload support for BLE#10
Open
TheAngryRaven wants to merge 11 commits intomasterfrom
Open
Add generic file management with upload support for BLE#10TheAngryRaven wants to merge 11 commits intomasterfrom
TheAngryRaven wants to merge 11 commits intomasterfrom
Conversation
- Add fileUploadChar (0x2A41) BLE characteristic for upload data
- Make bleSendFileList() generic with directory parameter
- Add upload functions: bleStartUpload(), bleUploadDataCallback(),
bleFinishUpload(), bleCancelUpload()
- Extend command handler with LIST:{path}, UPLOAD:{path}:{size},
UPLOAD_DONE, UPLOAD_CANCEL commands
- Update BLE_STOP() to clean up upload state
This enables the web app to manage both datalogs (root) and
track files (/TRACKS/) using the same generic BLE file operations.
https://claude.ai/code/session_01GMZQpxEgBp68v7n2ntKJtf
Owner
Author
|
@claude can you do a code review? |
Owner
Author
Add yml to branch
Owner
Author
Phase 1 critical fixes:
1. INTERRUPT DEADLOCK FIX (root cause of page-freeze bug)
- Removed noInterrupts() call from TACH_COUNT_PULSE ISR
- ISR was disabling ALL system interrupts, and if main loop was
delayed (e.g., during SD write), interrupts() never got called
- Now uses purely volatile flag gating - no system-wide interrupt disable
- Added detailed EMI/hardware shielding recommendations in comments
2. BUFFER OVERFLOW FIX
- Fixed char filepath[13] -> filepath[50] at location selection
- makeFullTrackPath creates paths up to 27 bytes, was overflowing
- Added FILEPATH_MAX constant for consistent buffer sizing
- Rewrote makeFullTrackPath to use snprintf for bounds safety
3. GPS NULL SAFETY
- Added gpsInitialized flag set only after successful GPS init
- GPS_SETUP now verifies allocation succeeded before continuing
- Added null checks to GPS_LOOP (main loop protection)
- Added null checks to helper functions (getGpsTimeInMilliseconds, etc.)
- Added null checks to display functions that access GPS data
- Graceful degradation: displays "GPS not available" instead of crashing
https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Phase 2 improvements:
1. STRNCPY BUFFER OVERFLOW FIX
- Fixed strncpy at location loading: was using sizeof(source) instead
of sizeof(destination), could overflow 13-byte buffer with 25-byte source
- Added explicit null-termination to ensure string safety
2. STANDARDIZED FILEPATH BUFFER SIZES
- Added FILEPATH_MAX constant (50 bytes) for consistent sizing
- Updated all filepath[] declarations to use FILEPATH_MAX
- Single point of change if path requirements ever increase
3. SD CARD ACCESS STATE MANAGEMENT
- Added SDAccessMode enum to track which subsystem owns SD access
- Added acquireSDAccess()/releaseSDAccess() functions
- Prevents race conditions between:
* Data logging (SD_ACCESS_LOGGING)
* Replay processing (SD_ACCESS_REPLAY)
* BLE file transfers (SD_ACCESS_BLE_TRANSFER)
* Track file parsing (SD_ACCESS_TRACK_PARSE)
- Shows user-friendly "SD card busy!" error instead of corrupt writes
- Proper cleanup on errors, disconnects, and mode switches
Integration points:
- GPS_LOOP: Acquires logging access before opening dataFile
- Logging stop: Releases access when user ends session
- Replay start: Acquires replay access before opening replayFile
- resetReplayState: Releases access when exiting replay
- BLE transfer: Acquires/releases for file transfers
- BLE disconnect: Releases if transfer was in progress
https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Phase 3 improvements for phantom button press prevention:
1. MULTI-SAMPLE BUTTON VERIFICATION
- New readButtonMultiSample() function takes 3 samples with 500μs delays
- ALL samples must show LOW (pressed) to register a button press
- Single EMI spike causing one false LOW reading is rejected
- Total sampling window: ~1ms (negligible latency)
2. IMPROVED DEBOUNCE TIMING
- Reduced antiBounceIntv from 500ms to 150ms
- Allows ~6 button presses per second (was only 2/sec)
- Much more responsive menu navigation
- Multi-sample verification handles noise rejection instead of long timeout
3. HARDWARE EMI RECOMMENDATIONS
- Added detailed comments for physical button hardening:
* RC low-pass filter: 10K + 100nF (~160Hz cutoff)
* Wire routing away from tach/ignition
* Ground wire shielding on ribbon cables
* Ferrite bead on button cable
The combination of hardware filtering (if implemented) and software
multi-sample verification should eliminate phantom button presses
even in electrically noisy racing environments.
https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Phase 4 - Data integrity and final cleanup:
1. GPS DATA TORN READ FIX (Root cause of CSV corruption)
- On 32-bit ARM, reading 64-bit doubles is NOT atomic
- GPS library could update gps->latitudeDegrees mid-read
- Result: half old value + half new value = "C"²ëB" garbage
- Fix: Snapshot all GPS values with interrupts disabled
- Applied to both CSV logging AND lap timer updates
2. GPS DATA VALIDATION
- Added range checks before logging:
* Latitude: -90 to 90
* Longitude: -180 to 180
* Altitude: -1000m to 50000m
* HDOP: 0 to 99.9
* Speed: 0 to 500 mph
* Satellites: 0 to 50
- Invalid samples are skipped with debug message
- Prevents garbage data from ever reaching the SD card
3. BUFFER SIZE INCREASES
- latStr/lngStr: 16 -> 24 bytes (safety margin)
- hdopStr: 8 -> 12 bytes
- speedStr/altStr: 12 -> 16 bytes
4. SPEED UPDATE FIX
- Moved gps_speed_mph update inside NMEA received block
- Previously updated every loop even without new data
- Now only updates when fresh GPS data arrives
The combination of atomic reads + validation ensures clean CSV data
even in electrically noisy racing environments with high GPS update rates.
https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Audited all noInterrupts()/interrupts() usage: REMOVED (unnecessary): - TACH_LOOP single 32-bit read of tachLastPulseUs On ARM Cortex-M4, aligned 32-bit reads are atomic by hardware. The ISR cannot interrupt mid-read, so protection is unnecessary overhead. KEPT (necessary): - TACH_LOOP read-modify-clear sequence (lines 228-234) This is a legitimate critical section: read flag, read value, clear flag. Without protection, ISR could write new data between read and clear, causing data loss. - GPS double reads (lines 3598-3603, 3625-3632) Defensive programming. GPS library *might* use DMA/interrupts internally. Overhead is ~1μs, doesn't hurt, protects against library changes. Added clarifying comments explaining WHY each critical section exists. https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Arduino's preprocessor generates function prototypes before processing enum definitions, causing "SDAccessMode not declared" errors. Fix: Changed from enum to #define constants with int parameters. Functionally identical, but compatible with Arduino's quirky preprocessor. https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Problem: With 150ms debounce, buttons were firing repeatedly while held because the code only checked if button was LOW, not if it had been released. Fix: Added edge detection via wasReleased flag in ButtonState. - Button must transition from released→pressed to register - Holding button down no longer causes repeated triggers - Increased debounce to 200ms for stability Before: Press and hold = fires every 150ms After: Press and hold = fires once, must release to fire again https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Root cause identified: Arduino's snprintf on NRF52840 doesn't support %llu format specifier for 64-bit integers. This caused the literal "lu,0,," garbage appearing in CSV output. Changes: - Manual 64-bit to string conversion for timestamp (avoids %llu) - Strengthened GPS validation (require satellites > 0, lat/lng != 0) - Added NaN/Inf checks on all GPS values - Added string validation after dtostrf to catch garbage characters - Fixed indentation in error handling block https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
Added missing } to close the GPGGA if block. https://claude.ai/code/session_019xucVnNpeYDjhLXySPXbyA
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This enables the web app to manage both datalogs (root) and track files (/TRACKS/) using the same generic BLE file operations.
https://claude.ai/code/session_01GMZQpxEgBp68v7n2ntKJtf