From: Scott Worley Date: Mon, 9 Oct 2023 21:53:25 +0000 (-0700) Subject: client: Stop sleeping separately for each re-send X-Git-Tag: v0.1.0~45 X-Git-Url: http://git.scottworley.com/tattlekey/commitdiff_plain/4db97133ae0498147735ef312fbd5cdea969e6da client: Stop sleeping separately for each re-send Instead, determine when the next send, in the whole pile of sends, needs to happen and sleep until then. This is the first functionality in this project that is more than simple glue code. This is the first non-trivial functionality that has some internal logic to it. I'd really like to write a test for it. This environment (C, CMake, pico-sdk) makes testing hard! :( I'd like to build the test as a host/native executable and run it on the build machine at build time, even though the primary/production use of the code under test would be on the pico, cross-compiled. I know testing this way is imperfect: It would fail to catch the entire class of bugs that are caused by the code under test making architecture assumptions that are valid on the build host but invalid on the cross target. But it would still be very useful for all other classes of bugs! This usage is very much not supported: 1. CMake doesn't support it at all: One toolchain per language: https://discourse.cmake.org/t/compile-unit-test-natively-and-cross-compile-code-for-embedded-target/3607 https://discourse.cmake.org/t/using-multiple-toolchains-in-the-same-project/1505 2. Even if I'm confident in my ability to write portable C that runs correctly on both build-host and cross-target architectures, I can't rely on pico-sdk's pheap library to be portable in this way. It is super-not-portable-at-all; it will never run on the build host. 3. If I use a different, portable heap library, I'm duplicating code already in the pico-sdk standard library. The pico is small enough already. I'd rather not deploy two heap libraries. 4. I could define an interface to a heap library & use the pico's heap library when deploying to the pico & some other heap library when testing on the native machine, but a. My interface-to-pico's-heap-library code goes untested. b. The interface-to-a-heap-library code would likely add overhead. c. That's a bunch of extra code to write. :( 5. Even if I worked through the interfacing-with-pico's-heap library issue and made this a separate, independent, portable library, I'd still be left with the problem of composing the two pieces. Do I build a library .so separately, spinning up a separate Debian VM for that? Does pico-sdk even support building libraries? Or do I copy the source of the library into the executable's source tree & make building the library part of the executable's build process? Ick. :( 6. Other option: Build a test intended to run on the pico. It's nuts that this is the least-nuts option. :( I am very discouraged. :( I don't even bother to keep this functionality separate with a clean interface for the test harness. :( So, no tests. :~( For now. Advice welcome. --- diff --git a/client/tattlekey.c b/client/tattlekey.c index 78d1dcf..9af9b5a 100644 --- a/client/tattlekey.c +++ b/client/tattlekey.c @@ -1,5 +1,6 @@ #include "pico/cyw43_arch.h" #include "pico/stdlib.h" +#include "pico/util/pheap.h" #include "pico/util/queue.h" #include "blink.h" @@ -13,6 +14,14 @@ typedef struct { u8_t send_count; } send_t; +uint32_t next_send(send_t *s) { return s->timestamp + s->send_count; } + +bool next_send_less_than(void *user_data, pheap_node_id_t a, + pheap_node_id_t b) { + send_t *sends = (send_t *)user_data; + return next_send(&sends[a]) < next_send(&sends[b]); +} + enum event_type { BUTTONPRESS, SEND }; typedef struct { enum event_type type; @@ -20,7 +29,6 @@ typedef struct { struct { uint32_t timestamp; } buttonpress; - send_t send; }; } event_t; @@ -53,30 +61,72 @@ static void button_pressed() { } } +static void time_to_send(uint _) { + /* This runs in interrupt context; don't linger. */ + event_t e; + e.type = SEND; + queue_try_add_ignoring_errors(&queue, &e); +} + +void service_sleeps(int alarm, send_t *sleeping_sends, pheap_t *sleeps_heap) { + hardware_alarm_cancel(alarm); + + while (1) { + uint32_t now = time_s(); + pheap_node_id_t i = ph_peek_head(sleeps_heap); + if (i == 0) + return; + send_t *send = &sleeping_sends[i]; + if (next_send(send) > now) { + uint32_t sleep_duration = next_send(send) - now; + if (hardware_alarm_set_target( + alarm, make_timeout_time_ms(sleep_duration * 1000))) + signal_error_by_blinking(); + return; + } + if (ph_remove_head(sleeps_heap, false) != i) + signal_error_by_blinking(); + uint32_t ago = now - send->timestamp; + send_report(send->seq, ago); + send->send_count++; + if (send->send_count < config_resend_count) + ph_insert_node(sleeps_heap, i); + else + ph_free_node(sleeps_heap, i); + } +} + void service_queue() { + int alarm = hardware_alarm_claim_unused(true); + if (alarm == -1) + signal_error_by_blinking(); + hardware_alarm_set_callback(alarm, time_to_send); + + send_t sleeping_sends[PICO_PHEAP_MAX_ENTRIES]; + pheap_t *sleeps_heap = + ph_create(PICO_PHEAP_MAX_ENTRIES, next_send_less_than, sleeping_sends); u16_t seq = 0; while (1) { + service_sleeps(alarm, sleeping_sends, sleeps_heap); + event_t e; queue_remove_blocking(&queue, &e); switch (e.type) { case BUTTONPRESS: { - event_t send_e; - send_e.type = SEND; - send_e.send.timestamp = e.buttonpress.timestamp; - send_e.send.seq = seq++; - send_e.send.send_count = 0; - queue_try_add_ignoring_errors(&queue, &send_e); + pheap_node_id_t i = ph_new_node(sleeps_heap); + if (i == 0) { + /* TODO: Don't drop new presses just because sleeps_heap is full of old + * presses. */ + break; + } + sleeping_sends[i].timestamp = e.buttonpress.timestamp; + sleeping_sends[i].seq = seq++; + sleeping_sends[i].send_count = 0; + ph_insert_node(sleeps_heap, i); } break; case SEND: { - uint32_t now = time_s(); - uint32_t ago = now - e.send.timestamp; - send_report(e.send.seq, ago); - e.send.send_count++; - if (e.send.send_count < config_resend_count) - queue_try_add_ignoring_errors(&queue, &e); - /* TODO: Sleep elsewhere. */ - signal(e.send.send_count == 1 ? 2 : 1, 100); - sleep_ms(config_resend_interval_ms); + /* OK, we're awake. Cool. Thanks! (We actually do the sends in the + * service_sleeps() call at the top of the loop.) */ } break; default: signal_error_by_blinking();