diff --git a/products/go/docs/ble_service.md b/products/go/docs/ble_service.md index ddf2590..291d686 100644 --- a/products/go/docs/ble_service.md +++ b/products/go/docs/ble_service.md @@ -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) @@ -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()`) @@ -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 @@ -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()` | @@ -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 @@ -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` | @@ -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; @@ -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()`. --- diff --git a/products/go/docs/storage_service.md b/products/go/docs/storage_service.md index 45650ce..10494fe 100644 --- a/products/go/docs/storage_service.md +++ b/products/go/docs/storage_service.md @@ -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 `/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 diff --git a/products/go/go_ble_client.md b/products/go/go_ble_client.md index 5be6a03..4de1a97 100644 --- a/products/go/go_ble_client.md +++ b/products/go/go_ble_client.md @@ -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 @@ -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) @@ -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 | @@ -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 @@ -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`. diff --git a/products/go/main/go_ble.cpp b/products/go/main/go_ble.cpp index 4accfe0..6980fdc 100644 --- a/products/go/main/go_ble.cpp +++ b/products/go/main/go_ble.cpp @@ -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; @@ -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; @@ -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; } diff --git a/products/go/tests/ble-integration/README.md b/products/go/tests/ble-integration/README.md index f5e14cd..4612aa8 100644 --- a/products/go/tests/ble-integration/README.md +++ b/products/go/tests/ble-integration/README.md @@ -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 @@ -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) diff --git a/products/go/tests/ble-integration/test_history.py b/products/go/tests/ble-integration/test_history.py index 87c2cb8..9d21971 100644 --- a/products/go/tests/ble-integration/test_history.py +++ b/products/go/tests/ble-integration/test_history.py @@ -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( @@ -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( @@ -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: @@ -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( @@ -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) diff --git a/products/go/tests/go_ble.tests.cpp b/products/go/tests/go_ble.tests.cpp index 3c17759..8d8b940 100644 --- a/products/go/tests/go_ble.tests.cpp +++ b/products/go/tests/go_ble.tests.cpp @@ -21,6 +21,8 @@ #include #include +static constexpr uint64_t EXPECTED_ROUTE_POINT_WIRE_SIZE = 56; + // =========================================================================== // Mock BLE types // =========================================================================== @@ -167,7 +169,10 @@ void StorageService::backup_cache() const {} void StorageService::restore_cache() {} bool StorageService::create_route(uint32_t /*session_id*/) { return true; } bool StorageService::resume_route(uint32_t /*session_id*/) { return true; } -bool StorageService::route_file_exists(uint32_t /*session_id*/) const { return false; } +bool StorageService::route_file_exists(uint32_t session_id) const { + return std::any_of(storage_spy::sessions.begin(), storage_spy::sessions.end(), + [session_id](const auto &session) { return session.id == session_id; }); +} bool StorageService::append_route_point(const RoutePoint & /*point*/) { return true; } void StorageService::end_route() {} bool StorageService::is_route_active() const { return false; } @@ -2427,6 +2432,37 @@ TEST_CASE("BLE: handle_history_start sends error for non-existent session") { CHECK(find_entry(entries, "err")->text_val == "session_not_found"); } +TEST_CASE("BLE: handle_history_start completes an empty session") { + storage_spy::reset(); + storage_spy::sessions = {{10001, 0, 0}}; + + StorageService storage(*null_cache_ptr, *null_nand_ptr); + BleService svc(nullptr, storage, default_ble_server); + MockBleCharacteristic history_char; + BleServiceTestAccess::set_history_char(svc, &history_char); + BleServiceTestAccess::set_connected(svc, true); + + svc.handle_history_start(10001); + + REQUIRE(history_char.all_values.size() == 2); + + const auto &started_value = history_char.all_values[0]; + REQUIRE(!started_value.empty()); + CHECK(started_value[0] == 0x00); + auto started_entries = decode_cbor_map(started_value.data() + 1, started_value.size() - 1); + CHECK(find_entry(started_entries, "type")->text_val == "started"); + CHECK(find_entry(started_entries, "session")->uint_val == 10001); + CHECK(find_entry(started_entries, "total")->uint_val == 0); + CHECK(find_entry(started_entries, "pt_size")->uint_val == EXPECTED_ROUTE_POINT_WIRE_SIZE); + + const auto &done_value = history_char.all_values[1]; + REQUIRE(!done_value.empty()); + CHECK(done_value[0] == 0x00); + auto done_entries = decode_cbor_map(done_value.data() + 1, done_value.size() - 1); + CHECK(find_entry(done_entries, "type")->text_val == "done"); + CHECK(find_entry(done_entries, "sent")->uint_val == 0); +} + TEST_CASE("BLE: handle_history_start streams points and sends done") { storage_spy::reset(); storage_spy::sessions = {{10001, 2, 1737000000}}; @@ -2653,6 +2689,28 @@ TEST_CASE("BLE: handle_history_delete succeeds and sends deleted response") { CHECK(find_entry(entries, "session")->uint_val == 10001); } +TEST_CASE("BLE: handle_history_delete deletes an empty session") { + storage_spy::reset(); + storage_spy::sessions = {{10001, 0, 0}}; + + StorageService storage(*null_cache_ptr, *null_nand_ptr); + BleService svc(nullptr, storage, default_ble_server); + MockBleCharacteristic history_char; + BleServiceTestAccess::set_history_char(svc, &history_char); + BleServiceTestAccess::set_connected(svc, true); + + svc.handle_history_delete(10001); + + CHECK(storage_spy::last_deleted_session_id == 10001); + REQUIRE(!history_char.last_value.empty()); + CHECK(history_char.last_value[0] == 0x00); + + auto entries = + decode_cbor_map(history_char.last_value.data() + 1, history_char.last_value.size() - 1); + CHECK(find_entry(entries, "type")->text_val == "deleted"); + CHECK(find_entry(entries, "session")->uint_val == 10001); +} + TEST_CASE("BLE: handle_history_delete sends error when storage delete fails") { storage_spy::reset(); storage_spy::sessions = {{10001, 150, 1737000000}}; diff --git a/vhub/Go.vhub.json b/vhub/Go.vhub.json index ef1a467..7380136 100644 --- a/vhub/Go.vhub.json +++ b/vhub/Go.vhub.json @@ -1,6 +1,6 @@ { "schema_version": "1.0", - "rev": "2e5fd97", + "rev": "edd5794", "product": { "slug": "airgradient-go", "name": "AirGradient Go", @@ -240,8 +240,8 @@ "sub_category": "GATT protocol", "applies_to": ["V1"], "description": "The production Portable GATT profile and command contract pass the hardware integration suite", - "expected_result": "The Portable BLE integration suite exits with zero failures or unexpected skips; route tests run against the seeded route, while the two co2_cal command tests are deselected. Debug capture contains one BLE Measures map with pm1, pm25, pm10, and pres. With a client MTU that requires a long read, BLE Config returns all 16 documented keys without truncation. The complete recorded BLE Config is restored.", - "notes": "Seed one completed route, bond the test host, and run pytest products/go/tests/ble-integration/ -v --log-cli-level=DEBUG with --deselect for TestConfigCommand::test_command_progress_and_result_format and ::test_read_after_command_returns_config_snapshot. Record the client and negotiated MTU used for the separate Read-Long check." + "expected_result": "The Portable BLE integration suite exits with zero failures or unexpected skips; route download tests run against the seeded non-empty route, while the two co2_cal command tests are deselected. Debug capture contains one BLE Measures map with pm1, pm25, pm10, and pres. With a client MTU that requires a long read, BLE Config returns all 16 documented keys without truncation. The complete recorded BLE Config is restored.", + "notes": "Seed one completed route with at least one point, bond the test host, and run pytest products/go/tests/ble-integration/ -v --log-cli-level=DEBUG with --deselect for TestConfigCommand::test_command_progress_and_result_format and ::test_read_after_command_returns_config_snapshot. Record the client and negotiated MTU used for the separate Read-Long check." }, { "id": "portable.provisioning.static-ip-verify-then-drop", @@ -279,6 +279,15 @@ "expected_result": "With seven completed routes, Portable BLE History list returns two pages: page 1 contains six sessions, page 2 contains one, both report tpg=2 and cnt=7, and IDs are not duplicated. Portable BLE History start for a route with at least eight points reports its exact total and pt_size=56, then sends sequential binary point indices. Discard the client receipt of the chunk beginning at index 4; after the initial done, Portable BLE History fill for indices 4 through 7 returns exactly those points and a second done with sent=4. Portable BLE History end returns ended.", "notes": "Seed exactly seven expendable completed routes and record each ID/point count. Use a client that can discard one notification without interrupting BLE. Delete the seeded routes after the case." }, + { + "id": "portable.history.empty-session", + "category": "Tracking & Storage", + "sub_category": "Portable BLE History", + "applies_to": ["V1"], + "description": "An empty completed route remains listable, downloadable, and deletable", + "expected_result": "Start tracking and stop before the first route point is recorded. Serial logs route closure with 0 points, and Portable BLE History list contains the exact session ID with pts=0 and ts=0. Starting that session returns started with the same ID, total=0, and pt_size=56, emits no binary point notification, and then returns done with sent=0. Portable BLE History end returns ended. Deleting the session returns deleted with the same ID; the next list reduces cnt by one, omits that ID, and retains an unrelated recorded session.", + "notes": "Use bonded BLE in Portable mode and seed one unrelated non-empty route. Set a long measurement cadence, start immediately after a completed measurement, and stop before the next one. Record the baseline list count and target session ID, confirm pts=0 before export, then restore the prior cadence and delete seeded data." + }, { "id": "portable.history.delete-success", "category": "Tracking & Storage",