Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions products/go/docs/ble_service.md
Original file line number Diff line number Diff line change
Expand Up @@ -712,8 +712,9 @@ The write buffer (256 bytes) fits approximately 50 point indices.
Deletes a single route file from NAND storage. The orchestrator rejects the
request with `"session_active"` if the session is currently being tracked.
If the session is being exported, the export is silently ended before
deletion. On success, the orchestrator sends an updated Status characteristic
value to reflect the changed flash usage.
deletion. Existing sessions with zero points are valid deletion targets. On
success, the orchestrator sends an updated Status characteristic value to
reflect the changed flash usage.

### Notify Responses (server -> phone)

Expand Down Expand Up @@ -741,6 +742,8 @@ includes `"pg"` (current page, 1-based), `"tpg"` (total pages), and
(session ID), `"pts"` (point count from `get_session_point_count()`),
and `"ts"` (start time from `get_session_start_time()`). If there are
no sessions, a single page with an empty `"sessions"` array is sent.
Session existence is based on the route file. A route stopped before its first
measurement remains listed with `"pts": 0` and `"ts": 0`.

#### Download Started (`handle_history_start()`)

Expand All @@ -751,6 +754,11 @@ no sessions, a single page with an empty `"sessions"` array is sent.
`"pt_size"` is always 56 (`ROUTE_POINT_WIRE_SIZE`), allowing the phone to
verify wire format compatibility.

Starting an existing zero-point session sends `"started"` with `"total": 0`,
emits no binary notifications, and immediately sends `"done"` with
`"sent": 0`. The client can then end the download or delete the session
normally.

#### Download Done (after `handle_history_start()` or `handle_history_fill()`)

```cbor
Expand All @@ -777,7 +785,7 @@ verify wire format compatibility.

| Error string | Cause | Sent by |
|---|---|---|
| `"session_not_found"` | Session ID does not exist (point count is 0) | `handle_history_start()`, `handle_history_delete()` |
| `"session_not_found"` | No route file exists for the requested session ID | `handle_history_start()`, `handle_history_delete()` |
| `"no_active_download"` | `fill` received but `_export_active` is false | `handle_history_fill()` |
| `"flash_error"` | `read_route_points()` returned 0 during stream | `handle_history_start()` |
| `"delete_failed"` | `delete_route()` returned false (unlink failed) | `handle_history_delete()` |
Expand Down Expand Up @@ -910,10 +918,10 @@ failed `setup_ble()` is non-fatal (advertise without OTA). See
| Method | Blocking? | Description |
|---|---|---|
| `handle_history_list()` | No | Reads sessions from storage, sends paginated CBOR session list notifications (6 per page). |
| `handle_history_start(session_id)` | **Yes** | Sends `"started"`, streams all points as binary, sends `"done"`. Aborts with `"error"` on flash failure. |
| `handle_history_start(session_id)` | **Yes** | Verifies the route file exists, sends `"started"`, streams all points as binary, then sends `"done"`. Existing empty sessions complete with zero points. Aborts with `"error"` on flash failure. |
| `handle_history_fill(point_indices, count)` | **Yes** | Sends binary notifications for requested points, then `"done"`. |
| `handle_history_end()` | No | Sets `_export_active = false`, sends `"ended"`. |
| `handle_history_delete(session_id)` | No | Ends export if active for this session, deletes route file, sends `"deleted"` or `"error"`. Caller must check active tracking conflict first. |
| `handle_history_delete(session_id)` | No | Verifies the route file exists, ends export if active for this session, deletes the route file, and sends `"deleted"` or `"error"`. Empty sessions are valid. Caller must check active tracking conflict first. |
| `notify_history_error(err)` | No | Sends a history error notification. Used by orchestrator for errors detected before delegation (e.g., `"session_active"`). |

History export reads raw route points and encodes those values directly. Both
Expand Down Expand Up @@ -992,10 +1000,10 @@ below 128 is unlikely in practice.
| Write callback with null data or zero length | Logged, write silently dropped | `on_config_write`, `on_history_write` |
| `notify_measures()` when not connected | Sets characteristic value for READ access, skips notification | `go_ble.cpp` |
| `encode_measures()` returns 0 | Warning logged, no notification sent | `go_ble.cpp:388` |
| History session not found (point count 0) | `"error": "session_not_found"` CBOR response | `handle_history_start()` |
| History route file does not exist | `"error": "session_not_found"` CBOR response | `handle_history_start()` |
| NAND read returns 0 points during stream | `"error": "flash_error"` CBOR response, `_export_active = false` | `handle_history_start()` |
| `fill` with no active download | `"error": "no_active_download"` CBOR response | `handle_history_fill()` |
| Delete session not found (point count 0) | `"error": "session_not_found"` CBOR response | `handle_history_delete()` |
| Delete route file does not exist | `"error": "session_not_found"` CBOR response | `handle_history_delete()` |
| Delete active tracking session | `"error": "session_active"` CBOR response | Orchestrator (`on_ble_history_write`) |
| Delete storage failure | `"error": "delete_failed"` CBOR response | `handle_history_delete()` |
| `notify()` returns false during stream | Retry with `RTOS::delay_ms(1)`, check `_connected` | `send_history_cbor`, `send_history_binary` |
Expand Down Expand Up @@ -1055,6 +1063,7 @@ The BLE service uses the following `StorageService` methods:
```cpp
uint16_t session_count() const;
uint16_t list_sessions(uint32_t *out, uint16_t max_count) const;
bool route_file_exists(uint32_t session_id) const;
uint32_t get_session_point_count(uint32_t session_id) const;
uint16_t read_route_points(uint32_t session_id, uint32_t offset,
RoutePoint *out, uint16_t count) const;
Expand All @@ -1065,9 +1074,10 @@ uint32_t used_kb() const;
```

History export uses `session_count()` to determine the total number of
sessions, then `list_sessions()` to read the IDs into a heap-allocated
vector. History delete uses `delete_route()`. Status reporting uses
`total_capacity_kb()` and `used_kb()`.
sessions, then `list_sessions()` to read the IDs into a heap-allocated vector.
History start and delete use `route_file_exists()` because a point count of zero
can mean either an existing empty route or a missing route. History delete uses
`delete_route()`. Status reporting uses `total_capacity_kb()` and `used_kb()`.

---

Expand Down
10 changes: 8 additions & 2 deletions products/go/docs/storage_service.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,21 @@ a route is already active, leaving the existing route untouched.
|---|---|
| `create_route(session_id)` | Open a brand-new route file. Refuses if the file already exists (no truncating-open) or if `stat()` fails with anything other than `ENOENT`. On success performs an immediate `fflush + fsync` of the empty file so the directory entry is durable on NAND before the orchestrator tells the user / phone the session started. Marked `[[nodiscard]]`. |
| `resume_route(session_id)` | Reopen an existing route file in append mode after deep-sleep wake. Truncates any torn trailing record (size not aligned to `sizeof(RoutePoint)`) via `ftruncate()` before opening so the next append lands on a clean boundary. Marked `[[nodiscard]]`. |
| `route_file_exists(session_id)` | Cheap `stat()` check used by the orchestrator's session-ID retry loop so collisions never surface to the user as storage errors. Returns `false` when NAND is not mounted. |
| `route_file_exists(session_id)` | Cheap `stat()` check used by the orchestrator's session-ID retry loop and BLE history start/delete. It distinguishes an existing empty route from a missing route. Returns `false` when NAND is not mounted. |
| `append_route_point(point)` | Write one `RoutePoint` via `fwrite`. Internally enforces the durability budget below — flushes + fsyncs at most every `CONFIG_TRACKING_FSYNC_INTERVAL_MS`, plus an unconditional sync on the very first post-open append. Returns `false` on `fwrite`, `fflush`, or `fsync` failure. Marked `[[nodiscard]]`. |
| `end_route()` | `fflush` + `fsync` + `fclose` (unconditional). Resets session ID, point count, and the budget anchor. Safe to call when no route is active (no-op). |
| `end_route()` | `fflush` + `fsync` + `fclose` (unconditional). Preserves an empty route as a valid completed session. Resets session ID, point count, and the budget anchor. Safe to call when no route is active (no-op). |
| `is_route_active()` | Returns `true` while a route file is open. |
| `current_route_point_count()` | Total points written in the current session (includes points from previous boots when resuming). Returns 0 when no route is active. |
| `clear_routes()` | Deletes all files under `<mount_path>/routes/`. Used by Clear Data and Factory Reset. Returns `true` when all route files are removed. |
| `total_capacity_kb()` | Total FATFS capacity in kilobytes for BLE status reporting. Uses `esp_vfs_fat_info()` on target and `statvfs()` under `TEST_HOST`. |
| `used_kb()` | Used FATFS capacity in kilobytes for BLE status reporting. Uses the same target/host split as `total_capacity_kb()`. |

Stopping tracking before the first append leaves a zero-point route file. The
file remains a valid completed session: session listing includes it, point
count and start time are both zero, and BLE history can download or delete it.
Callers must use `route_file_exists()` rather than point count to distinguish
this state from a missing session.

### Durability Budget

`append_route_point()` forces an `fflush + fsync` to NAND under three
Expand Down
16 changes: 13 additions & 3 deletions products/go/go_ble_client.md
Original file line number Diff line number Diff line change
Expand Up @@ -947,7 +947,8 @@ Request the list of stored route sessions. Can be sent at any time.
```

Start downloading all points for the specified session. The `"session"` value
is a session ID obtained from the session list.
is a session ID obtained from the session list. Existing sessions with zero
points are valid download targets.

#### Fill Missing Points

Expand Down Expand Up @@ -987,7 +988,8 @@ downloaded, the device silently ends the export before deleting.
After a successful delete, the device updates its Status characteristic
internally so the next read reflects the reduced `"used_kb"`.

Can be sent at any time (does not require an active download).
Can be sent at any time (does not require an active download). Listed sessions
with zero points can be deleted normally.

### 8.3 Notify Responses (Device -> Phone)

Expand Down Expand Up @@ -1017,7 +1019,7 @@ with pagination metadata. Collect pages until `"pg" == "tpg"`.
|---|---|---|
| `"id"` | uint | Session ID |
| `"pts"` | uint | Number of route points in this session |
| `"ts"` | uint | Session start time (unix seconds) |
| `"ts"` | uint | Session start time (unix seconds), or 0 when the session has no points |
| `"pg"` | uint | Current page number (1-based) |
| `"tpg"` | uint | Total number of pages |
| `"cnt"` | uint | Total number of sessions across all pages |
Expand All @@ -1027,6 +1029,10 @@ cap on the total number of sessions — all sessions stored on the device
are included. If the device has no sessions, a single page is sent with
an empty `"sessions"` array and `"cnt": 0`.

A route stopped before its first measurement is still a valid stored session.
It is included with `"pts": 0` and `"ts": 0` and remains downloadable and
deletable.

#### Download Started

Sent at the beginning of a `start` operation, before binary data streaming
Expand All @@ -1046,6 +1052,10 @@ The `"pt_size"` field allows the client to verify wire format compatibility.
If `"pt_size"` is not 56, the client should abort — the binary format is
incompatible.

For an empty session, `"total"` is 0. The device sends no binary chunks and
immediately follows `"started"` with `{"type": "done", "sent": 0}`. The
client should still send `{"op": "end"}` when it finishes the download.

#### Binary Data Chunks

Sent after `"started"`. Tagged with `0x01`.
Expand Down
11 changes: 5 additions & 6 deletions products/go/main/go_ble.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1106,9 +1106,8 @@ void BleService::handle_history_start(uint32_t session_id) {
_export_active = false;
}

// Check if session exists
uint32_t total_points = _storage.get_session_point_count(session_id);
if (total_points == 0) {
// Point count cannot distinguish an empty session from a missing one.
if (!_storage.route_file_exists(session_id)) {
// Send error: session not found
uint8_t buf[CBOR_BUF_SIZE];
CborEncoder encoder;
Expand All @@ -1125,6 +1124,8 @@ void BleService::handle_history_start(uint32_t session_id) {
return;
}

uint32_t total_points = _storage.get_session_point_count(session_id);

_export_session_id = session_id;
_export_active = true;

Expand Down Expand Up @@ -1301,9 +1302,7 @@ void BleService::handle_history_delete(uint32_t session_id) {
return;
}

// Check if session exists
uint32_t point_count = _storage.get_session_point_count(session_id);
if (point_count == 0) {
if (!_storage.route_file_exists(session_id)) {
notify_history_error(BLE_VAL_ERR_SESSION_NOT_FOUND);
return;
}
Expand Down
8 changes: 5 additions & 3 deletions products/go/tests/ble-integration/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ Covers read, write, delta-notify, and command operations:
**READ after a command still returns the full config snapshot** (not the
`cmd_result`)

### `test_history.py` — History Characteristic (8 tests)
### `test_history.py` — History Characteristic (10 tests)

Exercises the full download protocol. Tests that require stored sessions are
**skipped** when the device reports zero sessions.
Exercises the full download protocol. Download tests select a session with at
least one point and are **skipped** when the device has no non-empty sessions.

- **List**: writes `{"op": "list"}`, verifies `"sessions"` array with
`id`/`pts`/`ts` per entry
Expand All @@ -150,6 +150,8 @@ Exercises the full download protocol. Tests that require stored sessions are
- **End**: `"ended"` response after `{"op": "end"}`
- **Errors**: invalid session ID returns `"session_not_found"`; `fill` without
active download returns `"no_active_download"`
- **Delete**: a nonexistent session returns `"session_not_found"`; deleting a
listed non-active session returns `"deleted"` and removes only that session

### `test_device_info.py` — Device Information Service (5 tests)

Expand Down
38 changes: 19 additions & 19 deletions products/go/tests/ble-integration/test_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,32 +189,33 @@ class TestHistoryDownload:
"""Verify the full download flow: start -> stream -> done -> end."""

@pytest.fixture
async def first_session(
async def first_nonempty_session(
self,
ago_client: BleakClient,
history_notifications: NotificationCollector,
ago_notify_timeout: float,
) -> dict:
"""Get the first available session, skip if none exist."""
"""Get the first non-empty session, skip if none exist."""
sessions = await _list_sessions(
ago_client, history_notifications, ago_notify_timeout,
)
if not sessions:
pytest.skip("No sessions on device")
return sessions[0]
for session in sessions:
if session["pts"] > 0:
return session
pytest.skip("No non-empty sessions on device")

async def test_start_download(
self,
ago_client: BleakClient,
history_notifications: NotificationCollector,
ago_notify_timeout: float,
first_session: dict,
first_nonempty_session: dict,
):
"""Starting a download must return a 'started' CBOR response with
the correct session ID, total points, and pt_size=56.
"""
session_id = first_session["id"]
expected_pts = first_session["pts"]
session_id = first_nonempty_session["id"]
expected_pts = first_nonempty_session["pts"]

write_data = proto.encode_history_start(session_id)
await ago_client.write_gatt_char(
Expand Down Expand Up @@ -264,13 +265,13 @@ async def test_binary_data_format(
ago_client: BleakClient,
history_notifications: NotificationCollector,
ago_notify_timeout: float,
first_session: dict,
first_nonempty_session: dict,
):
"""Binary data notifications must have correct wire format:
tag 0x01, uint16_le point index, then N x 56-byte route points.
"""
session_id = first_session["id"]
expected_pts = first_session["pts"]
session_id = first_nonempty_session["id"]
expected_pts = first_nonempty_session["pts"]

write_data = proto.encode_history_start(session_id)
await ago_client.write_gatt_char(
Expand All @@ -282,8 +283,7 @@ async def test_binary_data_format(
history_notifications, timeout=download_timeout,
)

if not chunks:
pytest.skip("No binary chunks received (session may be empty)")
assert chunks, "No binary chunks received for non-empty session"

# Verify each binary chunk
for point_index, points in chunks:
Expand Down Expand Up @@ -325,11 +325,11 @@ async def test_download_done_count(
ago_client: BleakClient,
history_notifications: NotificationCollector,
ago_notify_timeout: float,
first_session: dict,
first_nonempty_session: dict,
):
"""The 'done' response 'sent' count must match 'total' from 'started'."""
session_id = first_session["id"]
expected_pts = first_session["pts"]
session_id = first_nonempty_session["id"]
expected_pts = first_nonempty_session["pts"]

write_data = proto.encode_history_start(session_id)
await ago_client.write_gatt_char(
Expand Down Expand Up @@ -363,11 +363,11 @@ async def test_end_download(
ago_client: BleakClient,
history_notifications: NotificationCollector,
ago_notify_timeout: float,
first_session: dict,
first_nonempty_session: dict,
):
"""Writing 'end' must return an 'ended' CBOR response."""
session_id = first_session["id"]
expected_pts = first_session["pts"]
session_id = first_nonempty_session["id"]
expected_pts = first_nonempty_session["pts"]

# Start a download first
write_data = proto.encode_history_start(session_id)
Expand Down
Loading
Loading