Skip to content

Commit 306c325

Browse files
committed
fix: preserve ride history storage errors
1 parent b79bf23 commit 306c325

8 files changed

Lines changed: 498 additions & 22 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ jobs:
3333
run: python3 tools/test_storage_csv_tail.py --compiler cc
3434
- name: Test Notes storage
3535
run: python3 tools/test_notes_io.py --compiler cc
36+
- name: Test ride history storage
37+
run: python3 tools/test_ride_history.py --compiler cc
3638
- name: Test capture reader
3739
run: |
3840
cc -std=c11 -Wall -Wextra -Werror -pedantic -I main main/capture_data.c tests/capture_data_test.c -o "$RUNNER_TEMP/capture_data_test"
@@ -131,6 +133,20 @@ jobs:
131133
if-no-files-found: warn
132134
retention-days: 14
133135

136+
- name: Retain ride history test evidence
137+
uses: actions/upload-artifact@v4
138+
if: always()
139+
with:
140+
name: ride-history-logs
141+
path: |
142+
build/ride-history/compile.log
143+
build/ride-history/result.log
144+
build/ride-history/source.json
145+
build/ride-history/ride_history.inc
146+
build/ride-history/storage_source.inc
147+
if-no-files-found: warn
148+
retention-days: 14
149+
134150
firmware:
135151
runs-on: ubuntu-latest
136152
container: espressif/idf:v5.4.2

‎ROADMAP.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ Phase 1 software progress is also underway:
6363
- [x] System reports the firmware/commit, reset reason, uptime, panel, heap, storage capacity, current OTA state, and persistent last OTA result.
6464
- [x] Notes and ride summaries use recoverable backup replacement; rides publish only after durable sync, and health logs repair incomplete final rows.
6565
- [x] Notes loads complete files up to 65,535 bytes, replacing a silent 2,047-byte prefix. Recovery/read/close errors, oversized files and NUL data disable Save; byte-length and SD checks run before storage changes. Its launcher leave hook clears editor ownership. Four preceding-source controls reproduce full-note truncation; 36 native file/fault cases and five real LVGL cases pass, including maximum ASCII/UTF-8 notes and 25 reopens without retained resources in the unchanged 96 KiB pool. Linux and Windows CI retain separate source-bound evidence. Physical SD/panel and power cuts remain open.
66+
- [x] Ride history reports recovery/open/read/close errors as unavailable instead of showing zero or partial totals. Summary appends check SD availability and preparation errors, reject failed read chunks before copying, and retain the first error through cleanup. All 50 native file/fault cases pass; the preceding callbacks fail 24 controls. Exact final/backup/temporary bytes and 25 reopens are checked with closed modeled handles. Linux CI retains source hashes and logs, and the unflashed firmware builds with unchanged configuration. Physical SD/panel and power cuts remain open.
6667
- [x] Shared CSV tail recovery now scans backwards in 512-byte blocks. Eight previous-source controls exceed stdio seek budgets; a 64 KiB partial tail drops from 65,538 seeks to 130 with the same retained prefix. All 32 native byte/boundary/fault cases, original storage checks and five MQTT/HTTP logging cases pass with closed modeled handles. The unflashed firmware builds with unchanged configuration. Linux CI retains source hashes and compilation/result logs; physical SD timing and power cuts remain open.
6768
- [x] Built-in SD paths, timestamps, units, 8.3-safe collision names, `.TMP`, and `.BAK` behavior are documented.
6869
- [x] Persisted brightness choices, a dimmed two-minute screen, and configurable 5/10/30-minute or Never screen-off behavior are implemented.

‎docs/architecture.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ CSV tail repair uses a 512-byte scratch buffer and scans backwards by blocks bef
8585

8686
Notes uses a temporary PSRAM buffer to load an entire file of up to 65,535 bytes and frees that buffer after LVGL copies the text. It applies the editor's character limit after loading, using LVGL's bulk text path. Recovery/read/close failures, oversized files and NUL data disable editing and Save; the save callback rechecks ownership, SD availability and byte length before changing paths. The launcher leave hook clears its editor pointer and permission before content deletion. The existing replacement helper still syncs/closes `NOTE.TMP`, retains the previous complete generation in `NOTE.BAK`, and reports publication failures. Real LVGL checks use the unchanged 96 KiB pool; physical SD and panel behavior remain open.
8787

88+
Ride history shows totals only after recovery, reading and closing the summary succeed; a missing index is valid empty history. Storage errors display an unavailable message and update the shared SD error. Appending checks known SD availability, directory creation and temporary-file removal before copying. A failed read chunk is rejected before writing, and the first read/write/open error survives source/destination close and temporary cleanup failures. The existing replacement helper still owns sync, backup, publication and rollback. Native tests verify exact file bytes and closed modeled handles; physical FAT/SD and panel behavior remain open.
89+
8890
See [Storage and compatibility contract](compatibility.md) and [Data formats](data-formats.md).
8991

9092
## Network and radio

‎docs/data-formats.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ Ride summary header:
5151
start_unix,duration_s,distance_km,work_kj,avg_power_w,max_power_w,avg_hr,max_hr
5252
```
5353

54+
History totals require successful backup recovery, open, read and close. A missing index shows empty history; storage errors show `History unavailable` instead of zero or partial totals. Summary appends check SD availability and preparation errors, reject failed read chunks before copying, and preserve the first error through cleanup. Failed cleanup can leave an unpublished `SUMMARY.TMP`; publication or rollback errors can also retain the temporary or backup generation. The individual ride CSV remains authoritative. Native files and controlled errors verify these boundaries; physical SD and power-loss behavior remain hardware checks.
55+
5456
Heart-rate header:
5557

5658
```text

‎docs/testing.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Python 3 and Node.js are needed for the quick checks. The configuration checks a
1212
| Relay, parsers, calculations, clipboard or file formats | [Core command reference](#core-command-reference); choose the affected module and any independent reference test. |
1313
| Storage writes or incomplete CSV rows | [Core command reference](#core-command-reference), [CSV tail recovery](#csv-tail-recovery) and a firmware build. |
1414
| Notes loading, saving or lifecycle | [Notes storage and UI](#notes-storage-and-ui) and a firmware build. |
15+
| Ride history loading or summary appends | [Ride history storage](#ride-history-storage) and a firmware build. |
1516
| Offline apps, SPI or I2C editor/result flows | [Offline and peripheral UI](#offline-and-peripheral-ui). |
1617
| Serial, saved serial logs or RTU Frames | [Serial and RTU UI](#serial-and-rtu-ui). |
1718
| Modbus TCP, NTP, Wake-on-LAN, UDP or HTTP fixtures | [Network fixtures and UI](#network-fixtures-and-ui). |
@@ -111,6 +112,18 @@ The optional real LVGL mode runs five additional cases against the cached pinned
111112

112113
Generated callbacks/config/storage source, full main/function/storage/header/test hashes and compile/result logs stay under the selected output directory. Linux CI retains the 36-case evidence as `notes-io-logs`. Windows CI runs the five real LVGL cases after the existing seven suites and retains separate `notes-ui-logs`; those five add to the prior 815 native checks. Both artifacts retain diagnostics when their test step fails.
113114

115+
## Ride history storage
116+
117+
```powershell
118+
python tools/test_ride_history.py --compiler 'C:/path/to/clang.exe'
119+
```
120+
121+
This portable fixture compiles the two actual ride-history callbacks extracted from `main/main.c` and the complete `storage_io.c`. Its 50 cases check missing/empty indexes, exact aggregate totals and appended rows, backup recovery, unavailable SD, preparation/open/read/write/flush/sync/close/publication/rollback errors and 25 history reopens. Every case independently checks final, backup and temporary bytes with no remaining modeled file handles. UI and I/O error boundaries are controlled; the files use the native filesystem.
122+
123+
The preceding callbacks fail 24 controls, including misleading totals after recovery/read/close errors, ignored SD/preparation errors and lost first errors during cleanup. Compounded read/copy/open failures retain their original error when later closes or removal fail. Missing read/write errno falls back to `EIO`; a successful earlier history read cannot supply a stale errno for a later failure. Multi-block and full-count read-error cases reject the failed chunk before copying. Cleanup failure can retain an unpublished partial temporary file, and publication/rollback failures can retain recovery generations. These checks do not validate every CSV field or prove physical FAT/SD latency, removal, power cuts or panel rendering.
124+
125+
Generated callbacks/storage source, full main/function/storage/header/test hashes and compile/result logs stay under `build/ride-history/`. Linux CI runs the fixture and retains these files as `ride-history-logs`, including diagnostics when compilation or execution fails. Its 50 cases are separate from the Windows native total.
126+
114127
## OTA manifests
115128

116129
```powershell

‎main/main.c‎

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6509,8 +6509,15 @@ static void govee_clicked(lv_event_t *event)
65096509
static void ride_load_history(void)
65106510
{
65116511
if (!ride_history) return;
6512-
storage_recover_replace(SD_PATH "/RIDES/SUMMARY.CSV", SD_PATH "/RIDES/SUMMARY.BAK");
6513-
FILE *file = fopen(SD_PATH "/RIDES/SUMMARY.CSV", "rb");
6512+
int error = sd_ready ? 0 : sd_error_snapshot();
6513+
if (!sd_ready && !error) error = ENODEV;
6514+
if (!error && storage_recover_replace(SD_PATH "/RIDES/SUMMARY.CSV", SD_PATH "/RIDES/SUMMARY.BAK") != 0)
6515+
error = errno ? errno : EIO;
6516+
FILE *file = NULL;
6517+
if (!error) {
6518+
file = fopen(SD_PATH "/RIDES/SUMMARY.CSV", "rb");
6519+
if (!file && errno != ENOENT) error = errno ? errno : EIO;
6520+
}
65146521
int rides = 0, best_power = 0;
65156522
long long start;
65166523
unsigned duration;
@@ -6520,15 +6527,23 @@ static void ride_load_history(void)
65206527
float total_distance = 0;
65216528
char line[160];
65226529
if (file) {
6523-
while (fgets(line, sizeof(line), file)) {
6530+
for (;;) {
6531+
errno = 0;
6532+
if (!fgets(line, sizeof(line), file)) break;
65246533
if (sscanf(line, "%lld,%u,%f,%f,%d,%d,%d,%d", &start, &duration, &distance,
65256534
&work, &average_power, &maximum_power, &average_hr, &maximum_hr) != 8) continue;
65266535
rides++;
65276536
total_seconds += duration;
65286537
total_distance += distance;
65296538
if (maximum_power > best_power) best_power = maximum_power;
65306539
}
6531-
fclose(file);
6540+
if (ferror(file)) error = errno ? errno : EIO;
6541+
if (fclose(file) != 0 && !error) error = errno ? errno : EIO;
6542+
}
6543+
if (error) {
6544+
sd_record_error(error);
6545+
lv_label_set_text_fmt(ride_history, "History unavailable: %s", strerror(error));
6546+
return;
65326547
}
65336548
char history_text[160];
65346549
snprintf(history_text, sizeof(history_text), "History: %d rides | %.1f mi | %.1f hr | best %d W",
@@ -6538,47 +6553,59 @@ static void ride_load_history(void)
65386553

65396554
static bool ride_append_summary(unsigned duration)
65406555
{
6541-
mkdir(SD_PATH "/RIDES", 0775);
6556+
if (!sd_ready) {
6557+
int error = sd_error_snapshot();
6558+
errno = error ? error : ENODEV;
6559+
return false;
6560+
}
6561+
if (mkdir(SD_PATH "/RIDES", 0775) != 0 && errno != EEXIST) return false;
65426562
const char *temporary_path = SD_PATH "/RIDES/SUMMARY.TMP";
65436563
const char *final_path = SD_PATH "/RIDES/SUMMARY.CSV";
65446564
const char *backup_path = SD_PATH "/RIDES/SUMMARY.BAK";
65456565
if (storage_recover_replace(final_path, backup_path) != 0) return false;
6546-
remove(temporary_path);
6566+
if (remove(temporary_path) != 0 && errno != ENOENT) return false;
65476567
FILE *source = fopen(final_path, "rb");
65486568
if (!source && errno != ENOENT) return false;
65496569
FILE *file = fopen(temporary_path, "wb");
65506570
if (!file) {
6571+
int error = errno ? errno : EIO;
65516572
if (source) fclose(source);
6573+
errno = error;
65526574
return false;
65536575
}
6554-
bool ok = true;
6576+
int error = 0;
65556577
bool empty = true;
65566578
int last = '\n';
65576579
char buffer[512];
6558-
while (source && ok) {
6580+
while (source && !error) {
6581+
errno = 0;
65596582
size_t length = fread(buffer, 1, sizeof(buffer), source);
6583+
if (ferror(source)) {
6584+
error = errno ? errno : EIO;
6585+
break;
6586+
}
65606587
if (length) {
65616588
empty = false;
65626589
last = (unsigned char)buffer[length - 1];
6563-
ok = fwrite(buffer, 1, length, file) == length;
6590+
if (fwrite(buffer, 1, length, file) != length) error = errno ? errno : EIO;
65646591
}
6565-
if (length < sizeof(buffer)) {
6566-
if (ferror(source)) ok = false;
6567-
break;
6568-
}
6569-
}
6570-
if (source && fclose(source) != 0) ok = false;
6571-
if (ok && !empty && last != '\n') ok = fputc('\n', file) != EOF;
6572-
if (ok && empty)
6573-
ok = fputs("start_unix,duration_s,distance_km,work_kj,avg_power_w,max_power_w,avg_hr,max_hr\n", file) >= 0;
6574-
if (ok)
6575-
ok = fprintf(file, "%lld,%u,%.3f,%.1f,%d,%d,%d,%d\n", (long long)ride_started_at, duration,
6592+
if (length < sizeof(buffer)) break;
6593+
}
6594+
if (source && fclose(source) != 0 && !error) error = errno ? errno : EIO;
6595+
if (!error && !empty && last != '\n' && fputc('\n', file) == EOF) error = errno ? errno : EIO;
6596+
if (!error && empty &&
6597+
fputs("start_unix,duration_s,distance_km,work_kj,avg_power_w,max_power_w,avg_hr,max_hr\n", file) < 0)
6598+
error = errno ? errno : EIO;
6599+
if (!error &&
6600+
fprintf(file, "%lld,%u,%.3f,%.1f,%d,%d,%d,%d\n", (long long)ride_started_at, duration,
65766601
ride_distance_km, ride_work_kj,
65776602
ride_power_samples ? (int)(ride_power_sum / ride_power_samples) : 0, ride_max_power,
6578-
ride_hr_samples_count ? (int)(ride_hr_sum / ride_hr_samples_count) : 0, ride_max_hr) >= 0;
6579-
if (!ok) {
6603+
ride_hr_samples_count ? (int)(ride_hr_sum / ride_hr_samples_count) : 0, ride_max_hr) < 0)
6604+
error = errno ? errno : EIO;
6605+
if (error) {
65806606
fclose(file);
65816607
remove(temporary_path);
6608+
errno = error;
65826609
return false;
65836610
}
65846611
return storage_commit_replace_file(&file, temporary_path, final_path, backup_path) == 0;

0 commit comments

Comments
 (0)