From 7570ac08a9b30c46670dfdb51f0e386021da98fc Mon Sep 17 00:00:00 2001 From: rusdacent <57439765+rusdacent@users.noreply.github.com> Date: Thu, 15 Oct 2020 19:50:21 +0300 Subject: [PATCH 1/3] Fix codestyle check pipeline (#178) * change continue-on-error to false * add manual trigger * revert manual trigger * test fail checkstyle * restore prev trigger and settings * fix syntax * fix exit code for codestyle * try to break pipeline * restore pipeline Co-authored-by: aanper --- .github/workflows/ci.yml | 2 +- applications/examples/fatfs_list.c | 3 ++- applications/examples/u8g2_example.c | 3 ++- applications/examples/u8g2_qrcode.c | 3 ++- applications/gui/gui_event.c | 7 ++++--- applications/menu/menu.c | 2 +- applications/tests/furi_valuemutex_test.c | 1 - applications/tests/minunit_test.c | 1 - core/api-basic/value-expanders.h | 12 +++--------- core/api-hal/api-spi.h | 17 ++++++++++------- docker/syntax_check.sh | 4 ++-- 11 files changed, 27 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd5465a8..acb79084 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: - name: Check syntax uses: ./.github/actions/docker - continue-on-error: true + continue-on-error: false with: run: /syntax_check.sh diff --git a/applications/examples/fatfs_list.c b/applications/examples/fatfs_list.c index b15db514..58e6730c 100644 --- a/applications/examples/fatfs_list.c +++ b/applications/examples/fatfs_list.c @@ -49,7 +49,8 @@ void fatfs_list(void* p) { furi_log = get_default_log(); - FuriRecordSubscriber* fb_record = furi_open_deprecated("u8g2_fb", false, false, NULL, NULL, NULL); + FuriRecordSubscriber* fb_record = + furi_open_deprecated("u8g2_fb", false, false, NULL, NULL, NULL); if(fb_record == NULL) { fuprintf(furi_log, "[widget][fatfs_list] cannot create fb record\n"); furiac_exit(NULL); diff --git a/applications/examples/u8g2_example.c b/applications/examples/u8g2_example.c index 02f35ecb..e77012d4 100644 --- a/applications/examples/u8g2_example.c +++ b/applications/examples/u8g2_example.c @@ -5,7 +5,8 @@ void u8g2_example(void* p) { FuriRecordSubscriber* log = get_default_log(); // open record - FuriRecordSubscriber* fb_record = furi_open_deprecated("u8g2_fb", false, false, NULL, NULL, NULL); + FuriRecordSubscriber* fb_record = + furi_open_deprecated("u8g2_fb", false, false, NULL, NULL, NULL); if(fb_record == NULL) { fuprintf(log, "[widget] cannot create fb record\n"); diff --git a/applications/examples/u8g2_qrcode.c b/applications/examples/u8g2_qrcode.c index 630199c1..2799df33 100644 --- a/applications/examples/u8g2_qrcode.c +++ b/applications/examples/u8g2_qrcode.c @@ -14,7 +14,8 @@ void u8g2_qrcode(void* p) { FuriRecordSubscriber* log = get_default_log(); // open record - FuriRecordSubscriber* fb_record = furi_open_deprecated("u8g2_fb", false, false, NULL, NULL, NULL); + FuriRecordSubscriber* fb_record = + furi_open_deprecated("u8g2_fb", false, false, NULL, NULL, NULL); // Allocate a chunk of memory to store the QR code // https://github.com/ricmoo/QRCode diff --git a/applications/gui/gui_event.c b/applications/gui/gui_event.c index 1d7caf64..a2ee801b 100644 --- a/applications/gui/gui_event.c +++ b/applications/gui/gui_event.c @@ -29,8 +29,8 @@ GUIEvent* gui_event_alloc() { assert(gui_event->mqueue); // Input - gui_event->input_event_record = - furi_open_deprecated("input_events", false, false, gui_event_input_events_callback, NULL, gui_event); + gui_event->input_event_record = furi_open_deprecated( + "input_events", false, false, gui_event_input_events_callback, NULL, gui_event); assert(gui_event->input_event_record != NULL); // Lock mutex gui_event->lock_mutex = osMutexNew(NULL); @@ -58,7 +58,8 @@ void gui_event_unlock(GUIEvent* gui_event) { } void gui_event_messsage_send(GUIEvent* gui_event, GUIMessage* message) { - assert(gui_event); assert(message); + assert(gui_event); + assert(message); osMessageQueuePut(gui_event->mqueue, message, 0, 0); } diff --git a/applications/menu/menu.c b/applications/menu/menu.c index f551c6af..8cafe6a4 100644 --- a/applications/menu/menu.c +++ b/applications/menu/menu.c @@ -187,7 +187,7 @@ void menu_task(void* p) { furiac_exit(NULL); } furiac_ready(); - + while(1) { MenuMessage m = menu_event_next(menu->event); diff --git a/applications/tests/furi_valuemutex_test.c b/applications/tests/furi_valuemutex_test.c index 960d1543..6279d188 100644 --- a/applications/tests/furi_valuemutex_test.c +++ b/applications/tests/furi_valuemutex_test.c @@ -39,7 +39,6 @@ void test_furi_valuemutex() { //read mutex blocking case } - /* TEST: concurrent access diff --git a/applications/tests/minunit_test.c b/applications/tests/minunit_test.c index f99b6204..37cc6b77 100644 --- a/applications/tests/minunit_test.c +++ b/applications/tests/minunit_test.c @@ -67,7 +67,6 @@ MU_TEST_SUITE(test_suite) { MU_RUN_TEST(mu_test_furi_create_open); MU_RUN_TEST(mu_test_furi_valuemutex); MU_RUN_TEST(mu_test_furi_concurrent_access); - } int run_minunit() { diff --git a/core/api-basic/value-expanders.h b/core/api-basic/value-expanders.h index cec0e75c..70adf856 100644 --- a/core/api-basic/value-expanders.h +++ b/core/api-basic/value-expanders.h @@ -12,15 +12,10 @@ void COPY_COMPOSE(void* ctx, void* state) { read_mutex((ValueMutex*)ctx, state, 0); } -typedef enum { - UiLayerBelowNotify - UiLayerNotify, - UiLayerAboveNotify -} UiLayer; +typedef enum { UiLayerBelowNotify UiLayerNotify, UiLayerAboveNotify } UiLayer; -ValueComposerHandle* add_compose_layer( - ValueComposer* composer, ValueComposerCallback cb, void* ctx, uint32_t layer -); +ValueComposerHandle* +add_compose_layer(ValueComposer* composer, ValueComposerCallback cb, void* ctx, uint32_t layer); bool remove_compose_layer(ValueComposerHandle* handle); @@ -44,7 +39,6 @@ typedef struct { PubSub pubsub; } ValueManager; - /* acquire value, changes it and send notify with current value. */ diff --git a/core/api-hal/api-spi.h b/core/api-hal/api-spi.h index b3bfe674..74559c54 100644 --- a/core/api-hal/api-spi.h +++ b/core/api-hal/api-spi.h @@ -17,13 +17,17 @@ For transmit/receive data use `spi_xfer` function. */ bool spi_xfer( SPI_HandleTypeDef* spi, - uint8_t* tx_data, uint8_t* rx_data, size_t len, - PubSubCallback cb, void* ctx); + uint8_t* tx_data, + uint8_t* rx_data, + size_t len, + PubSubCallback cb, + void* ctx); /* Blocking verison: */ -static inline bool spi_xfer_block(SPI_HandleTypeDef* spi, uint8_t* tx_data, uint8_t* rx_data, size_t len) { +static inline bool +spi_xfer_block(SPI_HandleTypeDef* spi, uint8_t* tx_data, uint8_t* rx_data, size_t len) { semaphoreInfo s; osSemaphore block = createSemaphoreStatic(s); if(!spi_xfer(spi, tx_data, rx_data, len, RELEASE_SEMAPHORE, (void*)block)) { @@ -52,15 +56,15 @@ typedef struct { ValueMutex* bus; ///< } SpiDevice; -## SPI IRQ device +##SPI IRQ device -/* + /* Many devices (like CC1101 and NFC) present as SPI bus and IRQ line. For work with it there is special entity `SpiIrqDevice`. Use `subscribe_pubsub` for subscribinq to irq events. */ -typedef struct { + typedef struct { ValueMutex* bus; ///< PubSub* irq; } SpiIrqDevice; @@ -75,7 +79,6 @@ typedef struct { ValueMutex* spi; ///< } DisplayBus; - typedef struct { ValueMutex* bus; ///< } DisplayDevice; diff --git a/docker/syntax_check.sh b/docker/syntax_check.sh index 3f54df83..0294b696 100755 --- a/docker/syntax_check.sh +++ b/docker/syntax_check.sh @@ -26,7 +26,7 @@ rust_syntax_rc=$? if [[ $rust_syntax_rc -eq 0 ]] && [[ $c_syntax_rc -eq 0 ]]; then echo "Code looks fine for me!" - exit 1 + exit 0 fi read -p "Do you want fix syntax? (y/n): " confirm && [[ $confirm == [yY] || $confirm == [yY][eE][sS] ]] || exit 1 @@ -38,4 +38,4 @@ cd $PROJECT_DIR # We use root in container and clang-format rewriting files. We'll need change owner to original local_user=$(stat -c '%u' .clang-format) $CLANG_FORMAT_BIN -style=file -i $C_FILES -chown $local_user $C_FILES \ No newline at end of file +chown $local_user $C_FILES From 59740349fa3318017eadacf50bd0b109792e3c0b Mon Sep 17 00:00:00 2001 From: DrZlo13 Date: Fri, 16 Oct 2020 03:36:15 +1000 Subject: [PATCH 2/3] furi memory managment (#177) * memory managment calls now forwarded to freertos heap * memory managment tests * local target test compability * rename heap.c file to heap_4.c for local target and explicity init heap in single thread context * rebase BlockLink_t struct * check mutex in local heap Co-authored-by: aanper --- applications/applications.mk | 1 + applications/tests/furi_memmgr_test.c | 99 +++++++ applications/tests/minunit_test.c | 9 + core/api-basic/memmgr.c | 51 ++++ core/api-basic/memmgr.h | 13 + core/flipper_v2.h | 4 +- firmware/targets/local/Src/heap_4.c | 389 ++++++++++++++++++++++++++ firmware/targets/local/Src/main.c | 8 + firmware/targets/local/fatfs/heap.h | 37 +++ firmware/targets/local/target.mk | 3 + 10 files changed, 613 insertions(+), 1 deletion(-) create mode 100644 applications/tests/furi_memmgr_test.c create mode 100644 core/api-basic/memmgr.c create mode 100644 core/api-basic/memmgr.h create mode 100644 firmware/targets/local/Src/heap_4.c create mode 100644 firmware/targets/local/fatfs/heap.h diff --git a/applications/applications.mk b/applications/applications.mk index 906f6690..c24adee3 100644 --- a/applications/applications.mk +++ b/applications/applications.mk @@ -26,6 +26,7 @@ C_SOURCES += $(APP_DIR)/tests/furi_record_test.c C_SOURCES += $(APP_DIR)/tests/test_index.c C_SOURCES += $(APP_DIR)/tests/minunit_test.c C_SOURCES += $(APP_DIR)/tests/furi_valuemutex_test.c +C_SOURCES += $(APP_DIR)/tests/furi_memmgr_test.c endif APP_EXAMPLE_BLINK ?= 0 diff --git a/applications/tests/furi_memmgr_test.c b/applications/tests/furi_memmgr_test.c new file mode 100644 index 00000000..99925be8 --- /dev/null +++ b/applications/tests/furi_memmgr_test.c @@ -0,0 +1,99 @@ +#include "minunit.h" +#include +#include +#include +// this test is not accurate, but gives a basic understanding +// that memory management is working fine + +// do not include memmgr.h here +// we also test that we are linking against stdlib +extern size_t memmgr_get_free_heap(void); +extern size_t memmgr_get_minimum_free_heap(void); + +// current heap managment realization consume: +// X bytes after allocate and 0 bytes after allocate and free, +// where X = sizeof(void*) + sizeof(size_t), look to BlockLink_t +const size_t heap_overhead_max_size = sizeof(void*) + sizeof(size_t); + +bool heap_equal(size_t heap_size, size_t heap_size_old) { + // heap borders with overhead + const size_t heap_low = heap_size_old - heap_overhead_max_size; + const size_t heap_high = heap_size_old + heap_overhead_max_size; + + // not extact, so we must test it against bigger numbers than "overhead size" + const bool result = ((heap_size >= heap_low) && (heap_size <= heap_high)); + + // debug allocation info + if(!result) { + printf("\n(hl: %zu) <= (p: %zu) <= (hh: %zu)\n", heap_low, heap_size, heap_high); + } + + return result; +} + +void test_furi_memmgr() { + size_t heap_size = 0; + size_t heap_size_old = 0; + const int alloc_size = 128; + + void* ptr = NULL; + void* original_ptr = NULL; + + // do not include furi memmgr.h case +#ifdef FURI_MEMMGR_GUARD + mu_fail("do not link against furi memmgr.h"); +#endif + + // allocate memory case + heap_size_old = memmgr_get_free_heap(); + ptr = malloc(alloc_size); + heap_size = memmgr_get_free_heap(); + mu_assert_pointers_not_eq(ptr, NULL); + mu_assert(heap_equal(heap_size, heap_size_old - alloc_size), "allocate failed"); + + // free memory case + heap_size_old = memmgr_get_free_heap(); + free(ptr); + ptr = NULL; + heap_size = memmgr_get_free_heap(); + mu_assert(heap_equal(heap_size, heap_size_old + alloc_size), "free failed"); + + // reallocate memory case + + // get filled array with some data + original_ptr = malloc(alloc_size); + mu_assert_pointers_not_eq(original_ptr, NULL); + for(int i = 0; i < alloc_size; i++) { + *(unsigned char*)(original_ptr + i) = i; + } + + // malloc array and copy data + ptr = malloc(alloc_size); + mu_assert_pointers_not_eq(ptr, NULL); + memcpy(ptr, original_ptr, alloc_size); + + // reallocate array + heap_size_old = memmgr_get_free_heap(); + ptr = realloc(ptr, alloc_size * 2); + heap_size = memmgr_get_free_heap(); + mu_assert(heap_equal(heap_size, heap_size_old - alloc_size), "reallocate failed"); + mu_assert_int_eq(memcmp(original_ptr, ptr, alloc_size), 0); + free(original_ptr); + free(ptr); + + // allocate and zero-initialize array (calloc) + original_ptr = malloc(alloc_size); + mu_assert_pointers_not_eq(original_ptr, NULL); + + for(int i = 0; i < alloc_size; i++) { + *(unsigned char*)(original_ptr + i) = 0; + } + heap_size_old = memmgr_get_free_heap(); + ptr = calloc(1, alloc_size); + heap_size = memmgr_get_free_heap(); + mu_assert(heap_equal(heap_size, heap_size_old - alloc_size), "callocate failed"); + mu_assert_int_eq(memcmp(original_ptr, ptr, alloc_size), 0); + + free(original_ptr); + free(ptr); +} \ No newline at end of file diff --git a/applications/tests/minunit_test.c b/applications/tests/minunit_test.c index 37cc6b77..631c1892 100644 --- a/applications/tests/minunit_test.c +++ b/applications/tests/minunit_test.c @@ -14,6 +14,7 @@ bool test_furi_mute_algorithm(); void test_furi_create_open(); void test_furi_valuemutex(); void test_furi_concurrent_access(); +void test_furi_memmgr(); static int foo = 0; @@ -54,6 +55,12 @@ MU_TEST(mu_test_furi_concurrent_access) { test_furi_concurrent_access(); } +MU_TEST(mu_test_furi_memmgr) { + // this test is not accurate, but gives a basic understanding + // that memory management is working fine + test_furi_memmgr(); +} + MU_TEST_SUITE(test_suite) { MU_SUITE_CONFIGURE(&test_setup, &test_teardown); @@ -67,6 +74,8 @@ MU_TEST_SUITE(test_suite) { MU_RUN_TEST(mu_test_furi_create_open); MU_RUN_TEST(mu_test_furi_valuemutex); MU_RUN_TEST(mu_test_furi_concurrent_access); + + MU_RUN_TEST(mu_test_furi_memmgr); } int run_minunit() { diff --git a/core/api-basic/memmgr.c b/core/api-basic/memmgr.c new file mode 100644 index 00000000..e67b7e7b --- /dev/null +++ b/core/api-basic/memmgr.c @@ -0,0 +1,51 @@ +#include "memmgr.h" +#include + +extern void* pvPortMalloc(size_t xSize); +extern void vPortFree(void* pv); +extern size_t xPortGetFreeHeapSize(void); +extern size_t xPortGetMinimumEverFreeHeapSize(void); + +void* malloc(size_t size) { + return pvPortMalloc(size); +} + +void free(void* ptr) { + vPortFree(ptr); +} + +void* realloc(void* ptr, size_t size) { + if(size == 0) { + vPortFree(ptr); + return NULL; + } + + void* p; + p = pvPortMalloc(size); + if(p) { + // TODO implement secure realloc + // insecure, but will do job in our case + if(ptr != NULL) { + memcpy(p, ptr, size); + vPortFree(ptr); + } + } + return p; +} + +void* calloc(size_t count, size_t size) { + void* ptr = pvPortMalloc(count * size); + if(ptr) { + // zero the memory + memset(ptr, 0, count * size); + } + return ptr; +} + +size_t memmgr_get_free_heap(void) { + return xPortGetFreeHeapSize(); +} + +size_t memmgr_get_minimum_free_heap(void) { + return xPortGetMinimumEverFreeHeapSize(); +} \ No newline at end of file diff --git a/core/api-basic/memmgr.h b/core/api-basic/memmgr.h new file mode 100644 index 00000000..70a32e4b --- /dev/null +++ b/core/api-basic/memmgr.h @@ -0,0 +1,13 @@ +#pragma once +#include + +// define for test case "link against furi memmgr" +#define FURI_MEMMGR_GUARD 1 + +void* malloc(size_t size); +void free(void* ptr); +void* realloc(void* ptr, size_t size); +void* calloc(size_t count, size_t size); + +size_t memmgr_get_free_heap(void); +size_t memmgr_get_minimum_free_heap(void); diff --git a/core/flipper_v2.h b/core/flipper_v2.h index f1079f71..d16cbacd 100644 --- a/core/flipper_v2.h +++ b/core/flipper_v2.h @@ -4,4 +4,6 @@ //#include "api-basic/flapp.h" #include "cmsis_os2.h" #include "api-basic/valuemutex.h" -//#include "api-basic/pubsub.h" \ No newline at end of file +//#include "api-basic/pubsub.h" + +#include "api-basic/memmgr.h" \ No newline at end of file diff --git a/firmware/targets/local/Src/heap_4.c b/firmware/targets/local/Src/heap_4.c new file mode 100644 index 00000000..04abcbf0 --- /dev/null +++ b/firmware/targets/local/Src/heap_4.c @@ -0,0 +1,389 @@ +/* + * FreeRTOS Kernel V10.2.1 + * Copyright (C) 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + * + * http://www.FreeRTOS.org + * http://aws.amazon.com/freertos + * + * 1 tab == 4 spaces! + */ + +/* + * A sample implementation of pvPortMalloc() and vPortFree() that combines + * (coalescences) adjacent memory blocks as they are freed, and in so doing + * limits memory fragmentation. + * + * See heap_1.c, heap_2.c and heap_3.c for alternative implementations, and the + * memory management pages of http://www.FreeRTOS.org for more information. + */ +#include "heap.h" + +osMutexId_t heap_managment_mutex = NULL; + +/* Block sizes must not get too small. */ +#define heapMINIMUM_BLOCK_SIZE ((size_t)(xHeapStructSize << 1)) + +/* Assumes 8bit bytes! */ +#define heapBITS_PER_BYTE ((size_t)8) + +/* Allocate the memory for the heap. */ +#if(configAPPLICATION_ALLOCATED_HEAP == 1) +/* The application writer has already defined the array used for the RTOS + heap - probably so it can be placed in a special segment or address. */ +extern uint8_t ucHeap[configTOTAL_HEAP_SIZE]; +#else +static uint8_t ucHeap[configTOTAL_HEAP_SIZE]; +#endif /* configAPPLICATION_ALLOCATED_HEAP */ + +/* Define the linked list structure. This is used to link free blocks in order +of their memory address. */ +typedef struct A_BLOCK_LINK { + struct A_BLOCK_LINK* pxNextFreeBlock; /*<< The next free block in the list. */ + size_t xBlockSize; /*<< The size of the free block. */ +} BlockLink_t; +/*-----------------------------------------------------------*/ + +/* + * Inserts a block of memory that is being freed into the correct position in + * the list of free memory blocks. The block being freed will be merged with + * the block in front it and/or the block behind it if the memory blocks are + * adjacent to each other. + */ +static void prvInsertBlockIntoFreeList(BlockLink_t* pxBlockToInsert); + +// this function is not thread-safe, so it must be called in single thread context +bool prvHeapInit(void); + +/*-----------------------------------------------------------*/ + +/* The size of the structure placed at the beginning of each allocated memory +block must by correctly byte aligned. */ +static const size_t xHeapStructSize = (sizeof(BlockLink_t) + ((size_t)(portBYTE_ALIGNMENT - 1))) & + ~((size_t)portBYTE_ALIGNMENT_MASK); + +/* Create a couple of list links to mark the start and end of the list. */ +static BlockLink_t xStart, *pxEnd = NULL; + +/* Keeps track of the number of free bytes remaining, but says nothing about +fragmentation. */ +static size_t xFreeBytesRemaining = 0U; +static size_t xMinimumEverFreeBytesRemaining = 0U; + +/* Gets set to the top bit of an size_t type. When this bit in the xBlockSize +member of an BlockLink_t structure is set then the block belongs to the +application. When the bit is free the block is still part of the free heap +space. */ +static size_t xBlockAllocatedBit = 0; + +/*-----------------------------------------------------------*/ + +void* pvPortMalloc(size_t xWantedSize) { + BlockLink_t *pxBlock, *pxPreviousBlock, *pxNewBlockLink; + void* pvReturn = NULL; + + acquire_memalloc_mutex(); + { + /* If this is the first call to malloc then the heap will require + initialisation to setup the list of free blocks. */ + if(pxEnd == NULL) { + prvHeapInit(); + } else { + mtCOVERAGE_TEST_MARKER(); + } + + /* Check the requested block size is not so large that the top bit is + set. The top bit of the block size member of the BlockLink_t structure + is used to determine who owns the block - the application or the + kernel, so it must be free. */ + if((xWantedSize & xBlockAllocatedBit) == 0) { + /* The wanted size is increased so it can contain a BlockLink_t + structure in addition to the requested amount of bytes. */ + if(xWantedSize > 0) { + xWantedSize += xHeapStructSize; + + /* Ensure that blocks are always aligned to the required number + of bytes. */ + if((xWantedSize & portBYTE_ALIGNMENT_MASK) != 0x00) { + /* Byte alignment required. */ + xWantedSize += (portBYTE_ALIGNMENT - (xWantedSize & portBYTE_ALIGNMENT_MASK)); + configASSERT((xWantedSize & portBYTE_ALIGNMENT_MASK) == 0); + } else { + mtCOVERAGE_TEST_MARKER(); + } + } else { + mtCOVERAGE_TEST_MARKER(); + } + + if((xWantedSize > 0) && (xWantedSize <= xFreeBytesRemaining)) { + /* Traverse the list from the start (lowest address) block until + one of adequate size is found. */ + pxPreviousBlock = &xStart; + pxBlock = xStart.pxNextFreeBlock; + while((pxBlock->xBlockSize < xWantedSize) && (pxBlock->pxNextFreeBlock != NULL)) { + pxPreviousBlock = pxBlock; + pxBlock = pxBlock->pxNextFreeBlock; + } + + /* If the end marker was reached then a block of adequate size + was not found. */ + if(pxBlock != pxEnd) { + /* Return the memory space pointed to - jumping over the + BlockLink_t structure at its start. */ + pvReturn = + (void*)(((uint8_t*)pxPreviousBlock->pxNextFreeBlock) + xHeapStructSize); + + /* This block is being returned for use so must be taken out + of the list of free blocks. */ + pxPreviousBlock->pxNextFreeBlock = pxBlock->pxNextFreeBlock; + + /* If the block is larger than required it can be split into + two. */ + if((pxBlock->xBlockSize - xWantedSize) > heapMINIMUM_BLOCK_SIZE) { + /* This block is to be split into two. Create a new + block following the number of bytes requested. The void + cast is used to prevent byte alignment warnings from the + compiler. */ + pxNewBlockLink = (void*)(((uint8_t*)pxBlock) + xWantedSize); + configASSERT((((size_t)pxNewBlockLink) & portBYTE_ALIGNMENT_MASK) == 0); + + /* Calculate the sizes of two blocks split from the + single block. */ + pxNewBlockLink->xBlockSize = pxBlock->xBlockSize - xWantedSize; + pxBlock->xBlockSize = xWantedSize; + + /* Insert the new block into the list of free blocks. */ + prvInsertBlockIntoFreeList(pxNewBlockLink); + } else { + mtCOVERAGE_TEST_MARKER(); + } + + xFreeBytesRemaining -= pxBlock->xBlockSize; + + if(xFreeBytesRemaining < xMinimumEverFreeBytesRemaining) { + xMinimumEverFreeBytesRemaining = xFreeBytesRemaining; + } else { + mtCOVERAGE_TEST_MARKER(); + } + + /* The block is being returned - it is allocated and owned + by the application and has no "next" block. */ + pxBlock->xBlockSize |= xBlockAllocatedBit; + pxBlock->pxNextFreeBlock = NULL; + } else { + mtCOVERAGE_TEST_MARKER(); + } + } else { + mtCOVERAGE_TEST_MARKER(); + } + } else { + mtCOVERAGE_TEST_MARKER(); + } + + traceMALLOC(pvReturn, xWantedSize); + } + release_memalloc_mutex(); + +#if(configUSE_MALLOC_FAILED_HOOK == 1) + { + if(pvReturn == NULL) { + extern void vApplicationMallocFailedHook(void); + vApplicationMallocFailedHook(); + } else { + mtCOVERAGE_TEST_MARKER(); + } + } +#endif + + configASSERT((((size_t)pvReturn) & (size_t)portBYTE_ALIGNMENT_MASK) == 0); + return pvReturn; +} +/*-----------------------------------------------------------*/ + +void vPortFree(void* pv) { + uint8_t* puc = (uint8_t*)pv; + BlockLink_t* pxLink; + + if(pv != NULL) { + /* The memory being freed will have an BlockLink_t structure immediately + before it. */ + puc -= xHeapStructSize; + + /* This casting is to keep the compiler from issuing warnings. */ + pxLink = (void*)puc; + + /* Check the block is actually allocated. */ + configASSERT((pxLink->xBlockSize & xBlockAllocatedBit) != 0); + configASSERT(pxLink->pxNextFreeBlock == NULL); + + if((pxLink->xBlockSize & xBlockAllocatedBit) != 0) { + if(pxLink->pxNextFreeBlock == NULL) { + /* The block is being returned to the heap - it is no longer + allocated. */ + pxLink->xBlockSize &= ~xBlockAllocatedBit; + + acquire_memalloc_mutex(); + { + /* Add this block to the list of free blocks. */ + xFreeBytesRemaining += pxLink->xBlockSize; + traceFREE(pv, pxLink->xBlockSize); + prvInsertBlockIntoFreeList(((BlockLink_t*)pxLink)); + } + release_memalloc_mutex(); + } else { + mtCOVERAGE_TEST_MARKER(); + } + } else { + mtCOVERAGE_TEST_MARKER(); + } + } +} +/*-----------------------------------------------------------*/ + +size_t xPortGetFreeHeapSize(void) { + return xFreeBytesRemaining; +} +/*-----------------------------------------------------------*/ + +size_t xPortGetMinimumEverFreeHeapSize(void) { + return xMinimumEverFreeBytesRemaining; +} +/*-----------------------------------------------------------*/ + +void vPortInitialiseBlocks(void) { + /* This just exists to keep the linker quiet. */ +} +/*-----------------------------------------------------------*/ + +bool prvHeapInit(void) { + BlockLink_t* pxFirstFreeBlock; + uint8_t* pucAlignedHeap; + size_t uxAddress; + size_t xTotalHeapSize = configTOTAL_HEAP_SIZE; + + /* Ensure the heap starts on a correctly aligned boundary. */ + uxAddress = (size_t)ucHeap; + + if((uxAddress & portBYTE_ALIGNMENT_MASK) != 0) { + uxAddress += (portBYTE_ALIGNMENT - 1); + uxAddress &= ~((size_t)portBYTE_ALIGNMENT_MASK); + xTotalHeapSize -= uxAddress - (size_t)ucHeap; + } + + pucAlignedHeap = (uint8_t*)uxAddress; + + /* xStart is used to hold a pointer to the first item in the list of free + blocks. The void cast is used to prevent compiler warnings. */ + xStart.pxNextFreeBlock = (void*)pucAlignedHeap; + xStart.xBlockSize = (size_t)0; + + /* pxEnd is used to mark the end of the list of free blocks and is inserted + at the end of the heap space. */ + uxAddress = ((size_t)pucAlignedHeap) + xTotalHeapSize; + uxAddress -= xHeapStructSize; + uxAddress &= ~((size_t)portBYTE_ALIGNMENT_MASK); + pxEnd = (void*)uxAddress; + pxEnd->xBlockSize = 0; + pxEnd->pxNextFreeBlock = NULL; + + /* To start with there is a single free block that is sized to take up the + entire heap space, minus the space taken by pxEnd. */ + pxFirstFreeBlock = (void*)pucAlignedHeap; + pxFirstFreeBlock->xBlockSize = uxAddress - (size_t)pxFirstFreeBlock; + pxFirstFreeBlock->pxNextFreeBlock = pxEnd; + + /* Only one block exists - and it covers the entire usable heap space. */ + xMinimumEverFreeBytesRemaining = pxFirstFreeBlock->xBlockSize; + xFreeBytesRemaining = pxFirstFreeBlock->xBlockSize; + + /* Work out the position of the top bit in a size_t variable. */ + xBlockAllocatedBit = ((size_t)1) << ((sizeof(size_t) * heapBITS_PER_BYTE) - 1); + + // now we can use malloc, so we init heap managment mutex + const osMutexAttr_t heap_managment_mutext_attr = { + .name = NULL, .attr_bits = 0, .cb_mem = NULL, .cb_size = 0U}; + + heap_managment_mutex = osMutexNew(&heap_managment_mutext_attr); + + return heap_managment_mutex != NULL; +} +/*-----------------------------------------------------------*/ + +static void prvInsertBlockIntoFreeList(BlockLink_t* pxBlockToInsert) { + BlockLink_t* pxIterator; + uint8_t* puc; + + /* Iterate through the list until a block is found that has a higher address + than the block being inserted. */ + for(pxIterator = &xStart; pxIterator->pxNextFreeBlock < pxBlockToInsert; + pxIterator = pxIterator->pxNextFreeBlock) { + /* Nothing to do here, just iterate to the right position. */ + } + + /* Do the block being inserted, and the block it is being inserted after + make a contiguous block of memory? */ + puc = (uint8_t*)pxIterator; + if((puc + pxIterator->xBlockSize) == (uint8_t*)pxBlockToInsert) { + pxIterator->xBlockSize += pxBlockToInsert->xBlockSize; + pxBlockToInsert = pxIterator; + } else { + mtCOVERAGE_TEST_MARKER(); + } + + /* Do the block being inserted, and the block it is being inserted before + make a contiguous block of memory? */ + puc = (uint8_t*)pxBlockToInsert; + if((puc + pxBlockToInsert->xBlockSize) == (uint8_t*)pxIterator->pxNextFreeBlock) { + if(pxIterator->pxNextFreeBlock != pxEnd) { + /* Form one big block from the two blocks. */ + pxBlockToInsert->xBlockSize += pxIterator->pxNextFreeBlock->xBlockSize; + pxBlockToInsert->pxNextFreeBlock = pxIterator->pxNextFreeBlock->pxNextFreeBlock; + } else { + pxBlockToInsert->pxNextFreeBlock = pxEnd; + } + } else { + pxBlockToInsert->pxNextFreeBlock = pxIterator->pxNextFreeBlock; + } + + /* If the block being inserted plugged a gab, so was merged with the block + before and the block after, then it's pxNextFreeBlock pointer will have + already been set, and should not be set here as that would make it point + to itself. */ + if(pxIterator != pxBlockToInsert) { + pxIterator->pxNextFreeBlock = pxBlockToInsert; + } else { + mtCOVERAGE_TEST_MARKER(); + } +} + +/* +at first run (heap init) it not work properly and prvHeapInit +is not thread-safe. But then we init mutex or die +*/ +void acquire_memalloc_mutex() { + if(heap_managment_mutex != NULL) { + osMutexAcquire(heap_managment_mutex, osWaitForever); + } +} + +void release_memalloc_mutex() { + if(heap_managment_mutex != NULL) { + osMutexRelease(heap_managment_mutex); + } +} \ No newline at end of file diff --git a/firmware/targets/local/Src/main.c b/firmware/targets/local/Src/main.c index 64b2d05b..d61403c2 100644 --- a/firmware/targets/local/Src/main.c +++ b/firmware/targets/local/Src/main.c @@ -1,3 +1,6 @@ +#include "heap.h" +#include "errno.h" + /* Flipper devices inc. @@ -7,5 +10,10 @@ Local fw build entry point. int app(); int main() { + // this function is not thread-safe, so it must be called in single thread context + if(!prvHeapInit()){ + return ENOMEM; + } + return app(); } \ No newline at end of file diff --git a/firmware/targets/local/fatfs/heap.h b/firmware/targets/local/fatfs/heap.h new file mode 100644 index 00000000..45c883eb --- /dev/null +++ b/firmware/targets/local/fatfs/heap.h @@ -0,0 +1,37 @@ +#include +#include +#include +#include + +#define configTOTAL_HEAP_SIZE ((size_t)(8192 * 16)) +#define configAPPLICATION_ALLOCATED_HEAP 0 +#define portBYTE_ALIGNMENT 8 + +#if portBYTE_ALIGNMENT == 8 +#define portBYTE_ALIGNMENT_MASK (0x0007) +#endif + +/* No test marker by default. */ +#ifndef mtCOVERAGE_TEST_MARKER +#define mtCOVERAGE_TEST_MARKER() +#endif + +/* No tracing by default. */ +#ifndef traceMALLOC +#define traceMALLOC(pvReturn, xWantedSize) +#endif + +/* No tracing by default. */ +#ifndef traceFREE +#define traceFREE(pvReturn, xBlockSize) +#endif + +/* No assert by default. */ +#ifndef configASSERT +#define configASSERT(var) +#endif + +bool prvHeapInit(void); + +void acquire_memalloc_mutex(); +void release_memalloc_mutex(); \ No newline at end of file diff --git a/firmware/targets/local/target.mk b/firmware/targets/local/target.mk index 968adca1..741b16ab 100644 --- a/firmware/targets/local/target.mk +++ b/firmware/targets/local/target.mk @@ -15,5 +15,8 @@ LDFLAGS += -pthread CFLAGS += -I$(TARGET_DIR)/fatfs C_SOURCES += $(TARGET_DIR)/fatfs/syscall.c +# memory manager +C_SOURCES += $(TARGET_DIR)/Src/heap_4.c + run: all $(OBJ_DIR)/$(PROJECT).elf \ No newline at end of file From 05ef19b07a5f7aa383676249a9fd1b3b99892d11 Mon Sep 17 00:00:00 2001 From: DrZlo13 Date: Fri, 16 Oct 2020 04:23:18 +1000 Subject: [PATCH 3/3] Pubsub core api feature (#174) * fixed inline functions for modern C standart * pubsub api, base version * basic test for pubsub * update applications.mk, add test file * more test for pubsub * remove unimplemented files, cleanup header file * remove legacy tests, check unsubscribe not call cb * implement deleting mutex, fail test * release mutex before deleting Co-authored-by: aanper --- applications/applications.mk | 1 + applications/tests/furi_pubsub_test.c | 56 ++++++++ applications/tests/furi_record_test.c | 194 -------------------------- applications/tests/minunit_test.c | 16 +-- core/api-basic/pubsub.c | 90 ++++++++++++ core/api-basic/pubsub.c.unimplemented | 48 ------- core/api-basic/pubsub.h | 34 +++-- core/flipper_v2.h | 4 +- firmware/targets/local/Inc/cmsis_os.h | 1 + firmware/targets/local/Src/lo_os.c | 12 ++ 10 files changed, 188 insertions(+), 268 deletions(-) create mode 100644 applications/tests/furi_pubsub_test.c create mode 100644 core/api-basic/pubsub.c delete mode 100644 core/api-basic/pubsub.c.unimplemented diff --git a/applications/applications.mk b/applications/applications.mk index c24adee3..9991b367 100644 --- a/applications/applications.mk +++ b/applications/applications.mk @@ -26,6 +26,7 @@ C_SOURCES += $(APP_DIR)/tests/furi_record_test.c C_SOURCES += $(APP_DIR)/tests/test_index.c C_SOURCES += $(APP_DIR)/tests/minunit_test.c C_SOURCES += $(APP_DIR)/tests/furi_valuemutex_test.c +C_SOURCES += $(APP_DIR)/tests/furi_pubsub_test.c C_SOURCES += $(APP_DIR)/tests/furi_memmgr_test.c endif diff --git a/applications/tests/furi_pubsub_test.c b/applications/tests/furi_pubsub_test.c new file mode 100644 index 00000000..4447f542 --- /dev/null +++ b/applications/tests/furi_pubsub_test.c @@ -0,0 +1,56 @@ +#include +#include +#include "flipper_v2.h" +#include "log.h" + +#include "minunit.h" + +const uint32_t context_value = 0xdeadbeef; +const uint32_t notify_value_0 = 0x12345678; +const uint32_t notify_value_1 = 0x11223344; + +uint32_t pubsub_value = 0; +uint32_t pubsub_context_value = 0; + +void test_pubsub_handler(void* arg, void* ctx) { + pubsub_value = *(uint32_t*)arg; + pubsub_context_value = *(uint32_t*)ctx; +} + +void test_furi_pubsub() { + bool result; + PubSub test_pubsub; + PubSubItem* test_pubsub_item; + + // init pubsub case + result = init_pubsub(&test_pubsub); + mu_assert(result, "init pubsub failed"); + + // subscribe pubsub case + test_pubsub_item = subscribe_pubsub(&test_pubsub, test_pubsub_handler, (void*)&context_value); + mu_assert_pointers_not_eq(test_pubsub_item, NULL); + + /// notify pubsub case + result = notify_pubsub(&test_pubsub, (void*)¬ify_value_0); + mu_assert(result, "notify pubsub failed"); + mu_assert_int_eq(pubsub_value, notify_value_0); + mu_assert_int_eq(pubsub_context_value, context_value); + + // unsubscribe pubsub case + result = unsubscribe_pubsub(test_pubsub_item); + mu_assert(result, "unsubscribe pubsub failed"); + + result = unsubscribe_pubsub(test_pubsub_item); + mu_assert(!result, "unsubscribe pubsub not failed"); + + /// notify unsubscribed pubsub case + result = notify_pubsub(&test_pubsub, (void*)¬ify_value_1); + mu_assert(result, "notify pubsub failed"); + mu_assert_int_not_eq(pubsub_value, notify_value_1); + + // delete pubsub case + result = delete_pubsub(&test_pubsub); + mu_assert(result, "unsubscribe pubsub failed"); + + // TODO test case that the pubsub_delete will remove pubsub from heap +} \ No newline at end of file diff --git a/applications/tests/furi_record_test.c b/applications/tests/furi_record_test.c index f3fdcf1c..4481602c 100644 --- a/applications/tests/furi_record_test.c +++ b/applications/tests/furi_record_test.c @@ -14,197 +14,3 @@ void test_furi_create_open() { void* record = furi_open("test/holding"); mu_assert_pointers_eq(record, &test_data); } - -/* -TEST: non-existent data -1. Try to open non-existent record -2. Check for NULL handler -3. Try to write/read, get error - -TODO: implement this test -*/ -bool test_furi_nonexistent_data() { - return true; -} - -/* -TEST: mute algorithm -1. Create "parent" application: - 1. Create pipe record - 2. Open watch handler: no_mute=false, solo=false, subscribe to data. - -2. Open handler A: no_mute=false, solo=false, NULL subscriber. Subscribe to state. -Try to write data to A and check subscriber. - -3. Open handler B: no_mute=true, solo=true, NULL subscriber. -Check A state cb get FlipperRecordStateMute. -Try to write data to A and check that subscriber get no data. (muted) -Try to write data to B and check that subscriber get data. - -TODO: test 3 not pass beacuse state callback not implemented - -4. Open hadler C: no_mute=false, solo=true, NULL subscriber. -Try to write data to A and check that subscriber get no data. (muted) -Try to write data to B and check that subscriber get data. (not muted because open with no_mute) -Try to write data to C and check that subscriber get data. - -5. Open handler D: no_mute=false, solo=false, NULL subscriber. -Try to write data to A and check that subscriber get no data. (muted) -Try to write data to B and check that subscriber get data. (not muted because open with no_mute) -Try to write data to C and check that subscriber get data. (not muted because D open without solo) -Try to write data to D and check that subscriber get data. - -6. Close C, close B. -Check A state cb get FlipperRecordStateUnmute -Try to write data to A and check that subscriber get data. (unmuted) -Try to write data to D and check that subscriber get data. - -TODO: test 6 not pass beacuse cleanup is not implemented -TODO: test 6 not pass because mute algorithm is unfinished. - -7. Exit "parent application" -Check A state cb get FlipperRecordStateDeleted - -TODO: test 7 not pass beacuse cleanup is not implemented -*/ - -static uint8_t mute_last_value = 0; -static FlipperRecordState mute_last_state = 255; - -void mute_record_cb(const void* value, size_t size, void* ctx) { - // hold value to static var - mute_last_value = *((uint8_t*)value); -} - -void mute_record_state_cb(FlipperRecordState state, void* ctx) { - mute_last_state = state; -} - -void furi_mute_parent_app(void* p) { - // 1. Create pipe record - if(!furi_create_deprecated("test/mute", NULL, 0)) { - printf("cannot create record\n"); - furiac_exit(NULL); - } - - // 2. Open watch handler: solo=false, no_mute=false, subscribe to data - FuriRecordSubscriber* watch_handler = - furi_open_deprecated("test/mute", false, false, mute_record_cb, NULL, NULL); - if(watch_handler == NULL) { - printf("cannot open watch handler\n"); - furiac_exit(NULL); - } - - while(1) { - // TODO we don't have thread sleep - delay(100000); - } -} - -bool test_furi_mute_algorithm() { - // 1. Create "parent" application: - FuriApp* parent_app = furiac_start(furi_mute_parent_app, "parent app", NULL); - - delay(2); // wait creating record - - // 2. Open handler A: solo=false, no_mute=false, NULL subscriber. Subscribe to state. - FuriRecordSubscriber* handler_a = - furi_open_deprecated("test/mute", false, false, NULL, mute_record_state_cb, NULL); - if(handler_a == NULL) { - printf("cannot open handler A\n"); - return false; - } - - uint8_t test_counter = 1; - - // Try to write data to A and check subscriber - if(!furi_write(handler_a, &test_counter, sizeof(uint8_t))) { - printf("write to A failed\n"); - return false; - } - - if(mute_last_value != test_counter) { - printf("value A mismatch: %d vs %d\n", mute_last_value, test_counter); - return false; - } - - // 3. Open handler B: solo=true, no_mute=true, NULL subscriber. - FuriRecordSubscriber* handler_b = - furi_open_deprecated("test/mute", true, true, NULL, NULL, NULL); - if(handler_b == NULL) { - printf("cannot open handler B\n"); - return false; - } - - // Check A state cb get FlipperRecordStateMute. - if(mute_last_state != FlipperRecordStateMute) { - printf("A state is not FlipperRecordStateMute: %d\n", mute_last_state); - return false; - } - - test_counter = 2; - - // Try to write data to A and check that subscriber get no data. (muted) - if(furi_write(handler_a, &test_counter, sizeof(uint8_t))) { - printf("A not muted\n"); - return false; - } - - if(mute_last_value == test_counter) { - printf("value A must be muted\n"); - return false; - } - - test_counter = 3; - - // Try to write data to B and check that subscriber get data. - if(!furi_write(handler_b, &test_counter, sizeof(uint8_t))) { - printf("write to B failed\n"); - return false; - } - - if(mute_last_value != test_counter) { - printf("value B mismatch: %d vs %d\n", mute_last_value, test_counter); - return false; - } - - // 4. Open hadler C: solo=true, no_mute=false, NULL subscriber. - FuriRecordSubscriber* handler_c = - furi_open_deprecated("test/mute", true, false, NULL, NULL, NULL); - if(handler_c == NULL) { - printf("cannot open handler C\n"); - return false; - } - - // TODO: Try to write data to A and check that subscriber get no data. (muted) - // TODO: Try to write data to B and check that subscriber get data. (not muted because open with no_mute) - // TODO: Try to write data to C and check that subscriber get data. - - // 5. Open handler D: solo=false, no_mute=false, NULL subscriber. - FuriRecordSubscriber* handler_d = - furi_open_deprecated("test/mute", false, false, NULL, NULL, NULL); - if(handler_d == NULL) { - printf("cannot open handler D\n"); - return false; - } - - // TODO: Try to write data to A and check that subscriber get no data. (muted) - // TODO: Try to write data to B and check that subscriber get data. (not muted because open with no_mute) - // TODO: Try to write data to C and check that subscriber get data. (not muted because D open without solo) - // TODO: Try to write data to D and check that subscriber get data. - - // 6. Close C, close B. - // TODO: Check A state cb get FlipperRecordStateUnmute - // TODO: Try to write data to A and check that subscriber get data. (unmuted) - // TODO: Try to write data to D and check that subscriber get data. - - // 7. Exit "parent application" - if(!furiac_kill(parent_app)) { - printf("kill parent_app fail\n"); - return false; - } - - // TODO: Check A state cb get FlipperRecordStateDeleted - - return true; -} \ No newline at end of file diff --git a/applications/tests/minunit_test.c b/applications/tests/minunit_test.c index 631c1892..472dfa7d 100644 --- a/applications/tests/minunit_test.c +++ b/applications/tests/minunit_test.c @@ -7,13 +7,12 @@ bool test_furi_ac_create_kill(); bool test_furi_ac_switch_exit(); -bool test_furi_nonexistent_data(); -bool test_furi_mute_algorithm(); - // v2 tests void test_furi_create_open(); void test_furi_valuemutex(); void test_furi_concurrent_access(); +void test_furi_pubsub(); + void test_furi_memmgr(); static int foo = 0; @@ -38,10 +37,6 @@ MU_TEST(mu_test_furi_ac_switch_exit) { mu_assert_int_eq(test_furi_ac_switch_exit(), true); } -MU_TEST(mu_test_furi_nonexistent_data) { - mu_assert_int_eq(test_furi_nonexistent_data(), true); -} - // v2 tests MU_TEST(mu_test_furi_create_open) { test_furi_create_open(); @@ -55,6 +50,10 @@ MU_TEST(mu_test_furi_concurrent_access) { test_furi_concurrent_access(); } +MU_TEST(mu_test_furi_pubsub) { + test_furi_pubsub(); +} + MU_TEST(mu_test_furi_memmgr) { // this test is not accurate, but gives a basic understanding // that memory management is working fine @@ -68,12 +67,11 @@ MU_TEST_SUITE(test_suite) { MU_RUN_TEST(mu_test_furi_ac_create_kill); MU_RUN_TEST(mu_test_furi_ac_switch_exit); - MU_RUN_TEST(mu_test_furi_nonexistent_data); - // v2 tests MU_RUN_TEST(mu_test_furi_create_open); MU_RUN_TEST(mu_test_furi_valuemutex); MU_RUN_TEST(mu_test_furi_concurrent_access); + MU_RUN_TEST(mu_test_furi_pubsub); MU_RUN_TEST(mu_test_furi_memmgr); } diff --git a/core/api-basic/pubsub.c b/core/api-basic/pubsub.c new file mode 100644 index 00000000..9b06705a --- /dev/null +++ b/core/api-basic/pubsub.c @@ -0,0 +1,90 @@ +#include "pubsub.h" + +bool init_pubsub(PubSub* pubsub) { + // mutex without name, + // no attributes (unfortunatly robust mutex is not supported by FreeRTOS), + // with dynamic memory allocation + const osMutexAttr_t value_mutex_attr = { + .name = NULL, .attr_bits = 0, .cb_mem = NULL, .cb_size = 0U}; + + pubsub->mutex = osMutexNew(&value_mutex_attr); + if(pubsub->mutex == NULL) return false; + + // construct list + list_pubsub_cb_init(pubsub->items); + + return true; +} + +bool delete_pubsub(PubSub* pubsub) { + if(osMutexAcquire(pubsub->mutex, osWaitForever) == osOK) { + bool result = osMutexDelete(pubsub->mutex) == osOK; + list_pubsub_cb_clear(pubsub->items); + return result; + } else { + return false; + } +} + +PubSubItem* subscribe_pubsub(PubSub* pubsub, PubSubCallback cb, void* ctx) { + if(osMutexAcquire(pubsub->mutex, osWaitForever) == osOK) { + // put uninitialized item to the list + PubSubItem* item = list_pubsub_cb_push_raw(pubsub->items); + + // initialize item + item->cb = cb; + item->ctx = ctx; + item->self = pubsub; + + // TODO unsubscribe pubsub on app exit + //flapp_on_exit(unsubscribe_pubsub, item); + + osMutexRelease(pubsub->mutex); + + return item; + } else { + return NULL; + } +} + +bool unsubscribe_pubsub(PubSubItem* pubsub_id) { + if(osMutexAcquire(pubsub_id->self->mutex, osWaitForever) == osOK) { + bool result = false; + + // iterate over items + list_pubsub_cb_it_t it; + for(list_pubsub_cb_it(it, pubsub_id->self->items); !list_pubsub_cb_end_p(it); + list_pubsub_cb_next(it)) { + const PubSubItem* item = list_pubsub_cb_cref(it); + + // if the iterator is equal to our element + if(item == pubsub_id) { + list_pubsub_cb_remove(pubsub_id->self->items, it); + result = true; + break; + } + } + + osMutexRelease(pubsub_id->self->mutex); + return result; + } else { + return false; + } +} + +bool notify_pubsub(PubSub* pubsub, void* arg) { + if(osMutexAcquire(pubsub->mutex, osWaitForever) == osOK) { + // iterate over subscribers + list_pubsub_cb_it_t it; + for(list_pubsub_cb_it(it, pubsub->items); !list_pubsub_cb_end_p(it); + list_pubsub_cb_next(it)) { + const PubSubItem* item = list_pubsub_cb_cref(it); + item->cb(arg, item->ctx); + } + + osMutexRelease(pubsub->mutex); + return true; + } else { + return false; + } +} diff --git a/core/api-basic/pubsub.c.unimplemented b/core/api-basic/pubsub.c.unimplemented deleted file mode 100644 index 77132f75..00000000 --- a/core/api-basic/pubsub.c.unimplemented +++ /dev/null @@ -1,48 +0,0 @@ -#include "pubsub.h" - -void init_pubsub(PubSub* pubsub) { - pubsub->count = 0; - - for(size_t i = 0; i < NUM_OF_CALLBACKS; i++) { - pubsub->items[i]. - } -} - -// TODO add mutex to reconfigurate PubSub -PubSubId* subscribe_pubsub(PubSub* pubsub, PubSubCallback cb, void* ctx) { - if(pubsub->count >= NUM_OF_CALLBACKS) return NULL; - - pubsub->count++; - PubSubItem* current = pubsub->items[pubsub->count]; - - current->cb = cb; - currrnt->ctx = ctx; - - pubsub->ids[pubsub->count].self = pubsub; - pubsub->ids[pubsub->count].item = current; - - flapp_on_exit(unsubscribe_pubsub, &(pubsub->ids[pubsub->count])); - - return current; -} - -void unsubscribe_pubsub(PubSubId* pubsub_id) { - // TODO: add, and rearrange all items to keep subscribers item continuous - // TODO: keep ids link actual - // TODO: also add mutex on every pubsub changes - - // trivial implementation for NUM_OF_CALLBACKS = 1 - if(NUM_OF_CALLBACKS != 1) return; - - if(pubsub_id != NULL || pubsub_id->self != NULL || pubsub_id->item != NULL) return; - - pubsub_id->self->count = 0; - pubsub_id->item = NULL; -} - -void notify_pubsub(PubSub* pubsub, void* arg) { - // iterate over subscribers - for(size_t i = 0; i < pubsub->count; i++) { - pubsub->items[i]->cb(arg, pubsub->items[i]->ctx); - } -} diff --git a/core/api-basic/pubsub.h b/core/api-basic/pubsub.h index bd38cc1f..1c235159 100644 --- a/core/api-basic/pubsub.h +++ b/core/api-basic/pubsub.h @@ -1,6 +1,7 @@ #pragma once -#include "flipper.h" +#include "flipper_v2.h" +#include "m-list.h" /* == PubSub == @@ -11,43 +12,46 @@ and also subscriber can set `void*` context pointer that pass into callback (you can see callback signature below). */ -typedef void(PubSubCallback*)(void*, void*); +typedef void (*PubSubCallback)(void*, void*); +typedef struct PubSubType PubSub; typedef struct { PubSubCallback cb; void* ctx; + PubSub* self; } PubSubItem; -typedef struct { - PubSub* self; - PubSubItem* item; -} PubSubId; +LIST_DEF(list_pubsub_cb, PubSubItem, M_POD_OPLIST); -typedef struct { - PubSubItem items[NUM_OF_CALLBACKS]; - PubSubId ids[NUM_OF_CALLBACKS]; ///< permanent links to item - size_t count; ///< count of callbacks -} PubSub; +struct PubSubType { + list_pubsub_cb_t items; + osMutexId_t mutex; +}; /* To create PubSub you should create PubSub instance and call `init_pubsub`. */ -void init_pubsub(PubSub* pubsub); +bool init_pubsub(PubSub* pubsub); + +/* +Since we use dynamic memory - we must explicity delete pubsub +*/ +bool delete_pubsub(PubSub* pubsub); /* Use `subscribe_pubsub` to register your callback. */ -PubSubId* subscribe_pubsub(PubSub* pubsub, PubSubCallback cb, void* ctx); +PubSubItem* subscribe_pubsub(PubSub* pubsub, PubSubCallback cb, void* ctx); /* Use `unsubscribe_pubsub` to unregister callback. */ -void unsubscribe_pubsub(PubSubId* pubsub_id); +bool unsubscribe_pubsub(PubSubItem* pubsub_id); /* Use `notify_pubsub` to notify subscribers. */ -void notify_pubsub(PubSub* pubsub, void* arg); +bool notify_pubsub(PubSub* pubsub, void* arg); /* diff --git a/core/flipper_v2.h b/core/flipper_v2.h index d16cbacd..9e3bc90e 100644 --- a/core/flipper_v2.h +++ b/core/flipper_v2.h @@ -4,6 +4,6 @@ //#include "api-basic/flapp.h" #include "cmsis_os2.h" #include "api-basic/valuemutex.h" -//#include "api-basic/pubsub.h" +#include "api-basic/pubsub.h" -#include "api-basic/memmgr.h" \ No newline at end of file +#include "api-basic/memmgr.h" diff --git a/firmware/targets/local/Inc/cmsis_os.h b/firmware/targets/local/Inc/cmsis_os.h index ebe04567..c1c574ab 100644 --- a/firmware/targets/local/Inc/cmsis_os.h +++ b/firmware/targets/local/Inc/cmsis_os.h @@ -94,5 +94,6 @@ typedef enum { osStatus_t osMutexAcquire (osMutexId_t mutex_id, uint32_t timeout); osStatus_t osMutexRelease (osMutexId_t mutex_id); +osStatus_t osMutexDelete (osMutexId_t mutex_id); #define osWaitForever portMAX_DELAY diff --git a/firmware/targets/local/Src/lo_os.c b/firmware/targets/local/Src/lo_os.c index e3603990..94c570a3 100644 --- a/firmware/targets/local/Src/lo_os.c +++ b/firmware/targets/local/Src/lo_os.c @@ -253,3 +253,15 @@ osStatus_t osMutexRelease (osMutexId_t mutex_id) { return osError; } } + +osStatus_t osMutexDelete (osMutexId_t mutex_id) { + osMutexRelease(mutex_id); + + int res = 0; + if((res = pthread_mutex_destroy(&mutex_id->mutex)) == 0) { + return osOK; + } else { + printf("res = %d\n", res); + return osError; + } +}