Skip to content

Commit

Permalink
[FL-3832] Use static synchronisation primitives (flipperdevices#3679)
Browse files Browse the repository at this point in the history
* Use static mutex
* Add static_assert checks
* Use static semaphore
* Fix formatting
* Use static stream buffer
* Use static timer
* Use static event group
* Increase allocation size for stream buffer
* Remove recursive bit from the mutex before freeing
* Prevent service tasks from ever returning
* Use static threads
* Do not realloc memory when changing stack size
* Use FuriSemaphore instead of raw FreeRTOS one in rpc_test
* Remove redundant includes
* Abolish FreeRTOS dynamic allocation
* Improve FuriMutex
* Improve FuriMessageQueue
* Remove redundant comments and parentheses
* Clean up code more
* Create service threads via a dedicated constructor
* Minor code improvements
* Update docs for FuriThread, FuriTimer
* Fix doxygen typo
* Use a bigger buffer for static StreamBuffer
* Furi: remove timer control block only when timer thread have completed all operations
---------

Co-authored-by: Aleksandr Kutuzov <[email protected]>
  • Loading branch information
gsurkov and skotopes authored Jun 5, 2024
1 parent 03196fa commit 20c4121
Show file tree
Hide file tree
Showing 28 changed files with 582 additions and 425 deletions.
43 changes: 21 additions & 22 deletions applications/debug/unit_tests/tests/rpc/rpc_test.c
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
#include <core/check.h>
#include <core/record.h>
#include <furi.h>
#include <stdint.h>

#include <FreeRTOS.h>
#include <semphr.h>

#include <rpc/rpc.h>
#include <rpc/rpc_i.h>
#include <cli/cli.h>
Expand Down Expand Up @@ -40,8 +35,8 @@ static uint32_t command_id = 0;
typedef struct {
RpcSession* session;
FuriStreamBuffer* output_stream;
SemaphoreHandle_t close_session_semaphore;
SemaphoreHandle_t terminate_semaphore;
FuriSemaphore* close_session_semaphore;
FuriSemaphore* terminate_semaphore;
uint32_t timeout;
} RpcSessionContext;

Expand Down Expand Up @@ -96,8 +91,8 @@ static void test_rpc_setup(void) {

rpc_session[0].output_stream = furi_stream_buffer_alloc(4096, 1);
rpc_session_set_send_bytes_callback(rpc_session[0].session, output_bytes_callback);
rpc_session[0].close_session_semaphore = xSemaphoreCreateBinary();
rpc_session[0].terminate_semaphore = xSemaphoreCreateBinary();
rpc_session[0].close_session_semaphore = furi_semaphore_alloc(1, 0);
rpc_session[0].terminate_semaphore = furi_semaphore_alloc(1, 0);
rpc_session_set_close_callback(rpc_session[0].session, test_rpc_session_close_callback);
rpc_session_set_terminated_callback(
rpc_session[0].session, test_rpc_session_terminated_callback);
Expand All @@ -116,8 +111,8 @@ static void test_rpc_setup_second_session(void) {

rpc_session[1].output_stream = furi_stream_buffer_alloc(1000, 1);
rpc_session_set_send_bytes_callback(rpc_session[1].session, output_bytes_callback);
rpc_session[1].close_session_semaphore = xSemaphoreCreateBinary();
rpc_session[1].terminate_semaphore = xSemaphoreCreateBinary();
rpc_session[1].close_session_semaphore = furi_semaphore_alloc(1, 0);
rpc_session[1].terminate_semaphore = furi_semaphore_alloc(1, 0);
rpc_session_set_close_callback(rpc_session[1].session, test_rpc_session_close_callback);
rpc_session_set_terminated_callback(
rpc_session[1].session, test_rpc_session_terminated_callback);
Expand All @@ -126,13 +121,15 @@ static void test_rpc_setup_second_session(void) {

static void test_rpc_teardown(void) {
furi_check(rpc_session[0].close_session_semaphore);
xSemaphoreTake(rpc_session[0].terminate_semaphore, 0);
furi_semaphore_acquire(rpc_session[0].terminate_semaphore, 0);
rpc_session_close(rpc_session[0].session);
furi_check(xSemaphoreTake(rpc_session[0].terminate_semaphore, portMAX_DELAY));
furi_check(
furi_semaphore_acquire(rpc_session[0].terminate_semaphore, FuriWaitForever) ==
FuriStatusOk);
furi_record_close(RECORD_RPC);
furi_stream_buffer_free(rpc_session[0].output_stream);
vSemaphoreDelete(rpc_session[0].close_session_semaphore);
vSemaphoreDelete(rpc_session[0].terminate_semaphore);
furi_semaphore_free(rpc_session[0].close_session_semaphore);
furi_semaphore_free(rpc_session[0].terminate_semaphore);
++command_id;
rpc_session[0].output_stream = NULL;
rpc_session[0].close_session_semaphore = NULL;
Expand All @@ -142,12 +139,14 @@ static void test_rpc_teardown(void) {

static void test_rpc_teardown_second_session(void) {
furi_check(rpc_session[1].close_session_semaphore);
xSemaphoreTake(rpc_session[1].terminate_semaphore, 0);
furi_semaphore_acquire(rpc_session[1].terminate_semaphore, 0);
rpc_session_close(rpc_session[1].session);
furi_check(xSemaphoreTake(rpc_session[1].terminate_semaphore, portMAX_DELAY));
furi_check(
furi_semaphore_acquire(rpc_session[1].terminate_semaphore, FuriWaitForever) ==
FuriStatusOk);
furi_stream_buffer_free(rpc_session[1].output_stream);
vSemaphoreDelete(rpc_session[1].close_session_semaphore);
vSemaphoreDelete(rpc_session[1].terminate_semaphore);
furi_semaphore_free(rpc_session[1].close_session_semaphore);
furi_semaphore_free(rpc_session[1].terminate_semaphore);
++command_id;
rpc_session[1].output_stream = NULL;
rpc_session[1].close_session_semaphore = NULL;
Expand Down Expand Up @@ -204,14 +203,14 @@ static void test_rpc_session_close_callback(void* context) {
furi_check(context);
RpcSessionContext* callbacks_context = context;

xSemaphoreGive(callbacks_context->close_session_semaphore);
furi_check(furi_semaphore_release(callbacks_context->close_session_semaphore) == FuriStatusOk);
}

static void test_rpc_session_terminated_callback(void* context) {
furi_check(context);
RpcSessionContext* callbacks_context = context;

xSemaphoreGive(callbacks_context->terminate_semaphore);
furi_check(furi_semaphore_release(callbacks_context->terminate_semaphore) == FuriStatusOk);
}

static void test_rpc_print_message_list(MsgList_t msg_list) {
Expand Down Expand Up @@ -1645,7 +1644,7 @@ static void test_rpc_feed_rubbish_run(

test_rpc_add_empty_to_list(expected, PB_CommandStatus_ERROR_DECODE, 0);

furi_check(!xSemaphoreTake(rpc_session[0].close_session_semaphore, 0));
furi_check(furi_semaphore_acquire(rpc_session[0].close_session_semaphore, 0) != FuriStatusOk);
test_rpc_encode_and_feed(input_before, 0);
test_send_rubbish(rpc_session[0].session, pattern, pattern_size, size);
test_rpc_encode_and_feed(input_after, 0);
Expand Down
4 changes: 2 additions & 2 deletions applications/debug/unit_tests/unit_test_api_table_i.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ static constexpr auto unit_tests_api_table = sort(create_array_t<sym_entry>(
API_METHOD(xQueueSemaphoreTake, BaseType_t, (QueueHandle_t, TickType_t)),
API_METHOD(vQueueDelete, void, (QueueHandle_t)),
API_METHOD(
xQueueGenericCreate,
xQueueGenericCreateStatic,
QueueHandle_t,
(const UBaseType_t, const UBaseType_t, const uint8_t)),
(const UBaseType_t, const UBaseType_t, uint8_t*, StaticQueue_t*, const uint8_t)),
API_METHOD(
xQueueGenericSend,
BaseType_t,
Expand Down
2 changes: 2 additions & 0 deletions applications/services/bt/bt_service/bt.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ int32_t bt_srv(void* p) {
FURI_LOG_W(TAG, "Skipping start in special boot mode");
ble_glue_wait_for_c2_start(FURI_HAL_BT_C2_START_TIMEOUT);
furi_record_create(RECORD_BT, bt);

furi_thread_suspend(furi_thread_get_current_id());
return 0;
}

Expand Down
2 changes: 2 additions & 0 deletions applications/services/desktop/desktop.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,8 @@ int32_t desktop_srv(void* p) {

if(furi_hal_rtc_get_boot_mode() != FuriHalRtcBootModeNormal) {
FURI_LOG_W(TAG, "Skipping start in special boot mode");

furi_thread_suspend(furi_thread_get_current_id());
return 0;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#include <furi.h>
#include <FreeRTOS.h>
#include <gui/scene_manager.h>

#include "../desktop_i.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#include <furi.h>
#include <stdint.h>
#include <stdio.h>
#include <FreeRTOS.h>
#include <projdefs.h>
#include <input/input.h>
#include <gui/canvas.h>
Expand Down
2 changes: 2 additions & 0 deletions applications/services/dolphin/dolphin.c
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ int32_t dolphin_srv(void* p) {

if(furi_hal_rtc_get_boot_mode() != FuriHalRtcBootModeNormal) {
FURI_LOG_W(TAG, "Skipping start in special boot mode");

furi_thread_suspend(furi_thread_get_current_id());
return 0;
}

Expand Down
2 changes: 2 additions & 0 deletions applications/services/power/power_service/power.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ int32_t power_srv(void* p) {

if(furi_hal_rtc_get_boot_mode() != FuriHalRtcBootModeNormal) {
FURI_LOG_W(TAG, "Skipping start in special boot mode");

furi_thread_suspend(furi_thread_get_current_id());
return 0;
}

Expand Down
23 changes: 16 additions & 7 deletions furi/core/event_flag.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,27 @@
#define FURI_EVENT_FLAG_MAX_BITS_EVENT_GROUPS 24U
#define FURI_EVENT_FLAG_INVALID_BITS (~((1UL << FURI_EVENT_FLAG_MAX_BITS_EVENT_GROUPS) - 1U))

struct FuriEventFlag {
StaticEventGroup_t container;
};

// IMPORTANT: container MUST be the FIRST struct member
static_assert(offsetof(FuriEventFlag, container) == 0);

FuriEventFlag* furi_event_flag_alloc(void) {
furi_check(!FURI_IS_IRQ_MODE());

EventGroupHandle_t handle = xEventGroupCreate();
furi_check(handle);
FuriEventFlag* instance = malloc(sizeof(FuriEventFlag));

furi_check(xEventGroupCreateStatic(&instance->container) == (EventGroupHandle_t)instance);

return ((FuriEventFlag*)handle);
return instance;
}

void furi_event_flag_free(FuriEventFlag* instance) {
furi_check(!FURI_IS_IRQ_MODE());
vEventGroupDelete((EventGroupHandle_t)instance);
free(instance);
}

uint32_t furi_event_flag_set(FuriEventFlag* instance, uint32_t flags) {
Expand All @@ -43,7 +52,7 @@ uint32_t furi_event_flag_set(FuriEventFlag* instance, uint32_t flags) {
}

/* Return event flags after setting */
return (rflags);
return rflags;
}

uint32_t furi_event_flag_clear(FuriEventFlag* instance, uint32_t flags) {
Expand All @@ -69,7 +78,7 @@ uint32_t furi_event_flag_clear(FuriEventFlag* instance, uint32_t flags) {
}

/* Return event flags before clearing */
return (rflags);
return rflags;
}

uint32_t furi_event_flag_get(FuriEventFlag* instance) {
Expand All @@ -85,7 +94,7 @@ uint32_t furi_event_flag_get(FuriEventFlag* instance) {
}

/* Return current event flags */
return (rflags);
return rflags;
}

uint32_t furi_event_flag_wait(
Expand Down Expand Up @@ -136,5 +145,5 @@ uint32_t furi_event_flag_wait(
}

/* Return event flags before clearing */
return (rflags);
return rflags;
}
2 changes: 1 addition & 1 deletion furi/core/event_flag.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
extern "C" {
#endif

typedef void FuriEventFlag;
typedef struct FuriEventFlag FuriEventFlag;

/** Allocate FuriEventFlag
*
Expand Down
4 changes: 0 additions & 4 deletions furi/core/memmgr_heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ task.h is included from an application file. */
#error This feature is broken, logging transport must be replaced with RTT
#endif

#if(configSUPPORT_DYNAMIC_ALLOCATION == 0)
#error This file must not be used if configSUPPORT_DYNAMIC_ALLOCATION is 0
#endif

/* Block sizes must not get too small. */
#define heapMINIMUM_BLOCK_SIZE ((size_t)(xHeapStructSize << 1))

Expand Down
46 changes: 18 additions & 28 deletions furi/core/message_queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,21 @@
#include <FreeRTOS.h>
#include <queue.h>

// Internal FreeRTOS member names
#define uxMessagesWaiting uxDummy4[0]
#define uxLength uxDummy4[1]
#define uxItemSize uxDummy4[2]

struct FuriMessageQueue {
// !!! Semi-Opaque type inheritance, Very Fragile, DO NOT MOVE !!!
StaticQueue_t container;

// !!! Data buffer, must be last in the structure, DO NOT MOVE !!!
uint8_t buffer[];
};

// IMPORTANT: container MUST be the FIRST struct member
static_assert(offsetof(FuriMessageQueue, container) == 0);
// IMPORTANT: buffer MUST be the LAST struct member
static_assert(offsetof(FuriMessageQueue, buffer) == sizeof(FuriMessageQueue));

FuriMessageQueue* furi_message_queue_alloc(uint32_t msg_count, uint32_t msg_size) {
furi_check((furi_kernel_is_irq_or_masked() == 0U) && (msg_count > 0U) && (msg_size > 0U));

Expand Down Expand Up @@ -75,8 +82,7 @@ FuriStatus
}
}

/* Return execution status */
return (stat);
return stat;
}

FuriStatus furi_message_queue_get(FuriMessageQueue* instance, void* msg_ptr, uint32_t timeout) {
Expand Down Expand Up @@ -114,31 +120,19 @@ FuriStatus furi_message_queue_get(FuriMessageQueue* instance, void* msg_ptr, uin
}
}

return (stat);
return stat;
}

uint32_t furi_message_queue_get_capacity(FuriMessageQueue* instance) {
furi_check(instance);

uint32_t capacity;

/* capacity = pxQueue->uxLength */
capacity = instance->container.uxDummy4[1];

/* Return maximum number of messages */
return (capacity);
return instance->container.uxLength;
}

uint32_t furi_message_queue_get_message_size(FuriMessageQueue* instance) {
furi_check(instance);

uint32_t size;

/* size = pxQueue->uxItemSize */
size = instance->container.uxDummy4[2];

/* Return maximum message size */
return (size);
return instance->container.uxItemSize;
}

uint32_t furi_message_queue_get_count(FuriMessageQueue* instance) {
Expand All @@ -153,8 +147,7 @@ uint32_t furi_message_queue_get_count(FuriMessageQueue* instance) {
count = uxQueueMessagesWaiting(hQueue);
}

/* Return number of queued messages */
return ((uint32_t)count);
return (uint32_t)count;
}

uint32_t furi_message_queue_get_space(FuriMessageQueue* instance) {
Expand All @@ -166,16 +159,14 @@ uint32_t furi_message_queue_get_space(FuriMessageQueue* instance) {
if(furi_kernel_is_irq_or_masked() != 0U) {
isrm = taskENTER_CRITICAL_FROM_ISR();

/* space = pxQueue->uxLength - pxQueue->uxMessagesWaiting; */
space = instance->container.uxDummy4[1] - instance->container.uxDummy4[0];
space = instance->container.uxLength - instance->container.uxMessagesWaiting;

taskEXIT_CRITICAL_FROM_ISR(isrm);
} else {
space = (uint32_t)uxQueueSpacesAvailable((QueueHandle_t)instance);
}

/* Return number of available slots */
return (space);
return space;
}

FuriStatus furi_message_queue_reset(FuriMessageQueue* instance) {
Expand All @@ -191,6 +182,5 @@ FuriStatus furi_message_queue_reset(FuriMessageQueue* instance) {
(void)xQueueReset(hQueue);
}

/* Return execution status */
return (stat);
return stat;
}
Loading

0 comments on commit 20c4121

Please sign in to comment.