Skip to content

Commit 027b5b0

Browse files
committed
fix: retry Ring history after storage errors
1 parent 306c325 commit 027b5b0

8 files changed

Lines changed: 575 additions & 36 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ jobs:
3535
run: python3 tools/test_notes_io.py --compiler cc
3636
- name: Test ride history storage
3737
run: python3 tools/test_ride_history.py --compiler cc
38+
- name: Test Ring history storage
39+
run: python3 tools/test_ring_history.py --compiler cc
3840
- name: Test capture reader
3941
run: |
4042
cc -std=c11 -Wall -Wextra -Werror -pedantic -I main main/capture_data.c tests/capture_data_test.c -o "$RUNNER_TEMP/capture_data_test"
@@ -147,6 +149,20 @@ jobs:
147149
if-no-files-found: warn
148150
retention-days: 14
149151

152+
- name: Retain Ring history test evidence
153+
uses: actions/upload-artifact@v4
154+
if: always()
155+
with:
156+
name: ring-history-logs
157+
path: |
158+
build/ring-history/compile.log
159+
build/ring-history/result.log
160+
build/ring-history/source.json
161+
build/ring-history/ring_history.inc
162+
build/ring-history/storage_source.inc
163+
if-no-files-found: warn
164+
retention-days: 14
165+
150166
firmware:
151167
runs-on: ubuntu-latest
152168
container: espressif/idf:v5.4.2

‎ROADMAP.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ Phase 1 software progress is also underway:
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.
6666
- [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.
67+
- [x] Ring logging commits its saved timestamp only after tail recovery and a complete, successfully closed read. Failed loads retain queued samples and defer history requests until a successful retry; known SD unavailability also blocks a cached checkpoint. Appends check directory errors and preserve the first seek/tell/header/row error through close and repair. All 59 native file/queue/fault cases pass; the preceding callbacks fail 32 controls. Checks verify exact bytes, retry/deduplication, timeout handling while blocked and 25 checkpoint reopens with closed modeled handles. Linux CI retains source hashes and logs. Physical SD, BLE, panel and power cuts remain open.
6768
- [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.
6869
- [x] Built-in SD paths, timestamps, units, 8.3-safe collision names, `.TMP`, and `.BAK` behavior are documented.
6970
- [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
@@ -87,6 +87,8 @@ Notes uses a temporary PSRAM buffer to load an entire file of up to 65,535 bytes
8787

8888
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.
8989

90+
Ring's saved heart-rate timestamp becomes loaded only after incomplete-tail repair, seek/size checks, the expected bounded tail read and close succeed. Read or close errors leave the checkpoint unchanged and retryable. Its existing health timer retains queued samples and defers history requests until that load succeeds, then filters timestamps against the recovered checkpoint. Known SD unavailability blocks queue processing even when a checkpoint is cached; timeout state still advances before the storage gate. Appends preserve the first error through close and tail repair. Native files and controlled queue/timer/SD boundaries verify this behavior; physical SD, BLE and scheduling remain open.
91+
9092
See [Storage and compatibility contract](compatibility.md) and [Data formats](data-formats.md).
9193

9294
## Network and radio

‎docs/data-formats.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ Heart-rate header:
5959
unix_time,local_time,bpm
6060
```
6161

62+
Ring loads its saved timestamp from the repaired final 512 bytes only after successful seek, size, complete read and close checks. A failed checkpoint load leaves queued samples pending and defers history requests until recovery; known SD unavailability also blocks a previously cached checkpoint. Directory and append failures preserve the first error through close and tail repair. An unconfirmed flush/sync/close can leave a complete row; a failed write or repair can retain an unfinished suffix. A retry can repeat a row whose persistence was unconfirmed, so consumers should use `unix_time` when handling duplicates. Native checks cover file bytes and controlled queue/error behavior; physical SD and BLE remain hardware checks.
63+
6264
I2C watch header:
6365

6466
```text

‎docs/testing.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Python 3 and Node.js are needed for the quick checks. The configuration checks a
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. |
1515
| Ride history loading or summary appends | [Ride history storage](#ride-history-storage) and a firmware build. |
16+
| Ring heart-rate logging, saved timestamps or queue processing | [Ring history storage](#ring-history-storage) and a firmware build. |
1617
| Offline apps, SPI or I2C editor/result flows | [Offline and peripheral UI](#offline-and-peripheral-ui). |
1718
| Serial, saved serial logs or RTU Frames | [Serial and RTU UI](#serial-and-rtu-ui). |
1819
| Modbus TCP, NTP, Wake-on-LAN, UDP or HTTP fixtures | [Network fixtures and UI](#network-fixtures-and-ui). |
@@ -124,6 +125,18 @@ The preceding callbacks fail 24 controls, including misleading totals after reco
124125

125126
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.
126127

128+
## Ring history storage
129+
130+
```powershell
131+
python tools/test_ring_history.py --compiler 'C:/path/to/clang.exe'
132+
```
133+
134+
This portable fixture compiles the three actual Ring append/checkpoint/health-timer callbacks extracted from `main/main.c` and the complete `storage_io.c`. Its 59 cases use native files to check exact headers/rows, missing/empty logs, bounded long-tail loads, incomplete-tail repair, seek/size/read/close failures, short reads without an error flag, full-count reads with an error flag, stored SD errors and directory/append/flush/sync/close/repair faults. Every case verifies exact file bytes with no remaining modeled file handles. Twenty-five checkpoint reopens retain the same bytes and close every read.
135+
136+
The preceding callbacks fail 32 controls, including prematurely caching failed reads, consuming samples against an uncertain checkpoint, ignoring directory errors and losing seek/tell/header/row errors during close. Timer cases retain queued samples and stop history requests after failed loads, retry successfully without appending historical duplicates, clear recovered errors, retain a failed append for retry and update timeout state while storage is blocked. A cached checkpoint cannot bypass known SD unavailability. Missing I/O errno falls back to `EIO`, and an earlier successful seek cannot supply stale errno for a failed read. Sync/close errors can follow a complete append; repair failure can retain an unfinished suffix. These checks do not promise rollback, exactly-once logging or power-loss durability.
137+
138+
Queue/timer and SD error APIs are controlled, time conversion uses UTC for independently expected rows, and no BLE traffic occurs. They do not run FreeRTOS/NimBLE or establish physical SD removal/full-card behavior, radio delivery, scheduling or panel rendering. Generated callbacks/storage source, full main/function/storage/header/test hashes and compile/result logs stay under `build/ring-history/`. Linux CI retains these files as `ring-history-logs`, including compiler/test failures; the 59 cases are separate from the Windows native total.
139+
127140
## OTA manifests
128141

129142
```powershell

‎main/main.c‎

Lines changed: 89 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,85 +1019,133 @@ static bool ring_hr_append_log(time_t sample_time, int heart_rate)
10191019
errno = error ? error : ENODEV;
10201020
return false;
10211021
}
1022-
mkdir(HEALTH_PATH, 0775);
1022+
errno = 0;
1023+
if (mkdir(HEALTH_PATH, 0775) != 0 && errno != EEXIST) {
1024+
int error = errno ? errno : EIO;
1025+
sd_record_error(error);
1026+
errno = error;
1027+
return false;
1028+
}
10231029
if (storage_repair_csv_tail(HEART_RATE_LOG) != 0) {
10241030
int error = errno ? errno : EIO;
10251031
sd_record_error(error);
10261032
errno = error;
10271033
return false;
10281034
}
1035+
errno = 0;
10291036
FILE *file = fopen(HEART_RATE_LOG, "ab+");
10301037
if (!file) {
10311038
int error = errno ? errno : EIO;
10321039
sd_record_error(error);
10331040
errno = error;
10341041
return false;
10351042
}
1036-
bool ok = fseek(file, 0, SEEK_END) == 0;
1037-
long size = ok ? ftell(file) : -1;
1038-
if (size < 0) ok = false;
1039-
if (ok && size == 0) ok = fputs("unix_time,local_time,bpm\n", file) >= 0;
1043+
int write_error = 0;
1044+
errno = 0;
1045+
if (fseek(file, 0, SEEK_END) != 0) write_error = errno ? errno : EIO;
1046+
long size = -1;
1047+
if (!write_error) {
1048+
errno = 0;
1049+
size = ftell(file);
1050+
if (size < 0) write_error = errno ? errno : EIO;
1051+
}
1052+
if (!write_error && size == 0) {
1053+
errno = 0;
1054+
if (fputs("unix_time,local_time,bpm\n", file) < 0) write_error = errno ? errno : EIO;
1055+
}
10401056
struct tm local;
10411057
char timestamp[24];
10421058
localtime_r(&sample_time, &local);
10431059
strftime(timestamp, sizeof(timestamp), "%Y-%m-%d %H:%M:%S", &local);
1044-
if (ok) ok = fprintf(file, "%lld,%s,%d\n", (long long)sample_time, timestamp, heart_rate) >= 0;
1045-
int write_error = 0;
1046-
if (ok && storage_sync_file(file) != 0) {
1047-
write_error = errno ? errno : EIO;
1048-
ok = false;
1060+
if (!write_error) {
1061+
errno = 0;
1062+
if (fprintf(file, "%lld,%s,%d\n", (long long)sample_time, timestamp, heart_rate) < 0)
1063+
write_error = errno ? errno : EIO;
10491064
}
1050-
if (fclose(file) != 0) {
1051-
if (!write_error) write_error = errno ? errno : EIO;
1052-
ok = false;
1065+
if (!write_error) {
1066+
errno = 0;
1067+
if (storage_sync_file(file) != 0) write_error = errno ? errno : EIO;
10531068
}
1054-
if (!ok) {
1055-
if (!write_error) write_error = errno ? errno : EIO;
1069+
errno = 0;
1070+
if (fclose(file) != 0 && !write_error) write_error = errno ? errno : EIO;
1071+
if (write_error) {
10561072
storage_repair_csv_tail(HEART_RATE_LOG);
10571073
sd_record_error(write_error);
10581074
errno = write_error;
10591075
}
1060-
return ok;
1076+
return write_error == 0;
10611077
}
10621078

1063-
static void ring_hr_load_last_saved(void)
1079+
static bool ring_hr_load_last_saved(void)
10641080
{
1065-
if (ring_hr_last_saved_loaded) return;
1081+
if (!sd_ready) {
1082+
int error = sd_error_snapshot();
1083+
errno = error ? error : ENODEV;
1084+
return false;
1085+
}
1086+
if (ring_hr_last_saved_loaded) return true;
10661087
if (storage_repair_csv_tail(HEART_RATE_LOG) != 0) {
1067-
sd_record_error(errno ? errno : EIO);
1068-
return;
1088+
int error = errno ? errno : EIO;
1089+
sd_record_error(error);
1090+
errno = error;
1091+
return false;
10691092
}
1070-
ring_hr_last_saved_loaded = true;
1093+
errno = 0;
10711094
FILE *file = fopen(HEART_RATE_LOG, "rb");
1072-
if (!file) return;
1095+
if (!file) {
1096+
if (errno == ENOENT) {
1097+
ring_hr_last_saved_loaded = true;
1098+
return true;
1099+
}
1100+
int error = errno ? errno : EIO;
1101+
sd_record_error(error);
1102+
errno = error;
1103+
return false;
1104+
}
10731105
char tail[513];
1074-
if (fseek(file, 0, SEEK_END) != 0) {
1075-
fclose(file);
1076-
return;
1106+
int error = 0;
1107+
errno = 0;
1108+
if (fseek(file, 0, SEEK_END) != 0) error = errno ? errno : EIO;
1109+
long end = 0, offset = 0;
1110+
if (!error) {
1111+
errno = 0;
1112+
end = ftell(file);
1113+
if (end < 0) error = errno ? errno : EIO;
1114+
else offset = end > (long)sizeof(tail) - 1 ? end - ((long)sizeof(tail) - 1) : 0;
10771115
}
1078-
long end = ftell(file);
1079-
long offset = end > (long)sizeof(tail) - 1 ? end - ((long)sizeof(tail) - 1) : 0;
1080-
if (end < 0 || fseek(file, offset, SEEK_SET) != 0) {
1081-
fclose(file);
1082-
return;
1116+
if (!error) {
1117+
errno = 0;
1118+
if (fseek(file, offset, SEEK_SET) != 0) error = errno ? errno : EIO;
1119+
}
1120+
size_t length = 0;
1121+
if (!error) {
1122+
errno = 0;
1123+
length = fread(tail, 1, (size_t)(end - offset), file);
1124+
if (ferror(file) || length != (size_t)(end - offset)) error = errno ? errno : EIO;
1125+
}
1126+
errno = 0;
1127+
if (fclose(file) != 0 && !error) error = errno ? errno : EIO;
1128+
if (error) {
1129+
sd_record_error(error);
1130+
errno = error;
1131+
return false;
10831132
}
1084-
size_t length = fread(tail, 1, sizeof(tail) - 1, file);
10851133
tail[length] = '\0';
1086-
fclose(file);
10871134
char *line = tail;
10881135
if (offset) {
10891136
line = strchr(line, '\n');
1090-
if (!line) return;
1091-
line++;
1137+
if (line) line++;
10921138
}
10931139
long long timestamp;
1094-
while (*line) {
1140+
while (line && *line) {
10951141
if (sscanf(line, "%lld,", &timestamp) == 1 && timestamp > ring_hr_last_saved)
10961142
ring_hr_last_saved = (time_t)timestamp;
10971143
line = strchr(line, '\n');
10981144
if (!line) break;
10991145
line++;
11001146
}
1147+
ring_hr_last_saved_loaded = true;
1148+
return true;
11011149
}
11021150

11031151
static int ring_request_history(void)
@@ -1161,7 +1209,12 @@ static void ring_health_tick(lv_timer_t *timer)
11611209
}
11621210
portEXIT_CRITICAL(&ring_lock);
11631211

1164-
ring_hr_load_last_saved();
1212+
if (!ring_hr_load_last_saved()) {
1213+
snprintf(ring_storage_error, sizeof(ring_storage_error),
1214+
"Heart-rate log not saved: %s", strerror(errno));
1215+
return;
1216+
}
1217+
ring_storage_error[0] = '\0';
11651218
ring_hr_sample_t sample;
11661219
while (ring_hr_samples && xQueuePeek(ring_hr_samples, &sample, 0) == pdTRUE) {
11671220
if (sample.timestamp > ring_hr_last_saved) {

0 commit comments

Comments
 (0)