diff --git a/CLAUDE.md b/CLAUDE.md index 1e3faa6..6146783 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,7 +32,7 @@ The simulator is a collection of host-side reimplementations of the firmware's h - **Orientation rotation lives in two places.** The firmware's renderer rotates content into the landscape framebuffer (90 CCW for `Portrait`). The simulator undoes that with `SDL_RenderCopyEx`. If you change one, change the other. The dst rect is landscape-shaped and centre-offset because `SDL_RenderCopyEx` rotates around the dst centre. - **HiDPI / dithering.** Set `SDL_HINT_RENDER_SCALE_QUALITY=1` *before* `SDL_CreateTexture`, plus `SDL_WINDOW_ALLOW_HIGHDPI` and `SDL_RenderSetLogicalSize`. Without all three, Bayer-dithered grays render as harsh black/white stripes on Retina. - **POSIX fds, not std::fstream, in [src/HalStorage.cpp](src/HalStorage.cpp).** This was a deliberate rewrite. fstream's separate get/put pointers, eofbit-blocks-seek behaviour, and write-only seek restrictions caused several silent-corruption bugs. Do not reintroduce fstream here. All paths are prefixed with `./fs_` so the simulated filesystem stays sandboxed under the binary's working directory; `/books/` on the SD card maps to `./fs_/books/`. Directory iteration skips only the special `.` and `..` entries; firmware applies its own hidden-file policy. -- **FreeRTOS shim.** [src/freertos/](src/freertos/) maps `xTaskCreate` to `std::thread`, task notifies to a condvar + counter, and `SemaphoreHandle_t` to `std::recursive_mutex`. A `thread_local SimTaskHandle*` lets each task thread find its own handle. +- **FreeRTOS shim.** [src/freertos/](src/freertos/) maps `xTaskCreate` to `std::thread`, task notifies to a condvar + counter, and `SemaphoreHandle_t` to a tagged handle with two arms, a `std::recursive_mutex` for `xSemaphoreCreateMutex` and a real binary semaphore for `xSemaphoreCreateBinary`. A `thread_local SimTaskHandle*` lets each task thread find its own handle. - **`_exit(0)` not `return 0`.** [src/simulator_main.cpp](src/simulator_main.cpp) ends with `_exit(0)` after `SDL_Quit()` to skip C++ global destructors. The render task is `[[noreturn]]`, so running destructors while it is mid-render races and produces a "quit unexpectedly" dialog. Keep this. - **Time uses `steady_clock`.** `millis()` / `micros()` in [src/Arduino.h](src/Arduino.h) deliberately use `steady_clock`, not `system_clock`, so wall-clock changes do not perturb timing. diff --git a/docs/freertos-shim.md b/docs/freertos-shim.md new file mode 100644 index 0000000..e7c8f82 --- /dev/null +++ b/docs/freertos-shim.md @@ -0,0 +1,295 @@ +# The FreeRTOS shim + +The simulator replaces the FreeRTOS API with host shims: `src/freertos/FreeRTOS.h`, +`src/freertos/task.h`, `src/freertos/semphr.h`. A task is a `std::thread` +(`src/freertos/task.h:26`), a mutex is a `std::recursive_mutex` +(`src/freertos/semphr.h:21`), a task notification is a condvar +(`src/freertos/FreeRTOS.h:47`). + +Two primitives were missing rather than simplified. This doc says what they +were, what replaced them, and what the change cost the existing timing. + +## Gap 1: no binary semaphore, and no timeout + +Read off the code, before this change: + +- `xSemaphoreCreateBinary()` did not exist. Only `xSemaphoreCreateMutex()` did. +- `xSemaphoreTake` ignored `ticksToWait` entirely (the parameter was unnamed and + unused). It always blocked until it got the mutex, then returned `true`. + +So firmware that creates a confirm semaphore and waits on it with a deadline +could not time out. The concrete case, in the firmware this simulator was +developed against: its BLE server creates the semaphore with +`xSemaphoreCreateBinary()`, `:621` polls it with `ticksToWait == 0` to drain a +stale give, `:629` waits 3000 ms and treats anything but `pdTRUE` as a dropped +reply, and `:75` gives it from the BLE callback thread. Under the old shim +`:261` did not compile, `:621` would have taken the mutex, and `:629` could +never have returned `pdFALSE`. + +## Gap 2: `vTaskDelay` was a no-op + +`inline void vTaskDelay(int) {}` -- an empty body, so every delay took zero +time. A firmware retry loop of 40 iterations x 25 ms ran instantly, and every +`yield every N rows` helper yielded nothing. + +## Gap 3: no `pdMS_TO_TICKS` + +Also absent, and not merely unused: that firmware passes every one of its +millisecond timeouts through it. Four call sites, all in its BLE server -- +`vTaskDelay(pdMS_TO_TICKS(20))`, `vTaskDelay(pdMS_TO_TICKS(50))`, +`xSemaphoreTake(sem, pdMS_TO_TICKS(kConfirmTimeoutMs))` with +`kConfirmTimeoutMs = 3000` (`:612`), and `:653` +`vTaskDelay(pdMS_TO_TICKS(kRetryDelayMs))` with `kRetryDelayMs = 25` (`:608`). +Without the macro that file does not compile against the shim at all, whatever +the semaphore does. + +## What was added + +**One handle type, two objects, tag dispatch.** `SemaphoreHandle_t` is now +`SimSemaphoreBase *` (`src/freertos/semphr.h:40`). `SimSemaphoreBase` carries a +`SimSemaphoreKind` tag; `SimMutex` and `SimBinarySemaphore` derive from it. The +mutex was not retyped and its internals were not touched: `SimMutex` still holds +the same `std::recursive_mutex`, `holder` and `holdCount` +(`src/freertos/semphr.h:21-30`), because `xSemaphoreGetMutexHolder` and +`xQueuePeek` read them. + +`xSemaphoreTake`, `xSemaphoreGive` and `xQueuePeek` are shared entry points, so +each one branches on the tag and leaves the mutex arm exactly as it was. + +**The binary semaphore** (`src/freertos/semphr.h:33-38`) is a `std::mutex` plus +a `std::condition_variable` plus one `bool available`, created empty: + +- `xSemaphoreTake(sem, portMAX_DELAY)` waits forever. +- `xSemaphoreTake(sem, n)` waits at most `n` ticks and returns `false` + (`pdFALSE`) if the token never arrived. +- `xSemaphoreTake(sem, 0)` tests once and returns. `wait_for` with a zero + duration evaluates the predicate and gives up, which is the non-blocking poll + the drain-a-stale-give call needs. +- A successful take clears the token, so the next take blocks again. +- `xSemaphoreGive` takes the semaphore's own lock, sets the token, drops the + lock, then notifies. It requires nothing of the calling thread, so a BLE + callback thread can give a semaphore the activity thread is waiting on. +- `xSemaphoreGive` on an already-full semaphore returns `false`, which is what + FreeRTOS does (`errQUEUE_FULL`). +- `xQueuePeek` on a binary semaphore reports whether a token is waiting. + `xSemaphoreGetMutexHolder` on one returns `nullptr`. + +`xSemaphoreCreateCounting` was **not** added. Nothing in the build asks for it. + +**Return type kept as `bool`.** Real FreeRTOS returns `BaseType_t`. The shim +already returned `bool` and every call site either ignores the result or +compares it against `pdTRUE` / `pdFALSE`, where `false == pdFALSE == 0` and +`true == pdTRUE == 1`. Leaving the signature alone keeps the mutex arm's diff to +zero. + +**No `vSemaphoreDelete`.** There was none before and there is none now, so +nothing ever deletes through the base pointer and the base needs no virtual +destructor (`src/freertos/semphr.h:44-46`). + +**`vTaskDelay` now sleeps** (`src/freertos/task.h:92`). A positive tick count +becomes a `std::this_thread::sleep_for`. Zero or negative yields instead of +sleeping, which is what FreeRTOS does with a zero delay. + +**`pdMS_TO_TICKS`** (`src/freertos/FreeRTOS.h:13-23`) lives in `FreeRTOS.h` +because that is where real FreeRTOS reaches it from -- it is defined in +`projdefs.h`, which `FreeRTOS.h` includes. The shim's expression is + +``` +#define pdMS_TO_TICKS(xTimeInMs) \ + ((uint32_t)((uint32_t)(xTimeInMs) / (uint32_t)portTICK_PERIOD_MS)) +``` + +which is FreeRTOS's own expression rewritten through the one macro this shim +has, not an invented one. In the pinned ESP-IDF (5.5.2.260206, on disk): + +- `components/freertos/FreeRTOS-Kernel/include/freertos/projdefs.h:46` defines + it as `((TickType_t)(xTimeInMs) * configTICK_RATE_HZ) / 1000U`, integer + division, so it rounds **down** and a sub-tick delay yields 0 ticks. +- `components/freertos/FreeRTOS-Kernel/portable/riscv/include/freertos/portmacro.h:126` + defines `portTICK_PERIOD_MS` as `1000 / configTICK_RATE_HZ`. + +Substituting the second into the first gives `xTimeInMs / portTICK_PERIOD_MS`. +Same rounding, and strictly safer: FreeRTOS multiplies before dividing and can +overflow a 32-bit tick type, a divide cannot. At `portTICK_PERIOD_MS == 1` it is +the identity, so `pdMS_TO_TICKS(n) == n`. + +The reduction is exact whenever `configTICK_RATE_HZ` divides 1000 evenly, which +every rate that yields a whole-millisecond `portTICK_PERIOD_MS` does. Checked by +running both expressions against each other over 10 tick rates x 14 millisecond +values: 140 pairs, 0 mismatches. A rate that does not divide 1000 evenly does +differ (at 333 Hz, `pdMS_TO_TICKS(3)` is 0 in FreeRTOS and 1 here), but such a +rate cannot be expressed by `portTICK_PERIOD_MS` in the first place, so the shim +cannot reach that case. + +**Nothing else was added.** Sweeping every FreeRTOS symbol the firmware +references against the shim turned up four more absentees, and none of them is a +gap the build has: + +- `xSemaphoreCreateRecursiveMutex`, `xSemaphoreTakeRecursive`, + `xSemaphoreGiveRecursive` -- used only in the firmware's `lib/hal/HalStorage.cpp`, + and `hal` is in the simulator env's `lib_ignore`. The simulator ships its own + `src/HalStorage.cpp`, which uses none of them. +- `vPortEnterCritical`, `xTaskPriorityDisinherit` -- appear in firmware comments + only, never called. + +## Tick rate + +One tick is one millisecond. Basis: `portTICK_PERIOD_MS` is defined as `1` and +is the only tick-rate definition in the shim (`src/freertos/FreeRTOS.h:11`). +There is no `configTICK_RATE_HZ` here. All three conversions go through +`portTICK_PERIOD_MS` rather than hardcoding 1 (`src/freertos/FreeRTOS.h:22-23`, +`src/freertos/semphr.h:58-59`, `src/freertos/task.h:98-99`), so redefining that +macro moves all three. + +This matches the firmware's own target config, which sets +`CONFIG_FREERTOS_HZ=1000`, so a tick is 1 ms on device too. + +Measured on this host: `vTaskDelay(1)` costs 1.057 ms per call over 1000 calls, +and `vTaskDelay(0)` costs 0.00066 ms. So the sleep overshoots its nominal +duration by about 6 percent. Verified by running. + +## Regression: what making `vTaskDelay` real cost + +A no-op `vTaskDelay` is load-bearing until proven otherwise, so the same gate +ran before and after. + +Gate: build the firmware's `simulator` env, then run a scripted session that +boots, enters the map on a persisted fix, and screenshots it. + +``` +pio run -e simulator +CROSSPOINT_SIM_INPUT_SCRIPT='1200:ENTER;12000:QUIT' \ +CROSSPOINT_SIM_SCREENSHOTS='6000:./qa-artifacts/map.bmp' \ + /program +``` + +| | before | after | +|---|---|---| +| build | SUCCESS | SUCCESS | +| unique compiler warnings | 4 | 4, same 4 | +| map render | 4 tiles, 2874 ways, 18 places, 22 ms | 4 tiles, 2874 ways, 18 places, 20 ms | +| screenshot | 480x800, 2 grey levels, 11.88 percent dark | byte-identical to before | +| process wall clock | 12.12 s | 12.12 s | + +Verified by running, 2026-08-23. The screenshot draws tiles, ways, place labels, +scale bar and compass in both runs, and the two BMPs compare byte for byte +equal. No gate behind an env var was needed; the real sleep is unconditional. + +**What the gate does not cover.** No `vTaskDelay` call site is on the boot -> +home -> map path, so the gate proves the change is harmless there, not +everywhere. The remaining call sites in the firmware, and their cost at +1.057 ms per tick, read off the code: + +- `lib/Xtc/Xtc.cpp:20` -- one tick every 8 thumbnail rows. A 480-row thumbnail + pays about 63 ms. +- `lib/PngToBmpConverter/PngToBmpConverter.cpp:80` -- one tick every 8 decoded + rows, same shape. +- `src/activities/reader/TxtReaderActivity.cpp:162` -- one tick per 20 indexed + pages. A 2000-page book pays about 106 ms. +- `lib/JpegToBmpConverter/JpegToBmpConverter.cpp:176` -- one tick every 4 file + IO operations. The IO count is not visible from the call site, so this is the + one site whose cost is **open**; a JPEG-heavy screen is what would show it. +- `src/activities/map/MapTransferReceiver.cpp:159` -- a poll loop with its own + millisecond deadline. It used to spin a core flat out; now it sleeps. Strictly + better. +- `src/activities/reader/KOReaderSyncActivity.cpp:56` -- 100 ticks, so about + 106 ms where it used to be 0. + +## Standalone checks + +At the time these were written the gate could not reach the binary semaphore: +the firmware gated that code behind a BLE capability flag the simulator build +did not set, so the file compiled to stubs. It was verified with a host program +compiled straight against these headers instead. Verified by running, +2026-08-23: + +- an empty binary semaphore, `ticksToWait == 0`, returns `pdFALSE` in under + 20 ms +- `ticksToWait == 300` returns `pdFALSE` after 300 ms +- a give from a second thread wakes a blocked 3000-tick take after 150 ms +- the token is consumed: the next zero-tick take fails again +- a second give on a full semaphore returns `pdFALSE` +- `vTaskDelay(200)` sleeps 200 ms, `vTaskDelay(0)` returns immediately + +`pdMS_TO_TICKS` was checked the same way, including the firmware's four call +sites verbatim. All 13 assertions passed, and the two `static_assert`s compiled, +which is what proves the macro is usable in a constant expression like the real +one: + +Captured verbatim. The `BlePositionServer.cpp:NNN` tags name call sites in +the downstream firmware this was checked against, not files in this repo. + +``` +pdMS_TO_TICKS(0) = 0 want 0 ok +pdMS_TO_TICKS(1) = 1 want 1 ok +pdMS_TO_TICKS(25) = 25 want 25 ok +pdMS_TO_TICKS(3000) = 3000 want 3000 ok +pdMS_TO_TICKS(4294967295u) = 4294967295 want 4294967295 ok (no overflow) +pdMS_TO_TICKS(10 + 15) = 25 want 25 ok (argument parenthesised) +pdMS_TO_TICKS(3000) / 2 = 1500 want 1500 ok (result parenthesised) +pdMS_TO_TICKS(20) = 20 want 20 ok (BlePositionServer.cpp:249) +pdMS_TO_TICKS(50) = 50 want 50 ok (BlePositionServer.cpp:404) +pdMS_TO_TICKS(kConfirmTimeoutMs) = 3000 want 3000 ok (BlePositionServer.cpp:629) +pdMS_TO_TICKS(kRetryDelayMs) = 25 want 25 ok (BlePositionServer.cpp:653) +xSemaphoreTake(sem, pdMS_TO_TICKS(3000)) != pdTRUE -> 1 after 3000 ms ok +vTaskDelay(pdMS_TO_TICKS(25)) slept 25 ms ok +``` + +The last two run the call sites' whole statements, not just the macro, so the +3000 ms confirm timeout that could never expire before now expires in 3000 ms. + +Mutex parity was checked the same way: one program exercising only the mutex API +(create, peek free, take, holder, recursive re-take, peek from another thread +while held, give, give again, holder cleared, null handle for all four entry +points) compiled against the old headers and the new ones and printed identical +output. + +Note one pre-existing quirk that parity run confirms is unchanged: `xQueuePeek` +on a mutex uses `try_lock`, and a `std::recursive_mutex` grants `try_lock` to +its own holder. So peek from the holding thread reports the mutex as free. Only +another thread sees it as taken. + +## Since exercised by real firmware, not only by a host program + +Later the same day the capability flag was turned on for the simulator build, +so the confirm-timeout path ran inside real firmware over a fake BLE link. +Verified by running, 2026-08-23: + +- **The 3000 ms wait is 3000 ms, 23 times in a row.** One multi-line reply at + the pessimistic MTU splits into 23 blocks, and against a peer that never + acknowledges anything each block cost one full timeout: 23 waits, gaps + measured at 2981.9 ms minimum and 3063.4 ms maximum, mean 3003.4 ms, 69.1 s + to drain. Timed on two independent clocks -- the peer's monotonic clock and + the firmware's own millisecond log -- and both agree. Under the old shim + every one of those waits returned success immediately, so the firmware's + whole give-up path was dead code that looked exercised. +- **A give from another thread wakes it, inside firmware.** The token is given + from the fake radio's callback thread and taken on the activity thread, which + is the arrangement the standalone check simulated. +- **`vTaskDelay` sleeping is what makes a retry budget mean anything.** A + 40-attempt, 25 ms retry loop is 1 s of real time now and was 0 s before. + +## A firmware file can use these names without including them + +Worth stating because the failure reads like a shim gap and is not. A firmware +translation unit compiled fine on device while using `portMUX_TYPE`, sixteen +`portENTER_CRITICAL`/`portEXIT_CRITICAL` sites, `vTaskDelay`, `pdMS_TO_TICKS` +and the whole semaphore API **without naming a single FreeRTOS header**. It +built because a different library it did include -- a Bluetooth stack -- pulls +`Arduino.h` in, and that drags FreeRTOS along with it. Replace that library +with a header-compatible fake that does not, and the file stops compiling. + +The symptom is unmistakable once you know it: every error is a +`was not declared in this scope` on a FreeRTOS name, and none is on a name +belonging to the library that was replaced. The fix is in the firmware, not +here -- name the three headers it actually uses: + +```cpp +#include +#include +#include +``` + +Those are the paths ESP-IDF owns, so the change is correct on both targets and +needs no conditional. Expect one of these per firmware file that has been +free-riding on some other library's include graph. diff --git a/src/freertos/FreeRTOS.h b/src/freertos/FreeRTOS.h index 84ca905..6613d6c 100644 --- a/src/freertos/FreeRTOS.h +++ b/src/freertos/FreeRTOS.h @@ -1,5 +1,6 @@ #pragma once #include +#include #include #include @@ -9,6 +10,18 @@ #define eIncrement 1 #define portTICK_PERIOD_MS 1 +// Real FreeRTOS defines this in projdefs.h, reached through FreeRTOS.h, as +// ( ( TickType_t ) ( xTimeInMs ) * configTICK_RATE_HZ ) / 1000 +// with integer division, so it rounds down and a sub-tick delay becomes 0 +// ticks. There is no configTICK_RATE_HZ here, and portTICK_PERIOD_MS above is +// the shim's one tick-rate definition. The port defines +// portTICK_PERIOD_MS == 1000 / configTICK_RATE_HZ, so substituting it reduces +// that expression exactly to a divide by it -- same rounding, and no +// multiply to overflow. At portTICK_PERIOD_MS == 1 it is the identity: +// pdMS_TO_TICKS(n) == n. +#define pdMS_TO_TICKS(xTimeInMs) \ + ((uint32_t)((uint32_t)(xTimeInMs) / (uint32_t)portTICK_PERIOD_MS)) + using BaseType_t = int; // ESP-IDF's portMUX_TYPE is a spinlock used with taskENTER_CRITICAL / diff --git a/src/freertos/semphr.h b/src/freertos/semphr.h index cc32125..bd03b35 100644 --- a/src/freertos/semphr.h +++ b/src/freertos/semphr.h @@ -1,54 +1,148 @@ #pragma once +#include +#include +#include #include #include "FreeRTOS.h" #include "task.h" +// FreeRTOS hands out mutexes and binary semaphores through the same +// SemaphoreHandle_t. They are different objects with different rules, so the +// handle carries a tag and the shared entry points below dispatch on it. +enum class SimSemaphoreKind { Mutex, Binary }; + +struct SimSemaphoreBase { + const SimSemaphoreKind kind; + explicit SimSemaphoreBase(SimSemaphoreKind k) : kind(k) {} +}; + // Use a real recursive mutex for the rendering semaphore. -struct SimMutex { +struct SimMutex : SimSemaphoreBase { + SimMutex() : SimSemaphoreBase(SimSemaphoreKind::Mutex) {} std::recursive_mutex mtx; // Track "holder" for xSemaphoreGetMutexHolder compatibility (not thread-safe, // good enough for simulator) TaskHandle_t holder = nullptr; uint16_t holdCount = 0; }; -typedef SimMutex *SemaphoreHandle_t; + +// A real binary semaphore: one token, created empty. A take with no token +// waits, and reports failure when ticksToWait runs out. A give may come from +// any thread, including one that never took it. +struct SimBinarySemaphore : SimSemaphoreBase { + SimBinarySemaphore() : SimSemaphoreBase(SimSemaphoreKind::Binary) {} + std::mutex mtx; + std::condition_variable cv; + bool available = false; +}; + +typedef SimSemaphoreBase *SemaphoreHandle_t; + +namespace sim_semaphore_detail { + +// Single non-virtual base, so a tag-checked downcast is exact. Nothing frees a +// handle in this shim (there is no vSemaphoreDelete), so no base-pointer delete +// happens and the base needs no virtual destructor. +inline SimMutex *asMutex(SemaphoreHandle_t sem) { + return static_cast(sem); +} +inline SimBinarySemaphore *asBinary(SemaphoreHandle_t sem) { + return static_cast(sem); +} + +// FreeRTOS timeouts are in ticks. portTICK_PERIOD_MS (FreeRTOS.h) is the one +// tick-rate definition this shim has, so convert through it rather than +// assuming a tick is a millisecond. +inline std::chrono::milliseconds ticksToDuration(uint32_t ticks) { + return std::chrono::milliseconds(static_cast(ticks) * + portTICK_PERIOD_MS); +} + +} // namespace sim_semaphore_detail inline SemaphoreHandle_t xSemaphoreCreateMutex() { return new SimMutex(); } -inline bool xSemaphoreTake(SemaphoreHandle_t sem, uint32_t /*ticksToWait*/) { +inline SemaphoreHandle_t xSemaphoreCreateBinary() { + return new SimBinarySemaphore(); +} + +// Returns pdTRUE (true) on success, pdFALSE (false) when ticksToWait expired. +// Only the binary semaphore can fail: a mutex take waits as long as it must, +// which is what every caller of the recursive mutex already assumes. +inline bool xSemaphoreTake(SemaphoreHandle_t sem, uint32_t ticksToWait) { if (!sem) return true; - sem->mtx.lock(); - sem->holder = xTaskGetCurrentTaskHandle(); - sem->holdCount++; + if (sem->kind == SimSemaphoreKind::Binary) { + auto *bin = sim_semaphore_detail::asBinary(sem); + std::unique_lock lk(bin->mtx); + const auto hasToken = [bin] { return bin->available; }; + if (ticksToWait == portMAX_DELAY) { + bin->cv.wait(lk, hasToken); + } else if (!bin->cv.wait_for( + lk, sim_semaphore_detail::ticksToDuration(ticksToWait), + hasToken)) { + // ticksToWait == 0 lands here too: wait_for tests the predicate once and + // returns, which is the non-blocking "drain a stale give" poll. + return false; + } + bin->available = false; + return true; + } + auto *mtx = sim_semaphore_detail::asMutex(sem); + mtx->mtx.lock(); + mtx->holder = xTaskGetCurrentTaskHandle(); + mtx->holdCount++; return true; } inline bool xSemaphoreGive(SemaphoreHandle_t sem) { if (!sem) return true; - if (sem->holdCount > 0) { - sem->holdCount--; + if (sem->kind == SimSemaphoreKind::Binary) { + auto *bin = sim_semaphore_detail::asBinary(sem); + bool wasEmpty; + { + std::lock_guard lk(bin->mtx); + wasEmpty = !bin->available; + bin->available = true; + } + // Notify outside the lock: the woken thread would only block on it again. + bin->cv.notify_one(); + // FreeRTOS reports pdFALSE for a give that overflows a full semaphore. + return wasEmpty; } - if (sem->holdCount == 0) { - sem->holder = nullptr; + auto *mtx = sim_semaphore_detail::asMutex(sem); + if (mtx->holdCount > 0) { + mtx->holdCount--; } - sem->mtx.unlock(); + if (mtx->holdCount == 0) { + mtx->holder = nullptr; + } + mtx->mtx.unlock(); return true; } inline TaskHandle_t xSemaphoreGetMutexHolder(SemaphoreHandle_t sem) { - return sem ? sem->holder : nullptr; + if (!sem || sem->kind != SimSemaphoreKind::Mutex) + return nullptr; + return sim_semaphore_detail::asMutex(sem)->holder; } // xQueuePeek on a mutex: returns pdTRUE if the mutex is available (not taken). +// On a binary semaphore, "available" means a token is waiting to be taken. inline int xQueuePeek(SemaphoreHandle_t sem, void *, uint32_t) { if (!sem) return pdTRUE; - bool locked = sem->mtx.try_lock(); + if (sem->kind == SimSemaphoreKind::Binary) { + auto *bin = sim_semaphore_detail::asBinary(sem); + std::lock_guard lk(bin->mtx); + return bin->available ? pdTRUE : pdFALSE; + } + auto *mtx = sim_semaphore_detail::asMutex(sem); + bool locked = mtx->mtx.try_lock(); if (locked) { - sem->mtx.unlock(); + mtx->mtx.unlock(); return pdTRUE; } return pdFALSE; diff --git a/src/freertos/task.h b/src/freertos/task.h index a0f24a2..8f98e04 100644 --- a/src/freertos/task.h +++ b/src/freertos/task.h @@ -1,4 +1,5 @@ #pragma once +#include #include #include "FreeRTOS.h" @@ -84,4 +85,16 @@ inline void vTaskDelete(TaskHandle_t h) { } inline unsigned int uxTaskGetStackHighWaterMark(TaskHandle_t) { return 2048; } inline void vTaskList(char *) {} -inline void vTaskDelay(int) {} +// vTaskDelay's argument is in FreeRTOS ticks, and portTICK_PERIOD_MS +// (FreeRTOS.h) is this shim's one tick-rate definition. Convert through it, do +// not assume one tick is one millisecond. A no-op here made every firmware +// retry loop and every yield-every-N-rows call run in zero time. +inline void vTaskDelay(int ticks) { + if (ticks <= 0) { + // FreeRTOS treats a zero delay as a yield, not a sleep. + std::this_thread::yield(); + return; + } + std::this_thread::sleep_for(std::chrono::milliseconds( + static_cast(ticks) * portTICK_PERIOD_MS)); +}