diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8d00f3f..1acddd32 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,9 @@ jobs: - name: Verify shared/platform UI boundaries run: python3 scripts/check_platform_ui_boundaries.py + - name: Verify shared SPI direct-call boundaries + run: python3 scripts/check_shared_spi_direct_calls.py + - name: Verify ESP stack hygiene run: python3 scripts/check_esp_stack_hygiene.py diff --git a/modules/core_sys/include/sys/bus_access_scope.h b/modules/core_sys/include/sys/bus_access_scope.h new file mode 100644 index 00000000..f6a07ffd --- /dev/null +++ b/modules/core_sys/include/sys/bus_access_scope.h @@ -0,0 +1,89 @@ +#pragma once + +#include "sys/runtime_async.h" + +namespace sys::runtime +{ + +class ScopedBusAccessToken +{ + public: + ScopedBusAccessToken(IBusArbiter& arbiter, const BusAcquireRequest& request) + : arbiter_(&arbiter), result_(arbiter.acquire(request)) + { + } + + ScopedBusAccessToken(const ScopedBusAccessToken&) = delete; + ScopedBusAccessToken& operator=(const ScopedBusAccessToken&) = delete; + + ScopedBusAccessToken(ScopedBusAccessToken&& other) noexcept + : arbiter_(other.arbiter_), result_(other.result_) + { + other.arbiter_ = nullptr; + other.result_.token.valid = false; + } + + ScopedBusAccessToken& operator=(ScopedBusAccessToken&& other) noexcept + { + if (this != &other) + { + release(); + arbiter_ = other.arbiter_; + result_ = other.result_; + other.arbiter_ = nullptr; + other.result_.token.valid = false; + } + return *this; + } + + ~ScopedBusAccessToken() + { + release(); + } + + bool acquired() const + { + return result_.status == BusAcquireStatus::Acquired && result_.token.valid; + } + + explicit operator bool() const + { + return acquired(); + } + + BusAcquireStatus status() const + { + return result_.status; + } + + const BusAcquireResult& result() const + { + return result_; + } + + const BusAccessToken& token() const + { + return result_.token; + } + + const BusDiagnostics& diagnostics() const + { + return result_.diagnostics; + } + + void release() + { + if (arbiter_ == nullptr || !result_.token.valid) + { + return; + } + arbiter_->release(result_.token); + result_.token.valid = false; + } + + private: + IBusArbiter* arbiter_ = nullptr; + BusAcquireResult result_{}; +}; + +} // namespace sys::runtime diff --git a/modules/core_sys/tests/test_runtime_async.cpp b/modules/core_sys/tests/test_runtime_async.cpp index 5f8bc440..dc7c6ba0 100644 --- a/modules/core_sys/tests/test_runtime_async.cpp +++ b/modules/core_sys/tests/test_runtime_async.cpp @@ -1,6 +1,8 @@ +#include "sys/bus_access_scope.h" #include "sys/runtime_async.h" #include +#include namespace { @@ -314,6 +316,91 @@ void test_storage_bus_arbiter_reports_degraded_after_timeouts() assert(arbiter.health().status == sys::runtime::StorageHealthStatus::Degraded); } +void test_scoped_bus_access_token_releases_on_destruction() +{ + FakeBusAdapter adapter; + sys::runtime::DefaultBusPolicyStrategy policy; + sys::runtime::StorageBusArbiter arbiter(adapter, policy); + + sys::runtime::BusAcquireRequest request{}; + request.resource = 8; + request.command_id = 12; + request.policy = sys::runtime::BusAccessPolicy::BackgroundWorkerBounded; + + { + sys::runtime::ScopedBusAccessToken scope(arbiter, request); + assert(scope.acquired()); + assert(static_cast(scope)); + assert(scope.status() == sys::runtime::BusAcquireStatus::Acquired); + assert(scope.token().owner == 12); + assert(scope.diagnostics().owner == 77); + assert(adapter.acquire_count == 1); + assert(adapter.release_count == 0); + } + + assert(adapter.release_count == 1); +} + +void test_scoped_bus_access_token_release_is_idempotent() +{ + FakeBusAdapter adapter; + sys::runtime::DefaultBusPolicyStrategy policy; + sys::runtime::StorageBusArbiter arbiter(adapter, policy); + + sys::runtime::BusAcquireRequest request{}; + request.policy = sys::runtime::BusAccessPolicy::BackgroundWorkerBounded; + + sys::runtime::ScopedBusAccessToken scope(arbiter, request); + assert(scope.acquired()); + scope.release(); + scope.release(); + assert(!scope.acquired()); + assert(adapter.release_count == 1); +} + +void test_scoped_bus_access_token_move_transfers_release() +{ + FakeBusAdapter adapter; + sys::runtime::DefaultBusPolicyStrategy policy; + sys::runtime::StorageBusArbiter arbiter(adapter, policy); + + sys::runtime::BusAcquireRequest request{}; + request.policy = sys::runtime::BusAccessPolicy::BackgroundWorkerBounded; + + { + sys::runtime::ScopedBusAccessToken original(arbiter, request); + assert(original.acquired()); + { + sys::runtime::ScopedBusAccessToken moved(std::move(original)); + assert(!original.acquired()); + assert(moved.acquired()); + assert(adapter.release_count == 0); + } + assert(adapter.release_count == 1); + } + + assert(adapter.release_count == 1); +} + +void test_scoped_bus_access_token_does_not_release_failed_acquire() +{ + FakeBusAdapter adapter; + adapter.acquire_ok = false; + sys::runtime::DefaultBusPolicyStrategy policy; + sys::runtime::StorageBusArbiter arbiter(adapter, policy); + + sys::runtime::BusAcquireRequest request{}; + request.policy = sys::runtime::BusAccessPolicy::BackgroundWorkerBounded; + + { + sys::runtime::ScopedBusAccessToken scope(arbiter, request); + assert(!scope.acquired()); + assert(scope.status() == sys::runtime::BusAcquireStatus::TimedOut); + } + + assert(adapter.release_count == 0); +} + } // namespace int main() @@ -328,5 +415,9 @@ int main() test_storage_bus_arbiter_uses_policy_timeout(); test_map_tile_policy_is_display_frame_critical(); test_storage_bus_arbiter_reports_degraded_after_timeouts(); + test_scoped_bus_access_token_releases_on_destruction(); + test_scoped_bus_access_token_release_is_idempotent(); + test_scoped_bus_access_token_move_transfers_release(); + test_scoped_bus_access_token_does_not_release_failed_acquire(); return 0; } diff --git a/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h b/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h index a4fae502..7a608272 100644 --- a/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h +++ b/modules/ui_map_runtime/include/ui_map_runtime/map_tiles/map_tile_async_runtime.h @@ -105,17 +105,31 @@ class IMapTileEventSink virtual bool publish(const MapTileAsyncEvent& event) = 0; }; +enum class MapTileReadStatus : uint8_t +{ + Ready, + Failed, + ResourceBusy, +}; + +struct MapTileReadResult +{ + MapTileReadStatus status = MapTileReadStatus::Failed; + std::size_t size = 0; + MapTileFormat format = MapTileFormat::Unknown; + int32_t error = -1; + bool bus_access_retained = true; +}; + class IMapTileWorkerBackend { public: virtual ~IMapTileWorkerBackend() = default; virtual MapTileLookupResult lookup(const MapTileRef& ref) = 0; - virtual bool read(const MapTileRef& ref, - uint8_t* buffer, - std::size_t capacity, - std::size_t& out_size, - MapTileFormat& out_format) = 0; + virtual MapTileReadResult read(const MapTileRef& ref, + uint8_t* buffer, + std::size_t capacity) = 0; }; struct MapTileStateSnapshot diff --git a/modules/ui_map_runtime/src/map_tiles/map_tile_async_runtime.cpp b/modules/ui_map_runtime/src/map_tiles/map_tile_async_runtime.cpp index 67d006d4..b589e2d5 100644 --- a/modules/ui_map_runtime/src/map_tiles/map_tile_async_runtime.cpp +++ b/modules/ui_map_runtime/src/map_tiles/map_tile_async_runtime.cpp @@ -177,22 +177,32 @@ bool MapTileWorker::execute(const LoadTileCommand& command, uint32_t now_ms) event.generation = command.runtime.generation; event.tile = command.tile; - std::size_t out_size = 0; - MapTileFormat out_format = MapTileFormat::Unknown; - const bool ok = backend_.read(command.tile, scratch_, scratch_size_, out_size, out_format); - bus_.release(acquired.token); + const MapTileReadResult read_result = + backend_.read(command.tile, scratch_, scratch_size_); + if (read_result.bus_access_retained) + { + bus_.release(acquired.token); + } - event.kind = ok ? MapTileAsyncEventKind::Ready : MapTileAsyncEventKind::Failed; - event.format = out_format; - event.payload_size = out_size; + const bool ok = read_result.status == MapTileReadStatus::Ready; + if (read_result.status == MapTileReadStatus::ResourceBusy) + { + event.kind = MapTileAsyncEventKind::ResourceBusy; + } + else + { + event.kind = ok ? MapTileAsyncEventKind::Ready : MapTileAsyncEventKind::Failed; + } + event.format = read_result.format; + event.payload_size = read_result.size; if (ok) { event.payload.ref = command.tile; - event.payload.format = out_format; + event.payload.format = read_result.format; event.payload.data = scratch_; - event.payload.size = out_size; + event.payload.size = read_result.size; } - event.error = ok ? 0 : -1; + event.error = ok ? 0 : read_result.error; (void)events_.publish(event); (void)now_ms; return ok; diff --git a/modules/ui_map_runtime/tests/test_map_tile_async_runtime.cpp b/modules/ui_map_runtime/tests/test_map_tile_async_runtime.cpp index 24a8bde5..5d29db4c 100644 --- a/modules/ui_map_runtime/tests/test_map_tile_async_runtime.cpp +++ b/modules/ui_map_runtime/tests/test_map_tile_async_runtime.cpp @@ -96,6 +96,9 @@ class FakeBackend final : public ui::map_tiles::IMapTileWorkerBackend public: bool available = true; bool read_ok = true; + ui::map_tiles::MapTileReadStatus read_status = ui::map_tiles::MapTileReadStatus::Ready; + int32_t read_error = -1; + bool bus_access_retained = true; int lookup_count = 0; int read_count = 0; @@ -110,25 +113,34 @@ class FakeBackend final : public ui::map_tiles::IMapTileWorkerBackend return result; } - bool read(const ui::map_tiles::MapTileRef& ref, - uint8_t* buffer, - std::size_t capacity, - std::size_t& out_size, - ui::map_tiles::MapTileFormat& out_format) override + ui::map_tiles::MapTileReadResult read(const ui::map_tiles::MapTileRef& ref, + uint8_t* buffer, + std::size_t capacity) override { ++read_count; (void)ref; - out_size = 0; - out_format = ui::map_tiles::MapTileFormat::Png; + ui::map_tiles::MapTileReadResult result{}; + result.status = read_status; + result.format = ui::map_tiles::MapTileFormat::Png; + result.error = read_error; + result.bus_access_retained = bus_access_retained; + if (read_status != ui::map_tiles::MapTileReadStatus::Ready) + { + return result; + } if (!available || !read_ok || !buffer || capacity < 3) { - return false; + result.status = ui::map_tiles::MapTileReadStatus::Failed; + result.error = -1; + result.bus_access_retained = true; + return result; } buffer[0] = 1; buffer[1] = 2; buffer[2] = 3; - out_size = 3; - return true; + result.size = 3; + result.error = 0; + return result; } }; @@ -379,6 +391,63 @@ void test_worker_missing_reads_once_without_lookup_probe() assert(events.events[0].payload_size == 0); } +void test_worker_resource_busy_read_publishes_resource_busy() +{ + FakeBackend backend; + backend.read_status = ui::map_tiles::MapTileReadStatus::ResourceBusy; + backend.read_error = + static_cast(sys::runtime::BusAcquireStatus::TimedOut); + FakeBusArbiter bus; + FakeEventSink events; + uint8_t scratch[8]{}; + ui::map_tiles::MapTileWorker worker(backend, bus, events, scratch, sizeof(scratch)); + + ui::map_tiles::LoadTileCommand command{}; + command.runtime.command_id = 12; + command.runtime.kind = sys::runtime::RuntimeCommandKind::MapTileLoad; + command.runtime.generation = 4; + command.runtime.priority = sys::runtime::RuntimePriority::Normal; + command.tile = make_tile(42); + + assert(!worker.execute(command, 320)); + assert(bus.acquire_count == 1); + assert(bus.release_count == 1); + assert(backend.read_count == 1); + assert(events.count == 1); + assert(events.events[0].kind == ui::map_tiles::MapTileAsyncEventKind::ResourceBusy); + assert(events.events[0].error == + static_cast(sys::runtime::BusAcquireStatus::TimedOut)); + assert(events.events[0].payload.data == nullptr); + assert(events.events[0].payload_size == 0); +} + +void test_worker_released_bus_read_skips_release() +{ + FakeBackend backend; + backend.read_status = ui::map_tiles::MapTileReadStatus::ResourceBusy; + backend.bus_access_retained = false; + backend.read_error = + static_cast(sys::runtime::BusAcquireStatus::TimedOut); + FakeBusArbiter bus; + FakeEventSink events; + uint8_t scratch[8]{}; + ui::map_tiles::MapTileWorker worker(backend, bus, events, scratch, sizeof(scratch)); + + ui::map_tiles::LoadTileCommand command{}; + command.runtime.command_id = 13; + command.runtime.kind = sys::runtime::RuntimeCommandKind::MapTileLoad; + command.runtime.generation = 4; + command.runtime.priority = sys::runtime::RuntimePriority::Normal; + command.tile = make_tile(43); + + assert(!worker.execute(command, 330)); + assert(bus.acquire_count == 1); + assert(bus.release_count == 0); + assert(backend.read_count == 1); + assert(events.count == 1); + assert(events.events[0].kind == ui::map_tiles::MapTileAsyncEventKind::ResourceBusy); +} + void test_runtime_and_worker_use_policy_strategy() { FakeCommandSink sink; @@ -413,6 +482,8 @@ int main() test_worker_busy_does_not_read_storage(); test_worker_success_publishes_ready(); test_worker_missing_reads_once_without_lookup_probe(); + test_worker_resource_busy_read_publishes_resource_busy(); + test_worker_released_bus_read_skips_release(); test_runtime_and_worker_use_policy_strategy(); return 0; } diff --git a/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp b/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp index 9577be46..2dafcfe1 100644 --- a/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp +++ b/platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp @@ -160,6 +160,8 @@ class PathOnlyMapTileFileSystem final : public ui::map_tiles::IMapTileFileSystem bool yield_map_tile_sd_bus_between_chunks(const char* path, std::size_t bytes_read, std::size_t total_bytes); +void reset_map_tile_sd_read_backpressure_state(); +void note_map_tile_sd_read_resource_busy(bool bus_access_retained); class SdMapTileFileSystem final : public ui::map_tiles::IMapTileFileSystem { @@ -180,6 +182,7 @@ class SdMapTileFileSystem final : public ui::map_tiles::IMapTileFileSystem std::size_t& out_size) const override { out_size = 0; + reset_map_tile_sd_read_backpressure_state(); if (!path || !buffer || capacity == 0) { return false; @@ -217,6 +220,7 @@ class SdMapTileFileSystem final : public ui::map_tiles::IMapTileFileSystem !yield_map_tile_sd_bus_between_chunks(path, total_read, target_size)) { out_size = total_read; + note_map_tile_sd_read_resource_busy(false); file.close(); return false; } @@ -246,6 +250,8 @@ constexpr uint32_t kMapTileDiagnosticLogIntervalMs = 1000; constexpr TickType_t kMapTileWorkerPostCommandYieldTicks = pdMS_TO_TICKS(32); constexpr TickType_t kMapTileSdChunkYieldTicks = pdMS_TO_TICKS(1); constexpr TickType_t kMapTileSdChunkReacquireTicks = pdMS_TO_TICKS(50); +constexpr uint32_t kMapTileSdChunkRetryReacquireMs = 100; +constexpr uint32_t kMapTileSdChunkReacquireBudgetMs = 2000; constexpr uint32_t kMapTileDisplaySpiSlowHoldMs = 8; constexpr uint32_t kMapTileDisplaySpiNormalCooldownMs = 160; constexpr uint32_t kMapTileDisplaySpiSlowCooldownMs = 1500; @@ -257,6 +263,8 @@ uint32_t g_map_tile_event_log_ms = 0; uint32_t g_map_tile_next_event_drain_ms = 0; uint32_t g_map_tile_chunk_yield_log_ms = 0; uint32_t g_map_tile_display_pressure_log_ms = 0; +bool g_map_tile_sd_read_resource_busy = false; +bool g_map_tile_sd_read_bus_access_retained = true; bool should_log_map_tile_diagnostic(uint32_t& last_ms, uint32_t now_ms) { @@ -289,6 +297,28 @@ void log_map_tile_display_pressure_pause(uint32_t now_ms, const char* stage) std::fflush(stdout); } +void reset_map_tile_sd_read_backpressure_state() +{ + g_map_tile_sd_read_resource_busy = false; + g_map_tile_sd_read_bus_access_retained = true; +} + +void note_map_tile_sd_read_resource_busy(bool bus_access_retained) +{ + g_map_tile_sd_read_resource_busy = true; + g_map_tile_sd_read_bus_access_retained = bus_access_retained; +} + +bool map_tile_sd_read_resource_busy() +{ + return g_map_tile_sd_read_resource_busy; +} + +bool map_tile_sd_read_bus_access_retained() +{ + return g_map_tile_sd_read_bus_access_retained; +} + bool yield_map_tile_sd_bus_between_chunks(const char* path, std::size_t bytes_read, std::size_t total_bytes) @@ -315,12 +345,45 @@ bool yield_map_tile_sd_bus_between_chunks(const char* path, std::fflush(stdout); } - while (!::platform::esp::common::shared_spi_lock_with_owner(pdMS_TO_TICKS(100), - "map_tile_sd")) + const uint32_t wait_start_ms = now_ms; + const uint32_t deadline_ms = wait_start_ms + kMapTileSdChunkReacquireBudgetMs; + while (static_cast(deadline_ms - sys::millis_now()) > 0) { + const uint32_t attempt_ms = sys::millis_now(); + const uint32_t remaining_ms = + static_cast(deadline_ms - attempt_ms) > 0 ? deadline_ms - attempt_ms + : 0; + if (remaining_ms == 0) + { + break; + } + const uint32_t wait_ms = + std::min(kMapTileSdChunkRetryReacquireMs, remaining_ms); + TickType_t wait_ticks = pdMS_TO_TICKS(wait_ms); + if (wait_ticks == 0) + { + wait_ticks = 1; + } + if (::platform::esp::common::shared_spi_lock_with_owner(wait_ticks, + "map_tile_sd")) + { + return true; + } vTaskDelay(kMapTileSdChunkYieldTicks); } - return true; + + const uint32_t timeout_ms = sys::millis_now() - wait_start_ms; + if (should_log_map_tile_diagnostic(g_map_tile_chunk_yield_log_ms, sys::millis_now())) + { + std::printf("[GPS][MAP][bus] chunk_reacquire_give_up path=%s bytes=%u/%u wait_ms=%lu budget_ms=%lu\n", + path ? path : "", + static_cast(bytes_read), + static_cast(total_bytes), + static_cast(timeout_ms), + static_cast(kMapTileSdChunkReacquireBudgetMs)); + std::fflush(stdout); + } + return false; } const char* map_tile_format_name(ui::map_tiles::MapTileFormat format) @@ -1096,23 +1159,39 @@ class EspMapTileWorkerBackend final : public ui::map_tiles::IMapTileWorkerBacken return source_.lookup(ref); } - bool read(const ui::map_tiles::MapTileRef& ref, - uint8_t* buffer, - std::size_t capacity, - std::size_t& out_size, - ui::map_tiles::MapTileFormat& out_format) override + ui::map_tiles::MapTileReadResult read(const ui::map_tiles::MapTileRef& ref, + uint8_t* buffer, + std::size_t capacity) override { + ui::map_tiles::MapTileReadResult result{}; + result.format = ui::map_tiles::mapTileFormatForLayer(ref.layer); + reset_map_tile_sd_read_backpressure_state(); if (map_tile_availability_memory().knownMissing(ref)) { - out_size = 0; - out_format = ui::map_tiles::mapTileFormatForLayer(ref.layer); - return false; + result.error = -1; + return result; } + std::size_t out_size = 0; + ui::map_tiles::MapTileFormat out_format = result.format; if (source_.read(ref, buffer, capacity, out_size, out_format)) { map_tile_availability_memory().markAvailable(ref); - return true; + result.status = ui::map_tiles::MapTileReadStatus::Ready; + result.size = out_size; + result.format = out_format; + result.error = 0; + return result; + } + + if (map_tile_sd_read_resource_busy()) + { + result.status = ui::map_tiles::MapTileReadStatus::ResourceBusy; + result.format = out_format; + result.error = + static_cast(sys::runtime::BusAcquireStatus::TimedOut); + result.bus_access_retained = map_tile_sd_read_bus_access_retained(); + return result; } const ui::map_tiles::MapTileLookupResult lookup = source_.lookup(ref); @@ -1120,7 +1199,10 @@ class EspMapTileWorkerBackend final : public ui::map_tiles::IMapTileWorkerBacken { map_tile_availability_memory().markMissing(ref); } - return false; + result.status = ui::map_tiles::MapTileReadStatus::Failed; + result.format = out_format; + result.error = -1; + return result; } private: diff --git a/scripts/check_shared_spi_direct_calls.py b/scripts/check_shared_spi_direct_calls.py new file mode 100644 index 00000000..cdac26b8 --- /dev/null +++ b/scripts/check_shared_spi_direct_calls.py @@ -0,0 +1,399 @@ +#!/usr/bin/env python3 +"""Freeze direct shared-SPI lock use to adapter boundaries.""" + +from __future__ import annotations + +from collections import Counter +from dataclasses import dataclass +import os +from pathlib import Path +import re +import subprocess +import sys + + +REPO_ROOT = Path(__file__).resolve().parent.parent + +SOURCE_SUFFIXES = {".c", ".cc", ".cpp", ".cxx", ".h", ".hpp", ".hh"} +EXCLUDED_DIR_NAMES = { + ".git", + ".pio", + ".pytest_cache", + ".tmp", + ".venv", + "__pycache__", + "build", + "dist", +} + + +@dataclass(frozen=True) +class DirectCallPattern: + name: str + pattern: re.Pattern[str] + + +@dataclass(frozen=True) +class Occurrence: + path: Path + relative: str + line_number: int + rule: str + line: str + + +DIRECT_CALL_PATTERNS = ( + DirectCallPattern("shared_spi_guard", re.compile(r"\bSharedSpiLockGuard\b")), + DirectCallPattern( + "shared_spi_lock_with_owner", + re.compile(r"\bshared_spi_lock_with_owner\s*\("), + ), + DirectCallPattern("shared_spi_unlock", re.compile(r"\bshared_spi_unlock\s*\(")), + DirectCallPattern( + "lilygo_display_spi_lock", + re.compile(r"\bLilyGoDispArduinoSPI::lock\s*\("), + ), + DirectCallPattern( + "lilygo_display_spi_unlock", + re.compile(r"\bLilyGoDispArduinoSPI::unlock\s*\("), + ), +) + + +# Permanent adapter boundaries are the only places allowed to own physical +# shared-SPI mutex semantics directly. +PERMANENT_ALLOWED_PATHS = { + "platform/esp/boards/src/display/DisplayInterface.cpp", + "platform/esp/common/include/platform/esp/common/shared_spi_bus_arbiter.h", + "platform/esp/common/include/platform/esp/common/shared_spi_lock.h", + "platform/esp/idf_common/src/shared_spi_lock.cpp", +} + + +# These files contain adapter-local direct calls, but we pin the exact current +# occurrences so future additions are explicit review points. +PERMANENT_ALLOWED_OCCURRENCES = { + ( + "platform/esp/arduino_common/src/LV_Helper_v9.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard spi_guard(", + ): 9, + ( + "platform/esp/arduino_common/src/storage/sd_card_runtime.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard guard_;", + ): 1, + ( + "platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp", + "shared_spi_lock_with_owner", + "::platform::esp::common::shared_spi_lock_with_owner(pdMS_TO_TICKS(timeout_ms),", + ): 1, + ( + "platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp", + "shared_spi_lock_with_owner", + "if (::platform::esp::common::shared_spi_lock_with_owner(kMapTileSdChunkReacquireTicks,", + ): 1, + ( + "platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp", + "shared_spi_lock_with_owner", + "if (::platform::esp::common::shared_spi_lock_with_owner(wait_ticks,", + ): 1, + ( + "platform/esp/arduino_common/src/ui/widgets/map/map_tiles.cpp", + "shared_spi_unlock", + "::platform::esp::common::shared_spi_unlock();", + ): 2, +} + + +# Transitional baseline: these are known active direct calls that still need +# migration to arbiter/token/backpressure paths. The check permits the current +# occurrences to shrink, but any new or changed business-layer direct call fails. +LEGACY_TRANSITION_OCCURRENCES = { + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + "if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(100)))", + ): 1, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(100), "radio_cfg"))', + ): 1, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(20), "radio_irq"))', + ): 2, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(20), "radio_rssi"))', + ): 3, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(20), "radio_rx"))', + ): 1, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(50), "radio_rx"))', + ): 2, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(50), "radio_tx"))', + ): 1, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(50), "radio_tx_finish"))', + ): 2, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_lock", + "if (LilyGoDispArduinoSPI::lock(portMAX_DELAY))", + ): 1, + ( + "boards/tdeck/src/tdeck_board.cpp", + "lilygo_display_spi_unlock", + "LilyGoDispArduinoSPI::unlock();", + ): 14, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + "if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(100)))", + ): 1, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(100), "radio_cfg"))', + ): 1, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(20), "radio_irq"))', + ): 2, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(20), "radio_rssi"))', + ): 3, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(20), "radio_rx"))', + ): 1, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(200), "radio_cfg"))', + ): 2, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(50), "radio_cfg"))', + ): 1, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(50), "radio_rx"))', + ): 2, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(50), "radio_tx"))', + ): 2, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + 'if (LilyGoDispArduinoSPI::lock(pdMS_TO_TICKS(50), "radio_tx_finish"))', + ): 2, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_lock", + "if (LilyGoDispArduinoSPI::lock(portMAX_DELAY))", + ): 1, + ( + "boards/tlora_pager/src/tlora_pager_board.cpp", + "lilygo_display_spi_unlock", + "LilyGoDispArduinoSPI::unlock();", + ): 18, + ( + "platform/esp/arduino_common/src/app_context_platform_bindings.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard spi_guard(", + ): 2, + ( + "platform/esp/arduino_common/src/chat/infra/contact_store.cpp", + "shared_spi_guard", + '::platform::esp::common::SharedSpiLockGuard spi_guard(kSdLoadWait, "contact_store_sd");', + ): 1, + ( + "platform/esp/arduino_common/src/chat/infra/contact_store.cpp", + "shared_spi_guard", + '::platform::esp::common::SharedSpiLockGuard spi_guard(kSdPersistWait, "contact_store_sd");', + ): 1, + ( + "platform/esp/arduino_common/src/chat/infra/meshtastic/node_store.cpp", + "shared_spi_guard", + '::platform::esp::common::SharedSpiLockGuard spi_guard(kSdLoadWait, "node_store_sd");', + ): 1, + ( + "platform/esp/arduino_common/src/chat/infra/meshtastic/node_store.cpp", + "shared_spi_guard", + '::platform::esp::common::SharedSpiLockGuard spi_guard(kSdPersistWait, "node_store_sd");', + ): 2, + ( + "platform/esp/arduino_common/src/gps/track_recorder.cpp", + "shared_spi_guard", + '::platform::esp::common::SharedSpiLockGuard spi_guard(0, "track_sd");', + ): 1, + ( + "platform/esp/arduino_common/src/gps/track_recorder.cpp", + "shared_spi_guard", + '::platform::esp::common::SharedSpiLockGuard spi_guard(kSdTransactionLockWait, "track_sd");', + ): 5, + ( + "platform/esp/arduino_common/src/gps/track_recorder.cpp", + "shared_spi_guard", + '::platform::esp::common::SharedSpiLockGuard spi_guard(spi_wait, "track_sd");', + ): 1, + ( + "platform/esp/arduino_common/src/platform_ui_usb_support_runtime.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard spi_guard(pdMS_TO_TICKS(50));", + ): 2, + ( + "platform/esp/arduino_common/src/sstv/sstv_service.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard guard(pdMS_TO_TICKS(200));", + ): 1, + ( + "platform/esp/arduino_common/src/ui/screens/team/team_ui_store.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard spi_guard(kTeamStoreLoadWait);", + ): 1, + ( + "platform/esp/arduino_common/src/ui/screens/team/team_ui_store.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard spi_guard(kTeamStoreReadWait);", + ): 2, + ( + "platform/esp/arduino_common/src/ui/screens/team/team_ui_store.cpp", + "shared_spi_guard", + "::platform::esp::common::SharedSpiLockGuard spi_guard(kTeamStoreWriteWait);", + ): 9, +} + + +def normalize_line(line: str) -> str: + return re.sub(r"\s+", " ", line.strip()) + + +def occurrence_key(occurrence: Occurrence) -> tuple[str, str, str]: + return (occurrence.relative, occurrence.rule, normalize_line(occurrence.line)) + + +def git_tracked_files() -> list[Path]: + try: + result = subprocess.run( + ["git", "ls-files", "--cached", "--others", "--exclude-standard"], + cwd=REPO_ROOT, + check=True, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + except (OSError, subprocess.CalledProcessError): + return [] + return [REPO_ROOT / line for line in result.stdout.splitlines() if line] + + +def walk_source_files() -> list[Path]: + tracked = git_tracked_files() + if tracked: + return [path for path in tracked if path.suffix.lower() in SOURCE_SUFFIXES] + + source_files: list[Path] = [] + for root, dir_names, file_names in os.walk(REPO_ROOT): + dir_names[:] = [name for name in dir_names if name not in EXCLUDED_DIR_NAMES] + root_path = Path(root) + for file_name in file_names: + path = root_path / file_name + if path.suffix.lower() in SOURCE_SUFFIXES: + source_files.append(path) + return source_files + + +def collect_occurrences() -> list[Occurrence]: + occurrences: list[Occurrence] = [] + for path in walk_source_files(): + relative = path.relative_to(REPO_ROOT).as_posix() + try: + lines = path.read_text(encoding="utf-8", errors="ignore").splitlines() + except OSError: + continue + for line_number, line in enumerate(lines, start=1): + for rule in DIRECT_CALL_PATTERNS: + if rule.pattern.search(line): + occurrences.append( + Occurrence( + path=path, + relative=relative, + line_number=line_number, + rule=rule.name, + line=line.rstrip(), + ) + ) + return occurrences + + +def collect_violations() -> tuple[list[Occurrence], int]: + permanent_remaining = Counter(PERMANENT_ALLOWED_OCCURRENCES) + legacy_remaining = Counter(LEGACY_TRANSITION_OCCURRENCES) + violations: list[Occurrence] = [] + legacy_count = 0 + + for occurrence in collect_occurrences(): + if occurrence.relative in PERMANENT_ALLOWED_PATHS: + continue + + key = occurrence_key(occurrence) + if permanent_remaining[key] > 0: + permanent_remaining[key] -= 1 + continue + + if legacy_remaining[key] > 0: + legacy_remaining[key] -= 1 + legacy_count += 1 + continue + + violations.append(occurrence) + + return violations, legacy_count + + +def main() -> int: + violations, legacy_count = collect_violations() + if not violations: + print( + "Shared SPI direct-call boundary check passed " + f"(legacy transition occurrences remaining: {legacy_count})." + ) + return 0 + + print("Shared SPI direct-call boundary check failed.") + print( + "Business/runtime code must acquire shared SPI through an arbiter/token " + "path; direct physical locks are limited to adapter boundaries." + ) + for violation in violations: + print(f"- [{violation.rule}] {violation.relative}:{violation.line_number}") + print(f" {violation.line.strip()}") + return 1 + + +if __name__ == "__main__": + sys.exit(main())